Skip to content

fix(openviking): sanitize skill memory input - #32663

Closed
ehz0ah wants to merge 1 commit into
NousResearch:mainfrom
ehz0ah:fix/search-query-safety
Closed

fix(openviking): sanitize skill memory input#32663
ehz0ah wants to merge 1 commit into
NousResearch:mainfrom
ehz0ah:fix/search-query-safety

Conversation

@ehz0ah

@ehz0ah ehz0ah commented May 26, 2026

Copy link
Copy Markdown
Contributor

Closes #32662

Summary

Hermes expands slash-skill invocations into model-facing messages that include the full skill body. The OpenViking memory provider was using that expanded text for background search and session sync, which made OpenViking receive prompt scaffolding instead of the user's actual instruction.

This keeps the global memory manager contract unchanged and limits the cleanup to the OpenViking provider boundary.

Changes

  • Extract the user instruction from single slash-skill invocations before OpenViking prefetch and session sync.
  • Extract the User instruction: field from skill-bundle turns before OpenViking prefetch.
  • Skip OpenViking prefetch/sync for synthetic skill turns that do not include a user instruction.
  • Add a marker-drift regression that checks the OpenViking parser markers against Hermes' skill scaffolding builders.
  • Add regression coverage that the MemoryManager still passes the original turn text to providers.

Test Plan

  • scripts/run_tests.sh tests/openviking_plugin/test_openviking.py tests/run_agent/test_memory_sync_interrupted.py

@ehz0ah
ehz0ah force-pushed the fix/search-query-safety branch from 04f6836 to 951155a Compare May 26, 2026 15:02
@ehz0ah ehz0ah changed the title fix(openviking): avoid using skill scaffolding as OV memory text fix(openviking): sanitize skill memory input May 26, 2026
@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 labels May 26, 2026
@ehz0ah
ehz0ah force-pushed the fix/search-query-safety branch 2 times, most recently from f33e57e to 6772149 Compare May 29, 2026 06:12
@ehz0ah

ehz0ah commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Note on alternative approach

The marker parsing in _derive_openviking_user_text keeps this PR scoped to the OpenViking plugin. A cleaner fix would be to pass user_instruction through skill invocations as a separate field so the plugin reads it directly without needing the markers. That would touch agent/skill_commands.py, agent/skill_bundles.py, and the cron skill-assembly path, which felt like too much for one PR. Happy to follow up with that version if you'd rather go that route.

@ehz0ah
ehz0ah force-pushed the fix/search-query-safety branch from 6772149 to bdff28b Compare June 16, 2026 03:40
teknium1 added a commit that referenced this pull request Jun 16, 2026
…viking

Generalizes #32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #47311. Your commit was cherry-picked onto current main with your authorship preserved (commit e3adbb5).

Thanks for the fix — and for the precise scoping note. While salvaging, an audit of all shipped memory providers showed the slash-skill scaffolding pollution wasn't openviking-specific: mem0, hindsight, retaindb, byterover, honcho, and supermemory all store/embed the raw user turn too. So we generalized your parser into a canonical extractor in agent/skill_commands.py and moved the strip up to the MemoryManager fan-out (commit c2c55c4), fixing every provider at once. Your openviking hook now delegates to that shared helper as defense-in-depth.

alanbratu pushed a commit to alanbratu/hermes-agent that referenced this pull request Jun 17, 2026
…viking

Generalizes NousResearch#32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
al3xar pushed a commit to al3xar/hermes-agent that referenced this pull request Jun 17, 2026
…viking

Generalizes NousResearch#32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…viking

Generalizes NousResearch#32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…viking

Generalizes NousResearch#32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…viking

Generalizes NousResearch#32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…viking

Generalizes NousResearch#32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…viking

Generalizes NousResearch#32663 (@ehz0ah). The slash-skill scaffolding pollution
affected every auto-syncing memory provider — mem0, hindsight, retaindb,
byterover, honcho, supermemory all store/embed the raw user turn, so a
/skill invocation poisoned their stores with the full skill body, not just
openviking.

- Lift the contributor's parser into agent/skill_commands.py as the canonical
  extract_user_instruction_from_skill_message(), co-located with the message
  builders so the markers can't drift.
- Strip once in MemoryManager.{prefetch_all,queue_prefetch_all,sync_all} —
  fixes the whole provider fan-out, bare /skill turns are skipped entirely.
- OpenViking's _derive_openviking_user_text() now delegates to the shared
  helper as defense-in-depth (no duplicated marker literals).
- Marker-drift regression now asserts against the canonical skill_commands
  constants; add manager-level coverage proving every provider gets clean text.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

OpenViking memory provider sends slash-skill scaffolding as search text

3 participants