From 9e4fe32d36fc84dd86f4d326d9de4db1e82739c6 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 21 Jun 2026 11:16:42 -0700 Subject: [PATCH] fix(session): opt the background-review fork out of session finalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The background-review fork (fires ~every 10 turns) pins review_agent.session_id = agent.session_id — the parent's LIVE id — for prefix-cache parity, then calls close(). With session finalization now in close(), that would end the still-active parent session mid-conversation. Set _end_session_on_close = False on the fork so the real owner (CLI close / gateway reset / cron) finalizes the session instead. Follow-up to the #12029 fix. --- agent/background_review.py | 7 ++++ tests/run_agent/test_background_review.py | 44 +++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/agent/background_review.py b/agent/background_review.py index c809b496065..fa4de508e19 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -575,6 +575,13 @@ def _run_review_in_thread( # if a future code path bypasses the cache. review_agent.session_start = agent.session_start review_agent.session_id = agent.session_id + # The fork shares the parent's live session_id (pinned above for + # prefix-cache parity). It is single-lifecycle and calls close() + # right after this run_conversation(); without opting out, close() + # would finalize the parent's still-active session row mid + # conversation (the review fires every ~10 turns). Leave session + # finalization to the real owner (CLI close / gateway reset / cron). + review_agent._end_session_on_close = False # Never let the review fork compress. It shares the parent's # session_id, so if it won a compression race it would rotate the # parent into a NEW child that the gateway never adopts (the fork diff --git a/tests/run_agent/test_background_review.py b/tests/run_agent/test_background_review.py index 8bce7e1507b..1198f4abe7f 100644 --- a/tests/run_agent/test_background_review.py +++ b/tests/run_agent/test_background_review.py @@ -76,6 +76,50 @@ def test_background_review_shuts_down_memory_provider_before_close(monkeypatch): ] +def test_background_review_fork_opts_out_of_session_finalization(monkeypatch): + """The review fork shares the parent's live session_id, so it must set + ``_end_session_on_close = False``. Otherwise close() (now finalizing owned + session rows) would end the still-active parent session mid-conversation + every time the review fires (~every 10 turns). Regression for #12029. + """ + seen = {} + + class FakeReviewAgent: + def __init__(self, **kwargs): + self._session_messages = [] + # Default matches AIAgent.__init__ (agent_init.py): owns its row. + self._end_session_on_close = True + + def __setattr__(self, name, value): + object.__setattr__(self, name, value) + if name == "_end_session_on_close": + seen["end_session_on_close"] = value + + def run_conversation(self, **kwargs): + # By the time the fork runs, the opt-out must already be applied. + seen["at_run_time"] = self._end_session_on_close + + def shutdown_memory_provider(self): + pass + + def close(self): + pass + + monkeypatch.setattr(run_agent_module, "AIAgent", FakeReviewAgent) + monkeypatch.setattr(run_agent_module.threading, "Thread", ImmediateThread) + + agent = _bare_agent() + + AIAgent._spawn_background_review( + agent, + messages_snapshot=[{"role": "user", "content": "hello"}], + review_memory=True, + ) + + assert seen.get("end_session_on_close") is False + assert seen.get("at_run_time") is False + + def test_background_review_summarizer_receives_captured_messages_after_close(monkeypatch): """The action summarizer must see review messages even after close cleanup.