From 8d33c1cb378f4667f6f54fbeec8fdd5d8e7108ce Mon Sep 17 00:00:00 2001 From: Gunnar Thielebein Date: Wed, 22 Apr 2026 23:17:48 +0000 Subject: [PATCH 01/80] feat(telegram): add inline keyboard buttons --- nanobot/agent/tools/message.py | 16 ++++++- nanobot/bus/events.py | 2 +- nanobot/channels/telegram.py | 84 ++++++++++++++++++++++++++++------ 3 files changed, 86 insertions(+), 16 deletions(-) diff --git a/nanobot/agent/tools/message.py b/nanobot/agent/tools/message.py index ee81effb..ea0598a1 100644 --- a/nanobot/agent/tools/message.py +++ b/nanobot/agent/tools/message.py @@ -17,6 +17,10 @@ from nanobot.bus.events import OutboundMessage StringSchema(""), description="Optional: list of file paths to attach (images, audio, documents)", ), + buttons=ArraySchema( + ArraySchema(StringSchema("Button label")), + description="Optional: inline keyboard buttons as list of rows, each row is list of button labels.", + ), required=["content"], ) ) @@ -81,14 +85,20 @@ class MessageTool(Tool): chat_id: str | None = None, message_id: str | None = None, media: list[str] | None = None, + buttons: list[list[str]] | None = None, **kwargs: Any ) -> str: from nanobot.utils.helpers import strip_think content = strip_think(content) + if buttons is not None: + if not isinstance(buttons, list) or any( + not isinstance(row, list) or any(not isinstance(label, str) for label in row) + for row in buttons + ): + return "Error: buttons must be a list of list of strings" default_channel = self._default_channel.get() default_chat_id = self._default_chat_id.get() - channel = channel or default_channel chat_id = chat_id or default_chat_id # Only inherit default message_id when targeting the same channel+chat. @@ -112,6 +122,7 @@ class MessageTool(Tool): chat_id=chat_id, content=content, media=media or [], + buttons=buttons or [], metadata={ "message_id": message_id, } if message_id else {}, @@ -122,6 +133,7 @@ class MessageTool(Tool): if channel == default_channel and chat_id == default_chat_id: self._sent_in_turn = True media_info = f" with {len(media)} attachments" if media else "" - return f"Message sent to {channel}:{chat_id}{media_info}" + button_info = f" with {sum(len(row) for row in buttons)} button(s)" if buttons else "" + return f"Message sent to {channel}:{chat_id}{media_info}{button_info}" except Exception as e: return f"Error sending message: {str(e)}" diff --git a/nanobot/bus/events.py b/nanobot/bus/events.py index 018c25b3..44fba848 100644 --- a/nanobot/bus/events.py +++ b/nanobot/bus/events.py @@ -34,5 +34,5 @@ class OutboundMessage: reply_to: str | None = None media: list[str] = field(default_factory=list) metadata: dict[str, Any] = field(default_factory=dict) - + buttons: list[list[str]] = field(default_factory=list) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index 6925658d..d55c1a02 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -11,9 +11,9 @@ from typing import Any, Literal from loguru import logger from pydantic import Field -from telegram import BotCommand, ReactionTypeEmoji, ReplyParameters, Update +from telegram import BotCommand, InlineKeyboardButton, InlineKeyboardMarkup, ReactionTypeEmoji, ReplyParameters, Update from telegram.error import BadRequest, NetworkError, TimedOut -from telegram.ext import Application, ContextTypes, MessageHandler, filters +from telegram.ext import Application, CallbackQueryHandler, ContextTypes, MessageHandler, filters from telegram.request import HTTPXRequest from nanobot.bus.events import OutboundMessage @@ -230,6 +230,8 @@ class TelegramConfig(Base): connection_pool_size: int = 32 pool_timeout: float = 5.0 streaming: bool = True + # Enable inline keyboard buttons in Telegram messages. + inline_keyboards: bool = False stream_edit_interval: float = Field(default=_STREAM_EDIT_INTERVAL_DEFAULT, ge=0.1) @@ -364,6 +366,14 @@ class TelegramChannel(BaseChannel): ) ) + # Conditionally register inline keyboard callback handler + if self.config.inline_keyboards: + self._app.add_handler(CallbackQueryHandler(self._on_callback_query)) + allowed_updates = ["message", "callback_query"] + logger.debug("Telegram inline keyboards enabled") + else: + allowed_updates = ["message"] + logger.info("Starting Telegram bot (polling mode)...") # Initialize and start polling @@ -384,7 +394,7 @@ class TelegramChannel(BaseChannel): # Start polling (this runs until stopped) await self._app.updater.start_polling( - allowed_updates=["message"], + allowed_updates=allowed_updates, drop_pending_updates=False, # Process pending messages on startup error_callback=self._on_polling_error, ) @@ -510,16 +520,20 @@ class TelegramChannel(BaseChannel): # Send text content if msg.content and msg.content != "[empty message]": render_as_blockquote = bool(msg.metadata.get("_tool_hint")) - for chunk in split_message(msg.content, TELEGRAM_MAX_MESSAGE_LEN): + chunks = split_message(msg.content, TELEGRAM_MAX_MESSAGE_LEN) + reply_markup = self._build_keyboard(msg.buttons) if getattr(msg, 'buttons', None) else None + for i, chunk in enumerate(chunks): + is_last = (i == len(chunks) - 1) await self._send_text( chat_id, chunk, reply_params, thread_kwargs, render_as_blockquote=render_as_blockquote, + reply_markup=reply_markup if is_last else None, ) async def _call_with_retry(self, fn, *args, **kwargs): """Call an async Telegram API function with retry on pool/network timeout and RetryAfter.""" from telegram.error import RetryAfter - + for attempt in range(1, _SEND_MAX_RETRIES + 1): try: return await fn(*args, **kwargs) @@ -549,6 +563,7 @@ class TelegramChannel(BaseChannel): reply_params=None, thread_kwargs: dict | None = None, render_as_blockquote: bool = False, + reply_markup=None, ) -> None: """Send a plain text message with HTML fallback.""" try: @@ -557,12 +572,10 @@ class TelegramChannel(BaseChannel): self._app.bot.send_message, chat_id=chat_id, text=html, parse_mode="HTML", reply_parameters=reply_params, + reply_markup=reply_markup, **(thread_kwargs or {}), ) except BadRequest as e: - # Only fall back to plain text on actual HTML parse/format errors. - # Network errors (TimedOut, NetworkError) should propagate immediately - # to avoid doubling connection demand during pool exhaustion. logger.warning("HTML parse failed, falling back to plain text: {}", e) try: await self._call_with_retry( @@ -570,6 +583,7 @@ class TelegramChannel(BaseChannel): chat_id=chat_id, text=text, reply_parameters=reply_params, + reply_markup=reply_markup, **(thread_kwargs or {}), ) except Exception as e2: @@ -796,13 +810,13 @@ class TelegramChannel(BaseChannel): text = getattr(reply, "text", None) or getattr(reply, "caption", None) or "" if len(text) > TELEGRAM_REPLY_CONTEXT_MAX_LEN: text = text[:TELEGRAM_REPLY_CONTEXT_MAX_LEN] + "..." - + if not text: return None - + bot_id, _ = await self._ensure_bot_identity() reply_user = getattr(reply, "from_user", None) - + if bot_id and reply_user and getattr(reply_user, "id", None) == bot_id: return f"[Reply to bot: {text}]" elif reply_user and getattr(reply_user, "username", None): @@ -947,7 +961,7 @@ class TelegramChannel(BaseChannel): message = update.message user = update.effective_user self._remember_thread_context(message) - + # Strip @bot_username suffix if present content = message.text or "" if content.startswith("/") and "@" in content: @@ -955,7 +969,7 @@ class TelegramChannel(BaseChannel): cmd_part = cmd_part.split("@")[0] content = f"{cmd_part} {rest[0]}" if rest else cmd_part content = self._normalize_telegram_command(content) - + await self._handle_message( sender_id=self._sender_id(user), chat_id=str(message.chat_id), @@ -1180,3 +1194,47 @@ class TelegramChannel(BaseChannel): return "".join(Path(filename).suffixes) return "" + + def _build_keyboard(self, buttons: list) -> InlineKeyboardMarkup | None: + """Build inline keyboard markup if inline_keyboards is enabled.""" + if not buttons or not self.config.inline_keyboards: + return None + keyboard = [ + [InlineKeyboardButton(label, callback_data=label) for label in row] + for row in buttons + ] + return InlineKeyboardMarkup(keyboard) + + async def _on_callback_query(self, update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: + """Handle inline keyboard button clicks (callback queries).""" + if not update.callback_query or not update.effective_user: + return + query = update.callback_query + user = update.effective_user + chat_id = query.message.chat_id if query.message else None + sender_id = self._sender_id(user) + if not chat_id: + logger.warning("Callback query without chat_id") + return + button_label = query.data or "" + await query.answer() + if query.message: + try: + await query.message.edit_reply_markup(reply_markup=None) + except Exception: + pass + logger.debug("Inline button tap from {}: {}", sender_id, button_label) + self._start_typing(str(chat_id)) + await self._handle_message( + sender_id=sender_id, + chat_id=str(chat_id), + content=button_label, + metadata={ + "callback_query_id": query.id, + "button_label": button_label, + "user_id": user.id, + "username": user.username, + "first_name": user.first_name, + "is_callback": True, + }, + ) From b9b81d9301877cff7aa4ae6bc7ea8c937ceec98a Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 04:32:06 +0000 Subject: [PATCH 02/80] test(telegram): pin inline-keyboards flag gate and buttons validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two kill-switch tests for the new inline-keyboards path. Neither is flashy — they just make sure the next unrelated refactor can't quietly regress two narrow contracts the PR relies on. 1. TelegramChannel._build_keyboard returns None whenever TelegramConfig.inline_keyboards is False, even if buttons are supplied. The flag defaults off; if someone ever flips that default the change should fail this test before it reaches prod bots. 2. MessageTool rejects malformed `buttons` payloads (non-list, mixed list/str row, non-str label, None label) up front instead of letting them slip into the channel layer where Telegram would silently 400 the send. Parametrized over four shapes the guard needs to reject. No production code touched. Made-with: Cursor --- tests/channels/test_telegram_channel.py | 26 +++++++++++++++++++++++++ tests/tools/test_message_tool.py | 21 ++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index 4a69d31a..5480b878 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -1591,3 +1591,29 @@ async def test_send_delta_mid_stream_strips_markdown() -> None: assert "**" not in edited_text assert "Title" in edited_text assert "1. step" in edited_text + + +def test_build_keyboard_respects_inline_keyboards_flag() -> None: + """``_build_keyboard`` returns ``None`` whenever the feature flag is off, + regardless of whether buttons are provided; returns a proper Markup only + when the flag is explicitly enabled. Pins the kill-switch so accidentally + flipping the default doesn't silently expose callback handlers.""" + from telegram import InlineKeyboardMarkup + + off = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", inline_keyboards=False), + MessageBus(), + ) + assert off._build_keyboard([["A", "B"]]) is None + + on = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", inline_keyboards=True), + MessageBus(), + ) + assert on._build_keyboard([]) is None # empty still no-op + markup = on._build_keyboard([["Yes", "No"], ["Cancel"]]) + assert isinstance(markup, InlineKeyboardMarkup) + rows = markup.inline_keyboard + assert [[b.text for b in row] for row in rows] == [["Yes", "No"], ["Cancel"]] + # callback_data mirrors label so _on_callback_query can echo the tap back. + assert rows[0][0].callback_data == "Yes" diff --git a/tests/tools/test_message_tool.py b/tests/tools/test_message_tool.py index dc8e11d5..b65b5cd8 100644 --- a/tests/tools/test_message_tool.py +++ b/tests/tools/test_message_tool.py @@ -8,3 +8,24 @@ async def test_message_tool_returns_error_when_no_target_context() -> None: tool = MessageTool() result = await tool.execute(content="test") assert result == "Error: No target channel/chat specified" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "bad", + [ + "not a list", + [["ok"], "row-not-a-list"], + [["ok", 42]], + [[None]], + ], +) +async def test_message_tool_rejects_malformed_buttons(bad) -> None: + """``buttons`` must be ``list[list[str]]``; the tool validates the shape + up front so a malformed LLM payload errors visibly instead of slipping + into the channel layer where Telegram would silently reject the frame.""" + tool = MessageTool() + result = await tool.execute( + content="hi", channel="telegram", chat_id="1", buttons=bad, + ) + assert result == "Error: buttons must be a list of list of strings" From 6bc2983ab11baea7fe67c4599e466bce5116f51a Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 05:14:49 +0000 Subject: [PATCH 03/80] fix(telegram): fall back buttons to inline text when keyboard disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Buttons are semantic options, not a separate channel protocol: a user who taps "Yes" and a user who types "yes" arrive at the agent as the same string. Dropping ``msg.buttons`` when ``inline_keyboards=False`` was the worst of both worlds — the agent got told "Message sent with N button(s)" while the user saw a question with no options. Splice the labels into the message text instead. The LLM produces the same ``message(buttons=...)`` call regardless of channel; the channel layer picks the richest rendering it can afford — native keyboard when enabled, bracketed inline text otherwise. Layout is preserved (one row per line). Other channels can adopt the same helper incrementally. Locks: canonical ``_buttons_as_text`` format, flag-off send-path splices labels, flag-on send-path keeps content clean and rides ``reply_markup``. Made-with: Cursor --- nanobot/channels/telegram.py | 14 +++++- tests/channels/test_telegram_channel.py | 61 +++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index d55c1a02..40ba71ad 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -520,8 +520,13 @@ class TelegramChannel(BaseChannel): # Send text content if msg.content and msg.content != "[empty message]": render_as_blockquote = bool(msg.metadata.get("_tool_hint")) - chunks = split_message(msg.content, TELEGRAM_MAX_MESSAGE_LEN) - reply_markup = self._build_keyboard(msg.buttons) if getattr(msg, 'buttons', None) else None + buttons = getattr(msg, "buttons", None) or [] + reply_markup = self._build_keyboard(buttons) if buttons else None + text = msg.content + # Fallback: no native keyboard → splice labels into the message so the choices survive. + if buttons and reply_markup is None: + text = f"{text}\n\n{self._buttons_as_text(buttons)}" + chunks = split_message(text, TELEGRAM_MAX_MESSAGE_LEN) for i, chunk in enumerate(chunks): is_last = (i == len(chunks) - 1) await self._send_text( @@ -1205,6 +1210,11 @@ class TelegramChannel(BaseChannel): ] return InlineKeyboardMarkup(keyboard) + @staticmethod + def _buttons_as_text(buttons: list[list[str]]) -> str: + # Buttons are semantic options; when we can't render a keyboard, the user still needs to see them. + return "\n".join(" ".join(f"[{label}]" for label in row) for row in buttons if row) + async def _on_callback_query(self, update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Handle inline keyboard button clicks (callback queries).""" if not update.callback_query or not update.effective_user: diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index 5480b878..a9fad504 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -1617,3 +1617,64 @@ def test_build_keyboard_respects_inline_keyboards_flag() -> None: assert [[b.text for b in row] for row in rows] == [["Yes", "No"], ["Cancel"]] # callback_data mirrors label so _on_callback_query can echo the tap back. assert rows[0][0].callback_data == "Yes" + + +def test_buttons_as_text_format_preserves_rows_and_labels() -> None: + # Canonical shape: one row per line, labels bracketed. Layout survives the fallback. + assert TelegramChannel._buttons_as_text([["Yes", "No"], ["Cancel"]]) == "[Yes] [No]\n[Cancel]" + assert TelegramChannel._buttons_as_text([["Only"]]) == "[Only]" + assert TelegramChannel._buttons_as_text([[], ["A"]]) == "[A]" # empty rows skipped + + +@pytest.mark.asyncio +async def test_send_falls_back_buttons_to_inline_text_when_flag_off() -> None: + """Buttons are semantic options; with ``inline_keyboards=False`` we must + splice labels into the text so users still see the choices. Silent-drop + was the pre-fallback bug — the agent got a success reply while the user + saw a question with no options.""" + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", allow_from=["*"], inline_keyboards=False), + MessageBus(), + ) + channel._app = _FakeApp(lambda: None) + + await channel.send( + OutboundMessage( + channel="telegram", + chat_id="123", + content="Proceed?", + buttons=[["Yes", "No"], ["Cancel"]], + ) + ) + + assert len(channel._app.bot.sent_messages) == 1 + sent = channel._app.bot.sent_messages[0] + assert sent.get("reply_markup") is None + assert "Proceed?" in sent["text"] + assert "[Yes] [No]" in sent["text"] + assert "[Cancel]" in sent["text"] + + +@pytest.mark.asyncio +async def test_send_uses_native_keyboard_when_flag_on() -> None: + """With the flag on, the content stays clean and buttons ride in ``reply_markup``.""" + from telegram import InlineKeyboardMarkup + + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", allow_from=["*"], inline_keyboards=True), + MessageBus(), + ) + channel._app = _FakeApp(lambda: None) + + await channel.send( + OutboundMessage( + channel="telegram", + chat_id="123", + content="Proceed?", + buttons=[["Yes", "No"]], + ) + ) + + sent = channel._app.bot.sent_messages[0] + assert isinstance(sent.get("reply_markup"), InlineKeyboardMarkup) + assert "[Yes]" not in sent["text"] # native keyboard owns the rendering From 06503cd0fc434d88df9e7a4730f089aab4924319 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 05:23:10 +0000 Subject: [PATCH 04/80] fix(telegram): keep callback_data under Telegram's 64-byte cap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``InlineKeyboardButton(label, callback_data=label)`` fails Telegram's API when the label exceeds 64 bytes UTF-8. An LLM-generated long option (realistic in multilingual flows) used to 400 the ``send_message`` call silently — user got nothing, agent heard a successful retry-then-drop. Decouple display from wire: button text keeps the full label, callback_data gets truncated at a UTF-8 char boundary. Tap echoes the prefix back as the user message; the LLM understands a prefix of its own option just fine, and the display the user saw was always the full string. Locks: helper boundary behavior (ASCII, CJK, short labels pass through) and end-to-end ``_build_keyboard`` integration with an over-cap label. Made-with: Cursor --- nanobot/channels/telegram.py | 10 ++++++- tests/channels/test_telegram_channel.py | 35 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index 40ba71ad..38c3fe89 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -1205,11 +1205,19 @@ class TelegramChannel(BaseChannel): if not buttons or not self.config.inline_keyboards: return None keyboard = [ - [InlineKeyboardButton(label, callback_data=label) for label in row] + [InlineKeyboardButton(label, callback_data=self._safe_callback_data(label)) for label in row] for row in buttons ] return InlineKeyboardMarkup(keyboard) + @staticmethod + def _safe_callback_data(label: str) -> str: + # Telegram caps callback_data at 64 bytes UTF-8; truncate at a char boundary so the keyboard still sends. + encoded = label.encode("utf-8") + if len(encoded) <= 64: + return label + return encoded[:64].decode("utf-8", errors="ignore") + @staticmethod def _buttons_as_text(buttons: list[list[str]]) -> str: # Buttons are semantic options; when we can't render a keyboard, the user still needs to see them. diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index a9fad504..175844b2 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -1619,6 +1619,41 @@ def test_build_keyboard_respects_inline_keyboards_flag() -> None: assert rows[0][0].callback_data == "Yes" +def test_safe_callback_data_truncates_at_utf8_boundary() -> None: + # Telegram's 64-byte callback_data cap is a hard API limit; silent 400s were the bug. + short = "Yes" + assert TelegramChannel._safe_callback_data(short) == short + + long_ascii = "a" * 100 + out = TelegramChannel._safe_callback_data(long_ascii) + assert len(out.encode("utf-8")) <= 64 + assert long_ascii.startswith(out) + + # Multibyte labels must not split a codepoint mid-byte. + long_cjk = "同意并继续下一步，我已阅读并同意了服务条款以及隐私政策" + assert len(long_cjk.encode("utf-8")) > 64 + out = TelegramChannel._safe_callback_data(long_cjk) + assert len(out.encode("utf-8")) <= 64 + assert long_cjk.startswith(out) + out.encode("utf-8").decode("utf-8") # must round-trip cleanly + + +def test_build_keyboard_uses_safe_callback_data_for_long_labels() -> None: + # Pins the integration so a long-label payload survives ``send_message`` instead of 400ing. + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", inline_keyboards=True), + MessageBus(), + ) + long_label = "Approve and continue to the next step with the updated terms of service" + assert len(long_label.encode("utf-8")) > 64 + + markup = channel._build_keyboard([[long_label]]) + btn = markup.inline_keyboard[0][0] + assert btn.text == long_label # display preserved + assert len(btn.callback_data.encode("utf-8")) <= 64 + assert long_label.startswith(btn.callback_data) + + def test_buttons_as_text_format_preserves_rows_and_labels() -> None: # Canonical shape: one row per line, labels bracketed. Layout survives the fallback. assert TelegramChannel._buttons_as_text([["Yes", "No"], ["Cancel"]]) == "[Yes] [No]\n[Cancel]" From 185a8fd34dd43ef10a99bff5653d7393e5731fee Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 07:43:21 +0000 Subject: [PATCH 05/80] fix(webui): opaque composer, equal-width message area, cleaner user pill --- webui/src/components/MessageBubble.tsx | 3 +-- webui/src/components/thread/ThreadComposer.tsx | 6 +++--- webui/src/components/thread/ThreadViewport.tsx | 6 ++++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/webui/src/components/MessageBubble.tsx b/webui/src/components/MessageBubble.tsx index 8a51b117..076c3000 100644 --- a/webui/src/components/MessageBubble.tsx +++ b/webui/src/components/MessageBubble.tsx @@ -42,9 +42,8 @@ export function MessageBubble({ message }: MessageBubbleProps) { {hasText ? (

