fix(agent): persist pre-flushed user context by exact row identity

[itsflownium]

What does this PR do?

Fixes #102194. When a CLI user turn is persisted before injected context is composed, reopening the session now replays the exact context sent to the provider. Repeated prompts such as ok cannot cause the previous turn's sidecar to be overwritten.

Changes Made

• Adapt fix(agent): row-addressed api_content backfill for pre-persisted user turns (#102194) #102411 by @JoaoMarcos44 to the current module layout, preserving the original commit author.
• Place the row-addressed store method in hermes_state_messages.py, resolving the predecessor's oversized-state-file blocker.
• Backfill known row identities in the turn-context helper; retain the existing in-place compaction fallback.
• Replace the predecessor's test suite with two behavioral tests, including a real database close/reopen and repeated-content protection.

How to Test

scripts/run_tests.sh tests/agent/test_api_content_row_addressed_backfill.py tests/agent/test_api_content_sidecar.py tests/agent/test_turn_context.py

macOS: 50 tests passed. The early-persistence regression fails on current main before the fix; the compaction and unpersisted-turn controls pass. Ruff and diff whitespace checks pass. The full repository suite was not run locally.

This is an adaptation of the existing fix onto the decomposed codebase; it should not land alongside an independent copy of #102411.

Coverage also exercises the real session flush that stamps the committed row ID onto the live message dictionary, before context is attached. Store-level assertions cover session/content mismatches, archived and non-user rows, invalid row IDs, and lone-surrogate handling without changing other rows.

[alt-glitch]

This was generated by AI during triage.

Related: same row-addressed api_content backfill mechanism as open #102411 (which this adapts, with @JoaoMarcos44's authorship kept), both fixing #102194. Delta here: the store method lives in hermes_state_messages.py (author says this clears the oversized-file blocker on #102411) and the unrelated context_compressor change from #102411 is dropped. Only one of the two should land; maintainer's call.

[Sahilvishnaliya]

Independent verification (Windows/py3.12, this PR's head)

Tests:

• tests/agent/test_api_content_row_addressed_backfill.py — 3/3 pass
• Broader -k "api_content or sidecar or turn_context" over tests/agent/ — 82 passed, 5 skipped, 0 failures

Design audit — this is the correct fix for #102194, and correctly targeted:

• The _row_id-or-in_place arming gate matches the invariant that matters: a positional backfill is only safe when the caller can prove the newest active user row belongs to this turn. Without _row_id, the stamp must not run — a repeated "ok"/"y"/"continue" turn makes the previous turn's row compare equal on content, and the positional update would overwrite that turn's sidecar with this turn's bytes (durable wrong-bytes replay, worse than the missing sidecar).
• set_message_api_content guards: row_id int-not-bool/positive, session match, role='user', active=1, content IS ? — verified by the parametrized test including the compacted variant via real archive_and_compact.
• Placement is right for current main: the method goes into SessionMessagesMixin (hermes_state_messages.py) beside set_latest_user_api_content, which is where it must live post-split. (The older #102411 base predates the split and patches the pre-split monolith — see my note there; this PR is currently the merge-clean one.)

Two suggestions, neither blocking:

1. Carry over #102411's guard tests. The store-method edge cases — wrong session, wrong content, unknown row id, archived (active=0) row, boolean row id, lone-surrogate write — are all covered by #102411's 281-line suite but not by this PR's two tests. Same design, same method name; those tests would apply almost verbatim and pin the guards against future regressions.
2. Early-flush path proof. Like #102411, the arming tests set _row_id manually. A test that drives the actual early-flush/close path (or a pointer to the sync_flushed_message_markers line that stamps _row_id onto the live dict in that race) would close the loop on the CLI-race half of the issue that neither candidate currently exercises end-to-end.

The SimpleNamespace-based _stamp_api_content_sidecar test approach is a nice lightweight harness — testing the stamp function directly avoids the heavyweight _build(agent) fixture.

[ehz0ah]

Reviewed the exact head and reapplied it cleanly to current upstream main. The focused PR suite passed 50 tests. The current-main compatibility suite passed 71 tests before the boundary probe. One verified blocking case remains, described inline.

[ehz0ah]

[P1] Preserve the durable clean-content key for pre-flushed API-only CLI turns

The CLI stages clean content, and close persistence can write that row before the worker starts. The worker then reuses the same dict but can replace its content with a voice prefix or model-switch note while _persist_user_message_override retains the clean value. This call compares the row against the API-only content, so set_message_api_content updates zero rows and leaves api_content NULL. With no memory or plugin injection, line 704 returns even earlier, so the API-only prefix is also lost. The next turn then replays different bytes and still breaks exact prefix parity.

I reproduced both paths through the real _flush_messages_to_session_db: persist the clean staged row first, then reuse the stamped dict with API-only content. Current head failed both probes. Derive the guarded row content with the same _override_replaces_content rule used by _db_flush_row, and retain differing API-only content as the sidecar even when composition returns None. Both probes and 85 adjacent tests passed with that narrow correction.

[kshitijk4poor]

Addressed in #105842: the sidecar stamp derives the durable row content through the same persist-override rule the flush uses (durable_user_row_content, shared by _db_flush_row and _stamp_api_content_sidecar), matches the row on those bytes, and stamps the live API-only bytes even when no memory/plugin context was injected. Covered by test_pre_flushed_api_only_turn_without_injections_preserves_sidecar through the real flush path.

[salch-cred]

Checked this PR's diff against the same close-flush-then-restore race @ehz0ah flagged on #103721 (duplicate-labeled against this PR) — this PR still has it.

This PR's _stamp_api_content_sidecar (and the new set_message_api_content) match/update the durable row using _turn_user_msg.get("content") — i.e. whatever is currently on the live turn dict at stamp time. But it doesn't touch _stage_turn_user_message, which is where the same dict can get its content overwritten: when a close/early flush has already persisted a CLI-staged dict's clean text (e.g. "hello") to a durable row (stamping _row_id on that dict), _stage_turn_user_message then restores the API-facing value (e.g. "[voice] hello") onto that same dict, with nothing recording what was actually written to disk.

So at stamp time, _turn_user_msg.get("content") is the restored/live value, not the persisted one — and:

• set_message_api_content(... WHERE ... content IS ?) guards on the live value, so it matches zero rows against the durable row that still holds the old clean text → silent no-op update.
• If there's no injected context this turn, compose_user_api_content returns None and the function returns before even reaching the backfill, so the stale row is never corrected either.

This is the same failure mode as #103721; I pushed a fix there (commit 7cf68e0 on fix/api-content-row-addressed-backfill-102194) that has _stage_turn_user_message stash the pre-overwrite persisted bytes as _turn_user_msg["_persisted_content"], and has _stamp_api_content_sidecar match/backfill against those stashed bytes instead of live content (falling back to live content as api_content when there's no injection but the persisted bytes are stale).

Flagging here since this PR is currently positioned as the canonical fix for #102194 — worth incorporating the same _stage_turn_user_message change before this lands, or the row-addressed backfill will still silently fail in that race.

[kshitijk4poor]

Thanks @itsflownium — landing this via #105842: the same row-addressed design (you adapted #102411 first onto the split layout — thank you). #105842 carries @JoaoMarcos44's original commit with the API-only pre-flushed turn and the _row_id/persist-lock race both closed; your carrier still had the API-only gap @ehz0ah found inline.

Closing this one so there is a single carrier for the issue; your credit is in the salvage PR body and in git history.