Skip to content

fix(gateway): preserve archived compaction history on /retry rewrite (#80216) - #80695

Closed
RelaxJonh wants to merge 1 commit into
NousResearch:mainfrom
RelaxJonh:fix/retry-preserves-archived-history-80216
Closed

fix(gateway): preserve archived compaction history on /retry rewrite (#80216)#80695
RelaxJonh wants to merge 1 commit into
NousResearch:mainfrom
RelaxJonh:fix/retry-preserves-archived-history-80216

Conversation

@RelaxJonh

Copy link
Copy Markdown
Contributor

Summary

rewrite_transcript() called replace_messages() with the default active_only=False, which DELETEd every row for the session — including soft-archived (active=0) compaction turns that in-place compaction deliberately keeps on disk for searchable history.

After any /retry on a compacted session, the archived pre-compaction history was gone permanently: hard ID gaps, empty archive queries, missing in_place_committed history. Nothing was logged.

Root Cause

gateway/slash_commands.py:2587 (_handle_retry_command):

truncated = history[:last_user_idx]
await self.async_session_store.rewrite_transcript(session_entry.session_id, truncated)

gateway/session.py rewrite_transcript then called:

self._db.replace_messages(session_id, messages)

with active_only defaulting to False, so the DELETE had no active clause and took the archived rows with it.

Fix

Probe has_archived_messages() before the rewrite and pass active_only=True when archives exist, mirroring the ACP adapter's _persist and the TUI prompt.submit path after #80195.

Testing

  • Verified the code compiles and the has_archived_messages method exists on SessionDB (line 7130 of hermes_state.py)
  • The fix is a minimal, surgical change: 1 file, 7 lines added, 1 removed
  • No behavioral change when no archived rows exist (the probe returns False, so active_only=False — same as before)

Fixes #80216

rewrite_transcript() called replace_messages() with the default
active_only=False, which DELETEd every row for the session — including
soft-archived (active=0) compaction turns that in-place compaction
deliberately keeps on disk for searchable history.

After any /retry on a compacted session, the archived pre-compaction
history was gone permanently: hard ID gaps, empty archive queries,
missing in_place_committed history. Nothing was logged.

Fix: probe has_archived_messages() before the rewrite and pass
active_only=True when archives exist, mirroring the ACP adapter's
_persist and the TUI prompt.submit path after NousResearch#80195.

Fixes NousResearch#80216
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter platform/discord Discord bot adapter platform/slack Slack app adapter platform/whatsapp WhatsApp Business adapter sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 7, 2026
@Adolanium

Copy link
Copy Markdown
Contributor

#80218 already fixes #80216 with a regression test and is linked from the issue. One design note for reviewers: this PR moves the probe inside rewrite_transcript, which changes behavior for every caller of the shared method (yuanbao recall, compress rotation). That auto-preserve shape was rejected in #57803 in favor of per-caller guards, and the probe variant has the fail-open/TOCTOU failure mode already discussed and fixed on #80218.

Also worth saying gently: with ~19.5k open PRs, every duplicate of an already-claimed issue costs real maintainer triage time. The PR template asks contributors to search existing PRs before opening one, and this issue has had a linked fix PR since before this one was created.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as a duplicate of #80218, which fixes the same bug (#80216) and was filed earlier by the issue reporter.

Both PRs correctly identified the root cause — rewrite_transcript() calling replace_messages() with the destructive default deletes soft-archived compaction rows on /retry. We went with #80218's approach for three reasons:

  1. It passes active_only=True unconditionally at the /retry call site rather than probing has_archived_messages() at rewrite time — the probe can race with a concurrent archive_and_compact(), and if it raises, the whole rewrite silently fails.
  2. The probe approach changes semantics for ALL rewrite_transcript callers, including yuanbao recall redaction, which intentionally purges archived rows (privacy fail-safe) — that caller must keep the destructive default.
  3. fix(gateway): preserve archived compaction history on /retry #80218 ships an end-to-end regression test against a real state.db.

Thanks for the correct diagnosis and the fix — appreciated even though the other PR won the tiebreak!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround platform/discord Discord bot adapter platform/slack Slack app adapter platform/telegram Telegram bot adapter platform/whatsapp WhatsApp Business adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: /retry permanently deletes archived compaction history on messaging platforms

4 participants