mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-24 21:04:48 -06:00
fix(session): flag persisted reminders delivered on resume
Persisted ``_reminders`` survive ``load_messages`` but the in-memory
``_reminders_delivered`` flag does not (it's session-scoped — set by
``_mark_reminders_delivered`` after each successful provider stream,
never persisted alongside the JSON column). Without a re-splice
guard at resume time, ``_apply_reminders_for_provider`` would walk
every loaded message, see ``_reminders`` set + the flag falsy, and
splice every historical ``<system-reminder>`` envelope onto the wire
on the very next user turn — leaking each reminder a second time, the
turn after it had already advised.
Mirror the post-stream hook in ``resume()``: every loaded message
that carries reminders has already been delivered (it survived to
disk), so flag it accordingly so ``_apply_reminders_for_provider``
short-circuits on the pass-through path.
Test pins the contract end-to-end — stage a workstream with a
persisted reminder, resume into a fresh session, append a live user
turn, run the wire transform, and assert the historical reminder
body does NOT land in the rendered output.
(cherry picked from commit f1466ca7e3)
This commit is contained in:
@@ -3311,6 +3311,57 @@ class TestReminderSidechannelIsolation:
|
||||
assert extracted_user == "first message body"
|
||||
assert "SECRET_NUDGE_TEXT" not in extracted_user
|
||||
|
||||
def test_resume_does_not_re_splice_persisted_reminders(self, tmp_db):
|
||||
"""Persisted reminders survive ``load_messages`` but the
|
||||
in-memory ``_reminders_delivered`` flag does not (it's
|
||||
session-scoped — the post-stream hook flips it after each
|
||||
successful provider call, and persistence skips
|
||||
leading-underscore siblings). Without a re-splice guard at
|
||||
resume time, ``_apply_reminders_for_provider`` would walk every
|
||||
loaded message, see ``_reminders`` set + ``_reminders_delivered``
|
||||
falsy, and splice every historical ``<system-reminder>`` envelope
|
||||
onto the wire on the very next user turn — leaking each
|
||||
reminder a second time, the turn after it had already advised.
|
||||
"""
|
||||
from turnstone.core.memory import register_workstream, save_message
|
||||
|
||||
# Stage a workstream with a persisted user-channel reminder.
|
||||
# Direct save_message so we control the exact reminders payload
|
||||
# without driving a real send().
|
||||
register_workstream("resume_no_resplice")
|
||||
save_message("resume_no_resplice", "user", "first turn", source="system_nudge")
|
||||
save_message(
|
||||
"resume_no_resplice",
|
||||
"user",
|
||||
"second turn",
|
||||
reminders=json.dumps(
|
||||
[{"type": "denial", "text": "HISTORICAL_REMINDER_BODY"}],
|
||||
separators=(",", ":"),
|
||||
),
|
||||
)
|
||||
save_message("resume_no_resplice", "assistant", "ok")
|
||||
|
||||
# Resume into a fresh session.
|
||||
session = _make_session()
|
||||
assert session.resume("resume_no_resplice") is True
|
||||
|
||||
# Sanity: the historical reminder is on the loaded message dict.
|
||||
loaded_user = next(
|
||||
m for m in session.messages if m.get("role") == "user" and m.get("_reminders")
|
||||
)
|
||||
assert loaded_user["_reminders"] == [{"type": "denial", "text": "HISTORICAL_REMINDER_BODY"}]
|
||||
|
||||
# Append a new live user turn (no reminders) and run the wire
|
||||
# transform. The output must NOT carry the historical reminder
|
||||
# body — every loaded message that had _reminders should already
|
||||
# be flagged delivered, so _apply_reminders_for_provider skips
|
||||
# them on the pass-through path.
|
||||
session.messages.append({"role": "user", "content": "live new turn"})
|
||||
wire = session._apply_reminders_for_provider(session.messages)
|
||||
rendered = "\n".join(m["content"] for m in wire if isinstance(m.get("content"), str))
|
||||
assert "HISTORICAL_REMINDER_BODY" not in rendered
|
||||
assert "<system-reminder>" not in rendered
|
||||
|
||||
|
||||
class TestSessionUIBaseUserReminderHook:
|
||||
"""``on_user_reminder`` enqueues a ``user_reminder`` SSE event with
|
||||
|
||||
@@ -1728,6 +1728,21 @@ class ChatSession:
|
||||
if not fork:
|
||||
self._ws_id = ws_id
|
||||
self.messages = messages
|
||||
# Persisted reminders ride the ``_reminders`` side-channel but
|
||||
# carry no in-memory ``_reminders_delivered`` flag (the flag is
|
||||
# session-scoped — set by ``_mark_reminders_delivered`` after
|
||||
# each successful provider stream, never persisted). Without
|
||||
# this re-splice guard, the very next user turn after resume
|
||||
# would walk the loaded history and re-render every historical
|
||||
# ``<system-reminder>`` block onto the wire — leaking each one a
|
||||
# second time, the turn after it had already advised. Mirror
|
||||
# the post-stream hook here: every loaded message that carries
|
||||
# reminders has already been delivered (it survived to disk),
|
||||
# so flag it accordingly so ``_apply_reminders_for_provider``
|
||||
# short-circuits on the pass-through path.
|
||||
for msg in self.messages:
|
||||
if msg.get("_reminders"):
|
||||
msg["_reminders_delivered"] = True
|
||||
self._read_files.clear()
|
||||
self._repeat_detector.clear()
|
||||
self._last_usage = None
|
||||
|
||||
Reference in New Issue
Block a user