From df37a361744cfeb8aad048071c99f2fddebdda2e Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sun, 26 Apr 2026 17:42:58 +0000 Subject: [PATCH 01/17] fix(agent): expose session timestamps in model context Include persisted turn timestamps when assembling LLM prompts so relative-date references like yesterday and today have concrete anchors. Made-with: Cursor --- nanobot/agent/loop.py | 4 +-- nanobot/agent/memory.py | 2 +- nanobot/session/manager.py | 21 ++++++++++- tests/agent/test_loop_save_turn.py | 5 ++- tests/agent/test_session_manager_history.py | 40 +++++++++++++++++++++ 5 files changed, 67 insertions(+), 5 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 3c893c38..3bc7c437 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -832,7 +832,7 @@ class AgentLoop: if is_subagent and self._persist_subagent_followup(session, msg): self.sessions.save(session) self._set_tool_context(channel, chat_id, msg.metadata.get("message_id")) - history = session.get_history(max_messages=0) + history = session.get_history(max_messages=0, include_timestamps=True) current_role = "assistant" if is_subagent else "user" # Subagent content is already in `history` above; passing it again @@ -901,7 +901,7 @@ class AgentLoop: if isinstance(message_tool, MessageTool): message_tool.start_turn() - history = session.get_history(max_messages=0) + history = session.get_history(max_messages=0, include_timestamps=True) pending_ask_id = pending_ask_user_id(history) if pending_ask_id: diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index 91160d4a..f11bd6af 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -494,7 +494,7 @@ class Consolidator: session_summary: str | None = None, ) -> tuple[int, str]: """Estimate current prompt size for the normal session history view.""" - history = session.get_history(max_messages=0) + history = session.get_history(max_messages=0, include_timestamps=True) channel, chat_id = (session.key.split(":", 1) if ":" in session.key else (None, None)) probe_messages = self._build_messages( history=history, diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index ddcfdea1..a9499046 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -30,6 +30,18 @@ class Session: metadata: dict[str, Any] = field(default_factory=dict) last_consolidated: int = 0 # Number of messages already consolidated to files + @staticmethod + def _annotate_message_time(message: dict[str, Any], content: Any) -> Any: + """Expose persisted turn timestamps to the model for relative-date reasoning.""" + timestamp = message.get("timestamp") + if ( + not timestamp + or message.get("role") not in {"user", "assistant"} + or not isinstance(content, str) + ): + return content + return f"[Message Time: {timestamp}]\n{content}" + def add_message(self, role: str, content: str, **kwargs: Any) -> None: """Add a message to the session.""" msg = { @@ -41,7 +53,12 @@ class Session: self.messages.append(msg) self.updated_at = datetime.now() - def get_history(self, max_messages: int = 500) -> list[dict[str, Any]]: + def get_history( + self, + max_messages: int = 500, + *, + include_timestamps: bool = False, + ) -> list[dict[str, Any]]: """Return unconsolidated messages for LLM input, aligned to a legal tool-call boundary.""" unconsolidated = self.messages[self.last_consolidated:] sliced = unconsolidated[-max_messages:] @@ -75,6 +92,8 @@ class Session: image_placeholder_text(p) for p in media if isinstance(p, str) and p ) content = f"{content}\n{breadcrumbs}" if content else breadcrumbs + if include_timestamps: + content = self._annotate_message_time(message, content) entry: dict[str, Any] = {"role": message["role"], "content": content} for key in ("tool_calls", "tool_call_id", "name", "reasoning_content"): if key in message: diff --git a/tests/agent/test_loop_save_turn.py b/tests/agent/test_loop_save_turn.py index 50951824..a79a6b01 100644 --- a/tests/agent/test_loop_save_turn.py +++ b/tests/agent/test_loop_save_turn.py @@ -535,7 +535,10 @@ async def test_system_subagent_followup_is_persisted_before_prompt_assembly(tmp_ ) non_system = [m for m in seen["initial_messages"] if m.get("role") != "system"] - assert [m["content"] for m in non_system[:2]] == ["question", "working"] + assert "question" in non_system[0]["content"] + assert "working" in non_system[1]["content"] + assert "[Message Time:" in non_system[0]["content"] + assert "[Message Time:" in non_system[1]["content"] assert non_system[2]["content"].count("subagent result") == 1 assert "Current Time:" in non_system[2]["content"] diff --git a/tests/agent/test_session_manager_history.py b/tests/agent/test_session_manager_history.py index 8b4d0740..3c2b68e3 100644 --- a/tests/agent/test_session_manager_history.py +++ b/tests/agent/test_session_manager_history.py @@ -194,6 +194,46 @@ def test_get_history_preserves_reasoning_content(): ] +def test_get_history_exposes_turn_timestamps_to_model(): + session = Session(key="test:timestamps") + session.messages.append({ + "role": "user", + "content": "10 点提醒是昨天发生的", + "timestamp": "2026-04-26T22:00:00", + }) + session.messages.append({ + "role": "assistant", + "content": "记下来了", + "timestamp": "2026-04-26T22:00:05", + }) + + history = session.get_history(max_messages=500, include_timestamps=True) + + assert history == [ + { + "role": "user", + "content": "[Message Time: 2026-04-26T22:00:00]\n10 点提醒是昨天发生的", + }, + { + "role": "assistant", + "content": "[Message Time: 2026-04-26T22:00:05]\n记下来了", + }, + ] + + +def test_get_history_does_not_annotate_tool_results_with_timestamps(): + session = Session(key="test:tool-timestamps") + session.messages.append({"role": "user", "content": "run tool"}) + session.messages.extend(_tool_turn("ts", 0)) + session.messages[-1]["timestamp"] = "2026-04-26T22:00:10" + + history = session.get_history(max_messages=500, include_timestamps=True) + + tool_result = history[-1] + assert tool_result["role"] == "tool" + assert tool_result["content"] == "ok" + + # --- Window cuts mid-group: assistant present but some tool results orphaned --- def test_window_cuts_mid_tool_group(): From 7037764186d146698d035e7786c2fab7341abc04 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sun, 26 Apr 2026 18:01:55 +0000 Subject: [PATCH 02/17] docs: clarify maintainer and contribution licensing --- CONTRIBUTING.md | 5 +++++ LICENSE | 2 +- README.md | 4 ++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eb4bca4b..51406a09 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -87,6 +87,11 @@ ruff check nanobot/ ruff format nanobot/ ``` +## Contribution License + +By submitting a contribution, you confirm that you have the right to submit it +and agree that it will be licensed under the project's MIT License. + ## Code Style We care about more than passing lint. We want nanobot to stay small, calm, and readable. diff --git a/LICENSE b/LICENSE index 24bdaccb..e06eb24c 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2025 nanobot contributors +Copyright (c) 2025-present Xubin Ren and the nanobot contributors Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/README.md b/README.md index 90a3d2f4..c9d89e88 100644 --- a/README.md +++ b/README.md @@ -282,6 +282,10 @@ PRs welcome! The codebase is intentionally small and readable. 🤗 - **More integrations** — Calendar and more - **Self-improvement** — Learn from feedback and mistakes +## Contact + +This project was started by [Xubin Ren](https://github.com/re-bin) as a personal open-source project and continues to be maintained in an individual capacity using personal resources, with contributions from the open-source community. Feel free to contact [xubinrencs@gmail.com](mailto:xubinrencs@gmail.com) for questions, ideas, or collaboration. + ### Contributors From 038a140ad38596ae7a5c58380d50dcdd3289e186 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sun, 26 Apr 2026 17:37:04 +0000 Subject: [PATCH 03/17] fix(slack): preserve thread context for proactive replies Capture Slack thread metadata for cron and message-tool deliveries so replies stay in the originating thread, and hydrate first thread mentions with recent Slack context. Made-with: Cursor --- docs/chat-apps.md | 2 +- nanobot/agent/loop.py | 44 ++++++++++--- nanobot/agent/tools/cron.py | 11 +++- nanobot/agent/tools/message.py | 22 +++++-- nanobot/channels/slack.py | 79 ++++++++++++++++++++++- nanobot/cli/commands.py | 9 ++- nanobot/cron/service.py | 4 ++ nanobot/cron/types.py | 2 + tests/agent/test_loop_tool_context.py | 90 +++++++++++++++++++++++++++ tests/channels/test_slack_channel.py | 50 +++++++++++++++ tests/cron/test_cron_service.py | 22 +++++++ tests/cron/test_cron_tool_list.py | 15 +++++ tests/tools/test_message_tool.py | 35 +++++++++++ 13 files changed, 366 insertions(+), 19 deletions(-) create mode 100644 tests/agent/test_loop_tool_context.py diff --git a/docs/chat-apps.md b/docs/chat-apps.md index 9332bdc0..96ef654c 100644 --- a/docs/chat-apps.md +++ b/docs/chat-apps.md @@ -434,7 +434,7 @@ Uses **Socket Mode** — no public URL required. **2. Configure the app** - **Socket Mode**: Toggle ON → Generate an **App-Level Token** with `connections:write` scope → copy it (`xapp-...`) -- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read` +- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read`, `channels:history`, `groups:history`, `im:history`, `mpim:history` - **Event Subscriptions**: Toggle ON → Subscribe to bot events: `message.im`, `message.channels`, `app_mention` → Save Changes - **App Home**: Scroll to **Show Tabs** → Enable **Messages Tab** → Check **"Allow users to send Slash commands and messages from the messages tab"** - **Install App**: Click **Install to Workspace** → Authorize → copy the **Bot Token** (`xoxb-...`) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 3c893c38..22ad14b2 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -76,6 +76,8 @@ class _LoopHook(AgentHook): channel: str = "cli", chat_id: str = "direct", message_id: str | None = None, + metadata: dict[str, Any] | None = None, + session_key: str | None = None, ) -> None: super().__init__(reraise=True) self._loop = agent_loop @@ -85,6 +87,8 @@ class _LoopHook(AgentHook): self._channel = channel self._chat_id = chat_id self._message_id = message_id + self._metadata = metadata or {} + self._session_key = session_key self._stream_buf = "" def wants_streaming(self) -> bool: @@ -127,7 +131,13 @@ class _LoopHook(AgentHook): 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) + self._loop._set_tool_context( + self._channel, + self._chat_id, + self._message_id, + self._metadata, + session_key=self._session_key, + ) async def after_iteration(self, context: AgentHookContext) -> None: if ( @@ -387,18 +397,24 @@ class AgentLoop: finally: self._mcp_connecting = False - def _set_tool_context(self, channel: str, chat_id: str, message_id: str | None = None) -> None: + def _set_tool_context( + self, channel: str, chat_id: str, + message_id: str | None = None, metadata: dict | None = None, + session_key: str | None = None, + ) -> None: """Update context for all tools that need routing info.""" - # Compute the effective session key (accounts for unified sessions) - # so that subagent results route to the correct pending queue. effective_key = UNIFIED_SESSION_KEY if self._unified_session else f"{channel}:{chat_id}" for name in ("message", "spawn", "cron", "my"): if tool := self.tools.get(name): if hasattr(tool, "set_context"): if name == "spawn": tool.set_context(channel, chat_id, effective_key=effective_key) + elif name == "cron": + tool.set_context(channel, chat_id, metadata=metadata, session_key=session_key) + elif name == "message": + tool.set_context(channel, chat_id, message_id, metadata=metadata) else: - tool.set_context(channel, chat_id, *([message_id] if name == "message" else [])) + tool.set_context(channel, chat_id) @staticmethod def _strip_think(text: str | None) -> str | None: @@ -464,6 +480,8 @@ class AgentLoop: channel: str = "cli", chat_id: str = "direct", message_id: str | None = None, + metadata: dict[str, Any] | None = None, + session_key: str | None = None, pending_queue: asyncio.Queue | None = None, ) -> tuple[str | None, list[str], list[dict], str, bool]: """Run the agent iteration loop. @@ -483,6 +501,8 @@ class AgentLoop: channel=channel, chat_id=chat_id, message_id=message_id, + metadata=metadata, + session_key=session_key, ) hook: AgentHook = ( CompositeHook([loop_hook] + self._extra_hooks) if self._extra_hooks else loop_hook @@ -831,7 +851,10 @@ class AgentLoop: is_subagent = msg.sender_id == "subagent" if is_subagent and self._persist_subagent_followup(session, msg): self.sessions.save(session) - self._set_tool_context(channel, chat_id, msg.metadata.get("message_id")) + self._set_tool_context( + channel, chat_id, msg.metadata.get("message_id"), + msg.metadata, session_key=key, + ) history = session.get_history(max_messages=0) current_role = "assistant" if is_subagent else "user" @@ -848,6 +871,8 @@ class AgentLoop: 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"), + metadata=msg.metadata, + session_key=key, pending_queue=pending_queue, ) self._save_turn(session, all_msgs, 1 + len(history)) @@ -896,7 +921,10 @@ class AgentLoop: session_summary=pending, ) - self._set_tool_context(msg.channel, msg.chat_id, msg.metadata.get("message_id")) + self._set_tool_context( + msg.channel, msg.chat_id, msg.metadata.get("message_id"), + msg.metadata, session_key=key, + ) if message_tool := self.tools.get("message"): if isinstance(message_tool, MessageTool): message_tool.start_turn() @@ -978,6 +1006,8 @@ class AgentLoop: channel=msg.channel, chat_id=msg.chat_id, message_id=msg.metadata.get("message_id"), + metadata=msg.metadata, + session_key=key, pending_queue=pending_queue, ) diff --git a/nanobot/agent/tools/cron.py b/nanobot/agent/tools/cron.py index 127ac6c9..46974d4e 100644 --- a/nanobot/agent/tools/cron.py +++ b/nanobot/agent/tools/cron.py @@ -60,12 +60,19 @@ class CronTool(Tool): self._default_timezone = default_timezone self._channel: ContextVar[str] = ContextVar("cron_channel", default="") self._chat_id: ContextVar[str] = ContextVar("cron_chat_id", default="") + self._metadata: ContextVar[dict] = ContextVar("cron_metadata", default={}) + self._session_key: ContextVar[str] = ContextVar("cron_session_key", default="") self._in_cron_context: ContextVar[bool] = ContextVar("cron_in_context", default=False) - def set_context(self, channel: str, chat_id: str) -> None: + def set_context( + self, channel: str, chat_id: str, + metadata: dict | None = None, session_key: str | None = None, + ) -> None: """Set the current session context for delivery.""" self._channel.set(channel) self._chat_id.set(chat_id) + self._metadata.set(metadata or {}) + self._session_key.set(session_key or f"{channel}:{chat_id}") def set_cron_context(self, active: bool): """Mark whether the tool is executing inside a cron job callback.""" @@ -199,6 +206,8 @@ class CronTool(Tool): channel=channel, to=chat_id, delete_after_run=delete_after, + channel_meta=self._metadata.get(), + session_key=self._session_key.get() or None, ) return f"Created job '{job.name}' (id: {job.id})" diff --git a/nanobot/agent/tools/message.py b/nanobot/agent/tools/message.py index ea7f91bc..fe097e11 100644 --- a/nanobot/agent/tools/message.py +++ b/nanobot/agent/tools/message.py @@ -41,17 +41,28 @@ class MessageTool(Tool): "message_default_message_id", default=default_message_id, ) + self._default_metadata: ContextVar[dict[str, Any]] = ContextVar( + "message_default_metadata", + default={}, + ) self._sent_in_turn_var: ContextVar[bool] = ContextVar("message_sent_in_turn", default=False) self._record_channel_delivery_var: ContextVar[bool] = ContextVar( "message_record_channel_delivery", default=False, ) - def set_context(self, channel: str, chat_id: str, message_id: str | None = None) -> None: + def set_context( + self, + channel: str, + chat_id: str, + message_id: str | None = None, + metadata: dict[str, Any] | None = None, + ) -> None: """Set the current message context.""" self._default_channel.set(channel) self._default_chat_id.set(chat_id) self._default_message_id.set(message_id) + self._default_metadata.set(metadata or {}) def set_send_callback(self, callback: Callable[[OutboundMessage], Awaitable[None]]) -> None: """Set the callback for sending messages.""" @@ -118,7 +129,8 @@ class MessageTool(Tool): # some channels (e.g. Feishu) use it to determine the target # conversation via their Reply API, which would route the message # to the wrong chat entirely. - if channel == default_channel and chat_id == default_chat_id: + same_target = channel == default_channel and chat_id == default_chat_id + if same_target: message_id = message_id or self._default_message_id.get() else: message_id = None @@ -129,9 +141,9 @@ class MessageTool(Tool): if not self._send_callback: return "Error: Message sending not configured" - metadata = { - "message_id": message_id, - } if message_id else {} + metadata = dict(self._default_metadata.get()) if same_target else {} + if message_id: + metadata["message_id"] = message_id if self._record_channel_delivery_var.get(): metadata["_record_channel_delivery"] = True diff --git a/nanobot/channels/slack.py b/nanobot/channels/slack.py index c68020ce..8d0d8709 100644 --- a/nanobot/channels/slack.py +++ b/nanobot/channels/slack.py @@ -38,6 +38,8 @@ class SlackConfig(Base): reply_in_thread: bool = True react_emoji: str = "eyes" done_emoji: str = "white_check_mark" + include_thread_context: bool = True + thread_context_limit: int = 20 allow_from: list[str] = Field(default_factory=list) group_policy: str = "mention" group_allow_from: list[str] = Field(default_factory=list) @@ -66,6 +68,7 @@ class SlackChannel(BaseChannel): self._socket_client: SocketModeClient | None = None self._bot_user_id: str | None = None self._target_cache: dict[str, str] = {} + self._thread_context_attempted: set[str] = set() async def start(self) -> None: """Start the Slack Socket Mode client.""" @@ -327,9 +330,11 @@ class SlackChannel(BaseChannel): text = self._strip_bot_mention(text) - thread_ts = event.get("thread_ts") + event_ts = event.get("ts") + raw_thread_ts = event.get("thread_ts") + thread_ts = raw_thread_ts if self.config.reply_in_thread and not thread_ts: - thread_ts = event.get("ts") + thread_ts = event_ts # Add :eyes: reaction to the triggering message (best-effort) try: if self._web_client and event.get("ts"): @@ -343,12 +348,20 @@ class SlackChannel(BaseChannel): # Thread-scoped session key for channel/group messages session_key = f"slack:{chat_id}:{thread_ts}" if thread_ts and channel_type != "im" else None + content = await self._with_thread_context( + text, + chat_id=chat_id, + channel_type=channel_type, + thread_ts=thread_ts, + raw_thread_ts=raw_thread_ts, + current_ts=event_ts, + ) try: await self._handle_message( sender_id=sender_id, chat_id=chat_id, - content=text, + content=content, metadata={ "slack": { "event": event, @@ -361,6 +374,66 @@ class SlackChannel(BaseChannel): except Exception: logger.exception("Error handling Slack message from {}", sender_id) + async def _with_thread_context( + self, + text: str, + *, + chat_id: str, + channel_type: str, + thread_ts: str | None, + raw_thread_ts: str | None, + current_ts: str | None, + ) -> str: + """Include thread history the first time the bot is pulled into a Slack thread.""" + if ( + not self.config.include_thread_context + or not self._web_client + or channel_type == "im" + or not raw_thread_ts + or not thread_ts + or current_ts == thread_ts + ): + return text + + key = f"{chat_id}:{thread_ts}" + if key in self._thread_context_attempted: + return text + self._thread_context_attempted.add(key) + + try: + response = await self._web_client.conversations_replies( + channel=chat_id, + ts=thread_ts, + limit=max(1, self.config.thread_context_limit), + ) + except Exception as e: + logger.warning("Slack thread context unavailable for {}: {}", key, e) + return text + + lines = self._format_thread_context( + response.get("messages", []), + current_ts=current_ts, + ) + if not lines: + return text + return "Slack thread context before this mention:\n" + "\n".join(lines) + f"\n\nCurrent message:\n{text}" + + def _format_thread_context(self, messages: list[dict[str, Any]], *, current_ts: str | None) -> list[str]: + lines: list[str] = [] + for item in messages: + if item.get("ts") == current_ts: + continue + if item.get("subtype"): + continue + sender = str(item.get("user") or item.get("bot_id") or "unknown") + if self._bot_user_id and sender == self._bot_user_id: + continue + text = str(item.get("text") or "").strip() + if not text: + continue + lines.append(f"- <@{sender}>: {self._strip_bot_mention(text)}") + return lines + async def _update_react_emoji(self, chat_id: str, ts: str | None) -> None: """Remove the in-progress reaction and optionally add a done reaction.""" if not self._web_client or not ts: diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index ce88ece5..f15df253 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -666,7 +666,9 @@ def _run_gateway( else f"{channel}:{chat_id}" ) - async def _deliver_to_channel(msg: OutboundMessage, *, record: bool = False) -> None: + async def _deliver_to_channel( + msg: OutboundMessage, *, record: bool = False, session_key: str | None = None, + ) -> None: """Publish a user-visible message and mirror it into that channel's session.""" metadata = dict(msg.metadata or {}) record = record or bool(metadata.pop("_record_channel_delivery", False)) @@ -687,7 +689,8 @@ def _run_gateway( and hasattr(session_manager, "get_or_create") and hasattr(session_manager, "save") ): - session = session_manager.get_or_create(_channel_session_key(msg.channel, msg.chat_id)) + key = session_key or _channel_session_key(msg.channel, msg.chat_id) + session = session_manager.get_or_create(key) session.add_message("assistant", msg.content, _channel_delivery=True) session_manager.save(session) await bus.publish_outbound(msg) @@ -757,8 +760,10 @@ def _run_gateway( channel=job.payload.channel or "cli", chat_id=job.payload.to, content=response, + metadata=dict(job.payload.channel_meta), ), record=True, + session_key=job.payload.session_key, ) return response diff --git a/nanobot/cron/service.py b/nanobot/cron/service.py index 165ce54d..0282aa1c 100644 --- a/nanobot/cron/service.py +++ b/nanobot/cron/service.py @@ -379,6 +379,8 @@ class CronService: channel: str | None = None, to: str | None = None, delete_after_run: bool = False, + channel_meta: dict | None = None, + session_key: str | None = None, ) -> CronJob: """Add a new job.""" _validate_schedule_for_add(schedule) @@ -395,6 +397,8 @@ class CronService: deliver=deliver, channel=channel, to=to, + channel_meta=channel_meta or {}, + session_key=session_key, ), state=CronJobState(next_run_at_ms=_compute_next_run(schedule, now)), created_at_ms=now, diff --git a/nanobot/cron/types.py b/nanobot/cron/types.py index c38542e1..24280da9 100644 --- a/nanobot/cron/types.py +++ b/nanobot/cron/types.py @@ -27,6 +27,8 @@ class CronPayload: deliver: bool = False channel: str | None = None # e.g. "whatsapp" to: str | None = None # e.g. phone number + channel_meta: dict = field(default_factory=dict) # channel-specific routing (e.g. Slack thread_ts) + session_key: str | None = None # original session key for correct session recording @dataclass diff --git a/tests/agent/test_loop_tool_context.py b/tests/agent/test_loop_tool_context.py new file mode 100644 index 00000000..e41bae35 --- /dev/null +++ b/tests/agent/test_loop_tool_context.py @@ -0,0 +1,90 @@ +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from nanobot.agent.loop import AgentLoop +from nanobot.bus.queue import MessageBus +from nanobot.providers.base import LLMResponse, ToolCallRequest + + +class _ContextRecordingTool: + name = "cron" + concurrency_safe = False + + def __init__(self) -> None: + self.contexts: list[dict] = [] + + def set_context( + self, + channel: str, + chat_id: str, + metadata: dict | None = None, + session_key: str | None = None, + ) -> None: + self.contexts.append({ + "channel": channel, + "chat_id": chat_id, + "metadata": metadata, + "session_key": session_key, + }) + + async def execute(self, **_kwargs) -> str: + return "created" + + +class _Tools: + def __init__(self, tool: _ContextRecordingTool) -> None: + self.tool = tool + + def get(self, name: str): + return self.tool if name == "cron" else None + + def get_definitions(self) -> list: + return [] + + def prepare_call(self, name: str, arguments: dict): + return (self.tool, arguments, None) if name == "cron" else (None, arguments, None) + + +@pytest.mark.asyncio +async def test_loop_hook_preserves_metadata_when_resetting_tool_context(tmp_path: Path) -> None: + provider = MagicMock() + calls = {"n": 0} + + async def chat_with_retry(**_kwargs): + calls["n"] += 1 + if calls["n"] == 1: + return LLMResponse( + content=None, + tool_calls=[ToolCallRequest(id="call_1", name="cron", arguments={"action": "add"})], + ) + return LLMResponse(content="done", tool_calls=[]) + + provider.chat_with_retry = chat_with_retry + provider.get_default_model.return_value = "test-model" + + loop = AgentLoop( + bus=MessageBus(), + provider=provider, + workspace=tmp_path, + model="test-model", + ) + cron = _ContextRecordingTool() + loop.tools = _Tools(cron) + + metadata = {"slack": {"thread_ts": "111.222", "channel_type": "channel"}} + await loop._run_agent_loop( + [], + channel="slack", + chat_id="C123", + metadata=metadata, + session_key="slack:C123:111.222", + ) + + assert cron.contexts[-1] == { + "channel": "slack", + "chat_id": "C123", + "metadata": metadata, + "session_key": "slack:C123:111.222", + } diff --git a/tests/channels/test_slack_channel.py b/tests/channels/test_slack_channel.py index 2e72c4e6..9b4d34c0 100644 --- a/tests/channels/test_slack_channel.py +++ b/tests/channels/test_slack_channel.py @@ -20,9 +20,11 @@ class _FakeAsyncWebClient: self.reactions_add_calls: list[dict[str, object | None]] = [] self.reactions_remove_calls: list[dict[str, object | None]] = [] self.conversations_list_calls: list[dict[str, object | None]] = [] + self.conversations_replies_calls: list[dict[str, object | None]] = [] self.users_list_calls: list[dict[str, object | None]] = [] self.conversations_open_calls: list[dict[str, object | None]] = [] self._conversations_pages: list[dict[str, object]] = [] + self._conversations_replies_response: dict[str, object] = {"messages": []} self._users_pages: list[dict[str, object]] = [] self._open_dm_response: dict[str, object] = {"channel": {"id": "D_OPENED"}} @@ -92,6 +94,10 @@ class _FakeAsyncWebClient: return self._conversations_pages.pop(0) return {"channels": [], "response_metadata": {"next_cursor": ""}} + async def conversations_replies(self, **kwargs): + self.conversations_replies_calls.append(kwargs) + return self._conversations_replies_response + async def users_list(self, **kwargs): self.users_list_calls.append(kwargs) if self._users_pages: @@ -316,3 +322,47 @@ async def test_send_raises_when_named_target_cannot_be_resolved() -> None: content="hello", ) ) + + +@pytest.mark.asyncio +async def test_with_thread_context_fetches_root_once() -> None: + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + channel._bot_user_id = "UBOT" + fake_web = _FakeAsyncWebClient() + fake_web._conversations_replies_response = { + "messages": [ + {"ts": "111.000", "user": "UROOT", "text": "drink water"}, + {"ts": "112.000", "user": "U2", "text": "good idea"}, + {"ts": "113.000", "user": "U3", "text": "<@UBOT> what did you see?"}, + ] + } + channel._web_client = fake_web + + content = await channel._with_thread_context( + "what did you see?", + chat_id="C123", + channel_type="channel", + thread_ts="111.000", + raw_thread_ts="111.000", + current_ts="113.000", + ) + + assert fake_web.conversations_replies_calls == [ + {"channel": "C123", "ts": "111.000", "limit": 20} + ] + assert "Slack thread context before this mention:" in content + assert "- <@UROOT>: drink water" in content + assert "- <@U2>: good idea" in content + assert "U3" not in content + assert content.endswith("Current message:\nwhat did you see?") + + second = await channel._with_thread_context( + "again", + chat_id="C123", + channel_type="channel", + thread_ts="111.000", + raw_thread_ts="111.000", + current_ts="114.000", + ) + assert second == "again" + assert len(fake_web.conversations_replies_calls) == 1 diff --git a/tests/cron/test_cron_service.py b/tests/cron/test_cron_service.py index 0e83b187..b41d5b15 100644 --- a/tests/cron/test_cron_service.py +++ b/tests/cron/test_cron_service.py @@ -43,6 +43,28 @@ def test_add_job_accepts_valid_timezone(tmp_path) -> None: assert job.state.next_run_at_ms is not None +def test_add_job_preserves_channel_meta_and_session_key(tmp_path) -> None: + service = CronService(tmp_path / "cron" / "jobs.json") + meta = {"slack": {"thread_ts": "1234567890.123456", "channel_type": "channel"}} + job = service.add_job( + name="thread test", + schedule=CronSchedule(kind="every", every_ms=60_000), + message="hello", + deliver=True, + channel="slack", + to="C123", + channel_meta=meta, + session_key="slack:C123:1234567890.123456", + ) + assert job.payload.channel_meta == meta + assert job.payload.session_key == "slack:C123:1234567890.123456" + + reloaded = service.get_job(job.id) + assert reloaded is not None + assert reloaded.payload.channel_meta == meta + assert reloaded.payload.session_key == "slack:C123:1234567890.123456" + + @pytest.mark.asyncio async def test_execute_job_records_run_history(tmp_path) -> None: store_path = tmp_path / "cron" / "jobs.json" diff --git a/tests/cron/test_cron_tool_list.py b/tests/cron/test_cron_tool_list.py index 5ffd4691..86eb95db 100644 --- a/tests/cron/test_cron_tool_list.py +++ b/tests/cron/test_cron_tool_list.py @@ -382,6 +382,21 @@ def test_add_job_empty_message_returns_actionable_error(tmp_path) -> None: assert "Retry including message=" in result +def test_add_job_captures_metadata_and_session_key(tmp_path) -> None: + """CronTool stores channel metadata and session_key when adding a job.""" + tool = _make_tool(tmp_path) + meta = {"slack": {"thread_ts": "111.222", "channel_type": "channel"}} + tool.set_context("slack", "C99", metadata=meta, session_key="slack:C99:111.222") + + result = tool._add_job("test", "say hi", 60, None, None, None) + assert "Created job" in result + + jobs = tool._cron.list_jobs() + assert len(jobs) == 1 + assert jobs[0].payload.channel_meta == meta + assert jobs[0].payload.session_key == "slack:C99:111.222" + + def test_list_excludes_disabled_jobs(tmp_path) -> None: tool = _make_tool(tmp_path) job = tool._cron.add_job( diff --git a/tests/tools/test_message_tool.py b/tests/tools/test_message_tool.py index 18a88121..feff43da 100644 --- a/tests/tools/test_message_tool.py +++ b/tests/tools/test_message_tool.py @@ -50,3 +50,38 @@ async def test_message_tool_marks_channel_delivery_only_when_enabled() -> None: assert sent[0].metadata == {} assert sent[1].metadata == {"_record_channel_delivery": True} + + +@pytest.mark.asyncio +async def test_message_tool_inherits_metadata_for_same_target() -> None: + sent: list[OutboundMessage] = [] + + async def _send(msg: OutboundMessage) -> None: + sent.append(msg) + + tool = MessageTool(send_callback=_send) + slack_meta = {"slack": {"thread_ts": "111.222", "channel_type": "channel"}} + tool.set_context("slack", "C123", metadata=slack_meta) + + await tool.execute(content="thread reply") + + assert sent[0].metadata == slack_meta + + +@pytest.mark.asyncio +async def test_message_tool_does_not_inherit_metadata_for_cross_target() -> None: + sent: list[OutboundMessage] = [] + + async def _send(msg: OutboundMessage) -> None: + sent.append(msg) + + tool = MessageTool(send_callback=_send) + tool.set_context( + "slack", + "C123", + metadata={"slack": {"thread_ts": "111.222", "channel_type": "channel"}}, + ) + + await tool.execute(content="channel reply", channel="slack", chat_id="C999") + + assert sent[0].metadata == {} From 6eb178113e2d5a2dcc7188ab6d66aefd470a566c Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 27 Apr 2026 10:01:32 +0800 Subject: [PATCH 04/17] fix(mcp): sanitize MCP capability names for model API compatibility MCP resource/prompt/tool names containing spaces or special characters (e.g. "PostgreSQL System Information") were forwarded verbatim to model provider APIs, causing validation errors from both Anthropic and OpenAI which require names matching ^[a-zA-Z0-9_-]{1,128}$. Add _sanitize_name() that replaces invalid characters with underscores and collapses consecutive underscores. Applied in MCPToolWrapper, MCPResourceWrapper, MCPPromptWrapper constructors and the enabled_tools filtering logic. Closes #3468 --- nanobot/agent/tools/mcp.py | 20 +++++-- tests/agent/test_runner.py | 9 ++- tests/tools/test_mcp_tool.py | 112 +++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 10 deletions(-) diff --git a/nanobot/agent/tools/mcp.py b/nanobot/agent/tools/mcp.py index 8db90a03..0e5b008f 100644 --- a/nanobot/agent/tools/mcp.py +++ b/nanobot/agent/tools/mcp.py @@ -2,6 +2,7 @@ import asyncio import os +import re import shutil from contextlib import AsyncExitStack from typing import Any @@ -28,6 +29,15 @@ _TRANSIENT_EXC_NAMES: frozenset[str] = frozenset(( _WINDOWS_SHELL_LAUNCHERS: frozenset[str] = frozenset(("npx", "npm", "pnpm", "yarn", "bunx")) +# Characters allowed in tool names by model providers (Anthropic, OpenAI, etc.). +# Replace anything outside [a-zA-Z0-9_-] with underscore and collapse runs. +_SANITIZE_RE = re.compile(r"_+") + + +def _sanitize_name(name: str) -> str: + """Sanitize an MCP-derived name for model API compatibility.""" + return _SANITIZE_RE.sub("_", re.sub(r"[^a-zA-Z0-9_-]", "_", name)) + def _is_transient(exc: BaseException) -> bool: """Check if an exception looks like a transient connection error.""" @@ -137,7 +147,7 @@ class MCPToolWrapper(Tool): def __init__(self, session, server_name: str, tool_def, tool_timeout: int = 30): self._session = session self._original_name = tool_def.name - self._name = f"mcp_{server_name}_{tool_def.name}" + self._name = _sanitize_name(f"mcp_{server_name}_{tool_def.name}") self._description = tool_def.description or tool_def.name raw_schema = tool_def.inputSchema or {"type": "object", "properties": {}} self._parameters = _normalize_schema_for_openai(raw_schema) @@ -221,7 +231,7 @@ class MCPResourceWrapper(Tool): def __init__(self, session, server_name: str, resource_def, resource_timeout: int = 30): self._session = session self._uri = resource_def.uri - self._name = f"mcp_{server_name}_resource_{resource_def.name}" + self._name = _sanitize_name(f"mcp_{server_name}_resource_{resource_def.name}") desc = resource_def.description or resource_def.name self._description = f"[MCP Resource] {desc}\nURI: {self._uri}" self._parameters: dict[str, Any] = { @@ -311,7 +321,7 @@ class MCPPromptWrapper(Tool): def __init__(self, session, server_name: str, prompt_def, prompt_timeout: int = 30): self._session = session self._prompt_name = prompt_def.name - self._name = f"mcp_{server_name}_prompt_{prompt_def.name}" + self._name = _sanitize_name(f"mcp_{server_name}_prompt_{prompt_def.name}") desc = prompt_def.description or prompt_def.name self._description = ( f"[MCP Prompt] {desc}\n" @@ -514,9 +524,9 @@ async def connect_mcp_servers( registered_count = 0 matched_enabled_tools: set[str] = set() available_raw_names = [tool_def.name for tool_def in tools.tools] - available_wrapped_names = [f"mcp_{name}_{tool_def.name}" for tool_def in tools.tools] + available_wrapped_names = [_sanitize_name(f"mcp_{name}_{tool_def.name}") for tool_def in tools.tools] for tool_def in tools.tools: - wrapped_name = f"mcp_{name}_{tool_def.name}" + wrapped_name = _sanitize_name(f"mcp_{name}_{tool_def.name}") if ( not allow_all_tools and tool_def.name not in enabled_tools diff --git a/tests/agent/test_runner.py b/tests/agent/test_runner.py index ffa5fda9..d4fdd7a0 100644 --- a/tests/agent/test_runner.py +++ b/tests/agent/test_runner.py @@ -1060,11 +1060,10 @@ async def test_next_turn_after_llm_error_keeps_turn_boundary(tmp_path): request_messages = provider.chat_with_retry.await_args_list[1].kwargs["messages"] non_system = [message for message in request_messages if message.get("role") != "system"] - assert non_system[0] == {"role": "user", "content": "first question"} - assert non_system[1] == { - "role": "assistant", - "content": _PERSISTED_MODEL_ERROR_PLACEHOLDER, - } + assert non_system[0]["role"] == "user" + assert "first question" in non_system[0]["content"] + assert non_system[1]["role"] == "assistant" + assert _PERSISTED_MODEL_ERROR_PLACEHOLDER in non_system[1]["content"] assert non_system[2]["role"] == "user" assert "second question" in non_system[2]["content"] diff --git a/tests/tools/test_mcp_tool.py b/tests/tools/test_mcp_tool.py index 7732f859..66f7b19a 100644 --- a/tests/tools/test_mcp_tool.py +++ b/tests/tools/test_mcp_tool.py @@ -13,6 +13,7 @@ from nanobot.agent.tools.mcp import ( MCPResourceWrapper, MCPToolWrapper, _normalize_windows_stdio_command, + _sanitize_name, connect_mcp_servers, ) from nanobot.agent.tools.registry import ToolRegistry @@ -798,3 +799,114 @@ async def test_connect_registers_resources_and_prompts( assert "mcp_test_tool_a" in registry.tool_names assert "mcp_test_resource_res_b" in registry.tool_names assert "mcp_test_prompt_prompt_c" in registry.tool_names + + +# --------------------------------------------------------------------------- +# _sanitize_name tests +# --------------------------------------------------------------------------- + + +def test_sanitize_name_replaces_spaces() -> None: + assert _sanitize_name("PostgreSQL System Information") == "PostgreSQL_System_Information" + + +def test_sanitize_name_replaces_special_characters() -> None: + assert _sanitize_name("foo.bar@baz!") == "foo_bar_baz_" + + +def test_sanitize_name_collapses_consecutive_underscores() -> None: + assert _sanitize_name("a b") == "a_b" + + +def test_sanitize_name_preserves_valid_characters() -> None: + assert _sanitize_name("my-tool_v2") == "my-tool_v2" + + +def test_sanitize_name_noop_for_already_clean_names() -> None: + assert _sanitize_name("mcp_server_tool") == "mcp_server_tool" + + +# --------------------------------------------------------------------------- +# Wrapper sanitization tests +# --------------------------------------------------------------------------- + + +def test_tool_wrapper_sanitizes_name() -> None: + tool_def = SimpleNamespace( + name="My Tool", + description="tool with spaces", + inputSchema={"type": "object", "properties": {}}, + ) + wrapper = MCPToolWrapper(SimpleNamespace(call_tool=None), "srv", tool_def) + assert wrapper.name == "mcp_srv_My_Tool" + + +def test_resource_wrapper_sanitizes_name() -> None: + resource_def = SimpleNamespace( + name="PostgreSQL System Information", + uri="file:///pg/info", + description="PG info", + ) + wrapper = MCPResourceWrapper(None, "srv", resource_def) + assert wrapper.name == "mcp_srv_resource_PostgreSQL_System_Information" + + +def test_prompt_wrapper_sanitizes_name() -> None: + prompt_def = SimpleNamespace( + name="design-schema", + description="Design schema", + arguments=None, + ) + # Hyphens are allowed, so this should pass through unchanged + wrapper = MCPPromptWrapper(None, "my server", prompt_def) + assert wrapper.name == "mcp_my_server_prompt_design-schema" + + +def test_tool_wrapper_preserves_original_name_for_mcp_call() -> None: + tool_def = SimpleNamespace( + name="My Tool", + description="tool with spaces", + inputSchema={"type": "object", "properties": {}}, + ) + wrapper = MCPToolWrapper(SimpleNamespace(call_tool=None), "srv", tool_def) + # The sanitized API-facing name differs from the original MCP name + assert wrapper.name == "mcp_srv_My_Tool" + assert wrapper._original_name == "My Tool" + + +@pytest.mark.asyncio +async def test_connect_mcp_servers_sanitizes_resource_names( + fake_mcp_runtime: dict[str, object | None], +) -> None: + fake_mcp_runtime["session"] = _make_fake_session_with_capabilities( + tool_names=[], + resource_names=["PostgreSQL System Information"], + prompt_names=[], + ) + registry = ToolRegistry() + stacks = await connect_mcp_servers( + {"test": MCPServerConfig(command="fake")}, + registry, + ) + for stack in stacks.values(): + await stack.aclose() + + assert "mcp_test_resource_PostgreSQL_System_Information" in registry.tool_names + + +@pytest.mark.asyncio +async def test_connect_mcp_servers_enabled_tools_matches_sanitized_name( + fake_mcp_runtime: dict[str, object | None], +) -> None: + fake_mcp_runtime["session"] = _make_fake_session_with_capabilities( + tool_names=["My Tool", "other"], + ) + registry = ToolRegistry() + stacks = await connect_mcp_servers( + {"test": MCPServerConfig(command="fake", enabled_tools=["mcp_test_My_Tool"])}, + registry, + ) + for stack in stacks.values(): + await stack.aclose() + + assert registry.tool_names == ["mcp_test_My_Tool"] From 4801f54f5b3fa5e5dec14ee078a2220a260b305a Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sun, 26 Apr 2026 18:50:05 +0000 Subject: [PATCH 05/17] fix(cron): persist channel_meta and session_key across reloads Without writing these fields into jobs.json, cron jobs created in a Slack thread lost their thread_ts (and original session_key) after the service was reloaded, so reminders fired into the channel root. Made-with: Cursor --- nanobot/cron/service.py | 8 ++++++++ tests/cron/test_cron_service.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/nanobot/cron/service.py b/nanobot/cron/service.py index 0282aa1c..1cc858ce 100644 --- a/nanobot/cron/service.py +++ b/nanobot/cron/service.py @@ -109,6 +109,12 @@ class CronService: deliver=j["payload"].get("deliver", False), channel=j["payload"].get("channel"), to=j["payload"].get("to"), + channel_meta=( + j["payload"].get("channelMeta") + or j["payload"].get("channel_meta") + or {} + ), + session_key=j["payload"].get("sessionKey") or j["payload"].get("session_key"), ), state=CronJobState( next_run_at_ms=j.get("state", {}).get("nextRunAtMs"), @@ -210,6 +216,8 @@ class CronService: "deliver": j.payload.deliver, "channel": j.payload.channel, "to": j.payload.to, + "channelMeta": j.payload.channel_meta, + "sessionKey": j.payload.session_key, }, "state": { "nextRunAtMs": j.state.next_run_at_ms, diff --git a/tests/cron/test_cron_service.py b/tests/cron/test_cron_service.py index b41d5b15..1f000dbd 100644 --- a/tests/cron/test_cron_service.py +++ b/tests/cron/test_cron_service.py @@ -65,6 +65,37 @@ def test_add_job_preserves_channel_meta_and_session_key(tmp_path) -> None: assert reloaded.payload.session_key == "slack:C123:1234567890.123456" +@pytest.mark.asyncio +async def test_channel_meta_and_session_key_survive_store_reload(tmp_path) -> None: + store_path = tmp_path / "cron" / "jobs.json" + service = CronService(store_path) + await service.start() + meta = {"slack": {"thread_ts": "1234567890.123456", "channel_type": "channel"}} + try: + job = service.add_job( + name="thread test", + schedule=CronSchedule(kind="every", every_ms=60_000), + message="hello", + deliver=True, + channel="slack", + to="C123", + channel_meta=meta, + session_key="slack:C123:1234567890.123456", + ) + finally: + service.stop() + + raw = json.loads(store_path.read_text(encoding="utf-8")) + payload = raw["jobs"][0]["payload"] + assert payload["channelMeta"] == meta + assert payload["sessionKey"] == "slack:C123:1234567890.123456" + + reloaded = CronService(store_path).get_job(job.id) + assert reloaded is not None + assert reloaded.payload.channel_meta == meta + assert reloaded.payload.session_key == "slack:C123:1234567890.123456" + + @pytest.mark.asyncio async def test_execute_job_records_run_history(tmp_path) -> None: store_path = tmp_path / "cron" / "jobs.json" From 1ef41052daebad614fc56115eb8f931968cd53e8 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sun, 26 Apr 2026 18:50:12 +0000 Subject: [PATCH 06/17] fix(cron): rephrase fire-time prompt so agent delivers a natural reminder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old prompt framed cron firing as a "task triggered" status report, which led the agent to reply with things like "Done ✅ 已提醒 U0AV8BJPV8D 喝水" — exposing the user id and reading like a system log instead of a friendly reminder. Reword it to instruct the agent to speak directly to the user and forbid status-style language. Made-with: Cursor --- nanobot/cli/commands.py | 8 +++++--- tests/cli/test_commands.py | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index f15df253..2b911d75 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -714,9 +714,11 @@ def _run_gateway( from nanobot.utils.evaluator import evaluate_response reminder_note = ( - "[Scheduled Task] Timer finished.\n\n" - f"Task '{job.name}' has been triggered.\n" - f"Scheduled instruction: {job.payload.message}" + "The scheduled time has arrived. Deliver this reminder to the user now, " + "as a brief and natural message in their language. Speak directly to them — " + "do not narrate progress, summarize, include user IDs, or add status reports " + "like 'Done' or 'Reminded'.\n\n" + f"Reminder: {job.payload.message}" ) cron_tool = agent.tools.get("cron") diff --git a/tests/cli/test_commands.py b/tests/cli/test_commands.py index 47b610da..6439c73f 100644 --- a/tests/cli/test_commands.py +++ b/tests/cli/test_commands.py @@ -1067,9 +1067,11 @@ def test_gateway_cron_evaluator_receives_scheduled_reminder_context( assert seen["provider"] is provider assert seen["model"] == "test-model" assert seen["task_context"] == ( - "[Scheduled Task] Timer finished.\n\n" - "Task 'stretch' has been triggered.\n" - "Scheduled instruction: Remind me to stretch." + "The scheduled time has arrived. Deliver this reminder to the user now, " + "as a brief and natural message in their language. Speak directly to them — " + "do not narrate progress, summarize, include user IDs, or add status reports " + "like 'Done' or 'Reminded'.\n\n" + "Reminder: Remind me to stretch." ) bus.publish_outbound.assert_awaited_once_with( OutboundMessage( From 1fe3f0eb229bc12b6637ede0ca5de2012644c074 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Sun, 26 Apr 2026 18:53:03 +0000 Subject: [PATCH 07/17] fix(restart): preserve channel metadata across /restart so reply lands in thread cmd_restart only persisted channel + chat_id across the os.execv boundary, so when the new process announced "Restart completed" the OutboundMessage had no Slack thread_ts and the reply fell back to the channel root. Serialize msg.metadata into NANOBOT_RESTART_NOTIFY_METADATA, restore it on the RestartNotice, and forward it to OutboundMessage so the completion message follows the same routing as the original /restart invocation. Made-with: Cursor --- nanobot/channels/manager.py | 1 + nanobot/command/builtin.py | 6 +++++- nanobot/utils/restart.py | 33 ++++++++++++++++++++++++++++++--- tests/utils/test_restart.py | 29 +++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/nanobot/channels/manager.py b/nanobot/channels/manager.py index 7110311b..ccac6306 100644 --- a/nanobot/channels/manager.py +++ b/nanobot/channels/manager.py @@ -172,6 +172,7 @@ class ChannelManager: channel=notice.channel, chat_id=notice.chat_id, content=format_restart_completed_message(notice.started_at_raw), + metadata=dict(notice.metadata or {}), ), )) diff --git a/nanobot/command/builtin.py b/nanobot/command/builtin.py index 87d4bf64..f46d7dbd 100644 --- a/nanobot/command/builtin.py +++ b/nanobot/command/builtin.py @@ -28,7 +28,11 @@ async def cmd_stop(ctx: CommandContext) -> OutboundMessage: async def cmd_restart(ctx: CommandContext) -> OutboundMessage: """Restart the process in-place via os.execv.""" msg = ctx.msg - set_restart_notice_to_env(channel=msg.channel, chat_id=msg.chat_id) + set_restart_notice_to_env( + channel=msg.channel, + chat_id=msg.chat_id, + metadata=dict(msg.metadata or {}), + ) async def _do_restart(): await asyncio.sleep(1) diff --git a/nanobot/utils/restart.py b/nanobot/utils/restart.py index 35b8cced..871667f0 100644 --- a/nanobot/utils/restart.py +++ b/nanobot/utils/restart.py @@ -2,12 +2,15 @@ from __future__ import annotations +import json import os import time -from dataclasses import dataclass +from dataclasses import dataclass, field +from typing import Any RESTART_NOTIFY_CHANNEL_ENV = "NANOBOT_RESTART_NOTIFY_CHANNEL" RESTART_NOTIFY_CHAT_ID_ENV = "NANOBOT_RESTART_NOTIFY_CHAT_ID" +RESTART_NOTIFY_METADATA_ENV = "NANOBOT_RESTART_NOTIFY_METADATA" RESTART_STARTED_AT_ENV = "NANOBOT_RESTART_STARTED_AT" @@ -16,6 +19,7 @@ class RestartNotice: channel: str chat_id: str started_at_raw: str + metadata: dict[str, Any] = field(default_factory=dict) def format_restart_completed_message(started_at_raw: str) -> str: @@ -30,11 +34,20 @@ def format_restart_completed_message(started_at_raw: str) -> str: return f"Restart completed{elapsed_suffix}." -def set_restart_notice_to_env(*, channel: str, chat_id: str) -> None: +def set_restart_notice_to_env( + *, channel: str, chat_id: str, metadata: dict[str, Any] | None = None, +) -> None: """Write restart notice env values for the next process.""" os.environ[RESTART_NOTIFY_CHANNEL_ENV] = channel os.environ[RESTART_NOTIFY_CHAT_ID_ENV] = chat_id os.environ[RESTART_STARTED_AT_ENV] = str(time.time()) + if metadata: + try: + os.environ[RESTART_NOTIFY_METADATA_ENV] = json.dumps(metadata, default=str) + except (TypeError, ValueError): + os.environ.pop(RESTART_NOTIFY_METADATA_ENV, None) + else: + os.environ.pop(RESTART_NOTIFY_METADATA_ENV, None) def consume_restart_notice_from_env() -> RestartNotice | None: @@ -42,9 +55,23 @@ def consume_restart_notice_from_env() -> RestartNotice | None: channel = os.environ.pop(RESTART_NOTIFY_CHANNEL_ENV, "").strip() chat_id = os.environ.pop(RESTART_NOTIFY_CHAT_ID_ENV, "").strip() started_at_raw = os.environ.pop(RESTART_STARTED_AT_ENV, "").strip() + metadata_raw = os.environ.pop(RESTART_NOTIFY_METADATA_ENV, "").strip() if not (channel and chat_id): return None - return RestartNotice(channel=channel, chat_id=chat_id, started_at_raw=started_at_raw) + metadata: dict[str, Any] = {} + if metadata_raw: + try: + parsed = json.loads(metadata_raw) + except (TypeError, ValueError): + parsed = None + if isinstance(parsed, dict): + metadata = parsed + return RestartNotice( + channel=channel, + chat_id=chat_id, + started_at_raw=started_at_raw, + metadata=metadata, + ) def should_show_cli_restart_notice(notice: RestartNotice, session_id: str) -> bool: diff --git a/tests/utils/test_restart.py b/tests/utils/test_restart.py index 48124d38..84427215 100644 --- a/tests/utils/test_restart.py +++ b/tests/utils/test_restart.py @@ -16,6 +16,7 @@ from nanobot.utils.restart import ( def test_set_and_consume_restart_notice_env_roundtrip(monkeypatch): monkeypatch.delenv("NANOBOT_RESTART_NOTIFY_CHANNEL", raising=False) monkeypatch.delenv("NANOBOT_RESTART_NOTIFY_CHAT_ID", raising=False) + monkeypatch.delenv("NANOBOT_RESTART_NOTIFY_METADATA", raising=False) monkeypatch.delenv("NANOBOT_RESTART_STARTED_AT", raising=False) set_restart_notice_to_env(channel="feishu", chat_id="oc_123") @@ -25,14 +26,42 @@ def test_set_and_consume_restart_notice_env_roundtrip(monkeypatch): assert notice.channel == "feishu" assert notice.chat_id == "oc_123" assert notice.started_at_raw + assert notice.metadata == {} # Consumed values should be cleared from env. assert consume_restart_notice_from_env() is None assert "NANOBOT_RESTART_NOTIFY_CHANNEL" not in os.environ assert "NANOBOT_RESTART_NOTIFY_CHAT_ID" not in os.environ + assert "NANOBOT_RESTART_NOTIFY_METADATA" not in os.environ assert "NANOBOT_RESTART_STARTED_AT" not in os.environ +def test_restart_notice_preserves_metadata_across_env(monkeypatch): + monkeypatch.delenv("NANOBOT_RESTART_NOTIFY_CHANNEL", raising=False) + monkeypatch.delenv("NANOBOT_RESTART_NOTIFY_CHAT_ID", raising=False) + monkeypatch.delenv("NANOBOT_RESTART_NOTIFY_METADATA", raising=False) + monkeypatch.delenv("NANOBOT_RESTART_STARTED_AT", raising=False) + + set_restart_notice_to_env( + channel="slack", + chat_id="C123", + metadata={"slack": {"thread_ts": "1700.42", "channel_type": "channel"}}, + ) + + notice = consume_restart_notice_from_env() + assert notice is not None + assert notice.metadata == { + "slack": {"thread_ts": "1700.42", "channel_type": "channel"} + } + assert "NANOBOT_RESTART_NOTIFY_METADATA" not in os.environ + + +def test_restart_notice_clears_stale_metadata(monkeypatch): + monkeypatch.setenv("NANOBOT_RESTART_NOTIFY_METADATA", '{"stale": true}') + set_restart_notice_to_env(channel="cli", chat_id="direct") + assert "NANOBOT_RESTART_NOTIFY_METADATA" not in os.environ + + def test_format_restart_completed_message_with_elapsed(monkeypatch): monkeypatch.setattr("nanobot.utils.restart.time.time", lambda: 102.0) assert format_restart_completed_message("100.0") == "Restart completed in 2.0s." From 5e9b9b98182434f8454da198c086fa05c964c6ac Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 03:47:34 +0000 Subject: [PATCH 08/17] fix(slack): skip thread context for slash commands so /restart is not buried _with_thread_context prepends conversation history to the message content. This turned "/restart" into "Slack thread context...\n\n Current message:\n/restart", which the command router could not match as a priority command. Skip the context enrichment when the stripped text starts with "/". Made-with: Cursor --- nanobot/channels/slack.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nanobot/channels/slack.py b/nanobot/channels/slack.py index 8d0d8709..c79c65e6 100644 --- a/nanobot/channels/slack.py +++ b/nanobot/channels/slack.py @@ -348,7 +348,8 @@ class SlackChannel(BaseChannel): # Thread-scoped session key for channel/group messages session_key = f"slack:{chat_id}:{thread_ts}" if thread_ts and channel_type != "im" else None - content = await self._with_thread_context( + is_slash = text.strip().startswith("/") + content = text if is_slash else await self._with_thread_context( text, chat_id=chat_id, channel_type=channel_type, From 8a0917db7a4bb4b82a29917b32323ba3a72809af Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 04:30:32 +0000 Subject: [PATCH 09/17] fix(slack): polish thread UX and media support --- README.md | 1 + docs/chat-apps.md | 4 +- nanobot/channels/slack.py | 100 +++++++++++++++++++++--- tests/channels/test_slack_channel.py | 113 +++++++++++++++++++++++++-- 4 files changed, 199 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index c9d89e88..439a963d 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,7 @@ nanobot agent - Want different LLM providers, web search, MCP, security settings, or more config options? See [Configuration](./docs/configuration.md) - Want to run nanobot in chat apps like Telegram, Discord, WeChat or Feishu? See [Chat Apps](./docs/chat-apps.md) +- Using Slack? Add `files:write` if you want nanobot to upload images, videos, or files. - Want Docker or Linux service deployment? See [Deployment](./docs/deployment.md) ## 🧪 WebUI (Development) diff --git a/docs/chat-apps.md b/docs/chat-apps.md index 96ef654c..0b64e421 100644 --- a/docs/chat-apps.md +++ b/docs/chat-apps.md @@ -434,11 +434,13 @@ Uses **Socket Mode** — no public URL required. **2. Configure the app** - **Socket Mode**: Toggle ON → Generate an **App-Level Token** with `connections:write` scope → copy it (`xapp-...`) -- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read`, `channels:history`, `groups:history`, `im:history`, `mpim:history` +- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read`, `files:write`, `channels:history`, `groups:history`, `im:history`, `mpim:history` - **Event Subscriptions**: Toggle ON → Subscribe to bot events: `message.im`, `message.channels`, `app_mention` → Save Changes - **App Home**: Scroll to **Show Tabs** → Enable **Messages Tab** → Check **"Allow users to send Slash commands and messages from the messages tab"** - **Install App**: Click **Install to Workspace** → Authorize → copy the **Bot Token** (`xoxb-...`) +> `files:write` is required for images, videos, and other file uploads. If you add it later, reinstall the Slack app to the workspace and restart nanobot so it uses the updated bot token. + **3. Configure nanobot** ```json diff --git a/nanobot/channels/slack.py b/nanobot/channels/slack.py index c79c65e6..5b00ed7e 100644 --- a/nanobot/channels/slack.py +++ b/nanobot/channels/slack.py @@ -16,6 +16,7 @@ from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus from nanobot.channels.base import BaseChannel from nanobot.config.schema import Base +from nanobot.utils.helpers import split_message class SlackDMConfig(Base): @@ -46,6 +47,9 @@ class SlackConfig(Base): dm: SlackDMConfig = Field(default_factory=SlackDMConfig) +SLACK_MAX_MESSAGE_LEN = 39_000 # Slack API allows ~40k; leave margin + + class SlackChannel(BaseChannel): """Slack channel using Socket Mode.""" @@ -59,6 +63,8 @@ class SlackChannel(BaseChannel): def default_config(cls) -> dict[str, Any]: return SlackConfig().model_dump(by_alias=True) + _THREAD_CONTEXT_CACHE_LIMIT = 10_000 + def __init__(self, config: Any, bus: MessageBus): if isinstance(config, dict): config = SlackConfig.model_validate(config) @@ -131,14 +137,17 @@ class SlackChannel(BaseChannel): else None ) - # Slack rejects empty text payloads. Keep media-only messages media-only, - # but send a single blank message when the bot has no text or files to send. if msg.content or not (msg.media or []): - await self._web_client.chat_postMessage( - channel=target_chat_id, - text=self._to_mrkdwn(msg.content) if msg.content else " ", - thread_ts=thread_ts_param, - ) + mrkdwn = self._to_mrkdwn(msg.content) if msg.content else " " + buttons = getattr(msg, "buttons", None) or [] + chunks = split_message(mrkdwn, SLACK_MAX_MESSAGE_LEN) + for index, chunk in enumerate(chunks): + kwargs: dict[str, Any] = dict( + channel=target_chat_id, text=chunk, thread_ts=thread_ts_param, + ) + if buttons and index == len(chunks) - 1: + kwargs["blocks"] = self._build_button_blocks(chunk, buttons) + await self._web_client.chat_postMessage(**kwargs) for media_path in msg.media or []: try: @@ -276,6 +285,9 @@ class SlackChannel(BaseChannel): req: SocketModeRequest, ) -> None: """Handle incoming Socket Mode requests.""" + if req.type == "interactive": + await self._on_block_action(client, req) + return if req.type != "events_api": return @@ -375,6 +387,37 @@ class SlackChannel(BaseChannel): except Exception: logger.exception("Error handling Slack message from {}", sender_id) + async def _on_block_action(self, client: SocketModeClient, req: SocketModeRequest) -> None: + """Handle button clicks from ask_user blocks.""" + await client.send_socket_mode_response(SocketModeResponse(envelope_id=req.envelope_id)) + payload = req.payload or {} + actions = payload.get("actions") or [] + if not actions: + return + value = str(actions[0].get("value") or "") + user_info = payload.get("user") or {} + sender_id = str(user_info.get("id") or "") + channel_info = payload.get("channel") or {} + chat_id = str(channel_info.get("id") or "") + if not sender_id or not chat_id or not value: + return + message_info = payload.get("message") or {} + thread_ts = message_info.get("thread_ts") or message_info.get("ts") + channel_type = self._infer_channel_type(chat_id) + if not self._is_allowed(sender_id, chat_id, channel_type): + return + session_key = f"slack:{chat_id}:{thread_ts}" if thread_ts else None + try: + await self._handle_message( + sender_id=sender_id, + chat_id=chat_id, + content=value, + metadata={"slack": {"thread_ts": thread_ts, "channel_type": channel_type}}, + session_key=session_key, + ) + except Exception: + logger.exception("Error handling Slack button click from {}", sender_id) + async def _with_thread_context( self, text: str, @@ -399,6 +442,8 @@ class SlackChannel(BaseChannel): key = f"{chat_id}:{thread_ts}" if key in self._thread_context_attempted: return text + if len(self._thread_context_attempted) >= self._THREAD_CONTEXT_CACHE_LIMIT: + self._thread_context_attempted.clear() self._thread_context_attempted.add(key) try: @@ -427,14 +472,36 @@ class SlackChannel(BaseChannel): if item.get("subtype"): continue sender = str(item.get("user") or item.get("bot_id") or "unknown") - if self._bot_user_id and sender == self._bot_user_id: - continue + is_bot = self._bot_user_id is not None and sender == self._bot_user_id + label = "bot" if is_bot else f"<@{sender}>" text = str(item.get("text") or "").strip() if not text: continue - lines.append(f"- <@{sender}>: {self._strip_bot_mention(text)}") + text = self._strip_bot_mention(text) + if len(text) > 500: + text = text[:500] + "…" + lines.append(f"- {label}: {text}") return lines + @staticmethod + def _build_button_blocks(text: str, buttons: list[list[str]]) -> list[dict[str, Any]]: + """Build Slack Block Kit blocks with action buttons for ask_user choices.""" + blocks: list[dict[str, Any]] = [ + {"type": "section", "text": {"type": "mrkdwn", "text": text[:3000]}}, + ] + elements = [] + for row in buttons: + for label in row: + elements.append({ + "type": "button", + "text": {"type": "plain_text", "text": label[:75]}, + "value": label[:75], + "action_id": f"ask_user_{label[:50]}", + }) + if elements: + blocks.append({"type": "actions", "elements": elements[:25]}) + return blocks + async def _update_react_emoji(self, chat_id: str, ts: str | None) -> None: """Remove the in-progress reaction and optionally add a done reaction.""" if not self._web_client or not ts: @@ -481,6 +548,19 @@ class SlackChannel(BaseChannel): return chat_id in self.config.group_allow_from return False + def is_allowed(self, sender_id: str) -> bool: + # Slack needs channel-aware policy checks, so _on_socket_request and + # _on_block_action call _is_allowed before handing off to BaseChannel. + return True + + @staticmethod + def _infer_channel_type(chat_id: str) -> str: + if chat_id.startswith("D"): + return "im" + if chat_id.startswith("G"): + return "group" + return "channel" + def _strip_bot_mention(self, text: str) -> str: if not text or not self._bot_user_id: return text diff --git a/tests/channels/test_slack_channel.py b/tests/channels/test_slack_channel.py index 9b4d34c0..f3905237 100644 --- a/tests/channels/test_slack_channel.py +++ b/tests/channels/test_slack_channel.py @@ -1,5 +1,8 @@ from __future__ import annotations +from types import SimpleNamespace +from unittest.mock import AsyncMock + import pytest # Check optional Slack dependencies before running tests @@ -10,7 +13,7 @@ except ImportError: from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus -from nanobot.channels.slack import SlackChannel, SlackConfig +from nanobot.channels.slack import SLACK_MAX_MESSAGE_LEN, SlackChannel, SlackConfig class _FakeAsyncWebClient: @@ -34,14 +37,16 @@ class _FakeAsyncWebClient: channel: str, text: str, thread_ts: str | None = None, + blocks: list[dict[str, object]] | None = None, ) -> None: - self.chat_post_calls.append( - { - "channel": channel, - "text": text, - "thread_ts": thread_ts, - } - ) + call: dict[str, object | None] = { + "channel": channel, + "text": text, + "thread_ts": thread_ts, + } + if blocks is not None: + call["blocks"] = blocks + self.chat_post_calls.append(call) async def files_upload_v2( self, @@ -155,6 +160,61 @@ async def test_send_omits_thread_for_dm_messages() -> None: assert fake_web.file_upload_calls[0]["thread_ts"] is None +@pytest.mark.asyncio +async def test_send_splits_long_messages() -> None: + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + fake_web = _FakeAsyncWebClient() + channel._web_client = fake_web + + await channel.send( + OutboundMessage( + channel="slack", + chat_id="C123", + content="x" * (SLACK_MAX_MESSAGE_LEN + 10), + ) + ) + + assert len(fake_web.chat_post_calls) == 2 + assert all(len(str(call["text"])) <= SLACK_MAX_MESSAGE_LEN for call in fake_web.chat_post_calls) + + +@pytest.mark.asyncio +async def test_send_renders_buttons_on_last_message_chunk() -> None: + channel = SlackChannel(SlackConfig(enabled=True), MessageBus()) + fake_web = _FakeAsyncWebClient() + channel._web_client = fake_web + + await channel.send( + OutboundMessage( + channel="slack", + chat_id="C123", + content="Choose one", + buttons=[["Yes", "No"]], + ) + ) + + assert len(fake_web.chat_post_calls) == 1 + blocks = fake_web.chat_post_calls[0]["blocks"] + assert isinstance(blocks, list) + assert blocks[-1] == { + "type": "actions", + "elements": [ + { + "type": "button", + "text": {"type": "plain_text", "text": "Yes"}, + "value": "Yes", + "action_id": "ask_user_Yes", + }, + { + "type": "button", + "text": {"type": "plain_text", "text": "No"}, + "value": "No", + "action_id": "ask_user_No", + }, + ], + } + + @pytest.mark.asyncio async def test_send_updates_reaction_when_final_response_sent() -> None: channel = SlackChannel(SlackConfig(enabled=True, react_emoji="eyes"), MessageBus()) @@ -333,6 +393,7 @@ async def test_with_thread_context_fetches_root_once() -> None: "messages": [ {"ts": "111.000", "user": "UROOT", "text": "drink water"}, {"ts": "112.000", "user": "U2", "text": "good idea"}, + {"ts": "112.500", "user": "UBOT", "text": "I'll remind you."}, {"ts": "113.000", "user": "U3", "text": "<@UBOT> what did you see?"}, ] } @@ -353,6 +414,7 @@ async def test_with_thread_context_fetches_root_once() -> None: assert "Slack thread context before this mention:" in content assert "- <@UROOT>: drink water" in content assert "- <@U2>: good idea" in content + assert "- bot: I'll remind you." in content assert "U3" not in content assert content.endswith("Current message:\nwhat did you see?") @@ -366,3 +428,38 @@ async def test_with_thread_context_fetches_root_once() -> None: ) assert second == "again" assert len(fake_web.conversations_replies_calls) == 1 + + +@pytest.mark.asyncio +async def test_slack_slash_command_skips_thread_context() -> None: + channel = SlackChannel(SlackConfig(enabled=True, allow_from=[]), MessageBus()) + channel._bot_user_id = "UBOT" + channel._with_thread_context = AsyncMock(return_value="wrapped") # type: ignore[method-assign] + channel._handle_message = AsyncMock() # type: ignore[method-assign] + client = SimpleNamespace(send_socket_mode_response=AsyncMock()) + req = SimpleNamespace( + type="events_api", + envelope_id="env-1", + payload={ + "event": { + "type": "app_mention", + "user": "U1", + "channel": "C123", + "text": "<@UBOT> /restart", + "thread_ts": "111.000", + "ts": "112.000", + } + }, + ) + + await channel._on_socket_request(client, req) + + channel._with_thread_context.assert_not_awaited() + channel._handle_message.assert_awaited_once() + assert channel._handle_message.await_args.kwargs["content"] == "/restart" + + +def test_slack_channel_uses_channel_aware_allow_policy() -> None: + channel = SlackChannel(SlackConfig(enabled=True, allow_from=[]), MessageBus()) + assert channel.is_allowed("U1") is True + assert channel._is_allowed("U1", "C123", "channel") is True From d89a824769572af64e0a27afe3dbfd14a6bb559f Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 04:40:22 +0000 Subject: [PATCH 10/17] docs(readme): keep Slack upload scope in chat app docs Keep the root README focused on the main setup path and leave Slack-specific upload permissions in the chat apps guide. Made-with: Cursor --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 439a963d..c9d89e88 100644 --- a/README.md +++ b/README.md @@ -190,7 +190,6 @@ nanobot agent - Want different LLM providers, web search, MCP, security settings, or more config options? See [Configuration](./docs/configuration.md) - Want to run nanobot in chat apps like Telegram, Discord, WeChat or Feishu? See [Chat Apps](./docs/chat-apps.md) -- Using Slack? Add `files:write` if you want nanobot to upload images, videos, or files. - Want Docker or Linux service deployment? See [Deployment](./docs/deployment.md) ## 🧪 WebUI (Development) From eeaec1f9516c823b4a56d1db631b26c147253429 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 06:23:43 +0000 Subject: [PATCH 11/17] fix(agent): prevent message time metadata from leaking into replies --- nanobot/templates/agent/identity.md | 4 ++++ tests/agent/test_context_prompt_cache.py | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/nanobot/templates/agent/identity.md b/nanobot/templates/agent/identity.md index a53be709..0000b51c 100644 --- a/nanobot/templates/agent/identity.md +++ b/nanobot/templates/agent/identity.md @@ -28,5 +28,9 @@ Output is rendered in a terminal. Avoid markdown headings and tables. Use plain - On broad searches, use `grep(output_mode="count")` to scope before requesting full content. {% include 'agent/_snippets/untrusted_content.md' %} +Historical messages may include `[Message Time: ...]` prefixes. Treat them as +metadata for chronology only; never quote, copy, or include those markers in +your response. + Reply directly with text for conversations. Only use the 'message' tool to send to a specific chat channel. 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/agent/test_context_prompt_cache.py b/tests/agent/test_context_prompt_cache.py index ea1052ca..0a7cb551 100644 --- a/tests/agent/test_context_prompt_cache.py +++ b/tests/agent/test_context_prompt_cache.py @@ -188,6 +188,16 @@ def test_identity_has_no_behavioral_instructions(tmp_path) -> None: assert "Execution Rules" not in identity +def test_system_prompt_treats_message_time_as_metadata(tmp_path) -> None: + workspace = _make_workspace(tmp_path) + builder = ContextBuilder(workspace) + + prompt = builder.build_system_prompt() + + assert "Historical messages may include `[Message Time: ...]` prefixes" in prompt + assert "never quote, copy, or include those markers" in prompt + + def test_default_soul_template_contains_execution_rules() -> None: """Default SOUL.md template must contain execution rules with act/plan layering.""" soul = (pkg_files("nanobot") / "templates" / "SOUL.md").read_text(encoding="utf-8") From 9b3e2524ac9a5a1812901789271821fded51ef18 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 27 Apr 2026 10:58:48 +0800 Subject: [PATCH 12/17] fix(agent): resolve relative media paths in MessageTool When deployed with Docker and workspace mounted as a volume, sending media files failed because relative paths (e.g. output/image.png) were not resolved against the workspace directory. The process CWD differs from the workspace in containerized environments, causing os.path.isfile checks to fail in channel handlers. Normalize relative media paths at the MessageTool entry point using get_workspace_path(). --- nanobot/agent/tools/message.py | 11 ++++ tests/tools/test_message_tool.py | 97 ++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/nanobot/agent/tools/message.py b/nanobot/agent/tools/message.py index fe097e11..f35f8fe2 100644 --- a/nanobot/agent/tools/message.py +++ b/nanobot/agent/tools/message.py @@ -1,11 +1,13 @@ """Message tool for sending messages to users.""" +import os from contextvars import ContextVar from typing import Any, Awaitable, Callable from nanobot.agent.tools.base import Tool, tool_parameters from nanobot.agent.tools.schema import ArraySchema, StringSchema, tool_parameters_schema from nanobot.bus.events import OutboundMessage +from nanobot.config.paths import get_workspace_path @tool_parameters( @@ -141,6 +143,15 @@ class MessageTool(Tool): if not self._send_callback: return "Error: Message sending not configured" + if media: + resolved = [] + for p in media: + if p.startswith(("http://", "https://")) or os.path.isabs(p): + resolved.append(p) + else: + resolved.append(str(get_workspace_path() / p)) + media = resolved + metadata = dict(self._default_metadata.get()) if same_target else {} if message_id: metadata["message_id"] = message_id diff --git a/tests/tools/test_message_tool.py b/tests/tools/test_message_tool.py index feff43da..d93219f0 100644 --- a/tests/tools/test_message_tool.py +++ b/tests/tools/test_message_tool.py @@ -1,7 +1,10 @@ +import os + import pytest from nanobot.agent.tools.message import MessageTool from nanobot.bus.events import OutboundMessage +from nanobot.config.paths import get_workspace_path @pytest.mark.asyncio @@ -85,3 +88,97 @@ async def test_message_tool_does_not_inherit_metadata_for_cross_target() -> None await tool.execute(content="channel reply", channel="slack", chat_id="C999") assert sent[0].metadata == {} + + +@pytest.mark.asyncio +async def test_message_tool_resolves_relative_media_paths() -> None: + sent: list[OutboundMessage] = [] + + async def _send(msg: OutboundMessage) -> None: + sent.append(msg) + + tool = MessageTool(send_callback=_send) + + await tool.execute( + content="see attached", + channel="telegram", + chat_id="1", + media=["output/image.png"], + ) + + expected = str(get_workspace_path() / "output/image.png") + assert sent[0].media == [expected] + + +@pytest.mark.asyncio +async def test_message_tool_passes_through_absolute_media_paths() -> None: + sent: list[OutboundMessage] = [] + + async def _send(msg: OutboundMessage) -> None: + sent.append(msg) + + tool = MessageTool(send_callback=_send) + + abs_path = os.path.abspath(os.path.join(os.sep, "tmp", "abs_image.png")) + + await tool.execute( + content="see attached", + channel="telegram", + chat_id="1", + media=[abs_path], + ) + + assert sent[0].media == [abs_path] + + +@pytest.mark.asyncio +async def test_message_tool_passes_through_url_media_paths() -> None: + sent: list[OutboundMessage] = [] + + async def _send(msg: OutboundMessage) -> None: + sent.append(msg) + + tool = MessageTool(send_callback=_send) + + url = "https://example.com/image.png" + + await tool.execute( + content="see attached", + channel="telegram", + chat_id="1", + media=[url], + ) + + assert sent[0].media == [url] + + +@pytest.mark.asyncio +async def test_message_tool_resolves_mixed_media_paths() -> None: + sent: list[OutboundMessage] = [] + + async def _send(msg: OutboundMessage) -> None: + sent.append(msg) + + tool = MessageTool(send_callback=_send) + + abs_path = os.path.abspath(os.path.join(os.sep, "tmp", "absolute.png")) + + await tool.execute( + content="see attached", + channel="telegram", + chat_id="1", + media=[ + "output/relative.png", + abs_path, + "https://example.com/url.png", + "http://example.com/http.png", + ], + ) + + expected_relative = str(get_workspace_path() / "output/relative.png") + assert sent[0].media == [ + expected_relative, + abs_path, + "https://example.com/url.png", + "http://example.com/http.png", + ] From 9b6f3d7abca53be514896fbf74cab10c650e6407 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 06:29:04 +0000 Subject: [PATCH 13/17] fix(agent): resolve message media against active workspace Made-with: Cursor --- nanobot/agent/loop.py | 2 +- nanobot/agent/tools/message.py | 5 ++++- tests/tools/test_message_tool.py | 20 ++++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index dce1890a..f6bd5e2e 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -368,7 +368,7 @@ class AgentLoop: WebSearchTool(config=self.web_config.search, proxy=self.web_config.proxy) ) self.tools.register(WebFetchTool(proxy=self.web_config.proxy)) - self.tools.register(MessageTool(send_callback=self.bus.publish_outbound)) + self.tools.register(MessageTool(send_callback=self.bus.publish_outbound, workspace=self.workspace)) self.tools.register(SpawnTool(manager=self.subagents)) if self.cron_service: self.tools.register( diff --git a/nanobot/agent/tools/message.py b/nanobot/agent/tools/message.py index f35f8fe2..6e3d037f 100644 --- a/nanobot/agent/tools/message.py +++ b/nanobot/agent/tools/message.py @@ -2,6 +2,7 @@ import os from contextvars import ContextVar +from pathlib import Path from typing import Any, Awaitable, Callable from nanobot.agent.tools.base import Tool, tool_parameters @@ -35,8 +36,10 @@ class MessageTool(Tool): default_channel: str = "", default_chat_id: str = "", default_message_id: str | None = None, + workspace: str | Path | None = None, ): self._send_callback = send_callback + self._workspace = Path(workspace).expanduser() if workspace is not None else get_workspace_path() self._default_channel: ContextVar[str] = ContextVar("message_default_channel", default=default_channel) self._default_chat_id: ContextVar[str] = ContextVar("message_default_chat_id", default=default_chat_id) self._default_message_id: ContextVar[str | None] = ContextVar( @@ -149,7 +152,7 @@ class MessageTool(Tool): if p.startswith(("http://", "https://")) or os.path.isabs(p): resolved.append(p) else: - resolved.append(str(get_workspace_path() / p)) + resolved.append(str(self._workspace / p)) media = resolved metadata = dict(self._default_metadata.get()) if same_target else {} diff --git a/tests/tools/test_message_tool.py b/tests/tools/test_message_tool.py index d93219f0..915fb0c9 100644 --- a/tests/tools/test_message_tool.py +++ b/tests/tools/test_message_tool.py @@ -110,6 +110,26 @@ async def test_message_tool_resolves_relative_media_paths() -> None: assert sent[0].media == [expected] +@pytest.mark.asyncio +async def test_message_tool_resolves_relative_media_paths_from_active_workspace(tmp_path) -> None: + sent: list[OutboundMessage] = [] + + async def _send(msg: OutboundMessage) -> None: + sent.append(msg) + + workspace = tmp_path / "workspace" + tool = MessageTool(send_callback=_send, workspace=workspace) + + await tool.execute( + content="see attached", + channel="telegram", + chat_id="1", + media=["output/image.png"], + ) + + assert sent[0].media == [str(workspace / "output/image.png")] + + @pytest.mark.asyncio async def test_message_tool_passes_through_absolute_media_paths() -> None: sent: list[OutboundMessage] = [] From 380309016a5547e85f4fea9c6cdc323ecdd73f35 Mon Sep 17 00:00:00 2001 From: mt-huerta <5499466+mt-huerta@users.noreply.github.com> Date: Sun, 26 Apr 2026 15:55:23 -0400 Subject: [PATCH 14/17] fix(agent): complete thread-session routing for spawn dispatch and system-channel branch Builds on PR #3463 (commit 038a140), which introduced metadata and session_key parameters through _LoopHook and _set_tool_context for the cron and message tools. Three downstream gaps remained: 1. _set_tool_context's body still computes effective_key from channel:chat_id and passes that to spawn, even when the caller provides a thread-scoped session_key. The new parameter is wired in for cron/message but spawn dispatch ignores it. Result: subagent announces from threaded callers carry a channel-only session_key_override, dropping thread_ts. 2. _process_message's system-channel branch loads the session via key = f"{channel}:{chat_id}", ignoring msg.session_key_override. So even when the announce InboundMessage carries the right override (after fix 1), the consumer side discards it and routes to the channel-level session. 3. The OutboundMessage returned from the system-channel branch has no metadata, so slack's outbound dispatcher has no thread_ts to use and posts the LLM's reply to the channel top-level rather than the originating thread. This change closes all three gaps with three small edits in loop.py. Behavior change: - Slack channels with reply_in_thread: true: subagent announces and follow-up replies now arrive in the originating thread session instead of leaking into the channel-level session. - Other channels constructing thread-scoped session keys (matrix threads, telegram thread mode, etc.): the session-loading and effective-key fixes apply identically since they're platform-agnostic. The outbound thread_ts reconstruction is slack-specific by virtue of the session-key format slack uses; other channels would benefit from the same pattern but are out of scope for this PR. - Unified session mode: no change. Falls back to UNIFIED_SESSION_KEY when session_key is not provided. - CLI / non-channel callers: no change. They don't pass session_key and the fallback to f"{channel}:{chat_id}" matches prior behavior. Reproducer (slack with reply_in_thread: true): 1. From a slack thread, send a message that triggers a subagent spawn. 2. Before fix: announce lands in slack:.jsonl session, parent agent in the thread never sees the completion event, eventual reply (if any) posts to the channel top-level, not the thread. 3. After fix: announce lands in slack::.jsonl, parent agent in the thread responds within seconds, reply posts in the thread. --- nanobot/agent/loop.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index f6bd5e2e..f2ec1e25 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -403,7 +403,16 @@ class AgentLoop: session_key: str | None = None, ) -> None: """Update context for all tools that need routing info.""" - effective_key = UNIFIED_SESSION_KEY if self._unified_session else f"{channel}:{chat_id}" + # When the caller threads a thread-scoped session_key (e.g. slack with + # reply_in_thread: true), honor it so spawn announces route back to + # the originating thread session. Falls back to unified mode or + # channel:chat_id for callers that don't have a thread-scoped key. + if session_key is not None: + effective_key = session_key + elif self._unified_session: + effective_key = UNIFIED_SESSION_KEY + else: + effective_key = f"{channel}:{chat_id}" for name in ("message", "spawn", "cron", "my"): if tool := self.tools.get(name): if hasattr(tool, "set_context"): @@ -830,7 +839,10 @@ class AgentLoop: msg.chat_id.split(":", 1) if ":" in msg.chat_id else ("cli", msg.chat_id) ) logger.info("Processing system message from {}", msg.sender_id) - key = f"{channel}:{chat_id}" + # Honor session_key_override so subagent announces from threaded + # callers route to the originating thread session, not the + # channel-level session derived from chat_id. + key = msg.session_key_override or f"{channel}:{chat_id}" session = self.sessions.get_or_create(key) if self._restore_runtime_checkpoint(session): self.sessions.save(session) @@ -885,11 +897,20 @@ class AgentLoop: options, channel, ) + # Reconstruct channel-specific metadata from session.key so the + # outbound reply lands in the originating thread (not the channel + # top-level). The announce InboundMessage carries only + # injected_event metadata; we recover thread_ts from the session + # key, which slack writes as "slack::". + outbound_metadata: dict[str, Any] = {} + if channel == "slack" and key.startswith("slack:") and key.count(":") >= 2: + outbound_metadata["slack"] = {"thread_ts": key.split(":", 2)[2]} return OutboundMessage( channel=channel, chat_id=chat_id, content=content, buttons=buttons, + metadata=outbound_metadata, ) # Extract document text from media at the processing boundary so all From 7dcf83e389c8139c5f147be78706b314885a66ab Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 06:35:40 +0000 Subject: [PATCH 15/17] test(agent): cover threaded subagent routing Made-with: Cursor --- tests/agent/test_loop_save_turn.py | 60 ++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/agent/test_loop_save_turn.py b/tests/agent/test_loop_save_turn.py index a79a6b01..13f6b60c 100644 --- a/tests/agent/test_loop_save_turn.py +++ b/tests/agent/test_loop_save_turn.py @@ -660,3 +660,63 @@ def test_subagent_followup_skips_empty_content() -> None: assert loop._persist_subagent_followup(session, msg) is False assert session.messages == [] + + +def test_set_tool_context_passes_thread_session_key_to_spawn(tmp_path: Path) -> None: + loop = _make_full_loop(tmp_path) + + loop._set_tool_context( + "slack", + "C123", + metadata={"slack": {"thread_ts": "1700.42", "channel_type": "channel"}}, + session_key="slack:C123:1700.42", + ) + + spawn_tool = loop.tools.get("spawn") + assert spawn_tool is not None + assert spawn_tool._session_key.get() == "slack:C123:1700.42" + + +@pytest.mark.asyncio +async def test_system_subagent_followup_uses_thread_session_and_slack_metadata(tmp_path: Path) -> None: + loop = _make_full_loop(tmp_path) + loop.consolidator.maybe_consolidate_by_tokens = AsyncMock(return_value=False) # type: ignore[method-assign] + + thread_session = loop.sessions.get_or_create("slack:C123:1700.42") + thread_session.add_message("user", "thread question") + loop.sessions.save(thread_session) + + seen: dict[str, list[dict]] = {} + + async def fake_run_agent_loop(initial_messages, **_kwargs): + seen["initial_messages"] = initial_messages + return ( + "done", + [], + [*initial_messages, {"role": "assistant", "content": "done"}], + "stop", + False, + ) + + loop._run_agent_loop = fake_run_agent_loop # type: ignore[method-assign] + + outbound = await loop._process_message( + InboundMessage( + channel="system", + sender_id="subagent", + chat_id="slack:C123", + content="subagent result", + session_key_override="slack:C123:1700.42", + metadata={"subagent_task_id": "sub-1"}, + ) + ) + + assert outbound is not None + assert outbound.channel == "slack" + assert outbound.chat_id == "C123" + assert outbound.metadata == {"slack": {"thread_ts": "1700.42"}} + assert "thread question" in seen["initial_messages"][1]["content"] + + loop.sessions.invalidate("slack:C123:1700.42") + persisted = loop.sessions.get_or_create("slack:C123:1700.42") + assert any(m.get("subagent_task_id") == "sub-1" for m in persisted.messages) From 620d9e4f31db740524c656ccb52f7af72fc03c85 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 07:11:11 +0000 Subject: [PATCH 16/17] fix(slack): accept inbound file_share messages without dropping them Slack inbound events with subtype=file_share were silently dropped, so nanobot never saw messages that included attachments. Allow file_share through, download Slack-private files using the bot token into the local media dir, and pass them to the agent as media paths plus a "[file: name]" / "[image: name]" placeholder in the content. Reject responses that look like Slack's login HTML so an auth page is never saved as if it were the user's file. Document the required files:read scope alongside files:write so installs that read attachments are not quietly missing the permission. --- docs/chat-apps.md | 4 +- nanobot/channels/slack.py | 73 ++++++++++++++++++++++++++-- tests/channels/test_slack_channel.py | 62 ++++++++++++++++++++++- 3 files changed, 132 insertions(+), 7 deletions(-) diff --git a/docs/chat-apps.md b/docs/chat-apps.md index 0b64e421..75e6f26f 100644 --- a/docs/chat-apps.md +++ b/docs/chat-apps.md @@ -434,12 +434,12 @@ Uses **Socket Mode** — no public URL required. **2. Configure the app** - **Socket Mode**: Toggle ON → Generate an **App-Level Token** with `connections:write` scope → copy it (`xapp-...`) -- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read`, `files:write`, `channels:history`, `groups:history`, `im:history`, `mpim:history` +- **OAuth & Permissions**: Add bot scopes: `chat:write`, `reactions:write`, `app_mentions:read`, `files:read`, `files:write`, `channels:history`, `groups:history`, `im:history`, `mpim:history` - **Event Subscriptions**: Toggle ON → Subscribe to bot events: `message.im`, `message.channels`, `app_mention` → Save Changes - **App Home**: Scroll to **Show Tabs** → Enable **Messages Tab** → Check **"Allow users to send Slash commands and messages from the messages tab"** - **Install App**: Click **Install to Workspace** → Authorize → copy the **Bot Token** (`xoxb-...`) -> `files:write` is required for images, videos, and other file uploads. If you add it later, reinstall the Slack app to the workspace and restart nanobot so it uses the updated bot token. +> `files:read` is required to read files users send to nanobot. `files:write` is required for nanobot to send images, videos, and other file uploads. If you add either scope later, reinstall the Slack app to the workspace and restart nanobot so it uses the updated bot token. **3. Configure nanobot** diff --git a/nanobot/channels/slack.py b/nanobot/channels/slack.py index 5b00ed7e..4c9c25ba 100644 --- a/nanobot/channels/slack.py +++ b/nanobot/channels/slack.py @@ -2,8 +2,10 @@ import asyncio import re +from pathlib import Path from typing import Any +import httpx from loguru import logger from pydantic import Field from slack_sdk.socket_mode.request import SocketModeRequest @@ -15,8 +17,9 @@ from slackify_markdown import slackify_markdown from nanobot.bus.events import OutboundMessage 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 split_message +from nanobot.utils.helpers import safe_filename, split_message class SlackDMConfig(Base): @@ -48,6 +51,8 @@ class SlackConfig(Base): SLACK_MAX_MESSAGE_LEN = 39_000 # Slack API allows ~40k; leave margin +SLACK_DOWNLOAD_TIMEOUT = 30.0 +_HTML_DOWNLOAD_PREFIXES = (b" tuple[str | None, str]: + """Download a Slack private file to the local media directory.""" + file_id = str(file_info.get("id") or "file") + name = str( + file_info.get("name") + or file_info.get("title") + or file_info.get("id") + or "slack-file" + ) + marker_type = "image" if str(file_info.get("mimetype") or "").startswith("image/") else "file" + marker = f"[{marker_type}: {name}]" + url = str(file_info.get("url_private_download") or file_info.get("url_private") or "") + if not url: + return None, f"[{marker_type}: {name}: missing download url]" + if not self.config.bot_token: + return None, f"[{marker_type}: {name}: missing bot token]" + + filename = safe_filename(f"{file_id}_{name}") + path = Path(get_media_dir("slack")) / filename + try: + async with httpx.AsyncClient(timeout=SLACK_DOWNLOAD_TIMEOUT, follow_redirects=True) as client: + response = await client.get( + url, + headers={"Authorization": f"Bearer {self.config.bot_token}"}, + ) + response.raise_for_status() + if self._looks_like_html_download(response): + raise ValueError("Slack returned HTML instead of file content") + path.write_bytes(response.content) + return str(path), marker + except Exception as e: + logger.warning("Failed to download Slack file {}: {}", file_id, e) + return None, f"[{marker_type}: {name}: download failed]" + + @staticmethod + def _looks_like_html_download(response: httpx.Response) -> bool: + content_type = response.headers.get("content-type", "").lower() + if "text/html" in content_type: + return True + preview = response.content[:256].lstrip().lower() + return preview.startswith(_HTML_DOWNLOAD_PREFIXES) + async def _on_block_action(self, client: SocketModeClient, req: SocketModeRequest) -> None: """Handle button clicks from ask_user blocks.""" await client.send_socket_mode_response(SocketModeResponse(envelope_id=req.envelope_id)) diff --git a/tests/channels/test_slack_channel.py b/tests/channels/test_slack_channel.py index f3905237..df7a7b56 100644 --- a/tests/channels/test_slack_channel.py +++ b/tests/channels/test_slack_channel.py @@ -3,6 +3,7 @@ from __future__ import annotations from types import SimpleNamespace from unittest.mock import AsyncMock +import httpx import pytest # Check optional Slack dependencies before running tests @@ -31,7 +32,7 @@ class _FakeAsyncWebClient: self._users_pages: list[dict[str, object]] = [] self._open_dm_response: dict[str, object] = {"channel": {"id": "D_OPENED"}} - async def chat_postMessage( + async def chat_postMessage( # noqa: N802 - mirrors Slack SDK method name self, *, channel: str, @@ -459,6 +460,65 @@ async def test_slack_slash_command_skips_thread_context() -> None: assert channel._handle_message.await_args.kwargs["content"] == "/restart" +@pytest.mark.asyncio +async def test_slack_file_share_downloads_media_and_reaches_agent() -> None: + channel = SlackChannel(SlackConfig(enabled=True, bot_token="xoxb-test"), MessageBus()) + channel._bot_user_id = "UBOT" + channel._web_client = _FakeAsyncWebClient() + channel._handle_message = AsyncMock() # type: ignore[method-assign] + channel._download_slack_file = AsyncMock( # type: ignore[method-assign] + return_value=("/tmp/report.pdf", "[file: report.pdf]") + ) + client = SimpleNamespace(send_socket_mode_response=AsyncMock()) + req = SimpleNamespace( + type="events_api", + envelope_id="env-file", + payload={ + "event": { + "type": "message", + "subtype": "file_share", + "user": "U1", + "channel": "D123", + "channel_type": "im", + "text": "please read this", + "ts": "1700000000.000100", + "files": [ + { + "id": "F123", + "name": "report.pdf", + "mimetype": "application/pdf", + "url_private_download": "https://files.slack.com/report.pdf", + } + ], + } + }, + ) + + await channel._on_socket_request(client, req) + + channel._download_slack_file.assert_awaited_once() + channel._handle_message.assert_awaited_once() + kwargs = channel._handle_message.await_args.kwargs + assert kwargs["content"] == "please read this\n[file: report.pdf]" + assert kwargs["media"] == ["/tmp/report.pdf"] + + +def test_slack_download_rejects_login_html() -> None: + html_response = httpx.Response( + 200, + headers={"content-type": "text/html; charset=utf-8"}, + content=b"Sign in to Slack", + ) + markdown_response = httpx.Response( + 200, + headers={"content-type": "text/markdown"}, + content=b"# PR Extraction Guide\n", + ) + + assert SlackChannel._looks_like_html_download(html_response) is True + assert SlackChannel._looks_like_html_download(markdown_response) is False + + def test_slack_channel_uses_channel_aware_allow_policy() -> None: channel = SlackChannel(SlackConfig(enabled=True, allow_from=[]), MessageBus()) assert channel.is_allowed("U1") is True From 311a7fe36ef161ffbe96afafe8f6b0160496b7d3 Mon Sep 17 00:00:00 2001 From: Xubin Ren Date: Mon, 27 Apr 2026 07:11:20 +0000 Subject: [PATCH 17/17] fix(session): stop training the model to parrot [Message Time: ...] Past assistant turns in history were prefixed with "[Message Time: ...]" just like user turns. The model treated these as in-context demos and started prefixing its own replies with the same marker, leaking metadata to the user. Prompt-level warnings could not beat dozens of prior assistant samples. Annotate only user turns and proactive deliveries (_channel_delivery=True, i.e. cron / heartbeat pushes whose timing is the whole point and which are too infrequent to act as demos). Adjacent user-side timestamps still pin every normal assistant reply for relative-time reasoning. The now-redundant identity.md warning is removed along with the demonstration source. --- nanobot/session/manager.py | 26 +++++++++--- nanobot/templates/agent/identity.md | 4 -- tests/agent/test_context_prompt_cache.py | 7 ++-- tests/agent/test_loop_save_turn.py | 6 ++- tests/agent/test_session_manager_history.py | 45 ++++++++++++++++++++- 5 files changed, 72 insertions(+), 16 deletions(-) diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index a9499046..d0479894 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -32,13 +32,27 @@ class Session: @staticmethod def _annotate_message_time(message: dict[str, Any], content: Any) -> Any: - """Expose persisted turn timestamps to the model for relative-date reasoning.""" + """Expose persisted turn timestamps to the model for relative-date reasoning. + + Annotating *every* assistant turn trains the model (via in-context + demonstrations) to start its own replies with the same + ``[Message Time: ...]`` prefix, which leaks metadata back to the user. + We therefore only annotate: + + * ``user`` turns — needed so the model can pin the conversation in time. + * proactive deliveries (``_channel_delivery=True``) — cron / heartbeat + assistant pushes that may sit hours away from the next user reply, + and are too infrequent to act as parroting demonstrations. + """ timestamp = message.get("timestamp") - if ( - not timestamp - or message.get("role") not in {"user", "assistant"} - or not isinstance(content, str) - ): + if not timestamp or not isinstance(content, str): + return content + role = message.get("role") + if role == "user": + pass + elif role == "assistant" and message.get("_channel_delivery"): + pass + else: return content return f"[Message Time: {timestamp}]\n{content}" diff --git a/nanobot/templates/agent/identity.md b/nanobot/templates/agent/identity.md index 0000b51c..a53be709 100644 --- a/nanobot/templates/agent/identity.md +++ b/nanobot/templates/agent/identity.md @@ -28,9 +28,5 @@ Output is rendered in a terminal. Avoid markdown headings and tables. Use plain - On broad searches, use `grep(output_mode="count")` to scope before requesting full content. {% include 'agent/_snippets/untrusted_content.md' %} -Historical messages may include `[Message Time: ...]` prefixes. Treat them as -metadata for chronology only; never quote, copy, or include those markers in -your response. - Reply directly with text for conversations. Only use the 'message' tool to send to a specific chat channel. 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/agent/test_context_prompt_cache.py b/tests/agent/test_context_prompt_cache.py index 0a7cb551..ec1ab543 100644 --- a/tests/agent/test_context_prompt_cache.py +++ b/tests/agent/test_context_prompt_cache.py @@ -188,14 +188,15 @@ def test_identity_has_no_behavioral_instructions(tmp_path) -> None: assert "Execution Rules" not in identity -def test_system_prompt_treats_message_time_as_metadata(tmp_path) -> None: +def test_system_prompt_does_not_warn_about_message_time_markers(tmp_path) -> None: + """Parroting is prevented by not annotating assistant turns in history; + no prompt-level warning about ``[Message Time: ...]`` is needed.""" workspace = _make_workspace(tmp_path) builder = ContextBuilder(workspace) prompt = builder.build_system_prompt() - assert "Historical messages may include `[Message Time: ...]` prefixes" in prompt - assert "never quote, copy, or include those markers" in prompt + assert "Message Time" not in prompt def test_default_soul_template_contains_execution_rules() -> None: diff --git a/tests/agent/test_loop_save_turn.py b/tests/agent/test_loop_save_turn.py index 13f6b60c..883bf704 100644 --- a/tests/agent/test_loop_save_turn.py +++ b/tests/agent/test_loop_save_turn.py @@ -537,8 +537,12 @@ async def test_system_subagent_followup_is_persisted_before_prompt_assembly(tmp_ non_system = [m for m in seen["initial_messages"] if m.get("role") != "system"] assert "question" in non_system[0]["content"] assert "working" in non_system[1]["content"] + # User turns carry the timestamp prefix so the model can reason about + # relative time. Assistant turns do NOT, otherwise the model treats those + # past replies as in-context examples and starts its own outputs with + # ``[Message Time: ...]`` (which then leaks back to the user). assert "[Message Time:" in non_system[0]["content"] - assert "[Message Time:" in non_system[1]["content"] + assert "[Message Time:" not in non_system[1]["content"] assert non_system[2]["content"].count("subagent result") == 1 assert "Current Time:" in non_system[2]["content"] diff --git a/tests/agent/test_session_manager_history.py b/tests/agent/test_session_manager_history.py index 3c2b68e3..d4461923 100644 --- a/tests/agent/test_session_manager_history.py +++ b/tests/agent/test_session_manager_history.py @@ -194,7 +194,13 @@ def test_get_history_preserves_reasoning_content(): ] -def test_get_history_exposes_turn_timestamps_to_model(): +def test_get_history_annotates_user_turns_but_not_assistant_turns(): + """Only user turns carry the timestamp prefix. + + Annotating assistant turns trains the model (via in-context examples) to + start its own replies with ``[Message Time: ...]``. User-side stamps are + enough to pin adjacent assistant replies for relative-time reasoning. + """ session = Session(key="test:timestamps") session.messages.append({ "role": "user", @@ -216,7 +222,42 @@ def test_get_history_exposes_turn_timestamps_to_model(): }, { "role": "assistant", - "content": "[Message Time: 2026-04-26T22:00:05]\n记下来了", + "content": "记下来了", + }, + ] + + +def test_get_history_annotates_proactive_assistant_deliveries_with_timestamps(): + """Cron / heartbeat assistant pushes still carry a timestamp prefix. + + These proactive deliveries can sit hours away from the next user reply, + so the model needs to know when they fired. They are rare enough that + they don't act as in-context demonstrations encouraging the model to + prefix its own normal replies with ``[Message Time: ...]``. + """ + session = Session(key="test:proactive-timestamps") + session.messages.append({ + "role": "assistant", + "content": "记得喝水", + "timestamp": "2026-04-26T15:00:00", + "_channel_delivery": True, + }) + session.messages.append({ + "role": "user", + "content": "好", + "timestamp": "2026-04-26T18:00:00", + }) + + history = session.get_history(max_messages=500, include_timestamps=True) + + assert history == [ + { + "role": "assistant", + "content": "[Message Time: 2026-04-26T15:00:00]\n记得喝水", + }, + { + "role": "user", + "content": "[Message Time: 2026-04-26T18:00:00]\n好", }, ]