Merge pull request #240 from huanghuang358/fix/s08-consecutive-user-messages

fix(s08): avoid consecutive user messages

agents/s08_background_tasks.py

@@ -185,15 +185,42 @@ TOOLS = [
 ]
 
 
+def append_user_notice(messages: list, text: str) -> None:
+    """Add an async notice without creating adjacent user messages."""
+    block = {"type": "text", "text": text}
+    if messages and messages[-1].get("role") == "user":
+        content = messages[-1].get("content", "")
+        if isinstance(content, list):
+            messages[-1]["content"] = [*content, block]
+        else:
+            messages[-1]["content"] = [
+                {"type": "text", "text": str(content)},
+                block,
+            ]
+        return
+    messages.append({"role": "user", "content": [block]})
+
+
+def inject_background_notifications(messages: list) -> int:
+    notifs = BG.drain_notifications()
+    if not notifs or not messages:
+        return 0
+    notif_text = "\n".join(
+        f"[bg:{n['task_id']}] {n['status']}: {n['result']}" for n in notifs
+    )
+    append_user_notice(
+        messages,
+        f"<background-results>\n{notif_text}\n</background-results>",
+    )
+    return len(notifs)
+
+
 def agent_loop(messages: list):
     while True:
-        # Drain background notifications and inject as system message before LLM call
+        # Drain background notifications and inject before the next LLM call.
-        notifs = BG.drain_notifications()
+        # Merge into the trailing user message when possible to avoid emitting
-        if notifs and messages:
+        # two consecutive user messages (which is messy for caching/debugging).
-            notif_text = "\n".join(
+        inject_background_notifications(messages)
-                f"[bg:{n['task_id']}] {n['status']}: {n['result']}" for n in notifs
-            )
-            messages.append({"role": "user", "content": f"<background-results>\n{notif_text}\n</background-results>"})
         response = client.messages.create(
             model=MODEL, system=SYSTEM, messages=messages,
             tools=TOOLS, max_tokens=8000,

agents/s_full.py

@@ -650,6 +650,45 @@ TOOLS = [
 ]
 
 
+def append_user_notice(messages: list, text: str) -> None:
+    """Add an async notice without creating adjacent user messages."""
+    block = {"type": "text", "text": text}
+    if messages and messages[-1].get("role") == "user":
+        content = messages[-1].get("content", "")
+        if isinstance(content, list):
+            messages[-1]["content"] = [*content, block]
+        else:
+            messages[-1]["content"] = [
+                {"type": "text", "text": str(content)},
+                block,
+            ]
+        return
+    messages.append({"role": "user", "content": [block]})
+
+
+def inject_pending_notifications(messages: list) -> int:
+    count = 0
+    notifs = BG.drain()
+    if notifs:
+        text = "\n".join(
+            f"[bg:{n['task_id']}] {n['status']}: {n['result']}" for n in notifs
+        )
+        append_user_notice(
+            messages,
+            f"<background-results>\n{text}\n</background-results>",
+        )
+        count += len(notifs)
+
+    inbox = BUS.read_inbox("lead")
+    if inbox:
+        append_user_notice(
+            messages,
+            f"<inbox>{json.dumps(inbox, indent=2)}</inbox>",
+        )
+        count += len(inbox)
+    return count
+
+
 # === SECTION: agent_loop ===
 def agent_loop(messages: list):
     rounds_without_todo = 0
@@ -659,15 +698,8 @@ def agent_loop(messages: list):
         if estimate_tokens(messages) > TOKEN_THRESHOLD:
             print("[auto-compact triggered]")
             messages[:] = auto_compact(messages)
-        # s08: drain background notifications
+        # s08/s10: fold asynchronous notices into one user turn.
-        notifs = BG.drain()
+        inject_pending_notifications(messages)
-        if notifs:
-            txt = "\n".join(f"[bg:{n['task_id']}] {n['status']}: {n['result']}" for n in notifs)
-            messages.append({"role": "user", "content": f"<background-results>\n{txt}\n</background-results>"})
-        # s10: check lead inbox
-        inbox = BUS.read_inbox("lead")
-        if inbox:
-            messages.append({"role": "user", "content": f"<inbox>{json.dumps(inbox, indent=2)}</inbox>"})
         # LLM call
         response = client.messages.create(
             model=MODEL, system=SYSTEM, messages=messages,

tests/test_s_full_background.py

@@ -8,10 +8,11 @@ from pathlib import Path
 
 
 REPO_ROOT = Path(__file__).resolve().parents[1]
-MODULE_PATH = REPO_ROOT / "agents" / "s_full.py"
+S08_MODULE_PATH = REPO_ROOT / "agents" / "s08_background_tasks.py"
+S_FULL_MODULE_PATH = REPO_ROOT / "agents" / "s_full.py"
 
 
-def load_s_full_module(temp_cwd: Path):
+def load_agent_module(temp_cwd: Path, module_path: Path, module_name: str):
     fake_anthropic = types.ModuleType("anthropic")
 
     class FakeAnthropic:
@@ -25,9 +26,9 @@ def load_s_full_module(temp_cwd: Path):
     previous_anthropic = sys.modules.get("anthropic")
     previous_dotenv = sys.modules.get("dotenv")
     previous_cwd = Path.cwd()
-    spec = importlib.util.spec_from_file_location("s_full_under_test", MODULE_PATH)
+    spec = importlib.util.spec_from_file_location(module_name, module_path)
     if spec is None or spec.loader is None:
-        raise RuntimeError(f"Unable to load {MODULE_PATH}")
+        raise RuntimeError(f"Unable to load {module_path}")
     module = importlib.util.module_from_spec(spec)
 
     sys.modules["anthropic"] = fake_anthropic
@@ -49,6 +50,18 @@ def load_s_full_module(temp_cwd: Path):
             sys.modules["dotenv"] = previous_dotenv
 
 
+def load_s08_module(temp_cwd: Path):
+    return load_agent_module(
+        temp_cwd,
+        S08_MODULE_PATH,
+        "s08_background_tasks_under_test",
+    )
+
+
+def load_s_full_module(temp_cwd: Path):
+    return load_agent_module(temp_cwd, S_FULL_MODULE_PATH, "s_full_under_test")
+
+
 class BackgroundManagerTests(unittest.TestCase):
     def test_check_returns_running_placeholder_when_result_is_none(self):
         with tempfile.TemporaryDirectory() as tmp:
@@ -63,5 +76,90 @@ class BackgroundManagerTests(unittest.TestCase):
             self.assertEqual(manager.check("abc123"), "[running] (running)")
 
 
+class NotificationInjectionTests(unittest.TestCase):
+    @staticmethod
+    def notification():
+        return {
+            "task_id": "bg-1",
+            "status": "completed",
+            "result": "BACKGROUND_OK",
+        }
+
+    def test_string_user_tail_receives_background_block(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            module = load_s08_module(Path(tmp))
+            module.BG = types.SimpleNamespace(
+                drain_notifications=lambda: [self.notification()]
+            )
+            messages = [{"role": "user", "content": "original request"}]
+
+            count = module.inject_background_notifications(messages)
+
+            self.assertEqual(count, 1)
+            self.assertEqual([message["role"] for message in messages], ["user"])
+            self.assertEqual(
+                messages[0]["content"][0],
+                {"type": "text", "text": "original request"},
+            )
+            self.assertIn("BACKGROUND_OK", messages[0]["content"][1]["text"])
+
+    def test_tool_result_user_tail_preserves_result_before_background_block(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            module = load_s08_module(Path(tmp))
+            module.BG = types.SimpleNamespace(
+                drain_notifications=lambda: [self.notification()]
+            )
+            tool_result = {
+                "type": "tool_result",
+                "tool_use_id": "tool-1",
+                "content": "tool output",
+            }
+            messages = [{"role": "user", "content": [tool_result]}]
+
+            module.inject_background_notifications(messages)
+
+            self.assertEqual([message["role"] for message in messages], ["user"])
+            self.assertEqual(messages[0]["content"][0], tool_result)
+            self.assertIn("BACKGROUND_OK", messages[0]["content"][1]["text"])
+
+    def test_assistant_tail_gets_one_following_user_message(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            module = load_s08_module(Path(tmp))
+            module.BG = types.SimpleNamespace(
+                drain_notifications=lambda: [self.notification()]
+            )
+            messages = [{"role": "assistant", "content": "working"}]
+
+            module.inject_background_notifications(messages)
+
+            self.assertEqual(
+                [message["role"] for message in messages],
+                ["assistant", "user"],
+            )
+            self.assertIn("BACKGROUND_OK", messages[1]["content"][0]["text"])
+
+    def test_full_agent_merges_background_and_inbox_into_one_user_turn(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            module = load_s_full_module(Path(tmp))
+            module.BG = types.SimpleNamespace(
+                drain=lambda: [self.notification()]
+            )
+            module.BUS = types.SimpleNamespace(
+                read_inbox=lambda recipient: [
+                    {"from": "reviewer", "to": recipient, "content": "INBOX_OK"}
+                ]
+            )
+            messages = [{"role": "user", "content": "original request"}]
+
+            count = module.inject_pending_notifications(messages)
+
+            self.assertEqual(count, 2)
+            self.assertEqual([message["role"] for message in messages], ["user"])
+            blocks = messages[0]["content"]
+            self.assertEqual(blocks[0], {"type": "text", "text": "original request"})
+            self.assertIn("BACKGROUND_OK", blocks[1]["text"])
+            self.assertIn("INBOX_OK", blocks[2]["text"])
+
+
 if __name__ == "__main__":
     unittest.main()