David/HERMES-GIELIE
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

stage-353: apply Opus SHOULD-FIX — unconditional parent_session_id stamp on compression rotation

Browse source 
Opus identified that PR #2227's preservation block had two related bugs in
the parent_session_id handling:

1. During preservation save: code did
     _old_parent = s.parent_session_id
     s.parent_session_id = None
     s.save(touch_updated_at=False, skip_index=True)
     s.parent_session_id = _old_parent
   The save persisted parent=None to disk. The in-memory restoration didn't
   reach the disk copy. Result: a /branch fork session that subsequently
   compressed lost its 'Forked from X' badge on the preserved old snapshot.

2. Stamping the continuation: code did
     if not s.parent_session_id:
         s.parent_session_id = old_sid
   The 'if not' guard skipped the stamp when the session already had a
   parent_session_id from a prior fork. Result: fork-of-fork compression
   broke lineage — the continuation jumped back to the original fork parent
   instead of the just-preserved immediate predecessor snapshot.

Fix (matches Opus's recommendation):
  - Remove the parent clearing during preservation save (preserve as-is)
  - Drop the 'if not' guard; always stamp continuation to old_sid

This makes the lineage chain consistent: new → old → old.parent → ... root.
Traversal from the continuation always walks through the just-preserved
snapshot to get to its parent's parent, never jumping over the snapshot.

Two new regression tests pin both invariants:
  - test_parent_session_id_stamped_unconditionally (no 'if not' guard)
  - test_old_session_parent_preserved_during_archive_save (no parent=None)

Both pass against the fix. All 8 tests in the file pass.
This commit is contained in:
Hermes Agent 2026-05-14 03:59:02 +00:00
parent ccfa3b2081
commit 3d34a72ee8
2 changed files with 52 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
api/streaming.py
View file

@ -3609,18 +3609,19 @@ def _run_agent_streaming(
existing_msgs = -1
if len(s.messages) > existing_msgs:
# In-memory messages are newer than the file — save.
# Preserve s.parent_session_id as-is (do NOT clear it):
# the OLD session's parent_session_id is its own
# pre-existing lineage (e.g. a /branch fork's
# parent_session_id back to the original). We must
# not overwrite that with None on disk. Stage-353
# Opus SHOULD-FIX: previous code cleared parent
# before save then restored after — but the on-disk
# copy persisted with parent=None, breaking
# fork-of-fork lineage traversal. (#2227 + #2223)
saved_sid = s.session_id
s.session_id = old_sid
try:
# Temporarily restore old sid so save()
# writes to old_path. Also stamp a
# parent_session_id linking back so lineage
# can be traced from the OLD session forward
# to the continuation session.
_old_parent = s.parent_session_id
s.parent_session_id = None
s.save(touch_updated_at=False, skip_index=True)
s.parent_session_id = _old_parent
logger.info(
"Preserved pre-compression session %s (%d messages) to disk",
old_sid, len(s.messages),
@ -3631,10 +3632,16 @@ def _run_agent_streaming(
s.session_id = saved_sid
except OSError:
logger.debug("Could not read old session file before preservation")
# Set parent_session_id on the continuation session so the
# frontend can traverse the lineage chain (old → new).
if not s.parent_session_id:
s.parent_session_id = old_sid
# Always link the continuation session to its immediate predecessor
# (the preserved snapshot). This OVERRIDES any prior
# parent_session_id because the new continuation IS the next link
# in the chain: traversal walks new → old → old.parent → ... root.
# Stage-353 Opus SHOULD-FIX: previous `if not s.parent_session_id`
# guard skipped this stamp on fork-of-fork compressions, so a
# subsequent traversal from the new continuation would jump
# over the just-preserved snapshot back to the original fork
# parent, losing access to the recoverable history in old_sid.json.
s.parent_session_id = old_sid
with LOCK:
if old_sid in SESSIONS:
SESSIONS[new_sid] = SESSIONS.pop(old_sid)

33
tests/test_issue2223_compression_no_rename.py
View file

@ -48,6 +48,39 @@ class TestNoRenameDuringCompression:
)
def test_parent_session_id_stamped_unconditionally(self):
"""Stage-353 Opus SHOULD-FIX: the continuation session's parent_session_id
must be stamped UNCONDITIONALLY to old_sid (the immediate predecessor).
The previous `if not s.parent_session_id` guard skipped the stamp on
fork-of-fork compressions (i.e. when the session already had a
parent_session_id from a prior /branch operation), so the continuation
would jump back to the original fork instead of the just-preserved
snapshot, losing access to the recoverable history in old_sid.json.
The fix removes the guard: continuation ALWAYS points to the preserved
snapshot. Traversal then walks new → old → old.parent → ... root.
"""
# The guarded form is the bug; the unconditional form is the fix.
assert "if not s.parent_session_id:\n s.parent_session_id = old_sid" not in streaming_src, (
"Guarded parent_session_id stamping resurfaced — breaks fork-of-fork "
"lineage traversal after compression"
)
def test_old_session_parent_preserved_during_archive_save(self):
"""Stage-353 Opus SHOULD-FIX: when preserving old_sid.json to disk, the
OLD session's parent_session_id must NOT be cleared.
Previous bug: code did `s.parent_session_id = None; s.save(); s.parent_session_id = _old_parent`.
The save persisted parent=None to disk; in-memory restoration didn't help.
Result: fork lineage badge ("Forked from X") disappeared on the old snapshot.
"""
# The clearing pattern must be gone.
assert "s.parent_session_id = None" not in streaming_src, (
"Clearing parent_session_id before preservation save resurfaced — "
"breaks fork lineage on the old snapshot"
)
class TestMergePreservesHistory:
"""_merge_display_messages_after_agent_result must preserve all previous
display messages when compression returns only a marker."""