Skip to content

fix(hindsight): replace destructive aretain_batch with safe file-based retain - #55819

Closed
kumaxs wants to merge 1 commit into
NousResearch:mainfrom
kumaxs:fix/hindsight-retain-safe-file-retain
Closed

fix(hindsight): replace destructive aretain_batch with safe file-based retain#55819
kumaxs wants to merge 1 commit into
NousResearch:mainfrom
kumaxs:fix/hindsight-retain-safe-file-retain

Conversation

@kumaxs

@kumaxs kumaxs commented Jun 30, 2026

Copy link
Copy Markdown

Summary

Background Review fork was calling hindsight_retain with the parent session_id, causing destructive overwrites of auto-retained session documents in Hindsight. The tool description was misleading — it advertised "Store information" without disclosing that every retain is a full document overwrite (not an incremental append) and that auto_retain already continuously maintains memories in the background.

Changes

1. plugins/memory/hindsight/__init__.py — Safe file-based retain

hindsight_retain tool now uses Hindsight's file retain API instead of direct aretain_batch. Each call generates a unique filename:

{agent}_{channel}_{user}_{session}_{uuid}.md

The UUID guarantees no collision with auto-retained session documents. File retain also enables richer content (markdown formatting, structured text).

2. plugins/memory/hindsight/__init__.py — Accurate tool description

Old: "Store information to long-term memory. Hindsight automatically extracts structured facts, resolves entities, and indexes for retrieval."

New: "This tool grants one dedicated extra memory record. Hindsight's auto_retain continuously maintains memories in the background — do not use this tool for routine memory retention. All long-term essential core information must be stored in memory.md; saving such content through this tool is prohibited under all circumstances."

3. agent/background_review.py — Guided prompt

The _MEMORY_REVIEW_PROMPT now tells the model to:

  • Write long-term essential core info to memory.md
  • Store important on-demand info to memory provider tools
  • Skip if nothing worth saving

Verification

  • Dry-run: filename generation verified (all components present, sanitization works)
  • Real API test: file retain submitted successfully against live Hindsight instance (operation tracked)
  • Syntax: all modified files pass py_compile
  • Auto-retain (sync_turn) is completely unaffected — it uses the internal aretain_batch API, not the tool

Closes #55816

@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels Jun 30, 2026
…d retain

Background Review fork was calling hindsight_retain with the parent
session_id, causing destructive overwrites of auto-retained session
documents in Hindsight. The tool description was misleading — it
advertised 'Store information' without disclosing the full-overwrite
semantics and that auto_retain already handles routine memory retention.

Changes:
- Replace hindsight_retain's direct aretain_batch with file-based retain
  that generates a unique filename per call:
  {agent}_{channel}_{user}_{session}_{uuid}.md — guaranteed no collision
  with auto-retained session documents.
- Rewrite RETAIN_SCHEMA description to accurately describe the tool as
  'one dedicated extra memory record' and prohibit storing core info
  that belongs in memory.md.
- Update Background Review MEMORY_REVIEW_PROMPT to guide correct tool
  selection: essential core info → memory.md, on-demand info → memory
  provider tools, skip if nothing worth saving.

Closes NousResearch#55816
@kumaxs
kumaxs force-pushed the fix/hindsight-retain-safe-file-retain branch from 277d36d to 9b7c3f2 Compare June 30, 2026 18:38
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for investigating the Hindsight overwrite risk.

Automated hermes-sweeper review found that the reported Background Review route is already isolated on current main:

  • agent/background_review.py:675-689 creates the review fork with skip_memory=True, explicitly preventing external memory-provider ingestion, recall, and sync side effects.
  • plugins/memory/hindsight/__init__.py:1710-1722 does not forward a model-supplied document_id through hindsight_retain; auto-retain supplies its session document identifier separately at :1681-1686.
  • This isolation was implemented by 4ece521bcf37401686e73f1d08ebaa87caaae05a (fix(run_agent): isolate background review fork from external memory plugins (#27190)).

Closing as implemented on main.

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 sweeper:implemented-on-main Sweeper: behavior already present on current main 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.

fix(hindsight): hindsight_retain destructive overwrite, misleading description, and Background Review abuse

3 participants