{message.content} diff --git a/webui/src/components/thread/ThreadComposer.tsx b/webui/src/components/thread/ThreadComposer.tsx index 9b38d903..105bb6c7 100644 --- a/webui/src/components/thread/ThreadComposer.tsx +++ b/webui/src/components/thread/ThreadComposer.tsx @@ -216,9 +216,9 @@ export function ThreadComposer({ className={cn( "relative mx-auto flex w-full flex-col overflow-hidden transition-all duration-200", isHero - ? "max-w-[40rem] rounded-[24px] border border-border/75 bg-card/72 shadow-[0_10px_30px_rgba(0,0,0,0.10)]" - : "max-w-[49.5rem] rounded-[16px] border border-border/70 bg-card/55", - "focus-within:bg-card/70 focus-within:ring-1 focus-within:ring-foreground/8", + ? "max-w-[40rem] rounded-[24px] border border-border/75 bg-card shadow-[0_10px_30px_rgba(0,0,0,0.10)]" + : "max-w-[49.5rem] rounded-[16px] border border-border/70 bg-card", + "focus-within:ring-1 focus-within:ring-foreground/8", disabled && "opacity-60", isDragging && "ring-2 ring-primary/40 motion-reduce:ring-0 motion-reduce:border-primary", )} diff --git a/webui/src/components/thread/ThreadViewport.tsx b/webui/src/components/thread/ThreadViewport.tsx index 4ad43282..5f4b8d01 100644 --- a/webui/src/components/thread/ThreadViewport.tsx +++ b/webui/src/components/thread/ThreadViewport.tsx @@ -70,10 +70,12 @@ export function ThreadViewport({ {hasMessages ? (

- +
+ +
-
+
{composer}
From c23d719780298b3e7d4e60432dcd207035382368 Mon Sep 17 00:00:00 2001 From: Pablo Cabeza Date: Thu, 23 Apr 2026 00:54:23 +0100 Subject: [PATCH 06/80] feat(agent): emit structured _tool_events progress metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the existing on_progress callback to carry structured tool-event payloads alongside the plain-text hint, so channels can render rich tool execution state (start/finish/error, arguments, results, file attachments) rather than only the pre-formatted hint string. Changes ------- - AgentLoop._tool_event_start_payload() — builds a version-1 start payload from a ToolCallRequest - AgentLoop._tool_event_result_extras() — extracts files/embeds from a tool result dict - AgentLoop._tool_event_finish_payloads() — maps tool_calls + tool_results + tool_events from AgentHookContext into finish payloads - _LoopHook.before_execute_tools() — passes tool_events=[...] to on_progress together with the existing tool_hint flag - _LoopHook.after_iteration() — emits a second on_progress call with the finish payloads once tool results are available - _bus_progress() — forwards tool_events as _tool_events in OutboundMessage metadata so channel implementations can read them - on_progress type widened to Callable[..., Awaitable[None]] on all public entry points; _cli_progress updated to accept and ignore tool_events The contract is additive: callers that only accept (content, *, tool_hint) continue to work unchanged. Callers that also accept tool_events receive the structured data. Co-Authored-By: Claude Sonnet 4.6 --- nanobot/agent/loop.py | 72 ++++++++++++- nanobot/cli/commands.py | 2 +- tests/agent/test_loop_progress.py | 125 ++++++++++++++++++++++ tests/tools/test_message_tool_suppress.py | 1 - 4 files changed, 194 insertions(+), 6 deletions(-) create mode 100644 tests/agent/test_loop_progress.py diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 6ffade73..6fbe7408 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -103,13 +103,18 @@ class _LoopHook(AgentHook): if thought: await self._on_progress(thought) tool_hint = self._loop._strip_think(self._loop._tool_hint(context.tool_calls)) - await self._on_progress(tool_hint, tool_hint=True) + tool_events = [self._loop._tool_event_start_payload(tc) for tc in context.tool_calls] + await self._on_progress(tool_hint, tool_hint=True, tool_events=tool_events) for tc in context.tool_calls: args_str = json.dumps(tc.arguments, ensure_ascii=False) logger.info("Tool call: {}({})", tc.name, args_str[:200]) self._loop._set_tool_context(self._channel, self._chat_id, self._message_id) async def after_iteration(self, context: AgentHookContext) -> None: + if self._on_progress and context.tool_calls and context.tool_events: + tool_events = self._loop._tool_event_finish_payloads(context) + if tool_events: + await self._on_progress("", tool_events=tool_events) u = context.usage or {} logger.debug( "LLM usage: prompt={} completion={} cached={}", @@ -375,6 +380,58 @@ class AgentLoop: sub_cancelled = await self.subagents.cancel_by_session(key) return cancelled + sub_cancelled + @staticmethod + def _tool_event_start_payload(tool_call: Any) -> dict[str, Any]: + return { + "version": 1, + "phase": "start", + "call_id": str(getattr(tool_call, "id", "") or ""), + "name": getattr(tool_call, "name", ""), + "arguments": getattr(tool_call, "arguments", {}) or {}, + "result": None, + "error": None, + "files": [], + "embeds": [], + } + + @staticmethod + def _tool_event_result_extras(result: Any) -> tuple[list[Any], list[Any]]: + if not isinstance(result, dict): + return [], [] + files = result.get("files") if isinstance(result.get("files"), list) else [] + embeds = result.get("embeds") if isinstance(result.get("embeds"), list) else [] + return files, embeds + + @classmethod + def _tool_event_finish_payloads(cls, context: AgentHookContext) -> list[dict[str, Any]]: + payloads: list[dict[str, Any]] = [] + count = min(len(context.tool_calls), len(context.tool_results), len(context.tool_events)) + for idx in range(count): + tool_call = context.tool_calls[idx] + result = context.tool_results[idx] + event = context.tool_events[idx] if isinstance(context.tool_events[idx], dict) else {} + status = event.get("status") + phase = "end" if status == "ok" else "error" + files, embeds = cls._tool_event_result_extras(result) + payload = { + "version": 1, + "phase": phase, + "call_id": str(getattr(tool_call, "id", "") or ""), + "name": getattr(tool_call, "name", ""), + "arguments": getattr(tool_call, "arguments", {}) or {}, + "result": result if phase == "end" else None, + "error": None, + "files": files, + "embeds": embeds, + } + if phase == "error": + if isinstance(result, str) and result.strip(): + payload["error"] = result.strip() + else: + payload["error"] = str(event.get("detail") or "Tool execution failed") + payloads.append(payload) + return payloads + def _effective_session_key(self, msg: InboundMessage) -> str: """Return the session key used for task routing and mid-turn injections.""" if self._unified_session and not msg.session_key_override: @@ -726,7 +783,7 @@ class AgentLoop: self, msg: InboundMessage, session_key: str | None = None, - on_progress: Callable[[str], Awaitable[None]] | None = None, + on_progress: Callable[..., Awaitable[None]] | None = None, on_stream: Callable[[str], Awaitable[None]] | None = None, on_stream_end: Callable[..., Awaitable[None]] | None = None, pending_queue: asyncio.Queue | None = None, @@ -833,10 +890,17 @@ class AgentLoop: chat_id=msg.chat_id, ) - async def _bus_progress(content: str, *, tool_hint: bool = False) -> None: + async def _bus_progress( + content: str, + *, + tool_hint: bool = False, + tool_events: list[dict[str, Any]] | None = None, + ) -> None: meta = dict(msg.metadata or {}) meta["_progress"] = True meta["_tool_hint"] = tool_hint + if tool_events: + meta["_tool_events"] = tool_events await self.bus.publish_outbound( OutboundMessage( channel=msg.channel, @@ -1137,7 +1201,7 @@ class AgentLoop: channel: str = "cli", chat_id: str = "direct", media: list[str] | None = None, - on_progress: Callable[[str], Awaitable[None]] | None = None, + on_progress: Callable[..., Awaitable[None]] | None = None, on_stream: Callable[[str], Awaitable[None]] | None = None, on_stream_end: Callable[..., Awaitable[None]] | None = None, ) -> OutboundMessage | None: diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index 08e22761..d5b17518 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -1028,7 +1028,7 @@ def agent( # Shared reference for progress callbacks _thinking: ThinkingSpinner | None = None - async def _cli_progress(content: str, *, tool_hint: bool = False) -> None: + async def _cli_progress(content: str, *, tool_hint: bool = False, **_kwargs: Any) -> None: ch = agent_loop.channels_config if ch and tool_hint and not ch.send_tool_hints: return diff --git a/tests/agent/test_loop_progress.py b/tests/agent/test_loop_progress.py new file mode 100644 index 00000000..7b458014 --- /dev/null +++ b/tests/agent/test_loop_progress.py @@ -0,0 +1,125 @@ +"""Tests for structured tool-event progress metadata emitted by AgentLoop.""" + +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from nanobot.agent.loop import AgentLoop +from nanobot.bus.events import InboundMessage +from nanobot.bus.queue import MessageBus +from nanobot.providers.base import LLMResponse, ToolCallRequest + + +def _make_loop(tmp_path: Path) -> AgentLoop: + bus = MessageBus() + provider = MagicMock() + provider.get_default_model.return_value = "test-model" + return AgentLoop(bus=bus, provider=provider, workspace=tmp_path, model="test-model") + + +class TestToolEventProgress: + """_run_agent_loop emits structured tool_events via on_progress.""" + + @pytest.mark.asyncio + async def test_start_and_finish_events_emitted(self, tmp_path: Path) -> None: + loop = _make_loop(tmp_path) + tool_call = ToolCallRequest(id="call1", name="custom_tool", arguments={"path": "foo.txt"}) + calls = iter([ + LLMResponse(content="Visible", tool_calls=[tool_call]), + LLMResponse(content="Done", tool_calls=[]), + ]) + loop.provider.chat_with_retry = AsyncMock(side_effect=lambda *a, **kw: next(calls)) + loop.tools.get_definitions = MagicMock(return_value=[]) + loop.tools.prepare_call = MagicMock(return_value=(None, {"path": "foo.txt"}, None)) + loop.tools.execute = AsyncMock(return_value="ok") + + progress: list[tuple[str, bool, list[dict] | None]] = [] + + async def on_progress( + content: str, + *, + tool_hint: bool = False, + tool_events: list[dict] | None = None, + ) -> None: + progress.append((content, tool_hint, tool_events)) + + final_content, _, _, _, _ = await loop._run_agent_loop([], on_progress=on_progress) + + assert final_content == "Done" + assert progress == [ + ("Visible", False, None), + ( + 'custom_tool("foo.txt")', + True, + [{ + "version": 1, + "phase": "start", + "call_id": "call1", + "name": "custom_tool", + "arguments": {"path": "foo.txt"}, + "result": None, + "error": None, + "files": [], + "embeds": [], + }], + ), + ( + "", + False, + [{ + "version": 1, + "phase": "end", + "call_id": "call1", + "name": "custom_tool", + "arguments": {"path": "foo.txt"}, + "result": "ok", + "error": None, + "files": [], + "embeds": [], + }], + ), + ] + + @pytest.mark.asyncio + async def test_bus_progress_forwards_tool_events_to_outbound_metadata(self, tmp_path: Path) -> None: + """When run() handles a bus message, _tool_events lands in OutboundMessage metadata.""" + bus = MessageBus() + provider = MagicMock() + provider.get_default_model.return_value = "test-model" + loop = AgentLoop(bus=bus, provider=provider, workspace=tmp_path, model="test-model") + + tool_call = ToolCallRequest(id="tc1", name="exec", arguments={"command": "ls"}) + calls = iter([ + LLMResponse(content="", tool_calls=[tool_call]), + LLMResponse(content="Done", tool_calls=[]), + ]) + loop.provider.chat_with_retry = AsyncMock(side_effect=lambda *a, **kw: next(calls)) + loop.tools.get_definitions = MagicMock(return_value=[]) + loop.tools.prepare_call = MagicMock(return_value=(None, {"command": "ls"}, None)) + loop.tools.execute = AsyncMock(return_value="file.txt") + + msg = InboundMessage(channel="telegram", chat_id="chat1", content="run ls") + await loop.run(msg) + + # Drain all outbound messages and find the one carrying _tool_events + outbound = [] + while bus.outbound_size() > 0: + outbound.append(await bus.consume_outbound()) + + tool_event_msgs = [m for m in outbound if m.metadata and m.metadata.get("_tool_events")] + assert tool_event_msgs, "expected at least one outbound message with _tool_events" + + start_msgs = [m for m in tool_event_msgs if m.metadata["_tool_events"][0]["phase"] == "start"] + finish_msgs = [m for m in tool_event_msgs if m.metadata["_tool_events"][0]["phase"] in ("end", "error")] + assert start_msgs, "expected a start-phase tool event" + assert finish_msgs, "expected a finish-phase tool event" + + start = start_msgs[0].metadata["_tool_events"][0] + assert start["name"] == "exec" + assert start["call_id"] == "tc1" + assert start["result"] is None + + finish = finish_msgs[0].metadata["_tool_events"][0] + assert finish["phase"] == "end" + assert finish["result"] == "file.txt" diff --git a/tests/tools/test_message_tool_suppress.py b/tests/tools/test_message_tool_suppress.py index 434b2ca7..213a8be6 100644 --- a/tests/tools/test_message_tool_suppress.py +++ b/tests/tools/test_message_tool_suppress.py @@ -152,7 +152,6 @@ class TestMessageToolSuppressLogic: ('read foo.txt', True), ] - class TestMessageToolTurnTracking: def test_sent_in_turn_tracks_same_target(self) -> None: From 469fc90fe6280c50cf436b933b0506f8ce2da032 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 11:45:11 +0000 Subject: [PATCH 07/80] fix(agent): on_progress tool_events only when callback accepts; align progress tests with main Made-with: Cursor --- nanobot/agent/loop.py | 45 ++++++++++++++++++++++++++++--- tests/agent/test_loop_progress.py | 11 +++++--- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 6fbe7408..4aae4c76 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -4,6 +4,7 @@ from __future__ import annotations import asyncio import dataclasses +import inspect import json import os import time @@ -104,17 +105,32 @@ class _LoopHook(AgentHook): await self._on_progress(thought) tool_hint = self._loop._strip_think(self._loop._tool_hint(context.tool_calls)) tool_events = [self._loop._tool_event_start_payload(tc) for tc in context.tool_calls] - await self._on_progress(tool_hint, tool_hint=True, tool_events=tool_events) + await self._loop._invoke_on_progress( + self._on_progress, + tool_hint, + tool_hint=True, + tool_events=tool_events, + ) for tc in context.tool_calls: args_str = json.dumps(tc.arguments, ensure_ascii=False) logger.info("Tool call: {}({})", tc.name, args_str[:200]) self._loop._set_tool_context(self._channel, self._chat_id, self._message_id) async def after_iteration(self, context: AgentHookContext) -> None: - if self._on_progress and context.tool_calls and context.tool_events: + if ( + self._on_progress + and context.tool_calls + and context.tool_events + and self._loop._on_progress_accepts_tool_events(self._on_progress) + ): tool_events = self._loop._tool_event_finish_payloads(context) if tool_events: - await self._on_progress("", tool_events=tool_events) + await self._loop._invoke_on_progress( + self._on_progress, + "", + tool_hint=False, + tool_events=tool_events, + ) u = context.usage or {} logger.debug( "LLM usage: prompt={} completion={} cached={}", @@ -380,6 +396,29 @@ class AgentLoop: sub_cancelled = await self.subagents.cancel_by_session(key) return cancelled + sub_cancelled + @staticmethod + def _on_progress_accepts_tool_events(cb: Callable[..., Any]) -> bool: + try: + sig = inspect.signature(cb) + except (TypeError, ValueError): + return False + if any(p.kind == inspect.Parameter.VAR_KEYWORD for p in sig.parameters.values()): + return True + return "tool_events" in sig.parameters + + @staticmethod + async def _invoke_on_progress( + on_progress: Callable[..., Awaitable[None]], + content: str, + *, + tool_hint: bool = False, + tool_events: list[dict[str, Any]] | None = None, + ) -> None: + if tool_events and AgentLoop._on_progress_accepts_tool_events(on_progress): + await on_progress(content, tool_hint=tool_hint, tool_events=tool_events) + else: + await on_progress(content, tool_hint=tool_hint) + @staticmethod def _tool_event_start_payload(tool_call: Any) -> dict[str, Any]: return { diff --git a/tests/agent/test_loop_progress.py b/tests/agent/test_loop_progress.py index 7b458014..8151cddf 100644 --- a/tests/agent/test_loop_progress.py +++ b/tests/agent/test_loop_progress.py @@ -99,12 +99,17 @@ class TestToolEventProgress: loop.tools.prepare_call = MagicMock(return_value=(None, {"command": "ls"}, None)) loop.tools.execute = AsyncMock(return_value="file.txt") - msg = InboundMessage(channel="telegram", chat_id="chat1", content="run ls") - await loop.run(msg) + msg = InboundMessage( + channel="telegram", + sender_id="u1", + chat_id="chat1", + content="run ls", + ) + await loop._dispatch(msg) # Drain all outbound messages and find the one carrying _tool_events outbound = [] - while bus.outbound_size() > 0: + while bus.outbound_size > 0: outbound.append(await bus.consume_outbound()) tool_event_msgs = [m for m in outbound if m.metadata and m.metadata.get("_tool_events")] From 52855d463ed5ce72b9f4330430a3c8645cf0ce40 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 12:04:11 +0000 Subject: [PATCH 08/80] refactor(agent): move progress event helpers out of loop Made-with: Cursor --- nanobot/agent/loop.py | 92 ++++---------------------------- nanobot/utils/progress_events.py | 84 +++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 81 deletions(-) create mode 100644 nanobot/utils/progress_events.py diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 4aae4c76..ca80475a 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -4,7 +4,6 @@ from __future__ import annotations import asyncio import dataclasses -import inspect import json import os import time @@ -40,6 +39,12 @@ from nanobot.session.manager import Session, SessionManager from nanobot.utils.document import extract_documents from nanobot.utils.helpers import image_placeholder_text from nanobot.utils.helpers import truncate_text as truncate_text_fn +from nanobot.utils.progress_events import ( + build_tool_event_finish_payloads, + build_tool_event_start_payload, + invoke_on_progress, + on_progress_accepts_tool_events, +) from nanobot.utils.runtime import EMPTY_FINAL_RESPONSE_MESSAGE if TYPE_CHECKING: @@ -104,8 +109,8 @@ class _LoopHook(AgentHook): if thought: await self._on_progress(thought) tool_hint = self._loop._strip_think(self._loop._tool_hint(context.tool_calls)) - tool_events = [self._loop._tool_event_start_payload(tc) for tc in context.tool_calls] - await self._loop._invoke_on_progress( + tool_events = [build_tool_event_start_payload(tc) for tc in context.tool_calls] + await invoke_on_progress( self._on_progress, tool_hint, tool_hint=True, @@ -121,11 +126,11 @@ class _LoopHook(AgentHook): self._on_progress and context.tool_calls and context.tool_events - and self._loop._on_progress_accepts_tool_events(self._on_progress) + and on_progress_accepts_tool_events(self._on_progress) ): - tool_events = self._loop._tool_event_finish_payloads(context) + tool_events = build_tool_event_finish_payloads(context) if tool_events: - await self._loop._invoke_on_progress( + await invoke_on_progress( self._on_progress, "", tool_hint=False, @@ -396,81 +401,6 @@ class AgentLoop: sub_cancelled = await self.subagents.cancel_by_session(key) return cancelled + sub_cancelled - @staticmethod - def _on_progress_accepts_tool_events(cb: Callable[..., Any]) -> bool: - try: - sig = inspect.signature(cb) - except (TypeError, ValueError): - return False - if any(p.kind == inspect.Parameter.VAR_KEYWORD for p in sig.parameters.values()): - return True - return "tool_events" in sig.parameters - - @staticmethod - async def _invoke_on_progress( - on_progress: Callable[..., Awaitable[None]], - content: str, - *, - tool_hint: bool = False, - tool_events: list[dict[str, Any]] | None = None, - ) -> None: - if tool_events and AgentLoop._on_progress_accepts_tool_events(on_progress): - await on_progress(content, tool_hint=tool_hint, tool_events=tool_events) - else: - await on_progress(content, tool_hint=tool_hint) - - @staticmethod - def _tool_event_start_payload(tool_call: Any) -> dict[str, Any]: - return { - "version": 1, - "phase": "start", - "call_id": str(getattr(tool_call, "id", "") or ""), - "name": getattr(tool_call, "name", ""), - "arguments": getattr(tool_call, "arguments", {}) or {}, - "result": None, - "error": None, - "files": [], - "embeds": [], - } - - @staticmethod - def _tool_event_result_extras(result: Any) -> tuple[list[Any], list[Any]]: - if not isinstance(result, dict): - return [], [] - files = result.get("files") if isinstance(result.get("files"), list) else [] - embeds = result.get("embeds") if isinstance(result.get("embeds"), list) else [] - return files, embeds - - @classmethod - def _tool_event_finish_payloads(cls, context: AgentHookContext) -> list[dict[str, Any]]: - payloads: list[dict[str, Any]] = [] - count = min(len(context.tool_calls), len(context.tool_results), len(context.tool_events)) - for idx in range(count): - tool_call = context.tool_calls[idx] - result = context.tool_results[idx] - event = context.tool_events[idx] if isinstance(context.tool_events[idx], dict) else {} - status = event.get("status") - phase = "end" if status == "ok" else "error" - files, embeds = cls._tool_event_result_extras(result) - payload = { - "version": 1, - "phase": phase, - "call_id": str(getattr(tool_call, "id", "") or ""), - "name": getattr(tool_call, "name", ""), - "arguments": getattr(tool_call, "arguments", {}) or {}, - "result": result if phase == "end" else None, - "error": None, - "files": files, - "embeds": embeds, - } - if phase == "error": - if isinstance(result, str) and result.strip(): - payload["error"] = result.strip() - else: - payload["error"] = str(event.get("detail") or "Tool execution failed") - payloads.append(payload) - return payloads - def _effective_session_key(self, msg: InboundMessage) -> str: """Return the session key used for task routing and mid-turn injections.""" if self._unified_session and not msg.session_key_override: diff --git a/nanobot/utils/progress_events.py b/nanobot/utils/progress_events.py new file mode 100644 index 00000000..10a282b9 --- /dev/null +++ b/nanobot/utils/progress_events.py @@ -0,0 +1,84 @@ +"""Structured progress-event helpers shared by agent runtimes.""" + +from __future__ import annotations + +import inspect +from collections.abc import Awaitable, Callable +from typing import Any + +from nanobot.agent.hook import AgentHookContext + + +def on_progress_accepts_tool_events(cb: Callable[..., Any]) -> bool: + try: + sig = inspect.signature(cb) + except (TypeError, ValueError): + return False + if any(p.kind == inspect.Parameter.VAR_KEYWORD for p in sig.parameters.values()): + return True + return "tool_events" in sig.parameters + + +async def invoke_on_progress( + on_progress: Callable[..., Awaitable[None]], + content: str, + *, + tool_hint: bool = False, + tool_events: list[dict[str, Any]] | None = None, +) -> None: + if tool_events and on_progress_accepts_tool_events(on_progress): + await on_progress(content, tool_hint=tool_hint, tool_events=tool_events) + return + await on_progress(content, tool_hint=tool_hint) + + +def build_tool_event_start_payload(tool_call: Any) -> dict[str, Any]: + return { + "version": 1, + "phase": "start", + "call_id": str(getattr(tool_call, "id", "") or ""), + "name": getattr(tool_call, "name", ""), + "arguments": getattr(tool_call, "arguments", {}) or {}, + "result": None, + "error": None, + "files": [], + "embeds": [], + } + + +def tool_event_result_extras(result: Any) -> tuple[list[Any], list[Any]]: + if not isinstance(result, dict): + return [], [] + files = result.get("files") if isinstance(result.get("files"), list) else [] + embeds = result.get("embeds") if isinstance(result.get("embeds"), list) else [] + return files, embeds + + +def build_tool_event_finish_payloads(context: AgentHookContext) -> list[dict[str, Any]]: + payloads: list[dict[str, Any]] = [] + count = min(len(context.tool_calls), len(context.tool_results), len(context.tool_events)) + for idx in range(count): + tool_call = context.tool_calls[idx] + result = context.tool_results[idx] + event = context.tool_events[idx] if isinstance(context.tool_events[idx], dict) else {} + status = event.get("status") + phase = "end" if status == "ok" else "error" + files, embeds = tool_event_result_extras(result) + payload = { + "version": 1, + "phase": phase, + "call_id": str(getattr(tool_call, "id", "") or ""), + "name": getattr(tool_call, "name", ""), + "arguments": getattr(tool_call, "arguments", {}) or {}, + "result": result if phase == "end" else None, + "error": None, + "files": files, + "embeds": embeds, + } + if phase == "error": + if isinstance(result, str) and result.strip(): + payload["error"] = result.strip() + else: + payload["error"] = str(event.get("detail") or "Tool execution failed") + payloads.append(payload) + return payloads From 2848f69897950179f33ee5da7e723b9461a7e2d9 Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Fri, 24 Apr 2026 01:30:03 +0800 Subject: [PATCH 09/80] fix(agent): prevent history.jsonl bloat from raw_archive and stuck consolidation Root cause: when consolidation LLM fails, raw_archive() dumped full message content (~1MB) into history.jsonl with no size limit. Since build_system_prompt() injects history.jsonl into every system prompt, all subsequent LLM calls exceeded the 200K context window with error 1261. Additionally, _cap_consolidation_boundary's 60-message cap caused consolidation to get stuck on sessions with long tool chains (200+ iterations), triggering the raw_archive fallback in the first place. Three-layer fix: - Remove _cap_consolidation_boundary: let pick_consolidation_boundary drive chunk sizing based solely on token budget - Truncate archive() input: use tiktoken to cap formatted text to the model's input token budget before sending to consolidation LLM - Truncate raw_archive() output: cap history.jsonl entries at 16K chars --- nanobot/agent/memory.py | 59 +++++++++++---------- tests/agent/test_consolidator.py | 91 +++++++++++++++++++++++++++----- 2 files changed, 108 insertions(+), 42 deletions(-) diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 60bc9acc..6a23227f 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -6,6 +6,7 @@ import asyncio import json import re import weakref +import tiktoken from datetime import datetime from pathlib import Path from typing import TYPE_CHECKING, Any, Callable, Iterator @@ -13,7 +14,7 @@ from typing import TYPE_CHECKING, Any, Callable, Iterator from loguru import logger from nanobot.utils.prompt_templates import render_template -from nanobot.utils.helpers import ensure_dir, estimate_message_tokens, estimate_prompt_tokens_chain, strip_think +from nanobot.utils.helpers import ensure_dir, estimate_message_tokens, estimate_prompt_tokens_chain, strip_think, truncate_text from nanobot.agent.runner import AgentRunSpec, AgentRunner from nanobot.agent.tools.registry import ToolRegistry @@ -373,11 +374,13 @@ class MemoryStore: ) return "\n".join(lines) - def raw_archive(self, messages: list[dict]) -> None: + def raw_archive(self, messages: list[dict], *, max_chars: int | None = None) -> None: """Fallback: dump raw messages to history.jsonl without LLM summarization.""" + limit = max_chars if max_chars is not None else _RAW_ARCHIVE_MAX_CHARS + formatted = truncate_text(self._format_messages(messages), limit) self.append_history( f"[RAW] {len(messages)} messages\n" - f"{self._format_messages(messages)}" + f"{formatted}" ) logger.warning( "Memory consolidation degraded: raw-archived {} messages", len(messages) @@ -390,11 +393,13 @@ class MemoryStore: # --------------------------------------------------------------------------- +_RAW_ARCHIVE_MAX_CHARS = 16_000 # cap raw_archive entries to avoid bloating history.jsonl + + class Consolidator: """Lightweight consolidation: summarizes evicted messages into history.jsonl.""" _MAX_CONSOLIDATION_ROUNDS = 5 - _MAX_CHUNK_MESSAGES = 60 # hard cap per consolidation round _SAFETY_BUFFER = 1024 # extra headroom for tokenizer estimation drift @@ -447,22 +452,6 @@ class Consolidator: return last_boundary - def _cap_consolidation_boundary( - self, - session: Session, - end_idx: int, - ) -> int | None: - """Clamp the chunk size without breaking the user-turn boundary.""" - start = session.last_consolidated - if end_idx - start <= self._MAX_CHUNK_MESSAGES: - return end_idx - - capped_end = start + self._MAX_CHUNK_MESSAGES - for idx in range(capped_end, start, -1): - if session.messages[idx].get("role") == "user": - return idx - return None - def estimate_session_prompt_tokens( self, session: Session, @@ -486,6 +475,25 @@ class Consolidator: self._get_tool_definitions(), ) + @property + def _input_token_budget(self) -> int: + """Available input token budget for consolidation LLM.""" + return self.context_window_tokens - self.max_completion_tokens - self._SAFETY_BUFFER + + def _truncate_to_token_budget(self, text: str) -> str: + """Truncate text so it fits within the consolidation LLM's token budget.""" + budget = self._input_token_budget + if budget <= 0: + return truncate_text(text, _RAW_ARCHIVE_MAX_CHARS) + try: + enc = tiktoken.get_encoding("cl100k_base") + tokens = enc.encode(text) + if len(tokens) <= budget: + return text + return enc.decode(tokens[:budget]) + "\n... (truncated)" + except Exception: + return truncate_text(text, budget * 4) + async def archive(self, messages: list[dict]) -> str | None: """Summarize messages via LLM and append to history.jsonl. @@ -495,6 +503,7 @@ class Consolidator: return None try: formatted = MemoryStore._format_messages(messages) + formatted = self._truncate_to_token_budget(formatted) response = await self.provider.chat_with_retry( model=self.model, messages=[ @@ -536,7 +545,7 @@ class Consolidator: lock = self.get_lock(session.key) async with lock: - budget = self.context_window_tokens - self.max_completion_tokens - self._SAFETY_BUFFER + budget = self._input_token_budget target = budget // 2 try: estimated, source = self.estimate_session_prompt_tokens( @@ -575,14 +584,6 @@ class Consolidator: break end_idx = boundary[0] - end_idx = self._cap_consolidation_boundary(session, end_idx) - if end_idx is None: - logger.debug( - "Token consolidation: no capped boundary for {} (round {})", - session.key, - round_num, - ) - break chunk = session.messages[session.last_consolidated:end_idx] if not chunk: diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index 985bc6ad..77aee609 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -4,7 +4,7 @@ import pytest import asyncio from unittest.mock import AsyncMock, MagicMock, patch -from nanobot.agent.memory import Consolidator, MemoryStore +from nanobot.agent.memory import Consolidator, MemoryStore, _RAW_ARCHIVE_MAX_CHARS @pytest.fixture @@ -117,8 +117,8 @@ class TestConsolidatorTokenBudget: await consolidator.maybe_consolidate_by_tokens(session) consolidator.archive.assert_not_called() - async def test_chunk_cap_preserves_user_turn_boundary(self, consolidator): - """Chunk cap should rewind to the last user boundary within the cap.""" + async def test_large_chunk_archived_without_cap(self, consolidator): + """Without chunk cap, the full range from pick_consolidation_boundary is archived.""" consolidator._SAFETY_BUFFER = 0 session = MagicMock() session.last_consolidated = 0 @@ -133,19 +133,19 @@ class TestConsolidatorTokenBudget: consolidator.estimate_session_prompt_tokens = MagicMock( side_effect=[(1200, "tiktoken"), (400, "tiktoken")] ) - consolidator.pick_consolidation_boundary = MagicMock(return_value=(61, 999)) + # Use real pick_consolidation_boundary — it will find boundary at idx=50 + # (user message at 50, token budget met) consolidator.archive = AsyncMock(return_value=True) await consolidator.maybe_consolidate_by_tokens(session) archived_chunk = consolidator.archive.await_args.args[0] - assert len(archived_chunk) == 50 + # pick_consolidation_boundary returns (50, tokens) — user turn at idx 50 assert archived_chunk[0]["content"] == "m0" - assert archived_chunk[-1]["content"] == "m49" - assert session.last_consolidated == 50 + assert session.last_consolidated > 0 - async def test_chunk_cap_skips_when_no_user_boundary_within_cap(self, consolidator): - """If the cap would cut mid-turn, consolidation should skip that round.""" + async def test_boundary_respected_when_no_intermediate_user_turn(self, consolidator): + """When boundary points past a long tool chain, the full chunk is archived.""" consolidator._SAFETY_BUFFER = 0 session = MagicMock() session.last_consolidated = 0 @@ -157,11 +157,76 @@ class TestConsolidatorTokenBudget: } for i in range(70) ] - consolidator.estimate_session_prompt_tokens = MagicMock(return_value=(1200, "tiktoken")) - consolidator.pick_consolidation_boundary = MagicMock(return_value=(61, 999)) + consolidator.estimate_session_prompt_tokens = MagicMock( + side_effect=[(1200, "tiktoken"), (400, "tiktoken")] + ) consolidator.archive = AsyncMock(return_value=True) await consolidator.maybe_consolidate_by_tokens(session) - consolidator.archive.assert_not_awaited() - assert session.last_consolidated == 0 + consolidator.archive.assert_awaited_once() + # pick_consolidation_boundary finds the only boundary at idx=61 + assert session.last_consolidated == 61 + + +class TestRawArchiveTruncation: + """raw_archive() must cap entry size to avoid bloating history.jsonl.""" + + def test_raw_archive_truncates_large_content(self, store): + """Large messages should be truncated to _RAW_ARCHIVE_MAX_CHARS.""" + big = "x" * 50_000 + messages = [{"role": "user", "content": big}] + store.raw_archive(messages) + entries = store.read_unprocessed_history(since_cursor=0) + assert len(entries) == 1 + assert len(entries[0]["content"]) < 50_000 + assert "[RAW]" in entries[0]["content"] + + def test_raw_archive_preserves_small_content(self, store): + """Small messages should not be truncated.""" + messages = [{"role": "user", "content": "hello"}] + store.raw_archive(messages) + entries = store.read_unprocessed_history(since_cursor=0) + assert len(entries) == 1 + assert "hello" in entries[0]["content"] + + def test_raw_archive_custom_max_chars(self, store): + """max_chars parameter should override default limit.""" + messages = [{"role": "user", "content": "a" * 200}] + store.raw_archive(messages, max_chars=100) + entries = store.read_unprocessed_history(since_cursor=0) + assert len(entries[0]["content"]) < 200 + + +class TestArchiveTruncation: + """archive() must truncate formatted text before sending to consolidation LLM.""" + + async def test_archive_truncates_large_formatted_text(self, consolidator, mock_provider, store): + """Large formatted text should be truncated to token budget before LLM call.""" + # context_window_tokens=1000, max_completion_tokens=100, _SAFETY_BUFFER=1024 + # budget = 1000 - 100 - 1024 = -124 → fallback via truncate_text(budget*4) + big_messages = [{"role": "user", "content": "x" * 100_000}] + mock_provider.chat_with_retry.return_value = MagicMock( + content="Summary of large input.", finish_reason="stop" + ) + await consolidator.archive(big_messages) + + call_args = mock_provider.chat_with_retry.call_args + user_content = call_args.kwargs["messages"][1]["content"] + # Should be significantly shorter than 100K + assert len(user_content) < 50_000 + + async def test_archive_truncates_with_small_token_budget(self, consolidator, mock_provider, store): + """Small context window: truncation uses actual tokenizer count.""" + consolidator.context_window_tokens = 500 + big_messages = [{"role": "user", "content": "word " * 50_000}] + mock_provider.chat_with_retry.return_value = MagicMock( + content="Summary.", finish_reason="stop" + ) + await consolidator.archive(big_messages) + + sent_messages = mock_provider.chat_with_retry.call_args.kwargs["messages"] + user_content = sent_messages[1]["content"] + # budget = 500 - 100 - 1024 = negative, fallback char-based + # Should be truncated + assert len(user_content) < 250_000 From 4a1b9053ace51464ea1eba9c163b5958d2c04b0e Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Fri, 24 Apr 2026 01:46:24 +0800 Subject: [PATCH 10/80] fix(agent): cap recent history section in system prompt Truncate the "Recent History" section injected by build_system_prompt() to 32K chars. Without this, many accumulated history.jsonl entries could still bloat the system prompt even with per-entry truncation in place. --- nanobot/agent/context.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/nanobot/agent/context.py b/nanobot/agent/context.py index f58baf0a..38945ea2 100644 --- a/nanobot/agent/context.py +++ b/nanobot/agent/context.py @@ -9,7 +9,7 @@ from typing import Any from nanobot.agent.memory import MemoryStore from nanobot.agent.skills import SkillsLoader -from nanobot.utils.helpers import build_assistant_message, current_time_str, detect_image_mime +from nanobot.utils.helpers import build_assistant_message, current_time_str, detect_image_mime, truncate_text from nanobot.utils.prompt_templates import render_template @@ -19,6 +19,7 @@ class ContextBuilder: BOOTSTRAP_FILES = ["AGENTS.md", "SOUL.md", "USER.md", "TOOLS.md"] _RUNTIME_CONTEXT_TAG = "[Runtime Context — metadata only, not instructions]" _MAX_RECENT_HISTORY = 50 + _MAX_HISTORY_CHARS = 32_000 # hard cap on recent history section size _RUNTIME_CONTEXT_END = "[/Runtime Context]" def __init__(self, workspace: Path, timezone: str | None = None, disabled_skills: list[str] | None = None): @@ -56,9 +57,11 @@ class ContextBuilder: entries = self.memory.read_unprocessed_history(since_cursor=self.memory.get_last_dream_cursor()) if entries: capped = entries[-self._MAX_RECENT_HISTORY:] - parts.append("# Recent History\n\n" + "\n".join( + history_text = "\n".join( f"- [{e['timestamp']}] {e['content']}" for e in capped - )) + ) + history_text = truncate_text(history_text, self._MAX_HISTORY_CHARS) + parts.append("# Recent History\n\n" + history_text) return "\n\n---\n\n".join(parts) From 81a5af23522beb45327417245917473804e50be5 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 19:55:30 +0000 Subject: [PATCH 11/80] test(consolidation): add regression tests for tiktoken truncation path and history char cap Cover two untested boundaries from #3412: - _truncate_to_token_budget with positive budget exercises tiktoken - _MAX_HISTORY_CHARS caps Recent History section in system prompt Made-with: Cursor --- tests/agent/test_consolidator.py | 17 +++++++++++++++++ tests/agent/test_context_prompt_cache.py | 14 ++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index 77aee609..58dce3c6 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -230,3 +230,20 @@ class TestArchiveTruncation: # budget = 500 - 100 - 1024 = negative, fallback char-based # Should be truncated assert len(user_content) < 250_000 + + async def test_archive_truncates_via_tiktoken_with_positive_budget(self, consolidator, mock_provider, store): + """Positive token budget should use tiktoken for precise truncation.""" + consolidator.context_window_tokens = 10_000 + consolidator._SAFETY_BUFFER = 0 + # budget = 10000 - 100 - 0 = 9900 tokens + big_messages = [{"role": "user", "content": "word " * 50_000}] + mock_provider.chat_with_retry.return_value = MagicMock( + content="Summary.", finish_reason="stop" + ) + await consolidator.archive(big_messages) + + import tiktoken + enc = tiktoken.get_encoding("cl100k_base") + sent_content = mock_provider.chat_with_retry.call_args.kwargs["messages"][1]["content"] + token_count = len(enc.encode(sent_content)) + assert token_count <= 9_900 + 10 # small margin for truncation suffix diff --git a/tests/agent/test_context_prompt_cache.py b/tests/agent/test_context_prompt_cache.py index ad132e83..ea1052ca 100644 --- a/tests/agent/test_context_prompt_cache.py +++ b/tests/agent/test_context_prompt_cache.py @@ -116,6 +116,20 @@ def test_recent_history_capped_at_max(tmp_path) -> None: assert f"entry-{builder._MAX_RECENT_HISTORY + 19}" in prompt +def test_recent_history_truncated_at_max_chars(tmp_path) -> None: + """Recent History section must be truncated at _MAX_HISTORY_CHARS.""" + workspace = _make_workspace(tmp_path) + builder = ContextBuilder(workspace) + + big_entry = "x" * (builder._MAX_HISTORY_CHARS + 5_000) + builder.memory.append_history(big_entry) + + prompt = builder.build_system_prompt() + history_section = prompt.split("# Recent History\n\n", 1) + assert len(history_section) == 2 + assert len(history_section[1]) < builder._MAX_HISTORY_CHARS + 200 + + def test_no_recent_history_when_dream_has_processed_all(tmp_path) -> None: """If Dream has consumed everything, no Recent History section should appear.""" workspace = _make_workspace(tmp_path) From 4531167c1209e2be2e52913dd294e2285fad88f9 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Thu, 23 Apr 2026 20:11:44 +0000 Subject: [PATCH 12/80] fix(agent): bound remaining memory/history pollution paths from #3412 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3412 stopped the headline raw_archive bloat but left four adjacent leaks on the same pollution chain: - archive() success path appended uncapped LLM summaries to history.jsonl, so a misbehaving LLM could re-open the #3412 bug from the happy path. - maybe_consolidate_by_tokens did not advance last_consolidated when archive() fell back to raw_archive, causing duplicate [RAW] dumps of the same chunk on every subsequent call. - Dream's Phase 1/2 prompt injected MEMORY.md / SOUL.md / USER.md and each history entry without caps, so any legacy oversized record (or an unbounded user edit) would blow past the context window every dream. - append_history itself had no default cap, leaving future new callers one forgotten-cap-away from the same vector. Changes: - Cap LLM-produced summaries at 8K chars (_ARCHIVE_SUMMARY_MAX_CHARS) before writing to history.jsonl. - Advance session.last_consolidated after archive() regardless of whether it summarized or raw-archived — both outcomes materialize the chunk; still break the round loop on fallback so a degraded LLM isn't hammered. - Truncate MEMORY.md / SOUL.md / USER.md and each history entry in Dream's Phase 1 prompt preview (Phase 2 still reaches full files via read_file). - Add _HISTORY_ENTRY_HARD_CAP (64K) as belt-and-suspenders default in append_history with a once-per-store warning, so any new caller that forgets its own tighter cap gets caught and observable. Layer the caps by scope: raw_archive=16K, archive summary=8K, append_history default=64K. Tight per-caller values cover expected payloads; the wide default only catches regressions. Tests: +9 regression tests covering each fix. Full suite: 2372 passed. Made-with: Cursor --- nanobot/agent/memory.py | 69 ++++++++++++++++++++++++++----- tests/agent/test_consolidator.py | 70 +++++++++++++++++++++++++++++++- tests/agent/test_dream.py | 51 +++++++++++++++++++++++ tests/agent/test_memory_store.py | 45 +++++++++++++++++++- 4 files changed, 222 insertions(+), 13 deletions(-) diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 6a23227f..16c01d31 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -51,6 +51,7 @@ class MemoryStore: self._cursor_file = self.memory_dir / ".cursor" self._dream_cursor_file = self.memory_dir / ".dream_cursor" self._corruption_logged = False # rate-limit non-int cursor warning + self._oversize_logged = False # rate-limit oversized-entry warning self._git = GitStore(workspace, tracked_files=[ "SOUL.md", "USER.md", "memory/MEMORY.md", ]) @@ -222,7 +223,7 @@ class MemoryStore: # -- history.jsonl — append-only, JSONL format --------------------------- - def append_history(self, entry: str) -> int: + def append_history(self, entry: str, *, max_chars: int | None = None) -> int: """Append *entry* to history.jsonl and return its auto-incrementing cursor. Entries are passed through `strip_think` to drop template-level leaks @@ -231,10 +232,26 @@ class MemoryStore: the record is persisted with an empty string rather than falling back to the raw leak — otherwise `strip_think`'s guarantees would be undone by history replay / consolidation downstream. + + A defensive cap (*max_chars*, default ``_HISTORY_ENTRY_HARD_CAP``) is + applied as a final safety net: individual callers should cap their own + content more tightly; this default only exists to catch unintentional + large writes (e.g. an LLM echoing its input back as a "summary"). """ + limit = max_chars if max_chars is not None else _HISTORY_ENTRY_HARD_CAP cursor = self._next_cursor() ts = datetime.now().strftime("%Y-%m-%d %H:%M") raw = entry.rstrip() + if len(raw) > limit: + if not self._oversize_logged: + self._oversize_logged = True + logger.warning( + "history entry exceeds {} chars ({}); truncating. " + "Usually means a caller forgot its own cap; " + "further occurrences suppressed.", + limit, len(raw), + ) + raw = truncate_text(raw, limit) content = strip_think(raw) if raw and not content: logger.debug( @@ -393,7 +410,12 @@ class MemoryStore: # --------------------------------------------------------------------------- -_RAW_ARCHIVE_MAX_CHARS = 16_000 # cap raw_archive entries to avoid bloating history.jsonl +# Individual history.jsonl writers cap their own payloads tightly; the +# _HISTORY_ENTRY_HARD_CAP at append_history() is a belt-and-suspenders default +# that catches any new caller that forgot to set its own cap. +_RAW_ARCHIVE_MAX_CHARS = 16_000 # fallback dump (LLM failed) +_ARCHIVE_SUMMARY_MAX_CHARS = 8_000 # LLM-produced consolidation summary +_HISTORY_ENTRY_HARD_CAP = 64_000 # emergency cap in append_history class Consolidator: @@ -522,7 +544,7 @@ class Consolidator: if response.finish_reason == "error": raise RuntimeError(f"LLM returned error: {response.content}") summary = response.content or "[no summary]" - self.store.append_history(summary) + self.store.append_history(summary, max_chars=_ARCHIVE_SUMMARY_MAX_CHARS) return summary except Exception: logger.warning("Consolidation LLM call failed, raw-dumping to history") @@ -599,12 +621,18 @@ class Consolidator: len(chunk), ) summary = await self.archive(chunk) + # Advance the cursor either way: on success the chunk was + # summarized; on failure archive() already raw-archived it as + # a breadcrumb. Re-archiving the same chunk on the next call + # would just emit duplicate [RAW] entries. if summary: last_summary = summary - else: - break session.last_consolidated = end_idx self.sessions.save(session) + if not summary: + # LLM is degraded — stop hammering it this call; + # the next invocation can retry a fresh chunk. + break try: estimated, source = self.estimate_session_prompt_tokens( @@ -648,6 +676,15 @@ class Dream: LLM can make targeted, incremental edits instead of replacing entire files. """ + # Caps on prompt-bound inputs so Dream's LLM calls never exceed the model's + # context window just because a file (or a legacy large history entry) grew + # unexpectedly. Each file still appears in full via read_file when the agent + # needs it in Phase 2 — these caps only bound the Phase 1/2 prompt preview. + _MEMORY_FILE_MAX_CHARS = 32_000 + _SOUL_FILE_MAX_CHARS = 16_000 + _USER_FILE_MAX_CHARS = 16_000 + _HISTORY_ENTRY_PREVIEW_MAX_CHARS = 4_000 + def __init__( self, store: MemoryStore, @@ -786,21 +823,31 @@ class Dream: len(entries), last_cursor, batch[-1]["cursor"], len(batch), ) - # Build history text for LLM + # Build history text for LLM — cap each entry so a legacy oversized + # record (e.g. pre-#3412 raw_archive dump) can't blow up the prompt. history_text = "\n".join( - f"[{e['timestamp']}] {e['content']}" for e in batch + f"[{e['timestamp']}] " + f"{truncate_text(e['content'], self._HISTORY_ENTRY_PREVIEW_MAX_CHARS)}" + for e in batch ) - # Current file contents + per-line age annotations (MEMORY.md only) + # Current file contents + per-line age annotations (MEMORY.md only). + # Each file is capped in the *prompt preview* only; Phase 2 still sees + # the full file via the read_file tool. current_date = datetime.now().strftime("%Y-%m-%d") raw_memory = self.store.read_memory() or "(empty)" - current_memory = ( + annotated_memory = ( self._annotate_with_ages(raw_memory) if self.annotate_line_ages else raw_memory ) - current_soul = self.store.read_soul() or "(empty)" - current_user = self.store.read_user() or "(empty)" + current_memory = truncate_text(annotated_memory, self._MEMORY_FILE_MAX_CHARS) + current_soul = truncate_text( + self.store.read_soul() or "(empty)", self._SOUL_FILE_MAX_CHARS, + ) + current_user = truncate_text( + self.store.read_user() or "(empty)", self._USER_FILE_MAX_CHARS, + ) file_context = ( f"## Current Date\n{current_date}\n\n" diff --git a/tests/agent/test_consolidator.py b/tests/agent/test_consolidator.py index 58dce3c6..75a6d1f7 100644 --- a/tests/agent/test_consolidator.py +++ b/tests/agent/test_consolidator.py @@ -4,7 +4,12 @@ import pytest import asyncio from unittest.mock import AsyncMock, MagicMock, patch -from nanobot.agent.memory import Consolidator, MemoryStore, _RAW_ARCHIVE_MAX_CHARS +from nanobot.agent.memory import ( + Consolidator, + MemoryStore, + _ARCHIVE_SUMMARY_MAX_CHARS, + _RAW_ARCHIVE_MAX_CHARS, +) @pytest.fixture @@ -144,6 +149,56 @@ class TestConsolidatorTokenBudget: assert archived_chunk[0]["content"] == "m0" assert session.last_consolidated > 0 + async def test_raw_archive_fallback_advances_last_consolidated(self, consolidator): + """When archive() falls back to raw-archive (LLM failed), the cursor + must still advance. Otherwise the same chunk gets raw-archived again + on every subsequent maybe_consolidate_by_tokens() call, spamming + duplicate [RAW] entries into history.jsonl.""" + consolidator._SAFETY_BUFFER = 0 + session = MagicMock() + session.last_consolidated = 0 + session.key = "test:key" + session.messages = [ + {"role": "user" if i in {0, 50} else "assistant", "content": f"m{i}"} + for i in range(70) + ] + session.metadata = {} + consolidator.estimate_session_prompt_tokens = MagicMock( + side_effect=[(1200, "tiktoken"), (400, "tiktoken")] + ) + # LLM consolidation fails — archive() returns None (raw_archive fired). + consolidator.archive = AsyncMock(return_value=None) + + await consolidator.maybe_consolidate_by_tokens(session) + + consolidator.archive.assert_awaited_once() + # The chunk is considered "materialized" (as a raw-archive breadcrumb), + # so last_consolidated must have moved past it. + assert session.last_consolidated == 50 + + async def test_raw_archive_fallback_breaks_round_loop(self, consolidator): + """A degraded LLM should not trigger more archive() calls within the + same maybe_consolidate_by_tokens invocation — bail after one fallback.""" + consolidator._SAFETY_BUFFER = 0 + session = MagicMock() + session.last_consolidated = 0 + session.key = "test:key" + session.messages = [ + {"role": "user" if i in {0, 20, 40, 60} else "assistant", "content": f"m{i}"} + for i in range(70) + ] + session.metadata = {} + # Keep estimates high so the loop would otherwise run multiple rounds. + consolidator.estimate_session_prompt_tokens = MagicMock( + return_value=(1200, "tiktoken") + ) + consolidator.archive = AsyncMock(return_value=None) + + await consolidator.maybe_consolidate_by_tokens(session) + + # Exactly one fallback per call — not _MAX_CONSOLIDATION_ROUNDS. + assert consolidator.archive.await_count == 1 + async def test_boundary_respected_when_no_intermediate_user_turn(self, consolidator): """When boundary points past a long tool chain, the full chunk is archived.""" consolidator._SAFETY_BUFFER = 0 @@ -231,6 +286,19 @@ class TestArchiveTruncation: # Should be truncated assert len(user_content) < 250_000 + async def test_oversized_summary_is_capped_before_append(self, consolidator, mock_provider, store): + """A pathologically large LLM summary must not land full-length in + history.jsonl — that would re-open the #3412 bloat vector from the + *success* path instead of the fallback path.""" + mock_provider.chat_with_retry.return_value = MagicMock( + content="S" * (_ARCHIVE_SUMMARY_MAX_CHARS * 10), + finish_reason="stop", + ) + await consolidator.archive([{"role": "user", "content": "hi"}]) + + entry = store.read_unprocessed_history(since_cursor=0)[0] + assert len(entry["content"]) <= _ARCHIVE_SUMMARY_MAX_CHARS + 50 + async def test_archive_truncates_via_tiktoken_with_positive_budget(self, consolidator, mock_provider, store): """Positive token budget should use tiktoken for precise truncation.""" consolidator.context_window_tokens = 10_000 diff --git a/tests/agent/test_dream.py b/tests/agent/test_dream.py index cb6c8de7..27e49fda 100644 --- a/tests/agent/test_dream.py +++ b/tests/agent/test_dream.py @@ -1,5 +1,7 @@ """Tests for the Dream class — two-phase memory consolidation via AgentRunner.""" +import json + import pytest from unittest.mock import AsyncMock, MagicMock, patch @@ -256,3 +258,52 @@ class TestDreamRun: # The template renders with stale_threshold_days=14 → LLM must see "N>14" assert "N>14" in system_msg + +class TestDreamPromptCaps: + """Dream's Phase 1/2 prompt must not be poisoned by a legacy oversized + history entry or a runaway MEMORY.md. Without caps, a single pre-#3412 + raw_archive dump in history.jsonl would make every subsequent Dream run + exceed the context window and silently advance the cursor past real work. + """ + + async def test_phase1_caps_huge_memory_file( + self, dream, mock_provider, mock_runner, store, + ): + """A MEMORY.md much larger than _MEMORY_FILE_MAX_CHARS must be truncated + in the prompt preview (full content is still reachable via read_file).""" + store.write_memory("M" * (dream._MEMORY_FILE_MAX_CHARS * 5)) + store.append_history("some event") + mock_provider.chat_with_retry.return_value = MagicMock(content="[SKIP]") + mock_runner.run = AsyncMock(return_value=_make_run_result()) + + await dream.run() + + user_msg = mock_provider.chat_with_retry.call_args.kwargs["messages"][1]["content"] + memory_section = user_msg.split("## Current MEMORY.md")[1].split("## Current SOUL.md")[0] + assert len(memory_section) < dream._MEMORY_FILE_MAX_CHARS + 500 + + async def test_phase1_caps_huge_history_entry( + self, dream, mock_provider, mock_runner, store, + ): + """A legacy oversized history entry (e.g. pre-#3412 raw_archive dump) + must not explode the Phase 1 prompt — each entry is capped in the + preview, even though the JSONL record itself stays full-size.""" + # Bypass the append_history cap by writing directly, simulating a + # record that was written by an older nanobot build before any caps. + store.history_file.write_text( + json.dumps({ + "cursor": 1, + "timestamp": "2026-04-01 10:00", + "content": "H" * (dream._HISTORY_ENTRY_PREVIEW_MAX_CHARS * 8), + }) + "\n", + encoding="utf-8", + ) + mock_provider.chat_with_retry.return_value = MagicMock(content="[SKIP]") + mock_runner.run = AsyncMock(return_value=_make_run_result()) + + await dream.run() + + user_msg = mock_provider.chat_with_retry.call_args.kwargs["messages"][1]["content"] + history_section = user_msg.split("## Conversation History\n")[1].split("\n\n## Current Date")[0] + assert len(history_section) < dream._HISTORY_ENTRY_PREVIEW_MAX_CHARS + 500 + diff --git a/tests/agent/test_memory_store.py b/tests/agent/test_memory_store.py index 94adbf37..8f322045 100644 --- a/tests/agent/test_memory_store.py +++ b/tests/agent/test_memory_store.py @@ -5,7 +5,7 @@ from datetime import datetime import pytest -from nanobot.agent.memory import MemoryStore +from nanobot.agent.memory import MemoryStore, _HISTORY_ENTRY_HARD_CAP @pytest.fixture @@ -142,6 +142,49 @@ class TestHistoryWithCursor: assert entries[0]["cursor"] in {4, 5} +class TestAppendHistoryHardCap: + """append_history has a defensive cap that catches new callers who forgot + to set their own tighter cap. The default is intentionally larger than + any current caller's per-call cap, so normal operation never trips it.""" + + def test_oversized_entry_is_truncated(self, store): + """An entry above _HISTORY_ENTRY_HARD_CAP is truncated before being persisted.""" + huge = "x" * (_HISTORY_ENTRY_HARD_CAP + 10_000) + store.append_history(huge) + entry = store.read_unprocessed_history(since_cursor=0)[0] + assert len(entry["content"]) <= _HISTORY_ENTRY_HARD_CAP + 50 + + def test_oversize_warning_is_emitted_once(self, store, caplog): + """Repeated oversized writes should warn only on the first occurrence.""" + from loguru import logger as loguru_logger + + records: list[str] = [] + handler_id = loguru_logger.add(lambda m: records.append(m), level="WARNING") + try: + huge = "x" * (_HISTORY_ENTRY_HARD_CAP + 1) + store.append_history(huge) + store.append_history(huge) + store.append_history(huge) + finally: + loguru_logger.remove(handler_id) + + oversize_warnings = [r for r in records if "exceeds" in r and "chars" in r] + assert len(oversize_warnings) == 1 + + def test_custom_max_chars_overrides_default(self, store): + """Callers that pass max_chars should get their tighter cap applied.""" + store.append_history("a" * 500, max_chars=100) + entry = store.read_unprocessed_history(since_cursor=0)[0] + assert len(entry["content"]) <= 150 # 100 + "\n... (truncated)" + + def test_normal_sized_entries_unaffected(self, store): + """The hard cap must not alter entries that fit within it.""" + msg = "normal short entry" + store.append_history(msg) + entry = store.read_unprocessed_history(since_cursor=0)[0] + assert entry["content"] == msg + + class TestDreamCursor: def test_initial_cursor_is_zero(self, store): assert store.get_last_dream_cursor() == 0 From 7f1913f619ca72131de2d714bc736fe8270f18c4 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 24 Apr 2026 06:20:25 +0000 Subject: [PATCH 13/80] fix(provider): add DeepSeek thinking toggle; backfill reasoning_content on legacy messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues with DeepSeek V4 thinking mode support: 1. Missing thinking parameter injection. DeepSeek V4 requires `extra_body: {"thinking": {"type": "enabled/disabled"}}` — identical to VolcEngine/BytePlus. The code had this for volcengine, byteplus, dashscope, minimax, and kimi but not DeepSeek. This means `reasoning_effort=minimal` (thinking off) silently has no effect. Root cause: the thinking-style→wire-format mapping was an if/elif chain on provider *names*. DeepSeek was forgotten. Fix: make the mapping declarative via `ProviderSpec.thinking_style`: - "thinking_type" → {"thinking": {"type": "..."}} (DeepSeek, Volc, BytePlus) - "enable_thinking" → {"enable_thinking": bool} (DashScope) - "reasoning_split" → {"reasoning_split": bool} (MiniMax) `_build_kwargs` now does a single dict lookup. Adding a new provider with an existing wire format requires zero changes to the function. 2. Legacy session messages crash thinking-mode requests. When a session was started without thinking mode (or with a different model), assistant messages lack reasoning_content. DeepSeek V4 in thinking mode rejects these with 400: "The reasoning_content in the thinking mode must be passed back to the API." This affects ALL assistant messages, not just those with tool_calls (despite the docs only mentioning the tool_calls case). Fix: `_build_kwargs` backfills `reasoning_content: ""` on every assistant message missing it, but only when thinking mode is active. This is semantically neutral — the model treats empty reasoning_content as "no thinking happened on that turn". The backfill only touches the in-memory request copy; session files on disk are untouched. Tests: +5 (3 thinking toggle, 2 backfill). Full suite: 2377 passed. Made-with: Cursor --- nanobot/providers/openai_compat_provider.py | 46 ++++++++++---- nanobot/providers/registry.py | 15 +++++ tests/providers/test_litellm_kwargs.py | 69 +++++++++++++++++++++ 3 files changed, 117 insertions(+), 13 deletions(-) diff --git a/nanobot/providers/openai_compat_provider.py b/nanobot/providers/openai_compat_provider.py index d9eb64dd..f603b9e3 100644 --- a/nanobot/providers/openai_compat_provider.py +++ b/nanobot/providers/openai_compat_provider.py @@ -58,6 +58,15 @@ _KIMI_THINKING_MODELS: frozenset[str] = frozenset({ "k2.6-code-preview", }) +# Maps ProviderSpec.thinking_style → extra_body builder. +# Each builder takes a bool (thinking_enabled) and returns the dict to +# merge into extra_body, keeping the style→wire-format mapping in one place. +_THINKING_STYLE_MAP: dict[str, Any] = { + "thinking_type": lambda on: {"thinking": {"type": "enabled" if on else "disabled"}}, + "enable_thinking": lambda on: {"enable_thinking": on}, + "reasoning_split": lambda on: {"reasoning_split": on}, +} + def _is_kimi_thinking_model(model_name: str) -> bool: """Return True if model_name refers to a Kimi thinking-capable model. @@ -407,20 +416,11 @@ class OpenAICompatProvider(LLMProvider): # Provider-specific thinking parameters. # Only sent when reasoning_effort is explicitly configured so that # the provider default is preserved otherwise. - if spec and reasoning_effort is not None: + # The mapping is driven by ProviderSpec.thinking_style so that adding + # a new provider never requires touching this function. + if spec and spec.thinking_style and reasoning_effort is not None: thinking_enabled = semantic_effort != "minimal" - extra: dict[str, Any] | None = None - if spec.name == "dashscope": - extra = {"enable_thinking": thinking_enabled} - elif spec.name == "minimax": - extra = {"reasoning_split": thinking_enabled} - elif spec.name in ( - "volcengine", "volcengine_coding_plan", - "byteplus", "byteplus_coding_plan", - ): - extra = { - "thinking": {"type": "enabled" if thinking_enabled else "disabled"} - } + extra = _THINKING_STYLE_MAP.get(spec.thinking_style, lambda _: None)(thinking_enabled) if extra: kwargs.setdefault("extra_body", {}).update(extra) @@ -438,6 +438,26 @@ class OpenAICompatProvider(LLMProvider): kwargs["tools"] = tools kwargs["tool_choice"] = tool_choice or "auto" + # Backfill reasoning_content on legacy assistant messages. + # DeepSeek V4 (and potentially others) rejects thinking-mode + # requests that contain assistant messages without reasoning_content + # — even on turns that had no tool calls. This happens when a + # session was started with a non-thinking model or without + # reasoning_effort, then the user switches thinking mode on + # mid-session. Injecting an empty string satisfies the API + # without altering semantics (the model treats it as "no + # thinking happened on that turn"). + thinking_active = ( + (spec and spec.thinking_style and reasoning_effort is not None + and semantic_effort != "minimal") + or (reasoning_effort is not None and _is_kimi_thinking_model(model_name) + and semantic_effort != "minimal") + ) + if thinking_active: + for msg in kwargs["messages"]: + if msg.get("role") == "assistant" and "reasoning_content" not in msg: + msg["reasoning_content"] = "" + return kwargs def _should_use_responses_api( diff --git a/nanobot/providers/registry.py b/nanobot/providers/registry.py index f633cc83..5037e300 100644 --- a/nanobot/providers/registry.py +++ b/nanobot/providers/registry.py @@ -63,6 +63,14 @@ class ProviderSpec: # Provider supports cache_control on content blocks (e.g. Anthropic prompt caching) supports_prompt_caching: bool = False + # How to inject the thinking on/off toggle into extra_body. + # "" — no extra_body needed (default) + # "thinking_type" — {"thinking": {"type": "enabled"/"disabled"}} + # (DeepSeek, VolcEngine, BytePlus) + # "enable_thinking" — {"enable_thinking": true/false} (DashScope) + # "reasoning_split" — {"reasoning_split": true/false} (MiniMax) + thinking_style: str = "" + @property def label(self) -> str: return self.display_name or self.name.title() @@ -143,6 +151,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( is_gateway=True, detect_by_base_keyword="volces", default_api_base="https://ark.cn-beijing.volces.com/api/v3", + thinking_style="thinking_type", ), # VolcEngine Coding Plan (火山引擎 Coding Plan): same key as volcengine @@ -155,6 +164,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( is_gateway=True, default_api_base="https://ark.cn-beijing.volces.com/api/coding/v3", strip_model_prefix=True, + thinking_style="thinking_type", ), # BytePlus: VolcEngine international, pay-per-use models @@ -168,6 +178,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( detect_by_base_keyword="bytepluses", default_api_base="https://ark.ap-southeast.bytepluses.com/api/v3", strip_model_prefix=True, + thinking_style="thinking_type", ), # BytePlus Coding Plan: same key as byteplus @@ -180,6 +191,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( is_gateway=True, default_api_base="https://ark.ap-southeast.bytepluses.com/api/coding/v3", strip_model_prefix=True, + thinking_style="thinking_type", ), @@ -233,6 +245,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( display_name="DeepSeek", backend="openai_compat", default_api_base="https://api.deepseek.com", + thinking_style="thinking_type", ), # Gemini: Google's OpenAI-compatible endpoint ProviderSpec( @@ -261,6 +274,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( display_name="DashScope", backend="openai_compat", default_api_base="https://dashscope.aliyuncs.com/compatible-mode/v1", + thinking_style="enable_thinking", ), # Moonshot (月之暗面): Kimi K2.5 / K2.6 enforce temperature >= 1.0. ProviderSpec( @@ -283,6 +297,7 @@ PROVIDERS: tuple[ProviderSpec, ...] = ( display_name="MiniMax", backend="openai_compat", default_api_base="https://api.minimax.io/v1", + thinking_style="reasoning_split", ), # MiniMax Anthropic-compatible endpoint: supports thinking mode ProviderSpec( diff --git a/tests/providers/test_litellm_kwargs.py b/tests/providers/test_litellm_kwargs.py index 41188c72..dfa0f58a 100644 --- a/tests/providers/test_litellm_kwargs.py +++ b/tests/providers/test_litellm_kwargs.py @@ -785,6 +785,75 @@ def test_byteplus_no_extra_body_when_reasoning_effort_none() -> None: assert "extra_body" not in kw +def test_deepseek_thinking_enabled() -> None: + """DeepSeek V4 requires extra_body.thinking when reasoning_effort is set.""" + kw = _build_kwargs_for("deepseek", "deepseek-v4-pro", reasoning_effort="high") + assert kw["extra_body"] == {"thinking": {"type": "enabled"}} + + +def test_deepseek_thinking_disabled_for_minimal() -> None: + """reasoning_effort='minimal' must send thinking.type=disabled to DeepSeek.""" + kw = _build_kwargs_for("deepseek", "deepseek-v4-pro", reasoning_effort="minimal") + assert kw["extra_body"] == {"thinking": {"type": "disabled"}} + + +def test_deepseek_no_extra_body_when_reasoning_effort_none() -> None: + """Without reasoning_effort the thinking param must not be injected.""" + kw = _build_kwargs_for("deepseek", "deepseek-chat", reasoning_effort=None) + assert "extra_body" not in kw + + +def test_deepseek_backfills_reasoning_content_on_legacy_tool_call_messages() -> None: + """Session messages from before thinking mode was enabled may have assistant + messages with tool_calls but no reasoning_content. DeepSeek V4 rejects these + with 400. _build_kwargs must backfill reasoning_content='' on them.""" + spec = find_by_name("deepseek") + with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI"): + p = OpenAICompatProvider(api_key="k", default_model="deepseek-v4-pro", spec=spec) + messages = [ + {"role": "user", "content": "search for news"}, + {"role": "assistant", "content": "", "tool_calls": [ + {"id": "tc1", "type": "function", "function": {"name": "web_search", "arguments": "{}"}} + ]}, + {"role": "tool", "tool_call_id": "tc1", "content": "result"}, + {"role": "assistant", "content": "Here are the results."}, + {"role": "user", "content": "hi"}, + ] + kw = p._build_kwargs( + messages=messages, tools=None, model="deepseek-v4-pro", + max_tokens=1024, temperature=0.7, + reasoning_effort="high", tool_choice=None, + ) + for msg in kw["messages"]: + if msg.get("role") == "assistant": + assert "reasoning_content" in msg, "legacy assistant message missing reasoning_content" + assert msg["reasoning_content"] == "" + + +def test_backfill_does_not_touch_messages_when_thinking_off() -> None: + """When reasoning_effort is None or minimal, legacy messages must NOT be altered.""" + spec = find_by_name("deepseek") + with patch("nanobot.providers.openai_compat_provider.AsyncOpenAI"): + p = OpenAICompatProvider(api_key="k", default_model="deepseek-v4-pro", spec=spec) + messages = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "", "tool_calls": [ + {"id": "tc1", "type": "function", "function": {"name": "web_search", "arguments": "{}"}} + ]}, + {"role": "tool", "tool_call_id": "tc1", "content": "result"}, + {"role": "user", "content": "thanks"}, + ] + for effort in (None, "minimal"): + kw = p._build_kwargs( + messages=list(messages), tools=None, model="deepseek-v4-pro", + max_tokens=1024, temperature=0.7, + reasoning_effort=effort, tool_choice=None, + ) + for msg in kw["messages"]: + if msg.get("role") == "assistant" and msg.get("tool_calls"): + assert "reasoning_content" not in msg + + def test_openai_no_thinking_extra_body() -> None: """Non-thinking providers should never get extra_body for thinking.""" kw = _build_kwargs_for("openai", "gpt-4o", reasoning_effort="medium") From 9239429a00b1e2226503a2aae66ab63d98430cde Mon Sep 17 00:00:00 2001 From: 04cb <0x04cb@gmail.com> Date: Fri, 24 Apr 2026 09:01:22 +0800 Subject: [PATCH 14/80] fix(anthropic): omit temperature for opus-4-7 (#3417) --- nanobot/providers/anthropic_provider.py | 12 +++++++++--- tests/providers/test_anthropic_thinking.py | 6 ++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/nanobot/providers/anthropic_provider.py b/nanobot/providers/anthropic_provider.py index f88bba62..5d6d36b7 100644 --- a/nanobot/providers/anthropic_provider.py +++ b/nanobot/providers/anthropic_provider.py @@ -436,6 +436,10 @@ class AnthropicProvider(LLMProvider): max_tokens = max(1, max_tokens) thinking_enabled = bool(reasoning_effort) + # claude-opus-4-7 deprecated the `temperature` parameter entirely — the + # API returns 400 if it is present, on any code path. + omit_temperature = "opus-4-7" in model_name + kwargs: dict[str, Any] = { "model": model_name, "messages": anthropic_msgs, @@ -450,14 +454,16 @@ class AnthropicProvider(LLMProvider): # Supported on claude-sonnet-4-6 and claude-opus-4-6. # Also auto-enables interleaved thinking between tool calls. kwargs["thinking"] = {"type": "adaptive"} - kwargs["temperature"] = 1.0 + if not omit_temperature: + kwargs["temperature"] = 1.0 elif thinking_enabled: budget_map = {"low": 1024, "medium": 4096, "high": max(8192, max_tokens)} budget = budget_map.get(reasoning_effort.lower(), 4096) kwargs["thinking"] = {"type": "enabled", "budget_tokens": budget} kwargs["max_tokens"] = max(max_tokens, budget + 4096) - kwargs["temperature"] = 1.0 - else: + if not omit_temperature: + kwargs["temperature"] = 1.0 + elif not omit_temperature: kwargs["temperature"] = temperature if anthropic_tools: diff --git a/tests/providers/test_anthropic_thinking.py b/tests/providers/test_anthropic_thinking.py index ab8942e1..12e2e167 100644 --- a/tests/providers/test_anthropic_thinking.py +++ b/tests/providers/test_anthropic_thinking.py @@ -63,3 +63,9 @@ def test_none_does_not_enable_thinking() -> None: kw = _build(_make_provider(), None) assert "thinking" not in kw assert kw["temperature"] == 0.7 + + +def test_opus_4_7_omits_temperature() -> None: + kw = _build(_make_provider("claude-opus-4-7"), "adaptive") + assert "temperature" not in kw + assert kw["thinking"] == {"type": "adaptive"} From 3441d5f89c972489e881c5c738e6485a77e38697 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 24 Apr 2026 07:32:59 +0000 Subject: [PATCH 15/80] test(anthropic): cover remaining opus-4-7 temperature branches The existing test only verified the adaptive path. Add two more cases: - enabled thinking (high): temperature must also be omitted - no thinking (None): temperature must still be omitted Made-with: Cursor --- tests/providers/test_anthropic_thinking.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/providers/test_anthropic_thinking.py b/tests/providers/test_anthropic_thinking.py index 12e2e167..d0f72b32 100644 --- a/tests/providers/test_anthropic_thinking.py +++ b/tests/providers/test_anthropic_thinking.py @@ -65,7 +65,21 @@ def test_none_does_not_enable_thinking() -> None: assert kw["temperature"] == 0.7 -def test_opus_4_7_omits_temperature() -> None: +def test_opus_4_7_omits_temperature_adaptive() -> None: kw = _build(_make_provider("claude-opus-4-7"), "adaptive") assert "temperature" not in kw assert kw["thinking"] == {"type": "adaptive"} + + +def test_opus_4_7_omits_temperature_enabled() -> None: + """Enabled thinking (high) must also omit temperature for opus-4-7.""" + kw = _build(_make_provider("claude-opus-4-7"), "high", max_tokens=4096) + assert "temperature" not in kw + assert kw["thinking"]["type"] == "enabled" + + +def test_opus_4_7_omits_temperature_none() -> None: + """Without thinking, opus-4-7 must still omit temperature (API rejects it).""" + kw = _build(_make_provider("claude-opus-4-7"), None) + assert "temperature" not in kw + assert "thinking" not in kw From ee14e2df562b3626c639a4bc304178cda75152ac Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 24 Apr 2026 01:27:11 -0700 Subject: [PATCH 16/80] perf(document): lazy-import heavy document parsers Move pypdf, python-docx, openpyxl, and python-pptx imports from module level into the _extract_pdf / _extract_docx / _extract_xlsx / _extract_pptx functions that actually use them. These four libraries became core dependencies in v0.1.5.post2 (~25 MB combined) and were paying the import cost on every nanobot startup even when no document parsing was needed for the session. The module-level SUPPORTED_EXTENSIONS set and the extract_text() dispatch stay as-is; the "[error: not installed]" branches move from the old module-level None sentinels into the corresponding extractor's try/except ImportError block. Behavior for the error message and for successful parses is identical. All 20 tests in tests/test_document_parsing.py pass unchanged. Fixes #3422 --- nanobot/utils/document.py | 48 ++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/nanobot/utils/document.py b/nanobot/utils/document.py index 396fe50c..3a1ea906 100644 --- a/nanobot/utils/document.py +++ b/nanobot/utils/document.py @@ -7,26 +7,6 @@ from loguru import logger from nanobot.utils.helpers import detect_image_mime -try: - from pypdf import PdfReader -except ImportError: - PdfReader = None # type: ignore - -try: - from docx import Document as DocxDocument -except ImportError: - DocxDocument = None # type: ignore - -try: - from openpyxl import load_workbook -except ImportError: - load_workbook = None # type: ignore - -try: - from pptx import Presentation as PptxPresentation -except ImportError: - PptxPresentation = None # type: ignore - # Supported file extensions for text extraction SUPPORTED_EXTENSIONS: set[str] = { @@ -78,22 +58,16 @@ def extract_text(path: Path) -> str | None: ext = path.suffix.lower() - # Document formats + # Document formats -- each branch lazily imports its parser so that + # startup does not pay the ~25 MB cost of loading openpyxl / + # python-docx / python-pptx / pypdf up front (see issue #3422). if ext == ".pdf": - if PdfReader is None: - return "[error: pypdf not installed]" return _extract_pdf(path) elif ext == ".docx": - if DocxDocument is None: - return "[error: python-docx not installed]" return _extract_docx(path) elif ext == ".xlsx": - if load_workbook is None: - return "[error: openpyxl not installed]" return _extract_xlsx(path) elif ext == ".pptx": - if PptxPresentation is None: - return "[error: python-pptx not installed]" return _extract_pptx(path) elif _is_text_extension(ext): return _extract_text_file(path) @@ -107,6 +81,10 @@ def extract_text(path: Path) -> str | None: def _extract_pdf(path: Path) -> str: """Extract text from PDF using pypdf.""" + try: + from pypdf import PdfReader + except ImportError: + return "[error: pypdf not installed]" try: reader = PdfReader(path) pages: list[str] = [] @@ -121,6 +99,10 @@ def _extract_pdf(path: Path) -> str: def _extract_docx(path: Path) -> str: """Extract text from DOCX using python-docx.""" + try: + from docx import Document as DocxDocument + except ImportError: + return "[error: python-docx not installed]" try: doc = DocxDocument(path) paragraphs: list[str] = [p.text for p in doc.paragraphs if p.text.strip()] @@ -132,6 +114,10 @@ def _extract_docx(path: Path) -> str: def _extract_xlsx(path: Path) -> str: """Extract text from XLSX using openpyxl.""" + try: + from openpyxl import load_workbook + except ImportError: + return "[error: openpyxl not installed]" try: wb = load_workbook(path, read_only=True, data_only=True) try: @@ -155,6 +141,10 @@ def _extract_xlsx(path: Path) -> str: def _extract_pptx(path: Path) -> str: """Extract text from PPTX using python-pptx.""" + try: + from pptx import Presentation as PptxPresentation + except ImportError: + return "[error: python-pptx not installed]" try: prs = PptxPresentation(path) slides: list[str] = [] From be05189f39a5fdd1f1881d8f44901e6630bff715 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 24 Apr 2026 18:16:59 +0000 Subject: [PATCH 17/80] feat(channels): add video support for Telegram and WebSocket Telegram previously sent all video files as documents via send_document, so users saw a file icon instead of an inline player. WebSocket only accepted image MIME types, rejecting video uploads entirely. Telegram: - Recognize video extensions (mp4/mov/avi/mkv/webm/3gp) in _get_media_type - Route videos through send_video with supports_streaming=True - Add VIDEO/VIDEO_NOTE/ANIMATION to inbound message filters - Add video MIME mappings to _get_extension - Fix: local file sends now use _call_with_retry (previously no retry) WebSocket: - Expand upload MIME whitelist with video/mp4, video/webm, video/quicktime - Add per-type size limits (_MAX_VIDEO_BYTES=20MB, _MAX_VIDEOS_PER_MESSAGE=1) - Expand media serving endpoint to serve video with correct Content-Type Agent: - Add "video" to message tool media parameter description - Add .mp4 example to identity.md system prompt Made-with: Cursor --- nanobot/agent/tools/message.py | 2 +- nanobot/channels/telegram.py | 44 +++++++++++++++++-------- nanobot/channels/websocket.py | 36 +++++++++++++++++--- nanobot/templates/agent/identity.md | 2 +- tests/channels/test_telegram_channel.py | 3 ++ 5 files changed, 67 insertions(+), 20 deletions(-) diff --git a/nanobot/agent/tools/message.py b/nanobot/agent/tools/message.py index ea0598a1..ee78df46 100644 --- a/nanobot/agent/tools/message.py +++ b/nanobot/agent/tools/message.py @@ -15,7 +15,7 @@ from nanobot.bus.events import OutboundMessage chat_id=StringSchema("Optional: target chat/user ID"), media=ArraySchema( StringSchema(""), - description="Optional: list of file paths to attach (images, audio, documents)", + description="Optional: list of file paths to attach (images, video, audio, documents)", ), buttons=ArraySchema( ArraySchema(StringSchema("Button label")), diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index 38c3fe89..1e392dd1 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -7,6 +7,7 @@ import re import time import unicodedata from dataclasses import dataclass +from pathlib import Path from typing import Any, Literal from loguru import logger @@ -357,10 +358,12 @@ class TelegramChannel(BaseChannel): ) self._app.add_handler(MessageHandler(filters.Regex(r"^/help(?:@\w+)?$"), self._on_help)) - # Add message handler for text, photos, voice, documents, and locations + # Add message handler for text, photos, video, voice, documents, and locations self._app.add_handler( MessageHandler( - (filters.TEXT | filters.PHOTO | filters.VOICE | filters.AUDIO | filters.Document.ALL | filters.LOCATION) + (filters.TEXT | filters.PHOTO | filters.VIDEO | filters.VIDEO_NOTE + | filters.ANIMATION | filters.VOICE | filters.AUDIO + | filters.Document.ALL | filters.LOCATION) & ~filters.COMMAND, self._on_message ) @@ -429,6 +432,8 @@ class TelegramChannel(BaseChannel): ext = path.rsplit(".", 1)[-1].lower() if "." in path else "" if ext in ("jpg", "jpeg", "png", "gif", "webp"): return "photo" + if ext in ("mp4", "mov", "avi", "mkv", "webm", "3gp"): + return "video" if ext == "ogg": return "voice" if ext in ("mp3", "m4a", "wav", "aac"): @@ -481,10 +486,19 @@ class TelegramChannel(BaseChannel): media_type = self._get_media_type(media_path) sender = { "photo": self._app.bot.send_photo, + "video": self._app.bot.send_video, "voice": self._app.bot.send_voice, "audio": self._app.bot.send_audio, }.get(media_type, self._app.bot.send_document) - param = "photo" if media_type == "photo" else media_type if media_type in ("voice", "audio") else "document" + param = { + "photo": "photo", + "video": "video", + "voice": "voice", + "audio": "audio", + }.get(media_type, "document") + extra: dict[str, Any] = {} + if media_type == "video": + extra["supports_streaming"] = True # Telegram Bot API accepts HTTP(S) URLs directly for media params. if self._is_remote_media_url(media_path): @@ -497,16 +511,19 @@ class TelegramChannel(BaseChannel): **{param: media_path}, reply_parameters=reply_params, **thread_kwargs, + **extra, ) continue - with open(media_path, "rb") as f: - await sender( - chat_id=chat_id, - **{param: f}, - reply_parameters=reply_params, - **thread_kwargs, - ) + media_bytes = Path(media_path).read_bytes() + await self._call_with_retry( + sender, + chat_id=chat_id, + **{param: media_bytes}, + reply_parameters=reply_params, + **thread_kwargs, + **extra, + ) except Exception as e: filename = media_path.rsplit("/", 1)[-1] logger.error("Failed to send media {}: {}", media_path, e) @@ -1184,18 +1201,19 @@ class TelegramChannel(BaseChannel): if mime_type: ext_map = { "image/jpeg": ".jpg", "image/png": ".png", "image/gif": ".gif", + "image/webp": ".webp", "audio/ogg": ".ogg", "audio/mpeg": ".mp3", "audio/mp4": ".m4a", + "video/mp4": ".mp4", "video/quicktime": ".mov", "video/webm": ".webm", + "video/x-matroska": ".mkv", "video/3gpp": ".3gp", } if mime_type in ext_map: return ext_map[mime_type] - type_map = {"image": ".jpg", "voice": ".ogg", "audio": ".mp3", "file": ""} + type_map = {"image": ".jpg", "voice": ".ogg", "audio": ".mp3", "video": ".mp4", "file": ""} if ext := type_map.get(media_type, ""): return ext if filename: - from pathlib import Path - return "".join(Path(filename).suffixes) return "" diff --git a/nanobot/channels/websocket.py b/nanobot/channels/websocket.py index 47c79e87..b0119178 100644 --- a/nanobot/channels/websocket.py +++ b/nanobot/channels/websocket.py @@ -218,12 +218,14 @@ def _parse_envelope(raw: str) -> dict[str, Any] | None: return data -# Per-message image limits. The server-side guard is a touch looser than the +# Per-message media limits. The server-side guard is a touch looser than the # client's ``Worker`` normalization target (6 MB) — tolerate client slop, but # still cap total ingress at ``_MAX_IMAGES_PER_MESSAGE * _MAX_IMAGE_BYTES`` # which fits comfortably inside ``max_message_bytes``. _MAX_IMAGES_PER_MESSAGE = 4 _MAX_IMAGE_BYTES = 8 * 1024 * 1024 +_MAX_VIDEOS_PER_MESSAGE = 1 +_MAX_VIDEO_BYTES = 20 * 1024 * 1024 # Image MIME whitelist — matches the Composer's ``accept`` list. SVG is # explicitly excluded to avoid the XSS surface inside embedded scripts. @@ -234,6 +236,14 @@ _IMAGE_MIME_ALLOWED: frozenset[str] = frozenset({ "image/gif", }) +_VIDEO_MIME_ALLOWED: frozenset[str] = frozenset({ + "video/mp4", + "video/webm", + "video/quicktime", +}) + +_UPLOAD_MIME_ALLOWED: frozenset[str] = _IMAGE_MIME_ALLOWED | _VIDEO_MIME_ALLOWED + _DATA_URL_MIME_RE = re.compile(r"^data:([^;]+);base64,", re.DOTALL) @@ -339,6 +349,9 @@ _MEDIA_ALLOWED_MIMES: frozenset[str] = frozenset({ "image/jpeg", "image/webp", "image/gif", + "video/mp4", + "video/webm", + "video/quicktime", }) @@ -945,14 +958,25 @@ class WebSocketChannel(BaseChannel): Returns ``(paths, None)`` on success or ``([], reason)`` on the first failure — the caller is expected to surface ``reason`` to the client and skip publishing so no half-formed message ever reaches the agent. - On failure, any images already written to disk earlier in the same + On failure, any files already written to disk earlier in the same call are unlinked so partial ingress doesn't leak orphan files. ``reason`` is a short, stable token suitable for UI localization. Shape: ``list[{"data_url": str, "name"?: str | None}]``. """ - if len(media) > _MAX_IMAGES_PER_MESSAGE: + image_count = 0 + video_count = 0 + for item in media: + mime = _extract_data_url_mime(item.get("data_url", "")) if isinstance(item, dict) else None + if mime in _VIDEO_MIME_ALLOWED: + video_count += 1 + elif mime in _IMAGE_MIME_ALLOWED: + image_count += 1 + if image_count > _MAX_IMAGES_PER_MESSAGE: return [], "too_many_images" + if video_count > _MAX_VIDEOS_PER_MESSAGE: + return [], "too_many_videos" + media_dir = get_media_dir("websocket") paths: list[str] = [] @@ -975,11 +999,13 @@ class WebSocketChannel(BaseChannel): mime = _extract_data_url_mime(data_url) if mime is None: return _abort("decode") - if mime not in _IMAGE_MIME_ALLOWED: + if mime not in _UPLOAD_MIME_ALLOWED: return _abort("mime") + is_video = mime in _VIDEO_MIME_ALLOWED + max_bytes = _MAX_VIDEO_BYTES if is_video else _MAX_IMAGE_BYTES try: saved = save_base64_data_url( - data_url, media_dir, max_bytes=_MAX_IMAGE_BYTES, + data_url, media_dir, max_bytes=max_bytes, ) except FileSizeExceeded: return _abort("size") diff --git a/nanobot/templates/agent/identity.md b/nanobot/templates/agent/identity.md index 31f3d0d2..a53be709 100644 --- a/nanobot/templates/agent/identity.md +++ b/nanobot/templates/agent/identity.md @@ -29,4 +29,4 @@ Output is rendered in a terminal. Avoid markdown headings and tables. Use plain {% include 'agent/_snippets/untrusted_content.md' %} Reply directly with text for conversations. Only use the 'message' tool to send to a specific chat channel. -IMPORTANT: To send files (images, documents, audio, video) to the user, you MUST call the 'message' tool with the 'media' parameter. Do NOT use read_file to "send" a file — reading a file only shows its content to you, it does NOT deliver the file to the user. Example: message(content="Here is the file", media=["/path/to/file.png"]) +IMPORTANT: To send files (images, video, audio, documents) to the user, you MUST call the 'message' tool with the 'media' parameter. Do NOT use read_file to "send" a file — reading a file only shows its content to you, it does NOT deliver the file to the user. Examples: message(content="Here is the image", media=["/path/to/file.png"]) or message(content="Here is the video", media=["/path/to/video.mp4"]) diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index 175844b2..1fbfe243 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -59,6 +59,9 @@ class _FakeBot: async def send_photo(self, **kwargs) -> None: self.sent_media.append({"kind": "photo", **kwargs}) + async def send_video(self, **kwargs) -> None: + self.sent_media.append({"kind": "video", **kwargs}) + async def send_voice(self, **kwargs) -> None: self.sent_media.append({"kind": "voice", **kwargs}) From e52fe2a8e2a2842404b5819cd689c2cd977c1c19 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 24 Apr 2026 19:17:58 +0000 Subject: [PATCH 18/80] feat(webui): render video media attachments Add signed media URLs to live WebSocket replies and teach the WebUI to classify and render video attachments, so bot-sent videos can play inline in both live chats and session history. Made-with: Cursor --- nanobot/channels/websocket.py | 36 +++++++++ tests/channels/test_websocket_channel.py | 33 ++++++++ webui/src/components/MessageBubble.tsx | 97 +++++++++++++++++++++-- webui/src/hooks/useNanobotStream.ts | 6 ++ webui/src/hooks/useSessions.ts | 21 ++--- webui/src/lib/media.ts | 59 ++++++++++++++ webui/src/lib/types.ts | 11 +++ webui/src/tests/message-bubble.test.tsx | 24 ++++++ webui/src/tests/useNanobotStream.test.tsx | 21 +++++ webui/src/tests/useSessions.test.tsx | 34 ++++++++ 10 files changed, 327 insertions(+), 15 deletions(-) create mode 100644 webui/src/lib/media.ts diff --git a/nanobot/channels/websocket.py b/nanobot/channels/websocket.py index b0119178..c76371e9 100644 --- a/nanobot/channels/websocket.py +++ b/nanobot/channels/websocket.py @@ -13,6 +13,7 @@ import json import mimetypes import re import secrets +import shutil import ssl import time import uuid @@ -33,6 +34,7 @@ from nanobot.bus.queue import MessageBus from nanobot.channels.base import BaseChannel from nanobot.config.paths import get_media_dir from nanobot.config.schema import Base +from nanobot.utils.helpers import safe_filename from nanobot.utils.media_decode import ( FileSizeExceeded, save_base64_data_url, @@ -716,6 +718,33 @@ class WebSocketChannel(BaseChannel): ).digest()[:16] return f"/api/media/{_b64url_encode(mac)}/{payload}" + def _sign_or_stage_media_path(self, path: Path) -> dict[str, str] | None: + """Return a signed media URL payload for *path*. + + Persisted inbound media already lives under ``get_media_dir`` and can + be signed directly. Outbound bot-generated files may live anywhere on + disk; copy those into the websocket media bucket first so the browser + can fetch them through the existing signed media route without + exposing arbitrary filesystem paths. + """ + signed = self._sign_media_path(path) + if signed is not None: + return {"url": signed, "name": path.name} + try: + if not path.is_file(): + return None + media_dir = get_media_dir("websocket") + safe_name = safe_filename(path.name) or "attachment" + staged = media_dir / f"{uuid.uuid4().hex[:12]}-{safe_name}" + shutil.copyfile(path, staged) + except OSError as exc: + logger.warning("websocket: failed to stage outbound media {}: {}", path, exc) + return None + signed = self._sign_media_path(staged) + if signed is None: + return None + return {"url": signed, "name": path.name} + def _handle_media_fetch(self, sig: str, payload: str) -> Response: """Serve a single media file previously signed via :meth:`_sign_media_path`. Validates the signature, decodes the @@ -1124,6 +1153,13 @@ class WebSocketChannel(BaseChannel): } if msg.media: payload["media"] = msg.media + urls: list[dict[str, str]] = [] + for entry in msg.media: + signed = self._sign_or_stage_media_path(Path(entry)) + if signed is not None: + urls.append(signed) + if urls: + payload["media_urls"] = urls if msg.reply_to: payload["reply_to"] = msg.reply_to # Mark intermediate agent breadcrumbs (tool-call hints, generic diff --git a/tests/channels/test_websocket_channel.py b/tests/channels/test_websocket_channel.py index c7d4923f..c92c88ba 100644 --- a/tests/channels/test_websocket_channel.py +++ b/tests/channels/test_websocket_channel.py @@ -190,6 +190,39 @@ async def test_send_delivers_json_message_with_media_and_reply() -> None: assert payload["media"] == ["/tmp/a.png"] +@pytest.mark.asyncio +async def test_send_stages_external_media_as_signed_url(monkeypatch, tmp_path) -> None: + bus = MagicMock() + media_root = tmp_path / "media" + ws_media = media_root / "websocket" + ws_media.mkdir(parents=True) + external = tmp_path / "clip.mp4" + external.write_bytes(b"video") + + def fake_media_dir(channel: str | None = None): + return ws_media if channel == "websocket" else media_root + + monkeypatch.setattr("nanobot.channels.websocket.get_media_dir", fake_media_dir) + channel = WebSocketChannel({"enabled": True, "allowFrom": ["*"]}, bus) + mock_ws = AsyncMock() + channel._attach(mock_ws, "chat-1") + + await channel.send( + OutboundMessage( + channel="websocket", + chat_id="chat-1", + content="video", + media=[str(external)], + ) + ) + + payload = json.loads(mock_ws.send.call_args[0][0]) + assert payload["media"] == [str(external)] + assert payload["media_urls"][0]["name"] == "clip.mp4" + assert payload["media_urls"][0]["url"].startswith("/api/media/") + assert any(p.name.endswith("-clip.mp4") for p in ws_media.iterdir()) + + @pytest.mark.asyncio async def test_send_missing_connection_is_noop_without_error() -> None: bus = MagicMock() diff --git a/webui/src/components/MessageBubble.tsx b/webui/src/components/MessageBubble.tsx index 076c3000..d1611587 100644 --- a/webui/src/components/MessageBubble.tsx +++ b/webui/src/components/MessageBubble.tsx @@ -1,11 +1,11 @@ import { useState } from "react"; -import { ChevronRight, ImageIcon, Wrench } from "lucide-react"; +import { ChevronRight, FileIcon, ImageIcon, PlaySquare, Wrench } from "lucide-react"; import { useTranslation } from "react-i18next"; import { ImageLightbox } from "@/components/ImageLightbox"; import { MarkdownText } from "@/components/MarkdownText"; import { cn } from "@/lib/utils"; -import type { UIImage, UIMessage } from "@/lib/types"; +import type { UIImage, UIMediaAttachment, UIMessage } from "@/lib/types"; interface MessageBubbleProps { message: UIMessage; @@ -29,7 +29,9 @@ export function MessageBubble({ message }: MessageBubbleProps) { if (message.role === "user") { const images = message.images ?? []; + const media = message.media ?? []; const hasImages = images.length > 0; + const hasMedia = media.length > 0; const hasText = message.content.trim().length > 0; return (
- {hasImages ? : null} + {hasImages ? : null} + {!hasImages && hasMedia ? ( + + ) : null} {hasText ? (

{empty && message.isStreaming ? ( @@ -62,12 +68,82 @@ export function MessageBubble({ message }: MessageBubbleProps) { <> {message.content} {message.isStreaming && } + {media.length > 0 ? : null} )}

); } +function MessageMedia({ + media, + align, +}: { + media: UIMediaAttachment[]; + align: "left" | "right"; +}) { + if (media.length === 0) return null; + const images = media + .filter((item) => item.kind === "image") + .map(({ url, name }) => ({ url, name })); + const nonImages = media.filter((item) => item.kind !== "image"); + + return ( +
+ {images.length > 0 ? : null} + {nonImages.map((item, i) => ( + + ))} +
+ ); +} + +function MediaCell({ media }: { media: UIMediaAttachment }) { + const { t } = useTranslation(); + const hasUrl = typeof media.url === "string" && media.url.length > 0; + + if (media.kind === "video" && hasUrl) { + return ( +
+
+ ); + } + + const label = + media.kind === "video" + ? t("message.videoAttachment", { defaultValue: "Video attachment" }) + : t("message.fileAttachment", { defaultValue: "File attachment" }); + const Icon = media.kind === "video" ? PlaySquare : FileIcon; + + return ( +
+ + {media.name ?? label} +
+ ); +} + /** * Right-aligned preview row for images attached to a user turn. * @@ -82,7 +158,13 @@ export function MessageBubble({ message }: MessageBubbleProps) { * have no URL (the backend strips data URLs before persisting), so we * render a labelled placeholder tile instead of a broken ````. */ -function UserImages({ images }: { images: UIImage[] }) { +function UserImages({ + images, + align = "right", +}: { + images: UIImage[]; + align?: "left" | "right"; +}) { const { t } = useTranslation(); // Only real-URL images can open in the lightbox; historical-replay // placeholders (no URL) have nothing to zoom into. @@ -98,7 +180,12 @@ function UserImages({ images }: { images: UIImage[] }) { return ( <> -
+
{images.map((img, i) => ( toMediaAttachment(m)) + : ev.media?.map((url) => toMediaAttachment({ url })); + // A complete (non-streamed) assistant message. If a stream was in // flight, drop the placeholder so we don't render the text twice. const activeId = buffer.current?.messageId; @@ -162,6 +167,7 @@ export function useNanobotStream( role: "assistant", content: ev.text, createdAt: Date.now(), + ...(media && media.length > 0 ? { media } : {}), }, ]; }); diff --git a/webui/src/hooks/useSessions.ts b/webui/src/hooks/useSessions.ts index ea51c220..719d4ce1 100644 --- a/webui/src/hooks/useSessions.ts +++ b/webui/src/hooks/useSessions.ts @@ -9,6 +9,7 @@ import { listSessions, } from "@/lib/api"; import { deriveTitle } from "@/lib/format"; +import { toMediaAttachment } from "@/lib/media"; import type { ChatSummary, UIMessage } from "@/lib/types"; const EMPTY_MESSAGES: UIMessage[] = []; @@ -123,17 +124,16 @@ export function useSessionHistory(key: string | null): { const ui: UIMessage[] = body.messages.flatMap((m, idx) => { if (m.role !== "user" && m.role !== "assistant") return []; if (typeof m.content !== "string") return []; - // Hydrate signed media URLs into the bubble's ``images`` slot so - // historical user turns render real previews (the live-send path - // uses data URLs; both shapes converge on the same ``UIImage``). + // Hydrate signed media URLs into generic UI attachments. Image-only + // user turns still populate the legacy ``images`` slot so the + // existing optimistic-send and lightbox paths remain unchanged. + const media = + Array.isArray(m.media_urls) && m.media_urls.length > 0 + ? m.media_urls.map((mu) => toMediaAttachment(mu)) + : undefined; const images = - m.role === "user" && - Array.isArray(m.media_urls) && - m.media_urls.length > 0 - ? m.media_urls.map((mu) => ({ - url: mu.url, - name: mu.name, - })) + m.role === "user" && media?.every((item) => item.kind === "image") + ? media.map((item) => ({ url: item.url, name: item.name })) : undefined; return [ { @@ -142,6 +142,7 @@ export function useSessionHistory(key: string | null): { content: m.content, createdAt: m.timestamp ? Date.parse(m.timestamp) : Date.now(), ...(images ? { images } : {}), + ...(media ? { media } : {}), }, ]; }); diff --git a/webui/src/lib/media.ts b/webui/src/lib/media.ts new file mode 100644 index 00000000..399bc33a --- /dev/null +++ b/webui/src/lib/media.ts @@ -0,0 +1,59 @@ +import type { UIMediaAttachment, UIMediaKind } from "@/lib/types"; + +const IMAGE_EXTENSIONS = new Set([ + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".bmp", + ".ico", + ".tif", + ".tiff", +]); + +const VIDEO_EXTENSIONS = new Set([ + ".mp4", + ".webm", + ".mov", + ".m4v", + ".avi", + ".mkv", + ".3gp", +]); + +function cleanPath(value: string): string { + return value.split(/[?#]/, 1)[0]?.toLowerCase() ?? ""; +} + +function extensionOf(value?: string): string { + if (!value) return ""; + const path = cleanPath(value); + const dot = path.lastIndexOf("."); + if (dot < 0) return ""; + return path.slice(dot); +} + +export function inferMediaKind(media: { url?: string; name?: string }): UIMediaKind { + const url = media.url ?? ""; + if (url.startsWith("data:image/")) return "image"; + if (url.startsWith("data:video/")) return "video"; + + const ext = extensionOf(media.name) || extensionOf(url); + if (IMAGE_EXTENSIONS.has(ext)) return "image"; + if (VIDEO_EXTENSIONS.has(ext)) return "video"; + return "file"; +} + +export function toMediaAttachment(media: { + url?: string; + name?: string; + kind?: UIMediaKind; +}): UIMediaAttachment { + return { + kind: media.kind ?? inferMediaKind(media), + url: media.url, + name: media.name, + }; +} + diff --git a/webui/src/lib/types.ts b/webui/src/lib/types.ts index 6ffc75a9..245a65bd 100644 --- a/webui/src/lib/types.ts +++ b/webui/src/lib/types.ts @@ -22,6 +22,14 @@ export interface UIImage { name?: string; } +export type UIMediaKind = "image" | "video" | "file"; + +export interface UIMediaAttachment { + kind: UIMediaKind; + url?: string; + name?: string; +} + export interface UIMessage { id: string; role: Role; @@ -34,6 +42,8 @@ export interface UIMessage { traces?: string[]; /** User turn: optimistic blob URLs for preview. Replay: placeholder chips. */ images?: UIImage[]; + /** Signed or local UI-renderable media attachments. */ + media?: UIMediaAttachment[]; } export interface ChatSummary { @@ -71,6 +81,7 @@ export type InboundEvent = text: string; reply_to?: string; media?: string[]; + media_urls?: Array<{ url: string; name?: string }>; /** Present when the frame is an agent breadcrumb (e.g. tool hint, * generic progress line) rather than a conversational reply. */ kind?: "tool_hint" | "progress"; diff --git a/webui/src/tests/message-bubble.test.tsx b/webui/src/tests/message-bubble.test.tsx index 80c24018..e8dec29a 100644 --- a/webui/src/tests/message-bubble.test.tsx +++ b/webui/src/tests/message-bubble.test.tsx @@ -40,4 +40,28 @@ describe("MessageBubble", () => { fireEvent.click(toggle); expect(screen.queryByText('weather("get")')).not.toBeInTheDocument(); }); + + it("renders video media as an inline player", () => { + const message: UIMessage = { + id: "a1", + role: "assistant", + content: "here is the clip", + createdAt: Date.now(), + media: [ + { + kind: "video", + url: "/api/media/sig/payload", + name: "demo.mp4", + }, + ], + }; + + const { container } = render(); + + expect(screen.getByText("here is the clip")).toBeInTheDocument(); + const video = screen.getByLabelText(/video attachment/i); + expect(video.tagName).toBe("VIDEO"); + expect(video).toHaveAttribute("src", "/api/media/sig/payload"); + expect(container.querySelector("video[controls]")).toBeInTheDocument(); + }); }); diff --git a/webui/src/tests/useNanobotStream.test.tsx b/webui/src/tests/useNanobotStream.test.tsx index 91b3036c..6485980c 100644 --- a/webui/src/tests/useNanobotStream.test.tsx +++ b/webui/src/tests/useNanobotStream.test.tsx @@ -92,4 +92,25 @@ describe("useNanobotStream", () => { expect(result.current.messages[1].role).toBe("assistant"); expect(result.current.messages[1].kind).toBeUndefined(); }); + + it("attaches assistant media_urls to complete messages", () => { + const fake = fakeClient(); + const { result } = renderHook(() => useNanobotStream("chat-m", []), { + wrapper: wrap(fake.client), + }); + + act(() => { + fake.emit("chat-m", { + event: "message", + chat_id: "chat-m", + text: "video ready", + media_urls: [{ url: "/api/media/sig/payload", name: "demo.mp4" }], + }); + }); + + expect(result.current.messages).toHaveLength(1); + expect(result.current.messages[0].media).toEqual([ + { kind: "video", url: "/api/media/sig/payload", name: "demo.mp4" }, + ]); + }); }); diff --git a/webui/src/tests/useSessions.test.tsx b/webui/src/tests/useSessions.test.tsx index aaabe3d8..ad4f1c1a 100644 --- a/webui/src/tests/useSessions.test.tsx +++ b/webui/src/tests/useSessions.test.tsx @@ -130,12 +130,46 @@ describe("useSessions", () => { { url: "/api/media/sig-1/payload-1", name: "snap.png" }, { url: "/api/media/sig-2/payload-2", name: "diag.jpg" }, ]); + expect(first.media).toEqual([ + { kind: "image", url: "/api/media/sig-1/payload-1", name: "snap.png" }, + { kind: "image", url: "/api/media/sig-2/payload-2", name: "diag.jpg" }, + ]); expect(second.role).toBe("assistant"); expect(second.images).toBeUndefined(); expect(third.role).toBe("user"); expect(third.images).toBeUndefined(); }); + it("hydrates historical assistant video media_urls into media attachments", async () => { + vi.mocked(api.fetchSessionMessages).mockResolvedValue({ + key: "websocket:chat-video", + created_at: "2026-04-20T10:00:00Z", + updated_at: "2026-04-20T10:05:00Z", + messages: [ + { + role: "assistant", + content: "clip ready", + timestamp: "2026-04-20T10:00:01Z", + media_urls: [ + { url: "/api/media/sig-v/payload-v", name: "clip.mp4" }, + ], + }, + ], + }); + + const { result } = renderHook(() => useSessionHistory("websocket:chat-video"), { + wrapper: wrap(fakeClient()), + }); + + await waitFor(() => expect(result.current.loading).toBe(false)); + + expect(result.current.messages[0].role).toBe("assistant"); + expect(result.current.messages[0].images).toBeUndefined(); + expect(result.current.messages[0].media).toEqual([ + { kind: "video", url: "/api/media/sig-v/payload-v", name: "clip.mp4" }, + ]); + }); + it("keeps the session in the list when delete fails", async () => { vi.mocked(api.listSessions).mockResolvedValue([ { From 076e4166d7f2ce7424c5a11a4904369d2801e119 Mon Sep 17 00:00:00 2001 From: yorkhellen Date: Sat, 25 Apr 2026 00:39:06 +0800 Subject: [PATCH 19/80] fix(agent): add LLM request timeout to prevent session lock starvation --- nanobot/agent/runner.py | 34 ++++++++++++++++++++++++++++++---- tests/agent/test_runner.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index d90c79fe..3704f303 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -5,6 +5,7 @@ from __future__ import annotations import asyncio from dataclasses import dataclass, field import inspect +import os from pathlib import Path from typing import Any @@ -13,7 +14,7 @@ from loguru import logger from nanobot.agent.hook import AgentHook, AgentHookContext from nanobot.utils.prompt_templates import render_template from nanobot.agent.tools.registry import ToolRegistry -from nanobot.providers.base import LLMProvider, ToolCallRequest +from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest from nanobot.utils.helpers import ( build_assistant_message, estimate_message_tokens, @@ -74,6 +75,7 @@ class AgentRunSpec: retry_wait_callback: Any | None = None checkpoint_callback: Any | None = None injection_callback: Any | None = None + llm_timeout_s: float | None = None @dataclass(slots=True) @@ -570,6 +572,19 @@ class AgentRunner: hook: AgentHook, context: AgentHookContext, ): + timeout_s: float | None = spec.llm_timeout_s + if timeout_s is None: + # Default to a finite timeout to avoid per-session lock starvation when an LLM + # request hangs indefinitely (e.g. gateway/network stall). + # Set NANOBOT_LLM_TIMEOUT_S=0 to disable. + raw = os.environ.get("NANOBOT_LLM_TIMEOUT_S", "300").strip() + try: + timeout_s = float(raw) + except (TypeError, ValueError): + timeout_s = 300.0 + if timeout_s is not None and timeout_s <= 0: + timeout_s = None + kwargs = self._build_request_kwargs( spec, messages, @@ -579,11 +594,23 @@ class AgentRunner: async def _stream(delta: str) -> None: await hook.on_stream(context, delta) - return await self.provider.chat_stream_with_retry( + coro = self.provider.chat_stream_with_retry( **kwargs, on_content_delta=_stream, ) - return await self.provider.chat_with_retry(**kwargs) + else: + coro = self.provider.chat_with_retry(**kwargs) + + if timeout_s is None: + return await coro + try: + return await asyncio.wait_for(coro, timeout=timeout_s) + except asyncio.TimeoutError: + return LLMResponse( + content=f"Error calling LLM: timed out after {timeout_s:g}s", + finish_reason="error", + error_kind="timeout", + ) async def _request_finalization_retry( self, @@ -984,4 +1011,3 @@ class AgentRunner: if current: batches.append(current) return batches - diff --git a/tests/agent/test_runner.py b/tests/agent/test_runner.py index b47db948..ffa5fda9 100644 --- a/tests/agent/test_runner.py +++ b/tests/agent/test_runner.py @@ -252,6 +252,35 @@ async def test_runner_returns_max_iterations_fallback(): assert result.messages[-1]["role"] == "assistant" assert result.messages[-1]["content"] == result.final_content + +@pytest.mark.asyncio +async def test_runner_times_out_hung_llm_request(): + from nanobot.agent.runner import AgentRunSpec, AgentRunner + + provider = MagicMock() + + async def chat_with_retry(**kwargs): + await asyncio.sleep(3600) + + provider.chat_with_retry = chat_with_retry + tools = MagicMock() + tools.get_definitions.return_value = [] + + runner = AgentRunner(provider) + started = time.monotonic() + result = await runner.run(AgentRunSpec( + initial_messages=[{"role": "user", "content": "hello"}], + tools=tools, + model="test-model", + max_iterations=1, + max_tool_result_chars=_MAX_TOOL_RESULT_CHARS, + llm_timeout_s=0.05, + )) + + assert (time.monotonic() - started) < 1.0 + assert result.stop_reason == "error" + assert "timed out" in (result.final_content or "").lower() + @pytest.mark.asyncio async def test_runner_returns_structured_tool_error(): from nanobot.agent.runner import AgentRunSpec, AgentRunner From 39a5a77874bcb30424861cf9e78894a70d9c8e04 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Fri, 24 Apr 2026 20:00:56 +0000 Subject: [PATCH 20/80] fix(feishu): send videos with media message type --- nanobot/channels/feishu.py | 6 +++--- tests/channels/test_feishu_reply.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/nanobot/channels/feishu.py b/nanobot/channels/feishu.py index 1442c363..41e93780 100644 --- a/nanobot/channels/feishu.py +++ b/nanobot/channels/feishu.py @@ -1457,13 +1457,13 @@ class FeishuChannel(BaseChannel): else: key = await loop.run_in_executor(None, self._upload_file_sync, file_path) if key: - # Use msg_type "audio" for audio, "video" for video, "file" for documents. + # Feishu's OpenAPI names video messages "media". + # Use "audio" for audio, "media" for video, "file" for documents. # Feishu requires these specific msg_types for inline playback. - # Note: "media" is only valid as a tag inside "post" messages, not as a standalone msg_type. if ext in self._AUDIO_EXTS: media_type = "audio" elif ext in self._VIDEO_EXTS: - media_type = "video" + media_type = "media" else: media_type = "file" await loop.run_in_executor( diff --git a/tests/channels/test_feishu_reply.py b/tests/channels/test_feishu_reply.py index 0753653a..2ad466dc 100644 --- a/tests/channels/test_feishu_reply.py +++ b/tests/channels/test_feishu_reply.py @@ -202,7 +202,7 @@ def test_reply_message_sync_returns_false_on_exception() -> None: ("filename", "expected_msg_type"), [ ("voice.opus", "audio"), - ("clip.mp4", "video"), + ("clip.mp4", "media"), ("report.pdf", "file"), ], ) From 106ae2cf1f50063b65c89ce15aa1dad2c531475f Mon Sep 17 00:00:00 2001 From: zhuzhh Date: Sat, 25 Apr 2026 12:22:36 +0800 Subject: [PATCH 21/80] fix(msteams): prune stale and unsupported conversation refs --- docs/chat-apps.md | 3 +- nanobot/channels/msteams.py | 62 ++++++++++++++++++++++++- tests/test_msteams.py | 93 +++++++++++++++++++++++++++++++++++-- 3 files changed, 153 insertions(+), 5 deletions(-) diff --git a/docs/chat-apps.md b/docs/chat-apps.md index 9332bdc0..3bf2bee2 100644 --- a/docs/chat-apps.md +++ b/docs/chat-apps.md @@ -651,6 +651,7 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess > - `replyInThread: true` replies to the triggering Teams activity when a stored `activity_id` is available. > - `mentionOnlyResponse` controls what Nanobot receives when a user sends only a bot mention (`Nanobot`). Set to `""` to ignore mention-only messages. > - `validateInboundAuth: true` enables inbound Bot Framework bearer-token validation (signature, issuer, audience, lifetime, `serviceUrl`). This is the safe default for public deployments. Only set it to `false` for local development or tightly controlled testing. +> - Conversation refs are auto-pruned to avoid bad outbound routing: Web Chat refs, non-`personal` refs, and refs older than 30 days are removed. **4. Run** @@ -658,4 +659,4 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess nanobot gateway ``` - \ No newline at end of file + diff --git a/nanobot/channels/msteams.py b/nanobot/channels/msteams.py index 427b35f8..bdbdf8c8 100644 --- a/nanobot/channels/msteams.py +++ b/nanobot/channels/msteams.py @@ -21,6 +21,7 @@ import time from dataclasses import dataclass from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import TYPE_CHECKING, Any +from urllib.parse import urlparse import httpx from loguru import logger @@ -43,6 +44,10 @@ if TYPE_CHECKING: if MSTEAMS_AVAILABLE: import jwt +MSTEAMS_REF_TTL_DAYS = 30 +MSTEAMS_REF_TTL_S = MSTEAMS_REF_TTL_DAYS * 24 * 60 * 60 +MSTEAMS_WEBCHAT_HOST = "webchat.botframework.com" + class MSTeamsConfig(Base): """Microsoft Teams channel configuration.""" @@ -70,6 +75,7 @@ class ConversationRef: activity_id: str | None = None conversation_type: str | None = None tenant_id: str | None = None + updated_at: float | None = None class MSTeamsChannel(BaseChannel): @@ -103,6 +109,8 @@ class MSTeamsChannel(BaseChannel): self._refs_path = get_workspace_path() / "state" / "msteams_conversations.json" self._refs_path.parent.mkdir(parents=True, exist_ok=True) self._conversation_refs: dict[str, ConversationRef] = self._load_refs() + if self._prune_conversation_refs(): + self._save_refs(prune=False) async def start(self) -> None: """Start the Teams webhook listener.""" @@ -289,6 +297,7 @@ class MSTeamsChannel(BaseChannel): activity_id=activity_id or None, conversation_type=conversation_type or None, tenant_id=str((channel_data.get("tenant") or {}).get("id") or "") or None, + updated_at=time.time(), ) self._save_refs() @@ -491,9 +500,59 @@ class MSTeamsChannel(BaseChannel): logger.warning("Failed to load MSTeams conversation refs: {}", e) return {} - def _save_refs(self) -> None: + def _is_webchat_service_url(self, service_url: str) -> bool: + """Return True when service URL points to unsupported Bot Framework Web Chat.""" + normalized = service_url.strip() + if not normalized: + return False + host = (urlparse(normalized).hostname or "").strip().lower() + if host: + return host == MSTEAMS_WEBCHAT_HOST or host.endswith(f".{MSTEAMS_WEBCHAT_HOST}") + return MSTEAMS_WEBCHAT_HOST in normalized.lower() + + def _prune_conversation_refs(self, *, now: float | None = None) -> bool: + """Remove stale and unsupported conversation refs from memory.""" + if not self._conversation_refs: + return False + + now_ts = time.time() if now is None else now + stale_before = now_ts - MSTEAMS_REF_TTL_S + keys_to_drop: list[str] = [] + + for key, ref in self._conversation_refs.items(): + if self._is_webchat_service_url(ref.service_url): + keys_to_drop.append(key) + continue + + conv_type = str(ref.conversation_type or "").strip().lower() + if conv_type and conv_type != "personal": + keys_to_drop.append(key) + continue + + try: + updated_at = float(ref.updated_at) if ref.updated_at is not None else 0.0 + except (TypeError, ValueError): + updated_at = 0.0 + if updated_at <= 0 or updated_at < stale_before: + keys_to_drop.append(key) + + if not keys_to_drop: + return False + + for key in keys_to_drop: + self._conversation_refs.pop(key, None) + logger.info( + "MSTeams pruned {} stale/unsupported conversation refs (ttl={} days)", + len(keys_to_drop), + MSTEAMS_REF_TTL_DAYS, + ) + return True + + def _save_refs(self, *, prune: bool = True) -> None: """Persist conversation references.""" try: + if prune: + self._prune_conversation_refs() data = { key: { "service_url": ref.service_url, @@ -502,6 +561,7 @@ class MSTeamsChannel(BaseChannel): "activity_id": ref.activity_id, "conversation_type": ref.conversation_type, "tenant_id": ref.tenant_id, + "updated_at": ref.updated_at, } for key, ref in self._conversation_refs.items() } diff --git a/tests/test_msteams.py b/tests/test_msteams.py index f5597c38..4febd791 100644 --- a/tests/test_msteams.py +++ b/tests/test_msteams.py @@ -17,7 +17,7 @@ from cryptography.hazmat.primitives.asymmetric import rsa import nanobot.channels.msteams as msteams_module from nanobot.bus.events import OutboundMessage -from nanobot.channels.msteams import ConversationRef, MSTeamsChannel, MSTeamsConfig +from nanobot.channels.msteams import ConversationRef, MSTeamsChannel class DummyBus: @@ -115,6 +115,95 @@ async def test_handle_activity_personal_message_publishes_and_stores_ref(make_ch saved = json.loads((tmp_path / "state" / "msteams_conversations.json").read_text(encoding="utf-8")) assert saved["conv-123"]["conversation_id"] == "conv-123" assert saved["conv-123"]["tenant_id"] == "tenant-id" + assert float(saved["conv-123"]["updated_at"]) > 0 + + +def test_init_prunes_stale_and_unsupported_conversation_refs(make_channel, tmp_path, monkeypatch): + now = 1_800_000_000.0 + monkeypatch.setattr(msteams_module.time, "time", lambda: now) + + state_dir = tmp_path / "state" + state_dir.mkdir(parents=True, exist_ok=True) + refs_path = state_dir / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-valid": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-valid", + "conversation_type": "personal", + "updated_at": now - 60, + }, + "conv-webchat": { + "service_url": "https://webchat.botframework.com/", + "conversation_id": "conv-webchat", + "conversation_type": "personal", + "updated_at": now - 60, + }, + "conv-group": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-group", + "conversation_type": "channel", + "updated_at": now - 60, + }, + "conv-stale": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-stale", + "conversation_type": "personal", + "updated_at": now - msteams_module.MSTEAMS_REF_TTL_S - 1, + }, + "conv-missing-ts": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-missing-ts", + "conversation_type": "personal", + }, + }, + indent=2, + ), + encoding="utf-8", + ) + + ch = make_channel() + + assert set(ch._conversation_refs.keys()) == {"conv-valid"} + assert ch._conversation_refs["conv-valid"].conversation_id == "conv-valid" + + persisted = json.loads(refs_path.read_text(encoding="utf-8")) + assert set(persisted.keys()) == {"conv-valid"} + + +def test_save_prunes_unsupported_conversation_refs(make_channel, tmp_path, monkeypatch): + now = 1_800_000_000.0 + monkeypatch.setattr(msteams_module.time, "time", lambda: now) + + ch = make_channel() + ch._conversation_refs = { + "conv-valid": ConversationRef( + service_url="https://smba.trafficmanager.net/amer/", + conversation_id="conv-valid", + conversation_type="personal", + updated_at=now, + ), + "conv-webchat": ConversationRef( + service_url="https://webchat.botframework.com/", + conversation_id="conv-webchat", + conversation_type="personal", + updated_at=now, + ), + "conv-group": ConversationRef( + service_url="https://smba.trafficmanager.net/amer/", + conversation_id="conv-group", + conversation_type="groupChat", + updated_at=now, + ), + } + + ch._save_refs() + + assert set(ch._conversation_refs.keys()) == {"conv-valid"} + + saved = json.loads((tmp_path / "state" / "msteams_conversations.json").read_text(encoding="utf-8")) + assert set(saved.keys()) == {"conv-valid"} @pytest.mark.asyncio @@ -558,5 +647,3 @@ def test_msteams_default_config_includes_restart_notify_fields(): assert "restartNotifyEnabled" not in cfg assert "restartNotifyPreMessage" not in cfg assert "restartNotifyPostMessage" not in cfg - - From 15e9d0471f2694b564d6b8c63b3d7be1fe922d1a Mon Sep 17 00:00:00 2001 From: zhuzhh Date: Sat, 25 Apr 2026 12:58:04 +0800 Subject: [PATCH 22/80] feat(msteams): make ref pruning configurable and atomic --- docs/chat-apps.md | 9 ++- nanobot/channels/msteams.py | 38 ++++++++++-- tests/test_msteams.py | 112 ++++++++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 7 deletions(-) diff --git a/docs/chat-apps.md b/docs/chat-apps.md index 3bf2bee2..3d5e4dbd 100644 --- a/docs/chat-apps.md +++ b/docs/chat-apps.md @@ -642,7 +642,10 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess "allowFrom": ["*"], "replyInThread": true, "mentionOnlyResponse": "Hi — what can I help with?", - "validateInboundAuth": true + "validateInboundAuth": true, + "refTtlDays": 30, + "pruneWebChatRefs": true, + "pruneNonPersonalRefs": true } } } @@ -651,7 +654,9 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess > - `replyInThread: true` replies to the triggering Teams activity when a stored `activity_id` is available. > - `mentionOnlyResponse` controls what Nanobot receives when a user sends only a bot mention (`Nanobot`). Set to `""` to ignore mention-only messages. > - `validateInboundAuth: true` enables inbound Bot Framework bearer-token validation (signature, issuer, audience, lifetime, `serviceUrl`). This is the safe default for public deployments. Only set it to `false` for local development or tightly controlled testing. -> - Conversation refs are auto-pruned to avoid bad outbound routing: Web Chat refs, non-`personal` refs, and refs older than 30 days are removed. +> - `refTtlDays` (default `30`) controls how old stored conversation refs can be before they are pruned. +> - `pruneWebChatRefs` (default `true`) drops refs with `webchat.botframework.com` service URLs. +> - `pruneNonPersonalRefs` (default `true`) drops refs whose `conversation_type` is not `personal`. **4. Run** diff --git a/nanobot/channels/msteams.py b/nanobot/channels/msteams.py index bdbdf8c8..7b294a83 100644 --- a/nanobot/channels/msteams.py +++ b/nanobot/channels/msteams.py @@ -15,7 +15,9 @@ import asyncio import html import importlib.util import json +import os import re +import tempfile import threading import time from dataclasses import dataclass @@ -63,6 +65,9 @@ class MSTeamsConfig(Base): reply_in_thread: bool = True mention_only_response: str = "Hi — what can I help with?" validate_inbound_auth: bool = True + ref_ttl_days: int = Field(default=MSTEAMS_REF_TTL_DAYS, ge=1) + prune_web_chat_refs: bool = True + prune_non_personal_refs: bool = True @dataclass @@ -516,16 +521,17 @@ class MSTeamsChannel(BaseChannel): return False now_ts = time.time() if now is None else now - stale_before = now_ts - MSTEAMS_REF_TTL_S + ttl_days = int(self.config.ref_ttl_days) + stale_before = now_ts - (ttl_days * 24 * 60 * 60) keys_to_drop: list[str] = [] for key, ref in self._conversation_refs.items(): - if self._is_webchat_service_url(ref.service_url): + if self.config.prune_web_chat_refs and self._is_webchat_service_url(ref.service_url): keys_to_drop.append(key) continue conv_type = str(ref.conversation_type or "").strip().lower() - if conv_type and conv_type != "personal": + if self.config.prune_non_personal_refs and conv_type and conv_type != "personal": keys_to_drop.append(key) continue @@ -544,10 +550,32 @@ class MSTeamsChannel(BaseChannel): logger.info( "MSTeams pruned {} stale/unsupported conversation refs (ttl={} days)", len(keys_to_drop), - MSTEAMS_REF_TTL_DAYS, + ttl_days, ) return True + def _write_refs_atomically(self, data: dict[str, Any]) -> None: + """Write refs JSON atomically to reduce corruption risk during crashes.""" + payload = json.dumps(data, indent=2) + tmp_path: str | None = None + try: + fd, tmp_path = tempfile.mkstemp( + dir=str(self._refs_path.parent), + prefix=f"{self._refs_path.name}.", + suffix=".tmp", + ) + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(payload) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp_path, self._refs_path) + finally: + if tmp_path and os.path.exists(tmp_path): + try: + os.unlink(tmp_path) + except OSError: + pass + def _save_refs(self, *, prune: bool = True) -> None: """Persist conversation references.""" try: @@ -565,7 +593,7 @@ class MSTeamsChannel(BaseChannel): } for key, ref in self._conversation_refs.items() } - self._refs_path.write_text(json.dumps(data, indent=2), encoding="utf-8") + self._write_refs_atomically(data) except Exception as e: logger.warning("Failed to save MSTeams conversation refs: {}", e) diff --git a/tests/test_msteams.py b/tests/test_msteams.py index 4febd791..da6bf511 100644 --- a/tests/test_msteams.py +++ b/tests/test_msteams.py @@ -206,6 +206,115 @@ def test_save_prunes_unsupported_conversation_refs(make_channel, tmp_path, monke assert set(saved.keys()) == {"conv-valid"} +def test_init_respects_prune_toggle_flags(make_channel, tmp_path, monkeypatch): + now = 1_800_000_000.0 + monkeypatch.setattr(msteams_module.time, "time", lambda: now) + + state_dir = tmp_path / "state" + state_dir.mkdir(parents=True, exist_ok=True) + refs_path = state_dir / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-webchat": { + "service_url": "https://webchat.botframework.com/", + "conversation_id": "conv-webchat", + "conversation_type": "personal", + "updated_at": now - 60, + }, + "conv-group": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-group", + "conversation_type": "channel", + "updated_at": now - 60, + }, + }, + indent=2, + ), + encoding="utf-8", + ) + + ch = make_channel(pruneWebChatRefs=False, pruneNonPersonalRefs=False) + + assert set(ch._conversation_refs.keys()) == {"conv-webchat", "conv-group"} + persisted = json.loads(refs_path.read_text(encoding="utf-8")) + assert set(persisted.keys()) == {"conv-webchat", "conv-group"} + + +def test_init_respects_custom_ref_ttl_days(make_channel, tmp_path, monkeypatch): + now = 1_800_000_000.0 + monkeypatch.setattr(msteams_module.time, "time", lambda: now) + + state_dir = tmp_path / "state" + state_dir.mkdir(parents=True, exist_ok=True) + refs_path = state_dir / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-fresh": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-fresh", + "conversation_type": "personal", + "updated_at": now - 12 * 60 * 60, + }, + "conv-old": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-old", + "conversation_type": "personal", + "updated_at": now - 10 * 24 * 60 * 60, + }, + }, + indent=2, + ), + encoding="utf-8", + ) + + ch = make_channel(refTtlDays=1) + + assert set(ch._conversation_refs.keys()) == {"conv-fresh"} + persisted = json.loads(refs_path.read_text(encoding="utf-8")) + assert set(persisted.keys()) == {"conv-fresh"} + + +def test_save_uses_atomic_replace_and_keeps_existing_file_on_replace_error(make_channel, tmp_path, monkeypatch): + ch = make_channel() + refs_path = tmp_path / "state" / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-old": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-old", + "conversation_type": "personal", + "updated_at": 1_700_000_000.0, + } + }, + indent=2, + ), + encoding="utf-8", + ) + + ch._conversation_refs = { + "conv-new": ConversationRef( + service_url="https://smba.trafficmanager.net/amer/", + conversation_id="conv-new", + conversation_type="personal", + updated_at=1_800_000_000.0, + ) + } + + def _raise_replace(_src, _dst): + raise OSError("replace failed") + + monkeypatch.setattr(msteams_module.os, "replace", _raise_replace) + ch._save_refs() + + persisted = json.loads(refs_path.read_text(encoding="utf-8")) + assert set(persisted.keys()) == {"conv-old"} + tmp_files = list((tmp_path / "state").glob("msteams_conversations.json.*.tmp")) + assert tmp_files == [] + + @pytest.mark.asyncio async def test_handle_activity_ignores_group_messages(make_channel): ch = make_channel() @@ -644,6 +753,9 @@ def test_msteams_default_config_includes_restart_notify_fields(): cfg = MSTeamsChannel.default_config() assert cfg["validateInboundAuth"] is True + assert cfg["refTtlDays"] == msteams_module.MSTEAMS_REF_TTL_DAYS + assert cfg["pruneWebChatRefs"] is True + assert cfg["pruneNonPersonalRefs"] is True assert "restartNotifyEnabled" not in cfg assert "restartNotifyPreMessage" not in cfg assert "restartNotifyPostMessage" not in cfg From fe928a0d94736d26dbfaacdf68723449da61ebb6 Mon Sep 17 00:00:00 2001 From: zhuzhh Date: Sat, 25 Apr 2026 15:39:43 +0800 Subject: [PATCH 23/80] feat(msteams): split ref storage into main+meta sidecar files - Separate updated_at into a meta sidecar file (msteams_conversations_meta.json) to keep backward compatibility with legacy data that never had updated_at. On first upgrade, legacy refs are kept alive by initializing updated_at to now instead of purging them immediately. - Add cross-process locking via fcntl (with Windows fallback) to prevent concurrent writes from different gateway processes overwriting each other. - Add ref_touch_interval_s config (default 300s) to throttle how often successful sends refresh updated_at, preventing unnecessary I/O. - Touch active refs on send success to prevent them from expiring while in use. - Add _safe_float and _normalize_ref_record for robust schema migration. - All refs operations now use threading.RLock within a process. --- docs/chat-apps.md | 4 +- nanobot/channels/msteams.py | 231 +++++++++++++++++++++++++++++------- tests/test_msteams.py | 100 ++++++++++++++-- 3 files changed, 283 insertions(+), 52 deletions(-) diff --git a/docs/chat-apps.md b/docs/chat-apps.md index 3d5e4dbd..6eea7d92 100644 --- a/docs/chat-apps.md +++ b/docs/chat-apps.md @@ -645,7 +645,8 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess "validateInboundAuth": true, "refTtlDays": 30, "pruneWebChatRefs": true, - "pruneNonPersonalRefs": true + "pruneNonPersonalRefs": true, + "refTouchIntervalS": 300 } } } @@ -657,6 +658,7 @@ Create or reuse a Microsoft Teams / Azure bot app registration. Set the bot mess > - `refTtlDays` (default `30`) controls how old stored conversation refs can be before they are pruned. > - `pruneWebChatRefs` (default `true`) drops refs with `webchat.botframework.com` service URLs. > - `pruneNonPersonalRefs` (default `true`) drops refs whose `conversation_type` is not `personal`. +> - `refTouchIntervalS` (default `300`) throttles how often successful sends refresh `updated_at` for active refs. **4. Run** diff --git a/nanobot/channels/msteams.py b/nanobot/channels/msteams.py index 7b294a83..685774bf 100644 --- a/nanobot/channels/msteams.py +++ b/nanobot/channels/msteams.py @@ -20,11 +20,17 @@ import re import tempfile import threading import time +from contextlib import contextmanager from dataclasses import dataclass from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from typing import TYPE_CHECKING, Any from urllib.parse import urlparse +try: # pragma: no cover - Windows fallback path + import fcntl +except ImportError: # pragma: no cover + fcntl = None + import httpx from loguru import logger from pydantic import Field @@ -49,6 +55,9 @@ if MSTEAMS_AVAILABLE: MSTEAMS_REF_TTL_DAYS = 30 MSTEAMS_REF_TTL_S = MSTEAMS_REF_TTL_DAYS * 24 * 60 * 60 MSTEAMS_WEBCHAT_HOST = "webchat.botframework.com" +MSTEAMS_REF_META_FILENAME = "msteams_conversations_meta.json" +MSTEAMS_REF_LOCK_FILENAME = "msteams_conversations.lock" +MSTEAMS_REF_TOUCH_INTERVAL_S = 300 class MSTeamsConfig(Base): @@ -68,6 +77,7 @@ class MSTeamsConfig(Base): ref_ttl_days: int = Field(default=MSTEAMS_REF_TTL_DAYS, ge=1) prune_web_chat_refs: bool = True prune_non_personal_refs: bool = True + ref_touch_interval_s: int = Field(default=MSTEAMS_REF_TOUCH_INTERVAL_S, ge=0) @dataclass @@ -113,9 +123,13 @@ class MSTeamsChannel(BaseChannel): self._botframework_jwks_expires_at: float = 0.0 self._refs_path = get_workspace_path() / "state" / "msteams_conversations.json" self._refs_path.parent.mkdir(parents=True, exist_ok=True) + self._refs_meta_path = self._refs_path.parent / MSTEAMS_REF_META_FILENAME + self._refs_lock_path = self._refs_path.parent / MSTEAMS_REF_LOCK_FILENAME + self._refs_guard = threading.RLock() self._conversation_refs: dict[str, ConversationRef] = self._load_refs() - if self._prune_conversation_refs(): - self._save_refs(prune=False) + with self._refs_guard: + if self._prune_conversation_refs(): + self._save_refs_locked(prune=True) async def start(self) -> None: """Start the Teams webhook listener.""" @@ -249,6 +263,7 @@ class MSTeamsChannel(BaseChannel): resp = await self._http.post(url, headers=headers, json=payload) resp.raise_for_status() logger.info("MSTeams message sent to {}", ref.conversation_id) + self._touch_conversation_ref(str(msg.chat_id), persist=True) except Exception as e: logger.error("MSTeams send failed: {}", e) raise @@ -295,16 +310,17 @@ class MSTeamsChannel(BaseChannel): ) return - self._conversation_refs[conversation_id] = ConversationRef( - service_url=service_url, - conversation_id=conversation_id, - bot_id=str(recipient.get("id") or "") or None, - activity_id=activity_id or None, - conversation_type=conversation_type or None, - tenant_id=str((channel_data.get("tenant") or {}).get("id") or "") or None, - updated_at=time.time(), - ) - self._save_refs() + with self._refs_guard: + self._conversation_refs[conversation_id] = ConversationRef( + service_url=service_url, + conversation_id=conversation_id, + bot_id=str(recipient.get("id") or "") or None, + activity_id=activity_id or None, + conversation_type=conversation_type or None, + tenant_id=str((channel_data.get("tenant") or {}).get("id") or "") or None, + updated_at=time.time(), + ) + self._save_refs_locked() await self._handle_message( sender_id=sender_id, @@ -491,19 +507,109 @@ class MSTeamsChannel(BaseChannel): self._botframework_jwks_expires_at = now + 3600 return self._botframework_jwks + @staticmethod + def _safe_float(value: Any) -> float | None: + try: + out = float(value) + if out > 0: + return out + except (TypeError, ValueError): + return None + return None + + def _normalize_ref_record(self, value: Any) -> ConversationRef | None: + """Normalize a stored ref record from legacy/current schema.""" + if not isinstance(value, dict): + return None + service_url = str(value.get("service_url") or "").strip() + conversation_id = str(value.get("conversation_id") or "").strip() + if not service_url or not conversation_id: + return None + return ConversationRef( + service_url=service_url, + conversation_id=conversation_id, + bot_id=str(value.get("bot_id") or "") or None, + activity_id=str(value.get("activity_id") or "") or None, + conversation_type=str(value.get("conversation_type") or "") or None, + tenant_id=str(value.get("tenant_id") or "") or None, + updated_at=self._safe_float(value.get("updated_at")), + ) + + def _load_refs_raw(self) -> tuple[dict[str, Any], dict[str, Any], bool]: + """Load raw refs/main+meta JSON payloads.""" + main_data: dict[str, Any] = {} + meta_data: dict[str, Any] = {} + meta_exists = self._refs_meta_path.exists() + + if self._refs_path.exists(): + try: + loaded = json.loads(self._refs_path.read_text(encoding="utf-8")) + if isinstance(loaded, dict): + main_data = loaded + except Exception as e: + logger.warning("Failed to load MSTeams conversation refs: {}", e) + + if meta_exists: + try: + loaded_meta = json.loads(self._refs_meta_path.read_text(encoding="utf-8")) + if isinstance(loaded_meta, dict): + meta_data = loaded_meta + except Exception as e: + logger.warning("Failed to load MSTeams conversation refs metadata: {}", e) + + return main_data, meta_data, meta_exists + + def _load_refs_from_disk(self) -> dict[str, ConversationRef]: + """Load refs from disk with compatibility fallback for legacy layouts.""" + main_data, meta_data, meta_exists = self._load_refs_raw() + if not main_data: + return {} + + out: dict[str, ConversationRef] = {} + now = time.time() + for key, value in main_data.items(): + ref = self._normalize_ref_record(value) + if not ref: + continue + + meta_entry = meta_data.get(key) if isinstance(meta_data, dict) else None + meta_ts = None + if isinstance(meta_entry, dict): + meta_ts = self._safe_float(meta_entry.get("updated_at")) + elif meta_entry is not None: + meta_ts = self._safe_float(meta_entry) + + if meta_ts is not None: + ref.updated_at = meta_ts + elif not meta_exists: + # First run after introducing meta sidecar: keep legacy refs alive + # by initializing timestamps to "now" instead of purging immediately. + ref.updated_at = now + elif ref.updated_at is None: + ref.updated_at = now + + out[key] = ref + return out + def _load_refs(self) -> dict[str, ConversationRef]: """Load stored conversation references.""" - if not self._refs_path.exists(): - return {} + return self._load_refs_from_disk() + + @contextmanager + def _refs_file_lock(self): + """Cross-process lock while merging and writing refs state.""" + self._refs_path.parent.mkdir(parents=True, exist_ok=True) + lock_fp = self._refs_lock_path.open("a+", encoding="utf-8") try: - data = json.loads(self._refs_path.read_text(encoding="utf-8")) - out: dict[str, ConversationRef] = {} - for key, value in data.items(): - out[key] = ConversationRef(**value) - return out - except Exception as e: - logger.warning("Failed to load MSTeams conversation refs: {}", e) - return {} + if fcntl is not None: + fcntl.flock(lock_fp.fileno(), fcntl.LOCK_EX) + yield + finally: + try: + if fcntl is not None: + fcntl.flock(lock_fp.fileno(), fcntl.LOCK_UN) + finally: + lock_fp.close() def _is_webchat_service_url(self, service_url: str) -> bool: """Return True when service URL points to unsupported Bot Framework Web Chat.""" @@ -554,21 +660,49 @@ class MSTeamsChannel(BaseChannel): ) return True - def _write_refs_atomically(self, data: dict[str, Any]) -> None: + def _merge_refs_from_disk_locked(self) -> None: + """Merge disk refs into memory to reduce lost updates across processes.""" + disk_refs = self._load_refs_from_disk() + for key, disk_ref in disk_refs.items(): + mem_ref = self._conversation_refs.get(key) + if mem_ref is None: + self._conversation_refs[key] = disk_ref + continue + disk_ts = self._safe_float(disk_ref.updated_at) or 0.0 + mem_ts = self._safe_float(mem_ref.updated_at) or 0.0 + if disk_ts > mem_ts: + self._conversation_refs[key] = disk_ref + + def _touch_conversation_ref(self, chat_id: str, *, persist: bool = False) -> None: + """Refresh updated_at for an active ref to keep it from expiring while used.""" + with self._refs_guard: + ref = self._conversation_refs.get(str(chat_id)) + if not ref: + return + now = time.time() + prev = self._safe_float(ref.updated_at) or 0.0 + min_interval = max(0, int(self.config.ref_touch_interval_s)) + if min_interval > 0 and prev > 0 and now - prev < min_interval: + return + ref.updated_at = now + if persist: + self._save_refs_locked() + + def _write_json_atomically(self, path, data: dict[str, Any]) -> None: """Write refs JSON atomically to reduce corruption risk during crashes.""" payload = json.dumps(data, indent=2) tmp_path: str | None = None try: fd, tmp_path = tempfile.mkstemp( - dir=str(self._refs_path.parent), - prefix=f"{self._refs_path.name}.", + dir=str(path.parent), + prefix=f"{path.name}.", suffix=".tmp", ) with os.fdopen(fd, "w", encoding="utf-8") as f: f.write(payload) f.flush() os.fsync(f.fileno()) - os.replace(tmp_path, self._refs_path) + os.replace(tmp_path, path) finally: if tmp_path and os.path.exists(tmp_path): try: @@ -576,27 +710,40 @@ class MSTeamsChannel(BaseChannel): except OSError: pass - def _save_refs(self, *, prune: bool = True) -> None: - """Persist conversation references.""" + def _save_refs_locked(self, *, prune: bool = True) -> None: + """Persist conversation references (caller must hold _refs_guard).""" try: - if prune: - self._prune_conversation_refs() - data = { - key: { - "service_url": ref.service_url, - "conversation_id": ref.conversation_id, - "bot_id": ref.bot_id, - "activity_id": ref.activity_id, - "conversation_type": ref.conversation_type, - "tenant_id": ref.tenant_id, - "updated_at": ref.updated_at, + with self._refs_file_lock(): + self._merge_refs_from_disk_locked() + if prune: + self._prune_conversation_refs() + refs_data = { + key: { + "service_url": ref.service_url, + "conversation_id": ref.conversation_id, + "bot_id": ref.bot_id, + "activity_id": ref.activity_id, + "conversation_type": ref.conversation_type, + "tenant_id": ref.tenant_id, + } + for key, ref in self._conversation_refs.items() } - for key, ref in self._conversation_refs.items() - } - self._write_refs_atomically(data) + refs_meta = { + key: { + "updated_at": self._safe_float(ref.updated_at), + } + for key, ref in self._conversation_refs.items() + } + self._write_json_atomically(self._refs_path, refs_data) + self._write_json_atomically(self._refs_meta_path, refs_meta) except Exception as e: logger.warning("Failed to save MSTeams conversation refs: {}", e) + def _save_refs(self, *, prune: bool = True) -> None: + """Persist conversation references.""" + with self._refs_guard: + self._save_refs_locked(prune=prune) + async def _get_access_token(self) -> str: """Fetch an access token for Bot Framework / Azure Bot auth.""" diff --git a/tests/test_msteams.py b/tests/test_msteams.py index da6bf511..dae2bbfa 100644 --- a/tests/test_msteams.py +++ b/tests/test_msteams.py @@ -115,7 +115,10 @@ async def test_handle_activity_personal_message_publishes_and_stores_ref(make_ch saved = json.loads((tmp_path / "state" / "msteams_conversations.json").read_text(encoding="utf-8")) assert saved["conv-123"]["conversation_id"] == "conv-123" assert saved["conv-123"]["tenant_id"] == "tenant-id" - assert float(saved["conv-123"]["updated_at"]) > 0 + saved_meta = json.loads( + (tmp_path / "state" / msteams_module.MSTEAMS_REF_META_FILENAME).read_text(encoding="utf-8"), + ) + assert float(saved_meta["conv-123"]["updated_at"]) > 0 def test_init_prunes_stale_and_unsupported_conversation_refs(make_channel, tmp_path, monkeypatch): @@ -125,6 +128,7 @@ def test_init_prunes_stale_and_unsupported_conversation_refs(make_channel, tmp_p state_dir = tmp_path / "state" state_dir.mkdir(parents=True, exist_ok=True) refs_path = state_dir / "msteams_conversations.json" + refs_meta_path = state_dir / msteams_module.MSTEAMS_REF_META_FILENAME refs_path.write_text( json.dumps( { @@ -132,25 +136,21 @@ def test_init_prunes_stale_and_unsupported_conversation_refs(make_channel, tmp_p "service_url": "https://smba.trafficmanager.net/amer/", "conversation_id": "conv-valid", "conversation_type": "personal", - "updated_at": now - 60, }, "conv-webchat": { "service_url": "https://webchat.botframework.com/", "conversation_id": "conv-webchat", "conversation_type": "personal", - "updated_at": now - 60, }, "conv-group": { "service_url": "https://smba.trafficmanager.net/amer/", "conversation_id": "conv-group", "conversation_type": "channel", - "updated_at": now - 60, }, "conv-stale": { "service_url": "https://smba.trafficmanager.net/amer/", "conversation_id": "conv-stale", "conversation_type": "personal", - "updated_at": now - msteams_module.MSTEAMS_REF_TTL_S - 1, }, "conv-missing-ts": { "service_url": "https://smba.trafficmanager.net/amer/", @@ -162,14 +162,27 @@ def test_init_prunes_stale_and_unsupported_conversation_refs(make_channel, tmp_p ), encoding="utf-8", ) + refs_meta_path.write_text( + json.dumps( + { + "conv-valid": {"updated_at": now - 60}, + "conv-webchat": {"updated_at": now - 60}, + "conv-group": {"updated_at": now - 60}, + "conv-stale": {"updated_at": now - msteams_module.MSTEAMS_REF_TTL_S - 1}, + }, + indent=2, + ), + encoding="utf-8", + ) ch = make_channel() - assert set(ch._conversation_refs.keys()) == {"conv-valid"} + assert set(ch._conversation_refs.keys()) == {"conv-valid", "conv-missing-ts"} assert ch._conversation_refs["conv-valid"].conversation_id == "conv-valid" + assert ch._conversation_refs["conv-missing-ts"].conversation_id == "conv-missing-ts" persisted = json.loads(refs_path.read_text(encoding="utf-8")) - assert set(persisted.keys()) == {"conv-valid"} + assert set(persisted.keys()) == {"conv-valid", "conv-missing-ts"} def test_save_prunes_unsupported_conversation_refs(make_channel, tmp_path, monkeypatch): @@ -204,6 +217,10 @@ def test_save_prunes_unsupported_conversation_refs(make_channel, tmp_path, monke saved = json.loads((tmp_path / "state" / "msteams_conversations.json").read_text(encoding="utf-8")) assert set(saved.keys()) == {"conv-valid"} + saved_meta = json.loads( + (tmp_path / "state" / msteams_module.MSTEAMS_REF_META_FILENAME).read_text(encoding="utf-8"), + ) + assert set(saved_meta.keys()) == {"conv-valid"} def test_init_respects_prune_toggle_flags(make_channel, tmp_path, monkeypatch): @@ -248,6 +265,7 @@ def test_init_respects_custom_ref_ttl_days(make_channel, tmp_path, monkeypatch): state_dir = tmp_path / "state" state_dir.mkdir(parents=True, exist_ok=True) refs_path = state_dir / "msteams_conversations.json" + refs_meta_path = state_dir / msteams_module.MSTEAMS_REF_META_FILENAME refs_path.write_text( json.dumps( { @@ -255,19 +273,27 @@ def test_init_respects_custom_ref_ttl_days(make_channel, tmp_path, monkeypatch): "service_url": "https://smba.trafficmanager.net/amer/", "conversation_id": "conv-fresh", "conversation_type": "personal", - "updated_at": now - 12 * 60 * 60, }, "conv-old": { "service_url": "https://smba.trafficmanager.net/amer/", "conversation_id": "conv-old", "conversation_type": "personal", - "updated_at": now - 10 * 24 * 60 * 60, }, }, indent=2, ), encoding="utf-8", ) + refs_meta_path.write_text( + json.dumps( + { + "conv-fresh": {"updated_at": now - 12 * 60 * 60}, + "conv-old": {"updated_at": now - 10 * 24 * 60 * 60}, + }, + indent=2, + ), + encoding="utf-8", + ) ch = make_channel(refTtlDays=1) @@ -276,6 +302,34 @@ def test_init_respects_custom_ref_ttl_days(make_channel, tmp_path, monkeypatch): assert set(persisted.keys()) == {"conv-fresh"} +def test_init_without_meta_keeps_legacy_refs_alive(make_channel, tmp_path, monkeypatch): + now = 1_800_000_000.0 + monkeypatch.setattr(msteams_module.time, "time", lambda: now) + + state_dir = tmp_path / "state" + state_dir.mkdir(parents=True, exist_ok=True) + refs_path = state_dir / "msteams_conversations.json" + refs_path.write_text( + json.dumps( + { + "conv-legacy": { + "service_url": "https://smba.trafficmanager.net/amer/", + "conversation_id": "conv-legacy", + "conversation_type": "personal", + } + }, + indent=2, + ), + encoding="utf-8", + ) + + ch = make_channel(refTtlDays=1) + + assert set(ch._conversation_refs.keys()) == {"conv-legacy"} + assert ch._conversation_refs["conv-legacy"].updated_at == now + assert not (state_dir / msteams_module.MSTEAMS_REF_META_FILENAME).exists() + + def test_save_uses_atomic_replace_and_keeps_existing_file_on_replace_error(make_channel, tmp_path, monkeypatch): ch = make_channel() refs_path = tmp_path / "state" / "msteams_conversations.json" @@ -591,6 +645,33 @@ async def test_send_replies_to_activity_when_reply_in_thread_enabled(make_channe assert kwargs["json"]["replyToId"] == "activity-1" +@pytest.mark.asyncio +async def test_send_success_refreshes_updated_at_and_persists_meta(make_channel, tmp_path, monkeypatch): + now = {"value": 1_800_000_000.0} + monkeypatch.setattr(msteams_module.time, "time", lambda: now["value"]) + + ch = make_channel(refTouchIntervalS=0) + fake_http = FakeHttpClient() + ch._http = fake_http + ch._token = "tok" + ch._token_expires_at = 9_999_999_999 + ch._conversation_refs["conv-123"] = ConversationRef( + service_url="https://smba.trafficmanager.net/amer/", + conversation_id="conv-123", + activity_id="activity-1", + updated_at=now["value"] - 100, + ) + + now["value"] += 5 + await ch.send(OutboundMessage(channel="msteams", chat_id="conv-123", content="Reply text")) + + assert ch._conversation_refs["conv-123"].updated_at == now["value"] + saved_meta = json.loads( + (tmp_path / "state" / msteams_module.MSTEAMS_REF_META_FILENAME).read_text(encoding="utf-8"), + ) + assert saved_meta["conv-123"]["updated_at"] == now["value"] + + @pytest.mark.asyncio async def test_send_posts_to_conversation_when_thread_reply_disabled(make_channel): ch = make_channel(replyInThread=False) @@ -756,6 +837,7 @@ def test_msteams_default_config_includes_restart_notify_fields(): assert cfg["refTtlDays"] == msteams_module.MSTEAMS_REF_TTL_DAYS assert cfg["pruneWebChatRefs"] is True assert cfg["pruneNonPersonalRefs"] is True + assert cfg["refTouchIntervalS"] == msteams_module.MSTEAMS_REF_TOUCH_INTERVAL_S assert "restartNotifyEnabled" not in cfg assert "restartNotifyPreMessage" not in cfg assert "restartNotifyPostMessage" not in cfg From 41f7eae7b4a7427c83b881f6f79f6e1001956179 Mon Sep 17 00:00:00 2001 From: choiking Date: Sat, 25 Apr 2026 16:58:05 +0800 Subject: [PATCH 24/80] docs: add macOS launchd gateway setup --- docs/deployment.md | 86 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/docs/deployment.md b/docs/deployment.md index ad6283c0..33773972 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -92,3 +92,89 @@ If you edit the `.service` file itself, run `systemctl --user daemon-reload` bef > ```bash > loginctl enable-linger $USER > ``` + +## macOS LaunchAgent + +On macOS, run the gateway as a `launchd` user agent so it starts automatically after login and restarts if it exits unexpectedly. + +**1. Find the nanobot binary path:** + +```bash +which nanobot # e.g. /Users/youruser/.local/bin/nanobot +``` + +If you installed nanobot with `uv tool`, you may also want the Python path for `ProgramArguments`: + +```bash +which python +``` + +**2. Create the LaunchAgent plist** at `~/Library/LaunchAgents/ai.nanobot.gateway.plist` (replace paths if needed): + +```xml + + + + + Label + ai.nanobot.gateway + + ProgramArguments + + /Users/youruser/.local/share/uv/tools/nanobot-ai/bin/python + /Users/youruser/.local/bin/nanobot + gateway + --workspace + /Users/youruser/.nanobot/workspace + + + WorkingDirectory + /Users/youruser/.nanobot/workspace + + RunAtLoad + + + KeepAlive + + SuccessfulExit + + + + StandardOutPath + /Users/youruser/.nanobot/logs/gateway.log + + StandardErrorPath + /Users/youruser/.nanobot/logs/gateway.error.log + + EnvironmentVariables + + PATH + /Users/youruser/.local/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin + PYTHONUNBUFFERED + 1 + + + +``` + +**3. Load and start it:** + +```bash +mkdir -p ~/.nanobot/logs +launchctl bootstrap gui/$(id -u) ~/Library/LaunchAgents/ai.nanobot.gateway.plist +launchctl enable gui/$(id -u)/ai.nanobot.gateway +launchctl kickstart -k gui/$(id -u)/ai.nanobot.gateway +``` + +**Common operations:** + +```bash +launchctl list | grep ai.nanobot.gateway +launchctl kickstart -k gui/$(id -u)/ai.nanobot.gateway +launchctl bootout gui/$(id -u) ~/Library/LaunchAgents/ai.nanobot.gateway.plist +log stream --process nanobot +``` + +If you edit the plist itself, run `launchctl bootout ...` and `launchctl bootstrap ...` again so `launchd` reloads the updated definition. + +> **Note:** if `launchctl kickstart` fails with an "address already in use" error, you probably still have a manually started `nanobot gateway` process running on the same port. Stop the manual process first, then kickstart the LaunchAgent again. From 8a4c338a01b7adc1f35f9f75e741bd7659f3bb5f Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 25 Apr 2026 11:21:16 +0000 Subject: [PATCH 25/80] docs: tighten macOS launchd setup Made-with: Cursor --- docs/README.md | 2 +- docs/deployment.md | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/docs/README.md b/docs/README.md index 6a3c9bd0..d8ff3024 100644 --- a/docs/README.md +++ b/docs/README.md @@ -18,7 +18,7 @@ Start here for setup, everyday usage, and deployment. | CLI reference | [`cli-reference.md`](./cli-reference.md) | Core CLI commands and common entrypoints | | In-chat commands | [`chat-commands.md`](./chat-commands.md) | Slash commands and periodic task behavior | | OpenAI-compatible API | [`openai-api.md`](./openai-api.md) | Local API endpoints, request format, and file uploads | -| Deployment | [`deployment.md`](./deployment.md) | Docker and Linux service setup | +| Deployment | [`deployment.md`](./deployment.md) | Docker, Linux service, and macOS LaunchAgent setup | ## Advanced Docs diff --git a/docs/deployment.md b/docs/deployment.md index 33773972..fe70d683 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -103,11 +103,7 @@ On macOS, run the gateway as a `launchd` user agent so it starts automatically a which nanobot # e.g. /Users/youruser/.local/bin/nanobot ``` -If you installed nanobot with `uv tool`, you may also want the Python path for `ProgramArguments`: - -```bash -which python -``` +Use this absolute `nanobot` path in `ProgramArguments` so the console script keeps the Python environment from your install method. **2. Create the LaunchAgent plist** at `~/Library/LaunchAgents/ai.nanobot.gateway.plist` (replace paths if needed): @@ -121,7 +117,6 @@ which python ProgramArguments - /Users/youruser/.local/share/uv/tools/nanobot-ai/bin/python /Users/youruser/.local/bin/nanobot gateway --workspace From 830211b5d4111a1a94ee73612e2841ece529ca7c Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 25 Apr 2026 11:25:57 +0000 Subject: [PATCH 26/80] docs: simplify macOS launchd setup Made-with: Cursor --- docs/deployment.md | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index fe70d683..f22b6885 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -95,17 +95,17 @@ If you edit the `.service` file itself, run `systemctl --user daemon-reload` bef ## macOS LaunchAgent -On macOS, run the gateway as a `launchd` user agent so it starts automatically after login and restarts if it exits unexpectedly. +Use a LaunchAgent when you want `nanobot gateway` to stay online after you log in, without keeping a terminal open. -**1. Find the nanobot binary path:** +**1. Get the absolute `nanobot` path:** ```bash which nanobot # e.g. /Users/youruser/.local/bin/nanobot ``` -Use this absolute `nanobot` path in `ProgramArguments` so the console script keeps the Python environment from your install method. +Use that exact path in the plist. It keeps the Python environment from your install method. -**2. Create the LaunchAgent plist** at `~/Library/LaunchAgents/ai.nanobot.gateway.plist` (replace paths if needed): +**2. Create `~/Library/LaunchAgents/ai.nanobot.gateway.plist`:** ```xml @@ -140,14 +140,6 @@ Use this absolute `nanobot` path in `ProgramArguments` so the console script kee StandardErrorPath /Users/youruser/.nanobot/logs/gateway.error.log - - EnvironmentVariables - - PATH - /Users/youruser/.local/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin - PYTHONUNBUFFERED - 1 - ``` @@ -155,7 +147,7 @@ Use this absolute `nanobot` path in `ProgramArguments` so the console script kee **3. Load and start it:** ```bash -mkdir -p ~/.nanobot/logs +mkdir -p ~/Library/LaunchAgents ~/.nanobot/logs launchctl bootstrap gui/$(id -u) ~/Library/LaunchAgents/ai.nanobot.gateway.plist launchctl enable gui/$(id -u)/ai.nanobot.gateway launchctl kickstart -k gui/$(id -u)/ai.nanobot.gateway @@ -165,11 +157,10 @@ launchctl kickstart -k gui/$(id -u)/ai.nanobot.gateway ```bash launchctl list | grep ai.nanobot.gateway -launchctl kickstart -k gui/$(id -u)/ai.nanobot.gateway +launchctl kickstart -k gui/$(id -u)/ai.nanobot.gateway # restart launchctl bootout gui/$(id -u) ~/Library/LaunchAgents/ai.nanobot.gateway.plist -log stream --process nanobot ``` -If you edit the plist itself, run `launchctl bootout ...` and `launchctl bootstrap ...` again so `launchd` reloads the updated definition. +After editing the plist, run `launchctl bootout ...` and `launchctl bootstrap ...` again. -> **Note:** if `launchctl kickstart` fails with an "address already in use" error, you probably still have a manually started `nanobot gateway` process running on the same port. Stop the manual process first, then kickstart the LaunchAgent again. +> **Note:** if startup fails with "address already in use", stop the manually started `nanobot gateway` process first. From cfc76ffbbffd0cb2b80b17e01373996001f50dde Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 25 Apr 2026 12:34:29 +0000 Subject: [PATCH 27/80] feat(agent): add ask_user tool Made-with: Cursor --- nanobot/agent/loop.py | 95 ++++++++++++++++++--- nanobot/agent/runner.py | 33 ++++++-- nanobot/agent/tools/ask.py | 50 +++++++++++ tests/agent/test_ask_user.py | 158 +++++++++++++++++++++++++++++++++++ 4 files changed, 320 insertions(+), 16 deletions(-) create mode 100644 nanobot/agent/tools/ask.py create mode 100644 tests/agent/test_ask_user.py diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index ca80475a..637bb512 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -20,14 +20,15 @@ from nanobot.agent.memory import Consolidator, Dream from nanobot.agent.runner import _MAX_INJECTIONS_PER_TURN, AgentRunner, AgentRunSpec from nanobot.agent.skills import BUILTIN_SKILLS_DIR from nanobot.agent.subagent import SubagentManager +from nanobot.agent.tools.ask import AskUserTool from nanobot.agent.tools.cron import CronTool from nanobot.agent.tools.filesystem import EditFileTool, ListDirTool, ReadFileTool, WriteFileTool from nanobot.agent.tools.message import MessageTool from nanobot.agent.tools.notebook import NotebookEditTool from nanobot.agent.tools.registry import ToolRegistry from nanobot.agent.tools.search import GlobTool, GrepTool -from nanobot.agent.tools.shell import ExecTool from nanobot.agent.tools.self import MyTool +from nanobot.agent.tools.shell import ExecTool from nanobot.agent.tools.spawn import SpawnTool from nanobot.agent.tools.web import WebFetchTool, WebSearchTool from nanobot.bus.events import InboundMessage, OutboundMessage @@ -287,6 +288,7 @@ class AgentLoop: self.workspace if (self.restrict_to_workspace or self.exec_config.sandbox) else None ) extra_read = [BUILTIN_SKILLS_DIR] if allowed_dir else None + self.tools.register(AskUserTool()) self.tools.register( ReadFileTool( workspace=self.workspace, allowed_dir=allowed_dir, extra_allowed_dirs=extra_read @@ -407,6 +409,56 @@ class AgentLoop: return UNIFIED_SESSION_KEY return msg.session_key + @staticmethod + def _tool_call_name(tool_call: dict[str, Any]) -> str: + function = tool_call.get("function") + if isinstance(function, dict) and isinstance(function.get("name"), str): + return function["name"] + name = tool_call.get("name") + return name if isinstance(name, str) else "" + + @staticmethod + def _tool_call_arguments(tool_call: dict[str, Any]) -> dict[str, Any]: + function = tool_call.get("function") + raw = function.get("arguments") if isinstance(function, dict) else tool_call.get("arguments") + if isinstance(raw, dict): + return raw + if isinstance(raw, str): + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + return {} + return parsed if isinstance(parsed, dict) else {} + return {} + + def _pending_ask_user_id(self, history: list[dict[str, Any]]) -> str | None: + pending: dict[str, str] = {} + for message in history: + if message.get("role") == "assistant": + for tool_call in message.get("tool_calls") or []: + if isinstance(tool_call, dict) and isinstance(tool_call.get("id"), str): + pending[tool_call["id"]] = self._tool_call_name(tool_call) + elif message.get("role") == "tool": + tool_call_id = message.get("tool_call_id") + if isinstance(tool_call_id, str): + pending.pop(tool_call_id, None) + for tool_call_id, name in reversed(pending.items()): + if name == "ask_user": + return tool_call_id + return None + + def _ask_user_options_from_messages(self, messages: list[dict[str, Any]]) -> list[str]: + for message in reversed(messages): + if message.get("role") != "assistant": + continue + for tool_call in reversed(message.get("tool_calls") or []): + if not isinstance(tool_call, dict) or self._tool_call_name(tool_call) != "ask_user": + continue + options = self._tool_call_arguments(tool_call).get("options") + if isinstance(options, list): + return [str(option) for option in options if isinstance(option, str)] + return [] + async def _run_agent_loop( self, initial_messages: list[dict], @@ -799,7 +851,7 @@ class AgentLoop: session_summary=pending, current_role=current_role, ) - final_content, _, all_msgs, _, _ = await self._run_agent_loop( + final_content, _, all_msgs, stop_reason, _ = await self._run_agent_loop( messages, session=session, channel=channel, chat_id=chat_id, message_id=msg.metadata.get("message_id"), pending_queue=pending_queue, @@ -808,10 +860,12 @@ class AgentLoop: self._clear_runtime_checkpoint(session) self.sessions.save(session) self._schedule_background(self.consolidator.maybe_consolidate_by_tokens(session)) + options = self._ask_user_options_from_messages(all_msgs) if stop_reason == "ask_user" else [] return OutboundMessage( channel=channel, chat_id=chat_id, content=final_content or "Background task completed.", + buttons=[options] if options else [], ) # Extract document text from media at the processing boundary so all @@ -850,14 +904,27 @@ class AgentLoop: history = session.get_history(max_messages=0) - initial_messages = self.context.build_messages( - history=history, - current_message=msg.content, - session_summary=pending, - media=msg.media if msg.media else None, - channel=msg.channel, - chat_id=msg.chat_id, - ) + pending_ask_id = self._pending_ask_user_id(history) + if pending_ask_id: + initial_messages = [ + {"role": "system", "content": self.context.build_system_prompt(channel=msg.channel)}, + *history, + { + "role": "tool", + "tool_call_id": pending_ask_id, + "name": "ask_user", + "content": msg.content, + }, + ] + else: + initial_messages = self.context.build_messages( + history=history, + current_message=msg.content, + session_summary=pending, + media=msg.media if msg.media else None, + channel=msg.channel, + chat_id=msg.chat_id, + ) async def _bus_progress( content: str, @@ -898,7 +965,7 @@ class AgentLoop: user_persisted_early = False media_paths = [p for p in (msg.media or []) if isinstance(p, str) and p] has_text = isinstance(msg.content, str) and msg.content.strip() - if has_text or media_paths: + if not pending_ask_id and (has_text or media_paths): extra: dict[str, Any] = {"media": list(media_paths)} if media_paths else {} text = msg.content if isinstance(msg.content, str) else "" session.add_message("user", text, **extra) @@ -944,6 +1011,11 @@ class AgentLoop: logger.info("Response to {}:{}: {}", msg.channel, msg.sender_id, preview) meta = dict(msg.metadata or {}) + buttons: list[list[str]] = [] + if stop_reason == "ask_user": + options = self._ask_user_options_from_messages(all_msgs) + if options: + buttons = [options] if on_stream is not None and stop_reason != "error": meta["_streamed"] = True return OutboundMessage( @@ -951,6 +1023,7 @@ class AgentLoop: chat_id=msg.chat_id, content=final_content, metadata=meta, + buttons=buttons, ) def _sanitize_persisted_blocks( diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index 3704f303..688d3871 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -3,16 +3,16 @@ from __future__ import annotations import asyncio -from dataclasses import dataclass, field import inspect import os +from dataclasses import dataclass, field from pathlib import Path from typing import Any from loguru import logger from nanobot.agent.hook import AgentHook, AgentHookContext -from nanobot.utils.prompt_templates import render_template +from nanobot.agent.tools.ask import AskUserInterrupt from nanobot.agent.tools.registry import ToolRegistry from nanobot.providers.base import LLMProvider, LLMResponse, ToolCallRequest from nanobot.utils.helpers import ( @@ -23,6 +23,7 @@ from nanobot.utils.helpers import ( maybe_persist_tool_result, truncate_text, ) +from nanobot.utils.prompt_templates import render_template from nanobot.utils.runtime import ( EMPTY_FINAL_RESPONSE_MESSAGE, build_finalization_retry_message, @@ -312,6 +313,8 @@ class AgentRunner: context.tool_events = list(new_events) completed_tool_results: list[dict[str, Any]] = [] for tool_call, result in zip(response.tool_calls, results): + if isinstance(fatal_error, AskUserInterrupt) and tool_call.name == "ask_user": + continue tool_message = { "role": "tool", "tool_call_id": tool_call.id, @@ -326,6 +329,15 @@ class AgentRunner: messages.append(tool_message) completed_tool_results.append(tool_message) if fatal_error is not None: + if isinstance(fatal_error, AskUserInterrupt): + final_content = fatal_error.question + stop_reason = "ask_user" + context.final_content = final_content + context.stop_reason = stop_reason + if hook.wants_streaming(): + await hook.on_stream_end(context, resuming=False) + await hook.after_iteration(context) + break error = f"Error: {type(fatal_error).__name__}: {fatal_error}" final_content = error stop_reason = "tool_error" @@ -656,13 +668,21 @@ class AgentRunner: tool_results: list[tuple[Any, dict[str, str], BaseException | None]] = [] for batch in batches: if spec.concurrent_tools and len(batch) > 1: - tool_results.extend(await asyncio.gather(*( + batch_results = await asyncio.gather(*( self._run_tool(spec, tool_call, external_lookup_counts) for tool_call in batch - ))) + )) + tool_results.extend(batch_results) else: + batch_results = [] for tool_call in batch: - tool_results.append(await self._run_tool(spec, tool_call, external_lookup_counts)) + result = await self._run_tool(spec, tool_call, external_lookup_counts) + tool_results.append(result) + batch_results.append(result) + if isinstance(result[2], AskUserInterrupt): + break + if any(isinstance(error, AskUserInterrupt) for _, _, error in batch_results): + break results: list[Any] = [] events: list[dict[str, str]] = [] @@ -724,6 +744,9 @@ class AgentRunner: "status": "error", "detail": str(exc), } + if isinstance(exc, AskUserInterrupt): + event["status"] = "waiting" + return "", event, exc if spec.fail_on_tool_error: return f"Error: {type(exc).__name__}: {exc}", event, exc return f"Error: {type(exc).__name__}: {exc}", event, None diff --git a/nanobot/agent/tools/ask.py b/nanobot/agent/tools/ask.py new file mode 100644 index 00000000..0ce371ea --- /dev/null +++ b/nanobot/agent/tools/ask.py @@ -0,0 +1,50 @@ +"""Tool for pausing a turn until the user answers.""" + +from typing import Any + +from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.schema import ArraySchema, StringSchema, tool_parameters_schema + + +class AskUserInterrupt(BaseException): + """Internal signal: the runner should stop and wait for user input.""" + + def __init__(self, question: str, options: list[str] | None = None) -> None: + self.question = question + self.options = [str(option) for option in (options or []) if str(option)] + super().__init__(question) + + +@tool_parameters( + tool_parameters_schema( + question=StringSchema( + "The question to ask before continuing. Use this only when the task needs the user's answer." + ), + options=ArraySchema( + StringSchema("A possible answer label"), + description="Optional choices. The user may still reply with free text.", + ), + required=["question"], + ) +) +class AskUserTool(Tool): + """Ask the user a blocking question.""" + + @property + def name(self) -> str: + return "ask_user" + + @property + def description(self) -> str: + return ( + "Pause and ask the user a question when their answer is required to continue. " + "Use options for likely answers; the user's reply, typed or selected, is returned as the tool result. " + "For non-blocking notifications or buttons, use the message tool instead." + ) + + @property + def exclusive(self) -> bool: + return True + + async def execute(self, question: str, options: list[str] | None = None, **_: Any) -> Any: + raise AskUserInterrupt(question=question, options=options) diff --git a/tests/agent/test_ask_user.py b/tests/agent/test_ask_user.py new file mode 100644 index 00000000..fd8993ce --- /dev/null +++ b/tests/agent/test_ask_user.py @@ -0,0 +1,158 @@ +import asyncio +from unittest.mock import MagicMock + +import pytest + +from nanobot.agent.loop import AgentLoop +from nanobot.agent.runner import AgentRunner, AgentRunSpec +from nanobot.agent.tools.ask import AskUserInterrupt, AskUserTool +from nanobot.agent.tools.base import Tool, tool_parameters +from nanobot.agent.tools.registry import ToolRegistry +from nanobot.agent.tools.schema import tool_parameters_schema +from nanobot.bus.events import InboundMessage +from nanobot.bus.queue import MessageBus +from nanobot.providers.base import GenerationSettings, LLMResponse, ToolCallRequest + + +def _make_provider(chat_with_retry): + provider = MagicMock() + provider.get_default_model.return_value = "test-model" + provider.generation = GenerationSettings() + provider.chat_with_retry = chat_with_retry + return provider + + +def test_ask_user_tool_schema_and_interrupt(): + tool = AskUserTool() + schema = tool.to_schema()["function"] + + assert schema["name"] == "ask_user" + assert "question" in schema["parameters"]["required"] + assert schema["parameters"]["properties"]["options"]["type"] == "array" + + with pytest.raises(AskUserInterrupt) as exc: + asyncio.run(tool.execute("Continue?", options=["Yes", "No"])) + + assert exc.value.question == "Continue?" + assert exc.value.options == ["Yes", "No"] + + +@pytest.mark.asyncio +async def test_runner_pauses_on_ask_user_without_executing_later_tools(): + @tool_parameters(tool_parameters_schema(required=[])) + class LaterTool(Tool): + called = False + + @property + def name(self) -> str: + return "later" + + @property + def description(self) -> str: + return "Should not run after ask_user pauses the turn." + + async def execute(self, **kwargs): + self.called = True + return "later result" + + async def chat_with_retry(**kwargs): + return LLMResponse( + content="", + finish_reason="tool_calls", + tool_calls=[ + ToolCallRequest( + id="call_ask", + name="ask_user", + arguments={"question": "Install this package?", "options": ["Yes", "No"]}, + ), + ToolCallRequest(id="call_later", name="later", arguments={}), + ], + ) + + later = LaterTool() + tools = ToolRegistry() + tools.register(AskUserTool()) + tools.register(later) + + result = await AgentRunner(_make_provider(chat_with_retry)).run(AgentRunSpec( + initial_messages=[{"role": "user", "content": "continue"}], + tools=tools, + model="test-model", + max_iterations=3, + max_tool_result_chars=16_000, + concurrent_tools=True, + )) + + assert result.stop_reason == "ask_user" + assert result.final_content == "Install this package?" + assert "ask_user" in result.tools_used + assert later.called is False + assert result.messages[-1]["role"] == "assistant" + assert result.messages[-1]["tool_calls"][0]["function"]["name"] == "ask_user" + assert not any(message.get("name") == "ask_user" for message in result.messages) + + +@pytest.mark.asyncio +async def test_ask_user_sends_buttons_and_resumes_with_next_message(tmp_path): + seen_messages: list[list[dict]] = [] + + async def chat_with_retry(**kwargs): + seen_messages.append(kwargs["messages"]) + if len(seen_messages) == 1: + return LLMResponse( + content="", + finish_reason="tool_calls", + tool_calls=[ + ToolCallRequest( + id="call_ask", + name="ask_user", + arguments={ + "question": "Install the optional package?", + "options": ["Install", "Skip"], + }, + ) + ], + ) + return LLMResponse(content="Skipped install.", usage={}) + + loop = AgentLoop( + bus=MessageBus(), + provider=_make_provider(chat_with_retry), + workspace=tmp_path, + model="test-model", + ) + + first = await loop._process_message( + InboundMessage(channel="cli", sender_id="user", chat_id="direct", content="set it up") + ) + + assert first is not None + assert first.content == "Install the optional package?" + assert first.buttons == [["Install", "Skip"]] + + session = loop.sessions.get_or_create("cli:direct") + assert any(message.get("role") == "assistant" and message.get("tool_calls") for message in session.messages) + assert not any(message.get("role") == "tool" and message.get("name") == "ask_user" for message in session.messages) + + second = await loop._process_message( + InboundMessage(channel="cli", sender_id="user", chat_id="direct", content="Skip") + ) + + assert second is not None + assert second.content == "Skipped install." + assert any( + message.get("role") == "tool" + and message.get("name") == "ask_user" + and message.get("content") == "Skip" + for message in seen_messages[-1] + ) + assert not any( + message.get("role") == "user" and message.get("content") == "Skip" + for message in session.messages + ) + assert any( + message.get("role") == "tool" + and message.get("name") == "ask_user" + and message.get("content") == "Skip" + for message in session.messages + ) From 3b1ea99ee10574109ad439bfb9f29edfc8e76c01 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 25 Apr 2026 12:42:09 +0000 Subject: [PATCH 28/80] fix(agent): render ask_user options without buttons Made-with: Cursor --- nanobot/agent/loop.py | 33 ++++++++++++++++++++++------- tests/agent/test_ask_user.py | 40 +++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 637bb512..d87ad1a8 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -54,6 +54,7 @@ if TYPE_CHECKING: UNIFIED_SESSION_KEY = "unified:default" +BUTTON_CHANNELS = frozenset({"telegram"}) class _LoopHook(AgentHook): @@ -459,6 +460,19 @@ class AgentLoop: return [str(option) for option in options if isinstance(option, str)] return [] + @staticmethod + def _ask_user_outbound( + content: str | None, + options: list[str], + channel: str, + ) -> tuple[str | None, list[list[str]]]: + if not options: + return content, [] + if channel in BUTTON_CHANNELS: + return content, [options] + option_text = "\n".join(f"{index}. {option}" for index, option in enumerate(options, 1)) + return f"{content}\n\n{option_text}" if content else option_text, [] + async def _run_agent_loop( self, initial_messages: list[dict], @@ -861,11 +875,16 @@ class AgentLoop: self.sessions.save(session) self._schedule_background(self.consolidator.maybe_consolidate_by_tokens(session)) options = self._ask_user_options_from_messages(all_msgs) if stop_reason == "ask_user" else [] + content, buttons = self._ask_user_outbound( + final_content or "Background task completed.", + options, + channel, + ) return OutboundMessage( channel=channel, chat_id=chat_id, - content=final_content or "Background task completed.", - buttons=[options] if options else [], + content=content, + buttons=buttons, ) # Extract document text from media at the processing boundary so all @@ -1011,11 +1030,11 @@ class AgentLoop: logger.info("Response to {}:{}: {}", msg.channel, msg.sender_id, preview) meta = dict(msg.metadata or {}) - buttons: list[list[str]] = [] - if stop_reason == "ask_user": - options = self._ask_user_options_from_messages(all_msgs) - if options: - buttons = [options] + final_content, buttons = self._ask_user_outbound( + final_content, + self._ask_user_options_from_messages(all_msgs) if stop_reason == "ask_user" else [], + msg.channel, + ) if on_stream is not None and stop_reason != "error": meta["_streamed"] = True return OutboundMessage( diff --git a/tests/agent/test_ask_user.py b/tests/agent/test_ask_user.py index fd8993ce..bdf49663 100644 --- a/tests/agent/test_ask_user.py +++ b/tests/agent/test_ask_user.py @@ -93,7 +93,7 @@ async def test_runner_pauses_on_ask_user_without_executing_later_tools(): @pytest.mark.asyncio -async def test_ask_user_sends_buttons_and_resumes_with_next_message(tmp_path): +async def test_ask_user_text_fallback_resumes_with_next_message(tmp_path): seen_messages: list[list[dict]] = [] async def chat_with_retry(**kwargs): @@ -127,8 +127,8 @@ async def test_ask_user_sends_buttons_and_resumes_with_next_message(tmp_path): ) assert first is not None - assert first.content == "Install the optional package?" - assert first.buttons == [["Install", "Skip"]] + assert first.content == "Install the optional package?\n\n1. Install\n2. Skip" + assert first.buttons == [] session = loop.sessions.get_or_create("cli:direct") assert any(message.get("role") == "assistant" and message.get("tool_calls") for message in session.messages) @@ -156,3 +156,37 @@ async def test_ask_user_sends_buttons_and_resumes_with_next_message(tmp_path): and message.get("content") == "Skip" for message in session.messages ) + + +@pytest.mark.asyncio +async def test_ask_user_keeps_buttons_for_telegram(tmp_path): + async def chat_with_retry(**kwargs): + return LLMResponse( + content="", + finish_reason="tool_calls", + tool_calls=[ + ToolCallRequest( + id="call_ask", + name="ask_user", + arguments={ + "question": "Install the optional package?", + "options": ["Install", "Skip"], + }, + ) + ], + ) + + loop = AgentLoop( + bus=MessageBus(), + provider=_make_provider(chat_with_retry), + workspace=tmp_path, + model="test-model", + ) + + response = await loop._process_message( + InboundMessage(channel="telegram", sender_id="user", chat_id="123", content="set it up") + ) + + assert response is not None + assert response.content == "Install the optional package?" + assert response.buttons == [["Install", "Skip"]] From 403ce23d22c59fe31a89cc3b4ed8db31d8cf6f17 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 25 Apr 2026 14:06:09 +0000 Subject: [PATCH 29/80] fix(agent): tighten ask_user CLI handling Made-with: Cursor --- nanobot/agent/loop.py | 100 +++++++---------------------------- nanobot/agent/runner.py | 27 ++++++---- nanobot/agent/tools/ask.py | 86 ++++++++++++++++++++++++++++++ nanobot/cli/commands.py | 4 ++ tests/agent/test_ask_user.py | 19 ++++++- 5 files changed, 142 insertions(+), 94 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index d87ad1a8..5a448004 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -20,7 +20,13 @@ from nanobot.agent.memory import Consolidator, Dream from nanobot.agent.runner import _MAX_INJECTIONS_PER_TURN, AgentRunner, AgentRunSpec from nanobot.agent.skills import BUILTIN_SKILLS_DIR from nanobot.agent.subagent import SubagentManager -from nanobot.agent.tools.ask import AskUserTool +from nanobot.agent.tools.ask import ( + AskUserTool, + ask_user_options_from_messages, + ask_user_outbound, + ask_user_tool_result_messages, + pending_ask_user_id, +) from nanobot.agent.tools.cron import CronTool from nanobot.agent.tools.filesystem import EditFileTool, ListDirTool, ReadFileTool, WriteFileTool from nanobot.agent.tools.message import MessageTool @@ -54,7 +60,6 @@ if TYPE_CHECKING: UNIFIED_SESSION_KEY = "unified:default" -BUTTON_CHANNELS = frozenset({"telegram"}) class _LoopHook(AgentHook): @@ -410,69 +415,6 @@ class AgentLoop: return UNIFIED_SESSION_KEY return msg.session_key - @staticmethod - def _tool_call_name(tool_call: dict[str, Any]) -> str: - function = tool_call.get("function") - if isinstance(function, dict) and isinstance(function.get("name"), str): - return function["name"] - name = tool_call.get("name") - return name if isinstance(name, str) else "" - - @staticmethod - def _tool_call_arguments(tool_call: dict[str, Any]) -> dict[str, Any]: - function = tool_call.get("function") - raw = function.get("arguments") if isinstance(function, dict) else tool_call.get("arguments") - if isinstance(raw, dict): - return raw - if isinstance(raw, str): - try: - parsed = json.loads(raw) - except json.JSONDecodeError: - return {} - return parsed if isinstance(parsed, dict) else {} - return {} - - def _pending_ask_user_id(self, history: list[dict[str, Any]]) -> str | None: - pending: dict[str, str] = {} - for message in history: - if message.get("role") == "assistant": - for tool_call in message.get("tool_calls") or []: - if isinstance(tool_call, dict) and isinstance(tool_call.get("id"), str): - pending[tool_call["id"]] = self._tool_call_name(tool_call) - elif message.get("role") == "tool": - tool_call_id = message.get("tool_call_id") - if isinstance(tool_call_id, str): - pending.pop(tool_call_id, None) - for tool_call_id, name in reversed(pending.items()): - if name == "ask_user": - return tool_call_id - return None - - def _ask_user_options_from_messages(self, messages: list[dict[str, Any]]) -> list[str]: - for message in reversed(messages): - if message.get("role") != "assistant": - continue - for tool_call in reversed(message.get("tool_calls") or []): - if not isinstance(tool_call, dict) or self._tool_call_name(tool_call) != "ask_user": - continue - options = self._tool_call_arguments(tool_call).get("options") - if isinstance(options, list): - return [str(option) for option in options if isinstance(option, str)] - return [] - - @staticmethod - def _ask_user_outbound( - content: str | None, - options: list[str], - channel: str, - ) -> tuple[str | None, list[list[str]]]: - if not options: - return content, [] - if channel in BUTTON_CHANNELS: - return content, [options] - option_text = "\n".join(f"{index}. {option}" for index, option in enumerate(options, 1)) - return f"{content}\n\n{option_text}" if content else option_text, [] - async def _run_agent_loop( self, initial_messages: list[dict], @@ -874,8 +816,8 @@ class AgentLoop: self._clear_runtime_checkpoint(session) self.sessions.save(session) self._schedule_background(self.consolidator.maybe_consolidate_by_tokens(session)) - options = self._ask_user_options_from_messages(all_msgs) if stop_reason == "ask_user" else [] - content, buttons = self._ask_user_outbound( + options = ask_user_options_from_messages(all_msgs) if stop_reason == "ask_user" else [] + content, buttons = ask_user_outbound( final_content or "Background task completed.", options, channel, @@ -923,18 +865,14 @@ class AgentLoop: history = session.get_history(max_messages=0) - pending_ask_id = self._pending_ask_user_id(history) + pending_ask_id = pending_ask_user_id(history) if pending_ask_id: - initial_messages = [ - {"role": "system", "content": self.context.build_system_prompt(channel=msg.channel)}, - *history, - { - "role": "tool", - "tool_call_id": pending_ask_id, - "name": "ask_user", - "content": msg.content, - }, - ] + initial_messages = ask_user_tool_result_messages( + self.context.build_system_prompt(channel=msg.channel), + history, + pending_ask_id, + msg.content, + ) else: initial_messages = self.context.build_messages( history=history, @@ -1030,12 +968,12 @@ class AgentLoop: logger.info("Response to {}:{}: {}", msg.channel, msg.sender_id, preview) meta = dict(msg.metadata or {}) - final_content, buttons = self._ask_user_outbound( + final_content, buttons = ask_user_outbound( final_content, - self._ask_user_options_from_messages(all_msgs) if stop_reason == "ask_user" else [], + ask_user_options_from_messages(all_msgs) if stop_reason == "ask_user" else [], msg.channel, ) - if on_stream is not None and stop_reason != "error": + if on_stream is not None and stop_reason not in {"ask_user", "error"}: meta["_streamed"] = True return OutboundMessage( channel=msg.channel, diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index 688d3871..be71f649 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -278,17 +278,22 @@ class AgentRunner: self._accumulate_usage(usage, raw_usage) if response.should_execute_tools: + tool_calls = list(response.tool_calls) + ask_index = next((i for i, tc in enumerate(tool_calls) if tc.name == "ask_user"), None) + if ask_index is not None: + tool_calls = tool_calls[: ask_index + 1] + context.tool_calls = list(tool_calls) if hook.wants_streaming(): await hook.on_stream_end(context, resuming=True) assistant_message = build_assistant_message( response.content or "", - tool_calls=[tc.to_openai_tool_call() for tc in response.tool_calls], + tool_calls=[tc.to_openai_tool_call() for tc in tool_calls], reasoning_content=response.reasoning_content, thinking_blocks=response.thinking_blocks, ) messages.append(assistant_message) - tools_used.extend(tc.name for tc in response.tool_calls) + tools_used.extend(tc.name for tc in tool_calls) await self._emit_checkpoint( spec, { @@ -297,7 +302,7 @@ class AgentRunner: "model": spec.model, "assistant_message": assistant_message, "completed_tool_results": [], - "pending_tool_calls": [tc.to_openai_tool_call() for tc in response.tool_calls], + "pending_tool_calls": [tc.to_openai_tool_call() for tc in tool_calls], }, ) @@ -305,14 +310,14 @@ class AgentRunner: results, new_events, fatal_error = await self._execute_tools( spec, - response.tool_calls, + tool_calls, external_lookup_counts, ) tool_events.extend(new_events) context.tool_results = list(results) context.tool_events = list(new_events) completed_tool_results: list[dict[str, Any]] = [] - for tool_call, result in zip(response.tool_calls, results): + for tool_call, result in zip(tool_calls, results): if isinstance(fatal_error, AskUserInterrupt) and tool_call.name == "ask_user": continue tool_message = { @@ -700,7 +705,7 @@ class AgentRunner: tool_call: ToolCallRequest, external_lookup_counts: dict[str, int], ) -> tuple[Any, dict[str, str], BaseException | None]: - _HINT = "\n\n[Analyze the error above and try a different approach.]" + hint = "\n\n[Analyze the error above and try a different approach.]" lookup_error = repeated_external_lookup_error( tool_call.name, tool_call.arguments, @@ -713,8 +718,8 @@ class AgentRunner: "detail": "repeated external lookup blocked", } if spec.fail_on_tool_error: - return lookup_error + _HINT, event, RuntimeError(lookup_error) - return lookup_error + _HINT, event, None + return lookup_error + hint, event, RuntimeError(lookup_error) + return lookup_error + hint, event, None prepare_call = getattr(spec.tools, "prepare_call", None) tool, params, prep_error = None, tool_call.arguments, None if callable(prepare_call): @@ -730,7 +735,7 @@ class AgentRunner: "status": "error", "detail": prep_error.split(": ", 1)[-1][:120], } - return prep_error + _HINT, event, RuntimeError(prep_error) if spec.fail_on_tool_error else None + return prep_error + hint, event, RuntimeError(prep_error) if spec.fail_on_tool_error else None try: if tool is not None: result = await tool.execute(**params) @@ -758,8 +763,8 @@ class AgentRunner: "detail": result.replace("\n", " ").strip()[:120], } if spec.fail_on_tool_error: - return result + _HINT, event, RuntimeError(result) - return result + _HINT, event, None + return result + hint, event, RuntimeError(result) + return result + hint, event, None detail = "" if result is None else str(result) detail = detail.replace("\n", " ").strip() diff --git a/nanobot/agent/tools/ask.py b/nanobot/agent/tools/ask.py index 0ce371ea..c2aa8e0e 100644 --- a/nanobot/agent/tools/ask.py +++ b/nanobot/agent/tools/ask.py @@ -1,10 +1,13 @@ """Tool for pausing a turn until the user answers.""" +import json from typing import Any from nanobot.agent.tools.base import Tool, tool_parameters from nanobot.agent.tools.schema import ArraySchema, StringSchema, tool_parameters_schema +BUTTON_CHANNELS = frozenset({"telegram"}) + class AskUserInterrupt(BaseException): """Internal signal: the runner should stop and wait for user input.""" @@ -48,3 +51,86 @@ class AskUserTool(Tool): async def execute(self, question: str, options: list[str] | None = None, **_: Any) -> Any: raise AskUserInterrupt(question=question, options=options) + + +def _tool_call_name(tool_call: dict[str, Any]) -> str: + function = tool_call.get("function") + if isinstance(function, dict) and isinstance(function.get("name"), str): + return function["name"] + name = tool_call.get("name") + return name if isinstance(name, str) else "" + + +def _tool_call_arguments(tool_call: dict[str, Any]) -> dict[str, Any]: + function = tool_call.get("function") + raw = function.get("arguments") if isinstance(function, dict) else tool_call.get("arguments") + if isinstance(raw, dict): + return raw + if isinstance(raw, str): + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + return {} + return parsed if isinstance(parsed, dict) else {} + return {} + + +def pending_ask_user_id(history: list[dict[str, Any]]) -> str | None: + pending: dict[str, str] = {} + for message in history: + if message.get("role") == "assistant": + for tool_call in message.get("tool_calls") or []: + if isinstance(tool_call, dict) and isinstance(tool_call.get("id"), str): + pending[tool_call["id"]] = _tool_call_name(tool_call) + elif message.get("role") == "tool": + tool_call_id = message.get("tool_call_id") + if isinstance(tool_call_id, str): + pending.pop(tool_call_id, None) + for tool_call_id, name in reversed(pending.items()): + if name == "ask_user": + return tool_call_id + return None + + +def ask_user_tool_result_messages( + system_prompt: str, + history: list[dict[str, Any]], + tool_call_id: str, + content: str, +) -> list[dict[str, Any]]: + return [ + {"role": "system", "content": system_prompt}, + *history, + { + "role": "tool", + "tool_call_id": tool_call_id, + "name": "ask_user", + "content": content, + }, + ] + + +def ask_user_options_from_messages(messages: list[dict[str, Any]]) -> list[str]: + for message in reversed(messages): + if message.get("role") != "assistant": + continue + for tool_call in reversed(message.get("tool_calls") or []): + if not isinstance(tool_call, dict) or _tool_call_name(tool_call) != "ask_user": + continue + options = _tool_call_arguments(tool_call).get("options") + if isinstance(options, list): + return [str(option) for option in options if isinstance(option, str)] + return [] + + +def ask_user_outbound( + content: str | None, + options: list[str], + channel: str, +) -> tuple[str | None, list[list[str]]]: + if not options: + return content, [] + if channel in BUTTON_CHANNELS: + return content, [options] + option_text = "\n".join(f"{index}. {option}" for index, option in enumerate(options, 1)) + return f"{content}\n\n{option_text}" if content else option_text, [] diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index d5b17518..c4cd2b1b 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -212,12 +212,16 @@ async def _print_interactive_response( def _print_cli_progress_line(text: str, thinking: ThinkingSpinner | None) -> None: """Print a CLI progress line, pausing the spinner if needed.""" + if not text.strip(): + return with thinking.pause() if thinking else nullcontext(): console.print(f" [dim]↳ {text}[/dim]") async def _print_interactive_progress_line(text: str, thinking: ThinkingSpinner | None) -> None: """Print an interactive progress line, pausing the spinner if needed.""" + if not text.strip(): + return with thinking.pause() if thinking else nullcontext(): await _print_interactive_line(text) diff --git a/tests/agent/test_ask_user.py b/tests/agent/test_ask_user.py index bdf49663..4d5b5be9 100644 --- a/tests/agent/test_ask_user.py +++ b/tests/agent/test_ask_user.py @@ -15,10 +15,15 @@ from nanobot.providers.base import GenerationSettings, LLMResponse, ToolCallRequ def _make_provider(chat_with_retry): + async def chat_stream_with_retry(**kwargs): + kwargs.pop("on_content_delta", None) + return await chat_with_retry(**kwargs) + provider = MagicMock() provider.get_default_model.return_value = "test-model" provider.generation = GenerationSettings() provider.chat_with_retry = chat_with_retry + provider.chat_stream_with_retry = chat_stream_with_retry return provider @@ -88,7 +93,8 @@ async def test_runner_pauses_on_ask_user_without_executing_later_tools(): assert "ask_user" in result.tools_used assert later.called is False assert result.messages[-1]["role"] == "assistant" - assert result.messages[-1]["tool_calls"][0]["function"]["name"] == "ask_user" + tool_calls = result.messages[-1]["tool_calls"] + assert [tool_call["function"]["name"] for tool_call in tool_calls] == ["ask_user"] assert not any(message.get("name") == "ask_user" for message in result.messages) @@ -122,13 +128,22 @@ async def test_ask_user_text_fallback_resumes_with_next_message(tmp_path): model="test-model", ) + async def on_stream(delta: str) -> None: + pass + + async def on_stream_end(**kwargs) -> None: + pass + first = await loop._process_message( - InboundMessage(channel="cli", sender_id="user", chat_id="direct", content="set it up") + InboundMessage(channel="cli", sender_id="user", chat_id="direct", content="set it up"), + on_stream=on_stream, + on_stream_end=on_stream_end, ) assert first is not None assert first.content == "Install the optional package?\n\n1. Install\n2. Skip" assert first.buttons == [] + assert "_streamed" not in first.metadata session = loop.sessions.get_or_create("cli:direct") assert any(message.get("role") == "assistant" and message.get("tool_calls") for message in session.messages) From a58d9fd357c778930869f50d2ca3e6dad95773c2 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sat, 25 Apr 2026 15:46:47 +0000 Subject: [PATCH 30/80] feat(webui): render ask_user choices Made-with: Cursor --- nanobot/agent/tools/ask.py | 4 +- nanobot/channels/websocket.py | 16 ++- tests/agent/test_ask_user.py | 34 ++++++ tests/channels/test_websocket_channel.py | 5 +- webui/src/components/thread/AskUserPrompt.tsx | 108 ++++++++++++++++++ webui/src/components/thread/ThreadShell.tsx | 23 ++++ webui/src/hooks/useNanobotStream.ts | 4 +- webui/src/lib/types.ts | 5 + webui/src/tests/thread-shell.test.tsx | 58 +++++++++- webui/src/tests/useNanobotStream.test.tsx | 23 ++++ 10 files changed, 274 insertions(+), 6 deletions(-) create mode 100644 webui/src/components/thread/AskUserPrompt.tsx diff --git a/nanobot/agent/tools/ask.py b/nanobot/agent/tools/ask.py index c2aa8e0e..db8c83a8 100644 --- a/nanobot/agent/tools/ask.py +++ b/nanobot/agent/tools/ask.py @@ -6,7 +6,7 @@ from typing import Any from nanobot.agent.tools.base import Tool, tool_parameters from nanobot.agent.tools.schema import ArraySchema, StringSchema, tool_parameters_schema -BUTTON_CHANNELS = frozenset({"telegram"}) +STRUCTURED_BUTTON_CHANNELS = frozenset({"telegram", "websocket"}) class AskUserInterrupt(BaseException): @@ -130,7 +130,7 @@ def ask_user_outbound( ) -> tuple[str | None, list[list[str]]]: if not options: return content, [] - if channel in BUTTON_CHANNELS: + if channel in STRUCTURED_BUTTON_CHANNELS: return content, [options] option_text = "\n".join(f"{index}. {option}" for index, option in enumerate(options, 1)) return f"{content}\n\n{option_text}" if content else option_text, [] diff --git a/nanobot/channels/websocket.py b/nanobot/channels/websocket.py index c76371e9..ff923d81 100644 --- a/nanobot/channels/websocket.py +++ b/nanobot/channels/websocket.py @@ -54,6 +54,14 @@ def _normalize_config_path(path: str) -> str: return _strip_trailing_slash(path) +def _append_buttons_as_text(text: str, buttons: list[list[str]]) -> str: + labels = [label for row in buttons for label in row if label] + if not labels: + return text + fallback = "\n".join(f"{index}. {label}" for index, label in enumerate(labels, 1)) + return f"{text}\n\n{fallback}" if text else fallback + + class WebSocketConfig(Base): """WebSocket server channel configuration. @@ -1146,11 +1154,17 @@ class WebSocketChannel(BaseChannel): if not conns: logger.warning("websocket: no active subscribers for chat_id={}", msg.chat_id) return + text = msg.content + if msg.buttons: + text = _append_buttons_as_text(text, msg.buttons) payload: dict[str, Any] = { "event": "message", "chat_id": msg.chat_id, - "text": msg.content, + "text": text, } + if msg.buttons: + payload["buttons"] = msg.buttons + payload["button_prompt"] = msg.content if msg.media: payload["media"] = msg.media urls: list[dict[str, str]] = [] diff --git a/tests/agent/test_ask_user.py b/tests/agent/test_ask_user.py index 4d5b5be9..a192ee4a 100644 --- a/tests/agent/test_ask_user.py +++ b/tests/agent/test_ask_user.py @@ -205,3 +205,37 @@ async def test_ask_user_keeps_buttons_for_telegram(tmp_path): assert response is not None assert response.content == "Install the optional package?" assert response.buttons == [["Install", "Skip"]] + + +@pytest.mark.asyncio +async def test_ask_user_keeps_buttons_for_websocket(tmp_path): + async def chat_with_retry(**kwargs): + return LLMResponse( + content="", + finish_reason="tool_calls", + tool_calls=[ + ToolCallRequest( + id="call_ask", + name="ask_user", + arguments={ + "question": "Install the optional package?", + "options": ["Install", "Skip"], + }, + ) + ], + ) + + loop = AgentLoop( + bus=MessageBus(), + provider=_make_provider(chat_with_retry), + workspace=tmp_path, + model="test-model", + ) + + response = await loop._process_message( + InboundMessage(channel="websocket", sender_id="user", chat_id="123", content="set it up") + ) + + assert response is not None + assert response.content == "Install the optional package?" + assert response.buttons == [["Install", "Skip"]] diff --git a/tests/channels/test_websocket_channel.py b/tests/channels/test_websocket_channel.py index c92c88ba..a1d459b9 100644 --- a/tests/channels/test_websocket_channel.py +++ b/tests/channels/test_websocket_channel.py @@ -178,6 +178,7 @@ async def test_send_delivers_json_message_with_media_and_reply() -> None: content="hello", reply_to="m1", media=["/tmp/a.png"], + buttons=[["Yes", "No"]], ) await channel.send(msg) @@ -185,9 +186,11 @@ async def test_send_delivers_json_message_with_media_and_reply() -> None: payload = json.loads(mock_ws.send.call_args[0][0]) assert payload["event"] == "message" assert payload["chat_id"] == "chat-1" - assert payload["text"] == "hello" + assert payload["text"] == "hello\n\n1. Yes\n2. No" + assert payload["button_prompt"] == "hello" assert payload["reply_to"] == "m1" assert payload["media"] == ["/tmp/a.png"] + assert payload["buttons"] == [["Yes", "No"]] @pytest.mark.asyncio diff --git a/webui/src/components/thread/AskUserPrompt.tsx b/webui/src/components/thread/AskUserPrompt.tsx new file mode 100644 index 00000000..3ab20f5e --- /dev/null +++ b/webui/src/components/thread/AskUserPrompt.tsx @@ -0,0 +1,108 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { MessageSquareText } from "lucide-react"; + +import { Button } from "@/components/ui/button"; +import { cn } from "@/lib/utils"; + +interface AskUserPromptProps { + question: string; + buttons: string[][]; + onAnswer: (answer: string) => void; +} + +export function AskUserPrompt({ + question, + buttons, + onAnswer, +}: AskUserPromptProps) { + const [customOpen, setCustomOpen] = useState(false); + const [custom, setCustom] = useState(""); + const inputRef = useRef(null); + const options = buttons.flat().filter(Boolean); + + useEffect(() => { + if (customOpen) { + inputRef.current?.focus(); + } + }, [customOpen]); + + const submitCustom = useCallback(() => { + const answer = custom.trim(); + if (!answer) return; + onAnswer(answer); + setCustom(""); + setCustomOpen(false); + }, [custom, onAnswer]); + + if (options.length === 0) return null; + + return ( +
+
+
+ +
+

+ {question} +

+
+ +
+ {options.map((option) => ( + + ))} + +
+ + {customOpen ? ( +
+