fix(agent): persist_session mutates copy, not live messages (#48677) - #48817
Closed
kyssta-exe wants to merge 1 commit into
Closed
fix(agent): persist_session mutates copy, not live messages (#48677)#48817kyssta-exe wants to merge 1 commit into
kyssta-exe wants to merge 1 commit into
Conversation
alt-glitch
added
type/bug
Collaborator
|
Duplicate of #48694 — both fix #48677 (early crash-resilience persist mutating the live |
teknium1
added
sweeper:risk-session-state
Contributor
|
Thanks for the careful diagnosis here, Kyssta — the writeup on #48677 was genuinely precise (the early crash-resilience persist mutating the shared We're closing this rather than salvaging it, though. The fix as written also conflicts with the current More fundamentally, we don't want to patch the observed-context delivery path here at all. It's built on the persist-override mechanism (send-wrapped-to-API, save-bare), and that mechanism is fighting itself in this seam. We'd rather rethink that delivery than bolt copy-on-persist onto it. Closing both this and #48677. Your authorship stays in the record via this PR. Appreciate the work. |
kshitijk4poor
pushed a commit
to kshitijk4poor/hermes-agent
that referenced
this pull request
Jul 1, 2026
… list (NousResearch#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in NousResearch#56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from NousResearch#48817; chokepoint reworked to coexist with the marker-based dedup (NousResearch#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
kshitijk4poor
pushed a commit
that referenced
this pull request
Jul 1, 2026
… list (#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in #56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from #48817; chokepoint reworked to coexist with the marker-based dedup (#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
waefrebeorn
pushed a commit
to waefrebeorn/slermes
that referenced
this pull request
Jul 2, 2026
… list (NousResearch#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in NousResearch#56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from NousResearch#48817; chokepoint reworked to coexist with the marker-based dedup (NousResearch#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
Jasper6439
pushed a commit
to Jasper6439/hermes-agent
that referenced
this pull request
Jul 5, 2026
… list (NousResearch#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in NousResearch#56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from NousResearch#48817; chokepoint reworked to coexist with the marker-based dedup (NousResearch#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
habarmc1223-sudo
pushed a commit
to habarmc1223-sudo/hermes-agent-fluxmem
that referenced
this pull request
Jul 8, 2026
… list (NousResearch#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in NousResearch#56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from NousResearch#48817; chokepoint reworked to coexist with the marker-based dedup (NousResearch#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
santhreal
pushed a commit
to santhreal/hermes-agent
that referenced
this pull request
Jul 13, 2026
… list (NousResearch#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in NousResearch#56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from NousResearch#48817; chokepoint reworked to coexist with the marker-based dedup (NousResearch#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
… list (NousResearch#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in NousResearch#56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from NousResearch#48817; chokepoint reworked to coexist with the marker-based dedup (NousResearch#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
leewenjie
pushed a commit
to leewenjie/hermes-agent
that referenced
this pull request
Aug 7, 2026
… list (NousResearch#48677) The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in NousResearch#56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from NousResearch#48817; chokepoint reworked to coexist with the marker-based dedup (NousResearch#50372). Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #48677. The early-crash-resilience persist in build_turn_context called _apply_persist_user_message_override on the live messages list, stripping observed group-chat context before the API call was built. Now _persist_session works on a shallow copy so the override only affects persistence, not the API request.