fix(agent): persist messages by intrinsic marker to stop id() reuse data loss (salvage #50372)

[hashbender]

Salvage of NousResearch#50372 (@rrevenanttt), rebased onto current main.

Issue

_flush_messages_to_session_db deduped persisted messages with a retained {id(msg)} set kept across turns. Once a flushed dict is dropped from the live list (scaffolding rewind / in-place compaction) and GC'd, CPython recycles its address onto a new assistant/tool dict whose id() collides with the stale entry — so the real turn is silently never written to state.db (NousResearch#46053-adjacent data loss).

Fix

Replace the retained id-set with an intrinsic _DB_PERSISTED_MARKER stamped on each dict. The id-set is demoted to a one-shot seed (valid only while the caller's objects are alive), translated to markers, and cleared after every flush — so no id() outlives a flush to alias a future message. The marker is _-prefixed so the wire sanitizers strip it before any request leaves the process. Preserves the existing _is_ephemeral_scaffolding skip landed since the PR was written.

Verification

2 new tests: (a) no id() lingers past a flush + all written dicts carry the marker; (b) a recycled-id collision still persists the real message. Mutation-checked: removing the seed reset fails the retention test. 31 persistence tests pass (identity-flush + sequence-repair).

Closes NousResearch#50372.

Co-authored-by: rrevenanttt 290873280+rrevenanttt@users.noreply.github.com

---

Mirror-of: NousResearch#56284
NousResearch#56284

[tenki-reviewer]

Review Complete

Files Reviewed: 2
Findings: 1

By Severity:

• 🟢 Low: 1

---

PR adds a one-shot seed deduplication mechanism to prevent stale id() values from suppressing new messages across turns, but the seed cleanup is placed inside a try block where an exception can leave stale IDs on the agent object.

Files Reviewed (2 files)

run_agent.py
tests/run_agent/test_identity_flush.py

[tenki-reviewer]

Risk: 🟡 Medium (25/100) — 1 low finding · 162 LOC across 2 files

---

Change Overview

This PR introduces a one-shot seed deduplication mechanism in _flush_messages_to_session_db (run_agent.py) to address a CPython id() reuse hazard. When a message dict is deallocated and its address is recycled for a new message, the old id() value could alias the new message and cause it to be incorrectly stamped as _db_persisted, silently skipping persistence. The fix seeds _flushed_db_message_ids from external callers (tests, gateway shutdown), translates them to durable _db_persisted markers during the flush, then clears the seed to prevent stale ids from surviving across turns.

A corresponding test (tests/run_agent/test_identity_flush.py) validates the seed-clearing invariant.

Key Finding

Exception handler bypasses seed cleanup (run_agent.py lines 1835-1838): The _flushed_db_message_ids reset is inside the try block after the flush loop. If append_message raises a transient exception (e.g., DB lock), control jumps to except Exception which only logs — the stale seed survives. On a subsequent flush, seed_ids reads from the uncleared set and could stamp a colliding new message with _db_persisted, causing silent data loss. The fix: move self._flushed_db_message_ids = set() to immediately after seed_ids is captured (after line 1763), so the seed is consumed before the flush loop begins.

Assessment

Low risk, comment-only. The production impact is minimal — currently only tests populate _flushed_db_message_ids with non-empty values — but the invariant violation makes the code fragile to any future production seeder. The core mechanism (seed → marker translation) is sound and the test correctly validates the critical seed-clearing path.

[tenki-reviewer]

🟢 Exception handler does not clear _flushed_db_message_ids seed, risking stale-id dedup error across retry paths (bug)

In _flush_messages_to_session_db (run_agent.py), the one-shot seed _flushed_db_message_ids is intended to be reset to an empty set after every flush. However, the reset at line 1835 is inside the try block. If an exception occurs during the flush loop — e.g., a transient DB lock timeout on append_message (line 1816) — control jumps to the except at line 1837, which only logs and falls through. The stale seed survives on the agent object. On a subsequent flush call in the same session, _last_flushed_db_idx > 0 and flushed_session_id matches, so seed_ids is read from the uncleared _flushed_db_message_ids. If that stale set contains an id matching a new message (address reuse in CPython), the new message is stamped _db_persisted and silently skipped — data loss. No current production code populates _flushed_db_message_ids with non-empty values (only tests do), but the invariant violation makes the code fragile to any future production seeder.

💡 Suggestion: Clear _flushed_db_message_ids before the message loop (immediately after capturing seed_ids at line 1763) rather than after, so the seed is consumed instantly and cannot survive an exception.

📋 Prompt for AI Agents

In run_agent.py _flush_messages_to_session_db, move the self._flushed_db_message_ids = set() to immediately after line 1763 (after seed_ids = set() in both branches), so the seed is consumed before the flush loop begins. This ensures an exception during the loop cannot leave stale IDs on the agent object. The _last_flushed_db_idx update can stay at line 1836 since it's only needed for successful flushes.