fix(agent): warn instead of silently spiraling on repeated identical tool calls

BUILDLOG.md

@@ -0,0 +1,33 @@
+# BUILDLOG
+
+Source of truth for what exists in this fork (`albatrossflyon-coder/nanobot`, tracking upstream `HKUDS/nanobot`) beyond upstream's own docs.
+
+## Tech Stack
+
+- **Languages:** Python, TypeScript
+- **Frameworks/Libraries:** FastAPI-style channel manager, React (webui), LangGraph-adjacent agent loop
+- **Dev Tools:** pytest, ruff, basedpyright
+
+---
+
+## 2026-08-11 — Tool-call-markup-leak fix: 4 original gaps closed, 3 new gaps found by code-review
+
+**Context:** A model finalizing with no tools offered could still emit literal `<tool_call><function=...>` text instead of a real answer, and that raw markup could reach a real user-facing channel. Confirmed live twice today via real email to Chris (`~/.nanobot/logs/gateway.log`, 14:02 and 14:27 CDT — `Response to email:...: <tool_call>` at INFO level, meaning it was never blocked; the filter that should have caught it was built but never actually committed/active in the running gateway).
+
+**4 gaps from the prior session's `/code-review` pass — all fixed and tested this session:**
+1. Dominant per-turn finalize path (`runner.py` `_run_core`, where most turns actually end) had no leak guard — only the max-iterations retry path did. Fixed: added `contains_leaked_tool_call_markup(clean)` check alongside the existing blank-content check, same pattern (fallback message, `stop_reason="leaked_tool_call_markup"`, drain injections, break).
+2. Only the email channel had the egress filter; 15 other channels didn't. Fixed by centralizing instead of propagating: `ChannelManager._send_once` (the single funnel all non-streaming channel sends pass through, confirmed via `find_references`/`search_text`) now runs the check once for all 17 channels. Removed the now-redundant duplicate check from `EmailChannel.send()`.
+3. Regex `<tool_call\b` missed the plural `<tool_calls>` wrapper tag. Verified live that **both** `<tool_calls>` and `</tool_calls>` failed to match (the prior session only caught the opening-tag case) — fixed to `<tool_calls?\b` / `</tool_calls?>`.
+4. The blocked-leak warning log wrote the raw leaked content (potentially shell commands/session IDs) unredacted. Fixed at the source: the new centralized check in `_send_once` never logs the raw content at all (length only).
+
+**Verification:** Full test suite 5914 passed / 44 skipped / 0 failed. `vuln-hunter scan_diff` clean except one unrelated pre-existing item (see below). New/moved tests: `tests/agent/test_runner_safety.py` (2 new — dominant-path leak rejection + clean-response negative case), `tests/channels/test_channel_manager_leak_filter.py` (3 new — centralized filter, plural-tag regression, normal-content passthrough), `nanobot/channels/email/tests/test_email_channel.py` (obsolete email-specific leak test removed, now covered at the manager level instead).
+
+**3 new findings from a fresh `/code-review high` pass after the fix — triaged with Chris, not silently shipped-and-disclosed:**
+
+1. **Streaming bypass (pre-existing, NOT introduced tonight, NOT fixed tonight).** The filter only runs in `_send_once`'s non-streaming branch. A leaked `<tool_call>` in a streaming response (webui/websocket) would already display live, token-by-token, before any finalize-time check runs — this gap existed before tonight's fix too (streaming had zero leak protection either way) and closing it properly needs mid-stream detection or buffering, a real design change, not a quick patch. **Status: open, tracked as a follow-up, not fixed.**
+2. **MessageTool-suppression behavior for the new `leaked_tool_call_markup` stop_reason was a genuine design question, not a clear bug** (`loop.py` `_assemble_outbound`, line ~1594). `empty_final_response` always suppresses the fallback notice when `MessageTool` already sent real content this turn. `leaked_tool_call_markup` was following the general rule instead (suppress only if no new injections occurred) — a code-review report initially described this backwards (claimed the leak notice gets silently dropped when the empty one doesn't; direct code tracing showed the opposite: in the `had_injections=True` case, the leak notice was the one that got delivered, `empty_final_response` was the one still suppressed). Chris's call: always suppress, matching `empty_final_response` — real content already went out via MessageTool, so a leak on the wrap-up has nothing useful to add. **Status: fixed 2026-08-11** — `stop_reason in ("empty_final_response", "leaked_tool_call_markup")` now both suppress unconditionally. New test: `tests/tools/test_message_tool_suppress.py::test_injected_followup_with_message_tool_suppresses_leaked_markup_notice`.
+3. **Regex `<function\s*=` / `TOOL_CALL:` can false-positive on legitimate prose** (e.g. an answer that explains or demonstrates the agent's own tool-call syntax). **Pre-existing** — both patterns were in `_LEAKED_TOOL_CALL_RE` before tonight; this session only touched the `<tool_call>`/`<tool_calls>` singular/plural portion. Checked `~/.nanobot/logs/gateway.log` for evidence this ever fired as a false positive in production — found none; the filter was never actually active before tonight (see Context above), so this risk hasn't manifested yet, but is real now that the filter is about to go live for real. **Status: open, not fixed tonight, worth a follow-up if it's ever observed firing on legitimate content.**
+
+**Also caught tonight:** checked GitHub notifications before pushing and found CI already failing on this same branch from an earlier commit (`6e8e2755`) — a `basedpyright --strict` error in `runner.py` (`append`/`sorted` on a partially-unknown type). A follow-up commit (`7cd2b29f`) already fixed that one but was stuck on GitHub's `action_required` approval gate, unverified. Ran `basedpyright` locally against all files touched tonight and found a **new** instance of the same error class in my own new code (`runner.py:817`, `len(clean)` where `clean: str | None` — the `is_blank_text` guard proves it's non-empty at runtime but basedpyright doesn't narrow through that call). Fixed (`len(clean or "")`). Re-ran clean: 0 errors across all 9 touched files.
+
+**Branch:** `fix/tool-call-loop-detection` in `C:\Repos\nanobot`, not yet committed as of this entry — pending Chris's sign-off per the `start-to-finish` skill.

nanobot/agent/loop.py

@@ -1591,7 +1591,10 @@ def _assemble_outbound(
         """Assemble the final outbound message from turn results."""
         # MessageTool suppression
         if (mt := self.tools.get("message")) and isinstance(mt, MessageTool) and mt._sent_in_turn:
-            if not had_injections or stop_reason == "empty_final_response":
+            if not had_injections or stop_reason in (
+                "empty_final_response",
+                "leaked_tool_call_markup",
+            ):
                 return None
 
         if log_content:

nanobot/agent/runner.py

@@ -4,6 +4,7 @@
 
 import asyncio
 import inspect
+import json
 import os
 from collections.abc import Awaitable, Callable, Iterable
 from copy import deepcopy
@@ -39,6 +40,7 @@
 from nanobot.utils.helpers import (
     IncrementalThinkExtractor,
     build_assistant_message,
+    contains_leaked_tool_call_markup,
     estimate_message_tokens,
     estimate_prompt_tokens_chain,
     extract_reasoning,
@@ -49,6 +51,7 @@
 from nanobot.utils.prompt_templates import render_template
 from nanobot.utils.runtime import (
     EMPTY_FINAL_RESPONSE_MESSAGE,
+    LEAKED_TOOL_CALL_FINAL_RESPONSE_MESSAGE,
     build_budget_exhausted_finalization_message,
     build_finalization_retry_message,
     build_goal_continue_message,
@@ -431,6 +434,10 @@ async def _run_core(
         external_lookup_counts: dict[str, int] = {}
         # Per-turn throttle for repeated attempts against the same outside target.
         workspace_violation_counts: dict[str, int] = {}
+        # Per-turn loop guard: the last few tool-call signatures (name + args),
+        # most recent last. Used to warn the model when it repeats the exact
+        # same call several times in a row instead of making progress.
+        recent_tool_signatures: list[str] = []
         empty_content_retries = 0
         # Segments from one uninterrupted length-recovery chain. Tool work or
         # injected user input starts a new logical answer and clears the chain.
@@ -523,6 +530,13 @@ async def _run_core(
                     response,
                 )
                 messages.append(assistant_message)
+
+                loop_warning = self._detect_tool_call_loop(
+                    response.tool_calls, recent_tool_signatures
+                ) or self._detect_intra_round_duplicate_calls(response.tool_calls)
+                if loop_warning:
+                    messages.append({"role": "system", "content": loop_warning})
+
                 await self._emit_checkpoint(
                     spec,
                     {
@@ -790,6 +804,35 @@ async def _run_core(
                     length_recovery_parts.clear()
                     continue
                 break
+            if contains_leaked_tool_call_markup(clean):
+                # Same last-mile safety net as the max-iterations finalize path
+                # (above): a model asked to finalize can still write literal
+                # <tool_call>... text instead of a real answer. This is the
+                # dominant path where most turns actually end, so it needs its
+                # own guard rather than relying on the retry-path check alone.
+                logger.warning(
+                    "Leaked tool-call markup in final response for {}; "
+                    "substituting fallback ({} chars)",
+                    spec.session_key or "default",
+                    len(clean or ""),
+                )
+                final_content = LEAKED_TOOL_CALL_FINAL_RESPONSE_MESSAGE
+                stop_reason = "leaked_tool_call_markup"
+                error = final_content
+                self._append_final_message(messages, final_content)
+                context.final_content = final_content
+                context.error = error
+                context.stop_reason = stop_reason
+                await hook.after_iteration(context)
+                should_continue, injection_cycles = await self._try_drain_injections(
+                    spec, messages, None, injection_cycles,
+                    phase="after leaked tool-call markup",
+                )
+                if should_continue:
+                    had_injections = True
+                    length_recovery_parts.clear()
+                    continue
+                break
 
             messages.append(
                 assistant_message
@@ -1218,12 +1261,14 @@ async def _try_finalize_after_max_iterations(
 
         raw_usage = self._usage_or_estimate(spec, retry_messages, response)
         self._accumulate_usage(usage, raw_usage)
-        if response.finish_reason == "error" or response.has_tool_calls:
+        leaked_tool_call = contains_leaked_tool_call_markup(response.content)
+        if response.finish_reason == "error" or response.has_tool_calls or leaked_tool_call:
             logger.warning(
                 "Budget-exhausted finalization returned finish_reason='{}' "
-                "with {} tool call(s) for {}; using fallback",
+                "with {} tool call(s){} for {}; using fallback",
                 response.finish_reason,
                 len(response.tool_calls),
+                " (leaked tool-call markup in content)" if leaked_tool_call else "",
                 spec.session_key or "default",
             )
             return None
@@ -1277,6 +1322,97 @@ def _max_iterations_fallback(spec: AgentRunSpec) -> str:
             max_iterations=spec.max_iterations,
         )
 
+    @staticmethod
+    def _single_call_signature(tc: ToolCallRequest) -> str:
+        """Build a stable signature for one tool call (name + arguments)."""
+        if isinstance(tc.arguments, str):
+            args_repr = tc.arguments
+        else:
+            try:
+                args_repr = json.dumps(tc.arguments, sort_keys=True, default=str)
+            except (TypeError, ValueError):
+                args_repr = str(tc.arguments)
+        return f"{tc.name}:{args_repr}"
+
+    @staticmethod
+    def _tool_call_signature(tool_calls: Iterable[ToolCallRequest]) -> str:
+        """Build a stable signature for one round of tool calls (name + args).
+
+        Order-independent (sorted) since concurrent_tools can execute several
+        calls in one round and their relative order isn't semantically
+        meaningful for loop detection.
+        """
+        parts = [AgentRunner._single_call_signature(tc) for tc in tool_calls]
+        return "|".join(sorted(parts))
+
+    @staticmethod
+    def _detect_intra_round_duplicate_calls(
+        tool_calls: Iterable[ToolCallRequest],
+        *,
+        threshold: int = 3,
+    ) -> str | None:
+        """Detect several identical calls batched into a single round.
+
+        Complements ``_detect_tool_call_loop``, which only tracks one
+        signature per whole round and so only catches a loop that repeats
+        across separate rounds -- it cannot see ``threshold`` identical
+        calls issued together in one round (e.g. three parallel read_file
+        calls with the same path), since that round produces its own
+        distinct joined signature just once. This checks within a single
+        round instead, so it fires the first time such a round occurs
+        rather than requiring it to repeat.
+        """
+        counts: dict[str, int] = {}
+        for tc in tool_calls:
+            signature = AgentRunner._single_call_signature(tc)
+            counts[signature] = counts.get(signature, 0) + 1
+        if not any(count >= threshold for count in counts.values()):
+            return None
+        return (
+            f"You have just made the exact same tool call {threshold}+ times "
+            "in a single turn (identical tool name and arguments, issued "
+            "together). Repeating an identical call will not produce a "
+            "different result. Stop repeating this action -- either try a "
+            "genuinely different approach, or report what you have found so "
+            "far and ask how to proceed."
+        )
+
+    @staticmethod
+    def _detect_tool_call_loop(
+        tool_calls: Iterable[ToolCallRequest],
+        recent_tool_signatures: list[str],
+        *,
+        threshold: int = 3,
+    ) -> str | None:
+        """Track recent tool-call signatures and return a warning once the
+        exact same round of calls repeats ``threshold`` times in a row.
+
+        Mutates ``recent_tool_signatures`` in place (bounded to ``threshold``
+        entries) and clears it after a warning fires, so the same loop won't
+        re-trigger the warning every single iteration once it's already been
+        flagged once.
+        """
+        signature = AgentRunner._tool_call_signature(tool_calls)
+        if not signature:
+            return None
+        recent_tool_signatures.append(signature)
+        if len(recent_tool_signatures) > threshold:
+            recent_tool_signatures.pop(0)
+        if (
+            len(recent_tool_signatures) == threshold
+            and len(set(recent_tool_signatures)) == 1
+        ):
+            recent_tool_signatures.clear()
+            return (
+                f"You have just made the exact same tool call(s) {threshold} times "
+                "in a row (identical tool name and arguments). Repeating an "
+                "identical call will not produce a different result. Stop "
+                "repeating this action -- either try a genuinely different "
+                "approach, or report what you have found so far and ask how "
+                "to proceed."
+            )
+        return None
+
     def _usage_or_estimate(
         self,
         spec: AgentRunSpec,

nanobot/channels/email/runtime.py

@@ -283,6 +283,9 @@ async def send(self, msg: OutboundMessage) -> None:
                 failed_attachments.append(f"[attachment: {filename} - send failed]")
                 self.logger.exception("Failed to attach file {}", media_path)
 
+        # Leaked tool-call markup is filtered centrally in ChannelManager._send_once
+        # before any channel's send() is invoked -- see contains_leaked_tool_call_markup
+        # usage there. No per-channel check needed here.
         content = msg.content or ""
         if failed_attachments:
             fallback = "\n".join(failed_attachments)

nanobot/channels/email/tests/test_email_channel.py

@@ -1867,3 +1867,41 @@ def send_message(self, msg: EmailMessage):
             attachment_parts.append(part)
     assert len(attachment_parts) == 1
     assert attachment_parts[0].get_filename() == "summary.pdf"
+
+
+@pytest.mark.asyncio
+async def test_send_passes_through_normal_content_unchanged(monkeypatch) -> None:
+    """Negative case for the leak filter -- ordinary replies, including ones
+    that mention code or angle brackets in prose, must not be mangled."""
+    sent_messages: list[EmailMessage] = []
+
+    class FakeSMTP:
+        def __init__(self, _host: str, _port: int, timeout: int = 30) -> None:
+            self.timeout = timeout
+
+        def __enter__(self):
+            return self
+
+        def __exit__(self, exc_type, exc, tb):
+            return False
+
+        def starttls(self, context=None):
+            return None
+
+        def login(self, _user: str, _pw: str):
+            return None
+
+        def send_message(self, msg: EmailMessage):
+            sent_messages.append(msg)
+
+    monkeypatch.setattr("nanobot.channels.email.runtime.smtplib.SMTP", lambda h, p, timeout=30: FakeSMTP(h, p, timeout=timeout))
+
+    normal = "Scan complete: 3 findings, all low severity. See <link> in the dashboard."
+    channel = EmailChannel(_make_config(), MessageBus())
+    await channel.send(
+        OutboundMessage(channel="email", chat_id="alice@example.com", content=normal)
+    )
+
+    assert len(sent_messages) == 1
+    body = sent_messages[0].get_body(preferencelist=("plain",)).get_content()
+    assert body.strip() == normal

nanobot/channels/manager.py

@@ -34,11 +34,13 @@
 )
 from nanobot.channels.registry import channel_default_enabled
 from nanobot.config.schema import Config
+from nanobot.utils.helpers import contains_leaked_tool_call_markup
 from nanobot.utils.restart import (
     RestartNotice,
     consume_restart_notice_from_env,
     format_restart_completed_message,
 )
+from nanobot.utils.runtime import LEAKED_TOOL_CALL_FINAL_RESPONSE_MESSAGE
 
 if TYPE_CHECKING:
     from nanobot.cron.service import CronService
@@ -835,6 +837,22 @@ async def _send_once(channel: BaseChannel, msg: OutboundMessage) -> None:
         elif isinstance(event, StreamEndEvent):
             await ChannelManager._send_stream_event(channel, msg, event)
         elif not isinstance(event, StreamedResponseEvent):
+            if contains_leaked_tool_call_markup(msg.content):
+                # Last-mile safety net for every channel: whatever produced
+                # this (a model finalizing with no tools offered and
+                # emitting tool-call-shaped text anyway, or any other path)
+                # should never reach a user-facing channel as raw unexecuted
+                # tool syntax. Content is deliberately not logged -- it may
+                # contain leaked tool arguments (shell commands, session
+                # IDs, etc).
+                logger.warning(
+                    "Blocked outbound message to {}:{} containing leaked "
+                    "tool-call markup ({} chars)",
+                    msg.channel,
+                    msg.chat_id,
+                    len(msg.content or ""),
+                )
+                msg.content = LEAKED_TOOL_CALL_FINAL_RESPONSE_MESSAGE
             await channel.send(msg)
 
     def _coalesce_stream_deltas(