From fcd5bbb3fbc161256a0cffdc47a04d7c1743b3d5 Mon Sep 17 00:00:00 2001
From: konsisumer <der@konsi.org>
Date: Fri, 21 Aug 2026 14:57:08 +0200
Subject: [PATCH] fix(agent): preserve foreground resources after background
 review

---
 agent/background_review.py                | 31 +++++++++--------------
 tests/run_agent/test_background_review.py | 19 +++++++++-----
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/agent/background_review.py b/agent/background_review.py
index 79848f479e..97795fd50f 100644
--- a/agent/background_review.py
+++ b/agent/background_review.py
@@ -1660,16 +1660,14 @@ def _run_review_in_thread(
             # summary still needs the completed review agent's tool results.
             review_messages = list(getattr(review_agent, "_session_messages", []))
 
-            # Tear down memory providers while stdout is still
-            # redirected so background thread teardown (Honcho flush,
-            # Hindsight sync, etc.) stays silent.  The finally block
-            # below is a safety net for the exception path.
+            # The fork shares the foreground session ID for prompt-cache
+            # parity.  Do not call close() or shutdown_memory_provider():
+            # both are session-bound lifecycle operations, and close() also
+            # kills registered terminal processes and cleans environments for
+            # that ID.  Releasing only this fork's clients leaves the live
+            # session and its child processes untouched.
             try:
-                review_agent.shutdown_memory_provider()
-            except Exception:
-                pass
-            try:
-                review_agent.close()
+                review_agent.release_clients()
             except Exception:
                 pass
             review_agent = None
@@ -1728,11 +1726,10 @@ def _run_review_in_thread(
             _log_review_completion(review_usage, "error")
         agent._emit_auxiliary_failure("background review", e)
     finally:
-        # Safety-net cleanup for the exception path.  Normal completion already
-        # shut down inside the thread-scoped silence above.  Re-enter the
-        # thread-scoped silence here so teardown output (Honcho flush, Hindsight
-        # sync, background thread joins) stays quiet even on the exception path,
-        # without blanking other threads' streams.
+        # Safety-net cleanup for the exception path. Normal completion already
+        # released its clients inside the thread-scoped silence above. Re-enter
+        # the thread-scoped silence here so exception-path cleanup output stays
+        # quiet without blanking other threads' streams.
         # Also a safety-net completion: covers exceptions raised during setup
         # before the request-phase finally. Both tracking cleanup and the
         # per-run completion publication are identity-scoped and idempotent.
@@ -1741,11 +1738,7 @@ def _run_review_in_thread(
             try:
                 with thread_scoped_silence():
                     try:
-                        review_agent.shutdown_memory_provider()
-                    except Exception:
-                        pass
-                    try:
-                        review_agent.close()
+                        review_agent.release_clients()
                     except Exception:
                         pass
             except Exception:
diff --git a/tests/run_agent/test_background_review.py b/tests/run_agent/test_background_review.py
index 930dfb7428..943513672d 100644
--- a/tests/run_agent/test_background_review.py
+++ b/tests/run_agent/test_background_review.py
@@ -185,7 +185,13 @@ def _install_relay_recorder(monkeypatch, review_run=None):
     return calls
 
 
-def test_background_review_shuts_down_memory_provider_before_close(monkeypatch):
+def test_background_review_releases_clients_without_closing_shared_session(monkeypatch):
+    """The review fork must not clean up resources owned by its parent session.
+
+    The fork uses the foreground session ID for prefix-cache parity.  Calling
+    ``close()`` would therefore kill that session's registered terminal
+    processes and tear down its environment when the review completes.
+    """
     events = []
 
     class FakeReviewAgent:
@@ -196,12 +202,12 @@ def test_background_review_shuts_down_memory_provider_before_close(monkeypatch):
         def run_conversation(self, **kwargs):
             events.append(("run_conversation", kwargs))
 
-        def shutdown_memory_provider(self):
-            events.append(("shutdown_memory_provider", None))
-
         def close(self):
             events.append(("close", None))
 
+        def release_clients(self):
+            events.append(("release_clients", None))
+
     monkeypatch.setattr(run_agent_module, "AIAgent", FakeReviewAgent)
     monkeypatch.setattr(run_agent_module.threading, "Thread", ImmediateThread)
 
@@ -216,8 +222,7 @@ def test_background_review_shuts_down_memory_provider_before_close(monkeypatch):
     assert [name for name, _payload in events] == [
         "init",
         "run_conversation",
-        "shutdown_memory_provider",
-        "close",
+        "release_clients",
     ]
 
 
@@ -725,7 +730,7 @@ def test_stale_review_cleanup_cannot_clear_or_signal_newer_review(monkeypatch):
             self.index = instance_count
             instance_count += 1
 
-        def shutdown_memory_provider(self):
+        def release_clients(self):
             if self.index == 0:
                 first_cleanup_entered.set()
                 assert allow_first_cleanup.wait(2.0)