Skip to content

feat(memory): Detect external DB/file drift and refuse mutations that would cause silent data loss #3021

@hamza-jeddad

Description

@hamza-jeddad

Background

Sub-issue of #3011.

Memory entries can be modified outside the memory tool — by a patch tool, a direct SQL client, a manual edit, or a concurrent agent session. If the in-process write path in pkg/tools/builtin/memory/ blindly overwrites the DB without checking for concurrent changes, it can silently discard externally-added content.

The reference implementation solves this with a drift detection guard: before any mutating write, re-read the current on-disk state and compare it to what the tool last saw. On a mismatch, refuse the write, save a timestamped backup of the current on-disk state, and return a structured error that tells the operator exactly what happened and how to recover.

Proposed design

1. What "drift" means for SQLite

For a SQLite-backed store, drift manifests as:

  • Row added externally: a row exists in the DB that was not in the tool's in-process cache when the write started.
  • Row modified externally: a row's content, category, or tier was changed since the tool last read it.
  • Row deleted externally: a row the tool was about to update or delete no longer exists.

SQLite provides rowid and column values for cheap comparison. A generation counter (monotonically incrementing integer in a meta table) is the lightest option:

CREATE TABLE IF NOT EXISTS memory_meta (
    key   TEXT PRIMARY KEY,
    value TEXT NOT NULL
);
INSERT OR IGNORE INTO memory_meta VALUES ('generation', '0');

Every mutating write (insert / update / delete from any source) bumps generation via a trigger. The in-process cache stores the generation number it last saw. Before any write, re-read generation; if it differs, drift has occurred.

2. Backup on drift

When drift is detected:

  1. Export the full current state of user_memories to a timestamped JSON file: <data_dir>/memory.bak.<unix_ts>.json.
  2. Log a warning with the backup path.
  3. Return a structured error — do not perform the write.
{
  "success": false,
  "error": "Refusing to write: memory DB was modified externally since last read (generation 4 → 7). A snapshot was saved to /path/to/memory.bak.1749123456.json. Resolve the drift first — review the backup, reconcile via add_memory/update_memory, then retry.",
  "drift_backup": "/path/to/memory.bak.1749123456.json",
  "remediation": "Open the .bak file, integrate missing entries via add_memory one at a time, then retry your original operation."
}

3. Snapshot format

{
  "timestamp": "2025-06-07T18:00:00Z",
  "generation_observed": 4,
  "generation_actual": 7,
  "entries": [
    { "id": "abc123", "content": "", "category": "preference", "tier": "memory", "created_at": "" }
  ]
}

4. Generation bump trigger

CREATE TRIGGER IF NOT EXISTS bump_generation_insert
    AFTER INSERT ON user_memories
BEGIN
    UPDATE memory_meta SET value = CAST(CAST(value AS INTEGER) + 1 AS TEXT)
    WHERE key = 'generation';
END;

-- same for UPDATE and DELETE triggers

5. Cache invalidation on detected drift

After emitting the drift error, reset the in-process cache to the current DB state so the next read is clean. This composes with the snapshot invalidation logic in #3017.

Implementation checklist

  • pkg/memory/database/sqlite/ — add memory_meta table + migration; add generation-bump triggers for INSERT, UPDATE, DELETE on user_memories
  • pkg/memory/database/ReadGeneration() (int64, error) helper
  • pkg/tools/builtin/memory/ — store last-seen generation in the tool's runtime state; check before every mutating write; on mismatch: export backup JSON, reset cache, return structured drift error
  • Backup writer: pkg/memory/database/backup.goExportSnapshot(db, path) error
  • Unit tests: simulate concurrent write via direct SQL; assert tool detects drift, backup file created with correct content, error returned, subsequent read reflects actual DB state
  • Integration test: two concurrent goroutines writing memories; assert no silent data loss (at least one write is rejected with drift error rather than overwriting the other silently)
  • go test -race passes on the memory package

Acceptance criteria

  • A write that would overwrite externally-added content is refused with a structured drift error
  • A .bak.<unix_ts>.json file is created containing the full DB state at the time of drift detection
  • The error message names the backup file path and gives remediation steps
  • The in-process cache is reset to actual DB state after drift detection
  • Writes that find no drift proceed normally with no observable overhead
  • ≥80% line coverage on the drift-detection code paths

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/agentFor work that has to do with the general agent loop/agentic features of the apparea/securityAuthentication, authorization, secrets, vulnerabilitiesarea/toolsFor features/issues/fixes related to the usage of built-in and MCP tools
    No fields configured for Enhancement.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions