Skip to content

fix(gateway): preserve transcript spool semantics - #129835

Open
Froraut wants to merge 1 commit into
NousResearch:mainfrom
Froraut:fix/shutdown-spool-transcript-codec-20261001
Open

Froraut wants to merge 1 commit into
NousResearch:mainfrom
Froraut:fix/shutdown-spool-transcript-codec-20261001

Conversation

@Froraut

@Froraut Froraut commented Oct 1, 2026

Copy link
Copy Markdown

Summary

Preserve full transcript message semantics when shutdown recovery spools are the only durable copy.

  • Introduce a versioned transcript codec for role/content plus tool calls, tool results, reasoning metadata, attachments, usage fields, timestamps, and durable message identity.
  • Use the same codec for shutdown history snapshots and cap-dropped transcript payloads.
  • Recover related transcript files in deterministic order and batch them atomically per session.
  • Keep invalid/incomplete payloads on disk instead of partially replaying and deleting them.
  • Retain compatibility with legacy text-only spool payloads.
  • Deduplicate message identities within recovered batches.

Verification

  • tests/gateway/test_session_messages_shutdown_preserve.py
  • tests/gateway/test_shutdown_flush.py
  • tests/gateway/test_pending_queue_spool.py

Result: 21 passed, 0 failed.

git diff --check passed.

Related work

#122718 fixes spool replay/drop ordering. This draft addresses the separate lossy message representation used by shutdown-history and cap-drop recovery; the two changes may need conflict resolution if both land.

Risk

The codec is versioned and legacy-compatible, but recovery is a data-integrity path. This remains a draft for broader CI and maintainer review.

Base: 663362680b6ffa4fbffeb58f6682564239a1953b

Co-authored-by: FroRaut 79215537+Froraut@users.noreply.github.com

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Oct 1, 2026
@Froraut
Froraut marked this pull request as ready for review October 1, 2026 23:21
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Batching the spool files per session is the right idea, and the UIDs make the idempotency
work. One precondition of the batch writer isn't met by the merge, though, and it turns a
recoverable situation into a total loss.

append_recovered_messages_batch documents itself as replaying one versioned envelope
(hermes_state_messages.py:485) and rejects a repeated message_uid outright
(:500-503). shutdown_flush.py:218-220 concatenates every spool file for the session into
a single call, so the input is no longer one envelope.

A cap-dropped file and an agent-history snapshot can hold the same live message: the
cap-dropped dict came out of _dirty_transcripts (session_transcript.py:139) and the
shutdown snapshot is _session_messages (run_shutdown.py:1172), and a message that was
enqueued and later persisted can sit in both. message_uid is deliberately not derived
from content, timestamp or tool-call ids (message_metadata.py:77-78) — it is minted once
and copied, which is exactly what makes it stable across those two snapshots. Same uid,
twice, in one batch.

Measured on the same two files, base vs head:

recovered files left
base 1 (already in db) pending-hist.json
head 0 both preserved

The batch that arrived was [UID-SHARED, UID-LOST, UID-SHARED, UID-LOST], where UID-LOST
is a message that exists only on disk — the exact data this recovery path exists to
save. Because the write is all-or-nothing, one duplicate costs the whole session, and every
subsequent boot repeats the failure on the same files.

So base recovered the messages it could; head recovers nothing. Worth noting the guard is
right to exist — it is the caller that must not hand it two envelopes at once. Deduping
merged messages by message_uid before the call keeps the crash-after-commit idempotency
the batching was for, and is a couple of lines at the merge point; alternatively the writer
could tolerate a repeated uid by keeping the first occurrence rather than raising, since
the later existing_uids filter already collapses anything genuinely in the DB.

Not asking to drop the batching — the atomic assistant/tool adjacency it buys is real, and
the (ts, seq, filename) ordering is a genuine improvement over sorting on a uuid4 name.

This branch has not been deployed

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

Labels

area/sessions Session lifecycle, resume, persistence, history comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists 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.

3 participants