diff --git a/stacklets/agent/config.json b/stacklets/agent/config.json index 1de17f02..cb2fa152 100644 --- a/stacklets/agent/config.json +++ b/stacklets/agent/config.json @@ -28,7 +28,7 @@ } }, "tools": { - "exec": {"enable": true}, + "exec": {"enable": true, "deny_patterns": ["\\blist\\s+delete\\b.*\\s--y"]}, "web": {"enable": false}, "file": {"enable": true}, "cli_apps": {"enable": false}, diff --git a/stacklets/agent/runtime/brief.py b/stacklets/agent/runtime/brief.py index 078cc5bd..896bb52e 100644 --- a/stacklets/agent/runtime/brief.py +++ b/stacklets/agent/runtime/brief.py @@ -26,6 +26,8 @@ from datetime import datetime from pathlib import Path +from turn import person_of + # Strip footnote/citation markers the curator leaves in generated pages, e.g. # "Safety Inspector [8]" -> "Safety Inspector". They are noise in a briefing. _CITE = re.compile(r"\s*\[\d+(?:,\s*\d+)*\]") @@ -165,12 +167,6 @@ def when_sent(now: datetime) -> str: return f"Sent {now:%A, %Y-%m-%d %H:%M} ({now.tzname()})." -# Who the agent is currently talking to, refreshed each turn by -# `brief_lines`. A tool call has no access to the Matrix message, so this -# is how a vault write learns whose name goes on the commit. -speaking_with = "" - - def brief_lines(msg, workspace) -> list[str]: """Return the briefing as a list of short runtime-context lines (may be empty).""" vault = Path(workspace) / "vault" @@ -181,14 +177,8 @@ def brief_lines(msg, workspace) -> list[str]: # WHO — the speaker. sender_id is an mxid like "@homer:simpson"; the localpart # is the vault person slug. This is the reliable, high-value part: it fixes # multi-user rooms where "me/my" must resolve to whoever actually spoke. - localpart = str(getattr(msg, "sender_id", "")).split(":")[0].lstrip("@") + localpart = person_of(getattr(msg, "sender_id", "")) if localpart: - # Remember who is speaking so a vault write made later in this turn is - # committed as them. The briefing is rebuilt every turn and always - # before any tool runs, so this is current by construction; a tool has - # no other route to the sender. See vault_write.py. - global speaking_with - speaking_with = localpart essence = _lead(vault / localpart / "about.md") head = f"You are speaking with {localpart} (@{localpart})." lines.append(f"{head} {essence}" if essence else head) diff --git a/stacklets/agent/runtime/list_tool.py b/stacklets/agent/runtime/list_tool.py index 84a1507a..920280c0 100644 --- a/stacklets/agent/runtime/list_tool.py +++ b/stacklets/agent/runtime/list_tool.py @@ -21,35 +21,40 @@ StringSchema, tool_parameters_schema, ) +from turn import Turn, current as current_turn class AwaitingYes: - """Lists whose delete the store refused because of open items, by turn. + """Lists whose delete the store refused because of open items, and who was told. The person has to say yes, the model cannot: on the rig (2026-10-12) the store refused a delete and the model sent `confirmed` itself in - the same turn. So a yes counts only in a later turn than the refusal, - which is a message from the person after they were told what is open. + the same turn. So a yes counts only from the person who asked, in the + same conversation, in a later turn than the refusal: their message + after they were told what is open. awaiting = AwaitingYes() - awaiting.refused("partyliste", turn="t1") - awaiting.allows("partyliste", turn="t1") # False: same turn - awaiting.allows("partyliste", turn="t2") # True + awaiting.refused("partyliste", Turn("homer", "!kitchen", "t1")) + awaiting.allows("partyliste", Turn("homer", "!kitchen", "t1")) # False: same turn + awaiting.allows("partyliste", Turn("bart", "!kitchen", "t2")) # False: not asked + awaiting.allows("partyliste", Turn("homer", "!kitchen", "t2")) # True """ def __init__(self) -> None: - self._refused_in: dict[str, str] = {} + self._refused_in: dict[tuple[str, str, str], str] = {} - def refused(self, slug: str, *, turn: str) -> None: - if turn: - self._refused_in[slug] = turn + def refused(self, slug: str, turn: Turn) -> None: + if turn.id: + self._refused_in[(turn.conversation, turn.person, slug)] = turn.id - def allows(self, slug: str, *, turn: str) -> bool: - refused_in = self._refused_in.get(slug, "") - return bool(refused_in and turn and turn != refused_in) + def allows(self, slug: str, turn: Turn) -> bool: + refused_in = self._refused_in.get((turn.conversation, turn.person, slug), "") + return bool(refused_in and turn.id and turn.id != refused_in) def done(self, slug: str) -> None: - self._refused_in.pop(slug, None) + """The list is gone, for everyone who was waiting on it.""" + for key in [k for k in self._refused_in if k[2] == slug]: + del self._refused_in[key] # What the store says when a list to delete still has open items. @@ -58,14 +63,6 @@ def done(self, slug: str) -> None: _awaiting_yes = AwaitingYes() -def _current_turn() -> str: - """The id of the turn this tool call belongs to, "" if nanobot has none.""" - from nanobot.agent.tools.context import current_request_context - - request = current_request_context() - return str(getattr(request, "turn_id", "") or "") - - @tool_parameters( tool_parameters_schema( list=StringSchema( @@ -168,20 +165,17 @@ async def execute( sections: "list[str] | None" = None, confirmed: bool | None = None, ) -> str: - try: - import brief - actor = getattr(brief, "speaking_with", "") or "someone" - except Exception: - actor = "someone" + now = current_turn() + actor = now.person or "someone" if op in ("create", "set"): args = _details_args(op, list, title, about, topic, sections) elif op == "delete": - slug, turn = (list or "").strip(), _current_turn() + slug = (list or "").strip() args = ["stack", "memory", "list", "delete", slug] # Only a real yes, in a later message than the refusal: nanobot # casts "true"/"false" but passes any other string through. - if confirmed is True and _awaiting_yes.allows(slug, turn=turn): + if confirmed is True and _awaiting_yes.allows(slug, now): args.append("--yes") else: args = _item_args(op, list, items, topic, section, people, due, to) @@ -202,7 +196,7 @@ async def execute( return f"Error: list edit failed with exit {proc.returncode}: {err or out}" if op == "delete": if _NEEDS_YES in out + err: - _awaiting_yes.refused(slug, turn=turn) + _awaiting_yes.refused(slug, now) return _ask_first(out or err) if proc.returncode == 0: _awaiting_yes.done(slug) diff --git a/stacklets/agent/runtime/open_question.py b/stacklets/agent/runtime/open_question.py index 47be5dcc..55f754ee 100644 --- a/stacklets/agent/runtime/open_question.py +++ b/stacklets/agent/runtime/open_question.py @@ -13,8 +13,12 @@ treated as before. Whatever else the agent says in the room closes the question, since it is no longer the last thing asked. +The question is for whoever sent the message it answers, which is not +always the last person to write in the room. + questions = OpenQuestions() - questions.asked("!kitchen", "@homer:simpson", at=100.0) + questions.heard("$ask", "@homer:simpson") + questions.replied("!kitchen", "$ask", "Sollen die auch weg?", at=100.0) questions.answers("!kitchen", "@marge:simpson", at=110.0) # False questions.answers("!kitchen", "@homer:simpson", at=120.0) # True, once """ @@ -28,6 +32,10 @@ # is not taken for one. WINDOW_SECONDS = 300 +# Senders of messages the agent handled, kept for their answers. Far more +# than are ever in flight at once in a household. +_REMEMBERED = 100 + # The end of a message, past trailing quotes, brackets, emoji and spaces. _TRAILING = re.compile(r"[\s\"'”“»«)\]*_~\U0001F300-\U0001FAFF☀-➿]+$") @@ -43,6 +51,22 @@ class OpenQuestions: def __init__(self, window: float = WINDOW_SECONDS) -> None: self._window = window self._open: dict[str, tuple[str, float]] = {} + self._senders: dict[str, str] = {} + + def heard(self, event_id: str, person: str) -> None: + """The agent is handling `event_id`, sent by `person`.""" + if event_id and person: + self._senders[event_id] = person + while len(self._senders) > _REMEMBERED: + del self._senders[next(iter(self._senders))] + + def replied(self, room: str, event_id: str, text: str, *, at: float) -> None: + """The agent said `text` in answer to `event_id`: a question opens, anything else closes.""" + person = self._senders.get(event_id, "") + if asks(text) and person: + self.asked(room, person, at=at) + else: + self.closed(room) def asked(self, room: str, person: str, *, at: float) -> None: if room and person: diff --git a/stacklets/agent/runtime/sitecustomize.py b/stacklets/agent/runtime/sitecustomize.py index eeec8687..e296b44c 100644 --- a/stacklets/agent/runtime/sitecustomize.py +++ b/stacklets/agent/runtime/sitecustomize.py @@ -49,12 +49,12 @@ 9. voice_trigger (voice_trigger.py) — a voice message in a shared room goes through nanobot's gate, is transcribed, and reaches the agent only if the - transcript addresses it by name, like typed text. The answer starts with - what was heard, quoted. + transcript addresses it by name, like typed text, or answers the agent's + question (10). The answer starts with what was heard, quoted. 10. open_question (open_question.py) — while the agent's last message in a room - is a question, the asked person's next message there reaches it without - its name. + is a question, the person whose message it answers reaches it with their + next message there, typed or spoken, without its name. 11. status_trace (status_trace.py) — a slow turn gets one status notice that grows a line as the agent moves on (looking, looking closer, writing it @@ -103,6 +103,9 @@ `MatrixChannel.send(self, msg: OutboundMessage)` (async, a dataclass whose `metadata["event_id"]` is the message being answered) open_question: the voice_trigger symbols, plus `room.room_id` and `event.sender` in the gate + answers: an answer is an `OutboundMessage` with `event` None or + `nanobot.bus.outbound_events.StreamedResponseEvent`; every notice + (progress, compaction, retry wait) carries its own event (`_is_answer`) status_trace: `nanobot.bus.outbound_events.ProgressEvent` (`tool_hint`, `tool_events[].name`), `nanobot.events.ContextCompactionEvent` (`phase`) reaching `MatrixChannel.send`, reaching `MatrixChannel.send` when `sendToolHints` is on, @@ -170,6 +173,25 @@ def _is_progress(msg) -> bool: return isinstance(getattr(msg, "event", None), ProgressEvent) +def _is_answer(msg) -> bool: + """True for the agent speaking to the family, not one of nanobot's notices. + + A notice carries text too ("Compressing context…", a tool hint, a retry + wait), so text alone does not make an answer. An answer carries no + event, or the one that marks it as streamed. + """ + if not msg.content: + return False + event = getattr(msg, "event", None) + if event is None: + return True + try: + from nanobot.bus.outbound_events import StreamedResponseEvent + except Exception: + return False + return isinstance(event, StreamedResponseEvent) + + # ── state_log: the message list per turn, for debugging (opt-in) ───────────── # AGENT_STATE_LOG=1 writes what nanobot built for each call to llm-state.log, # one line per message, to compare against the Matrix chat. Off by default: @@ -339,19 +361,26 @@ def _is_bot_mentioned_or_our_thread(self, event): class _Running: """One turn's trace: its status notice in Matrix, and the loop keeping it current.""" - def __init__(self, channel, room_id: str, metadata: dict) -> None: + def __init__(self, channel, room_id: str, metadata: dict, key: tuple[str, str]) -> None: self.trace = _status_trace.Trace(started=_time_trace.monotonic(), lang=_os_trace.environ.get("AGENT_LANGUAGE", "")) - self.channel, self.room_id, self.metadata = channel, room_id, metadata + self.channel, self.room_id, self.metadata, self.key = channel, room_id, metadata, key self.notice_id = "" self.task = _asyncio_trace.create_task(self._keep_current()) async def _keep_current(self) -> None: - while True: + while not self.trace.over(at=_time_trace.monotonic()): await _asyncio_trace.sleep(_TRACE_TICK) lines = self.trace.due(at=_time_trace.monotonic()) if lines: await self._show("\n".join(lines)) + self.stop() + + def stop(self) -> None: + """Let go of the room; the notice stays as it is.""" + if _traces.get(self.key) is self: + del _traces[self.key] + self.task.cancel() async def _show(self, text: str) -> None: notice = {"msgtype": "m.notice", "body": text} @@ -378,9 +407,13 @@ async def _handle_and_trace(self, sender_id, chat_id, content, *args, **kwargs): metadata = kwargs.get("metadata") or {} key = _trace_key(chat_id, metadata) if key in _traces: - _traces.pop(key).task.cancel() - _traces[key] = _Running(self, chat_id, metadata) - return await _orig_handle_trace(self, sender_id, chat_id, content, *args, **kwargs) + _traces[key].stop() + running = _traces[key] = _Running(self, chat_id, metadata, key) + try: + return await _orig_handle_trace(self, sender_id, chat_id, content, *args, **kwargs) + except BaseException: + running.stop() + raise async def _send_traced(self, msg): key = _trace_key(msg.chat_id, msg.metadata) @@ -396,8 +429,8 @@ async def _send_traced(self, msg): if running and msg.event.phase == "started": running.trace.tidying() return None - if running and msg.content and not _is_progress(msg): - _traces.pop(key).task.cancel() + if running and _is_answer(msg): + running.stop() return await _orig_send_trace(self, msg) _matrix_trace.MatrixChannel._handle_message = _handle_and_trace @@ -407,6 +440,47 @@ async def _send_traced(self, msg): _log.exception("status-trace shim could not attach (nanobot internals changed?)") +# ── open_question: the answer to the agent's own question needs no name ───── +# While the agent's last message in a room asks someone something, that +# person's next message there reaches the agent without its name. The +# question is for whoever sent the message it answers. Installed inside the +# voice gate, so a spoken answer passes the way a typed one does. See +# open_question.py. +try: + import time as _time_question + + import nanobot.channels.matrix.runtime as _matrix_question + from open_question import OpenQuestions as _OpenQuestions + + _open_questions = _OpenQuestions() + _orig_should_process_q = _matrix_question.MatrixChannel._should_process_message + _orig_handle_q = _matrix_question.MatrixChannel._handle_message + _orig_send_q = _matrix_question.MatrixChannel.send + + def _should_process_or_answer(self, room, event): + if _orig_should_process_q(self, room, event): + return True + return _open_questions.answers(getattr(room, "room_id", ""), event.sender, + at=_time_question.monotonic()) + + async def _handle_and_remember(self, sender_id, chat_id, content, *args, **kwargs): + _open_questions.heard((kwargs.get("metadata") or {}).get("event_id", ""), sender_id) + return await _orig_handle_q(self, sender_id, chat_id, content, *args, **kwargs) + + async def _send_and_remember(self, msg): + if _is_answer(msg): + _open_questions.replied(msg.chat_id, (msg.metadata or {}).get("event_id", ""), + msg.content, at=_time_question.monotonic()) + return await _orig_send_q(self, msg) + + _matrix_question.MatrixChannel._should_process_message = _should_process_or_answer + _matrix_question.MatrixChannel._handle_message = _handle_and_remember + _matrix_question.MatrixChannel.send = _send_and_remember + _log.info("open-question shim active") +except Exception: + _log.exception("open-question shim could not attach (nanobot internals changed?)") + + # ── voice_trigger: a voice message is addressed by what was said ───────────── # nanobot's gate runs before transcription, and a voice message's text is # "Voice message", so in a shared room it never reached the agent. Let it @@ -451,10 +525,9 @@ async def _handle_message(self, sender_id, chat_id, content, *args, **kwargs): async def _send(self, msg): # The answer carries the voice message's event id for threading; the - # first one with text gets what was heard in front of it. Progress - # (a tool hint, see status_trace) is not the answer. + # first answer gets what was heard in front of it, not a notice. heard = _pending_voice.answering((msg.metadata or {}).get("event_id", "")) \ - if msg.content and not _is_progress(msg) else "" + if _is_answer(msg) else "" if heard: msg = _dataclasses_voice.replace(msg, content=_quoted(heard, msg.content)) return await _orig_send_voice(self, msg) @@ -467,51 +540,6 @@ async def _send(self, msg): _log.exception("voice-trigger shim could not attach (nanobot internals changed?)") -# ── open_question: the answer to the agent's own question needs no name ───── -# While the agent's last message in a room asks someone something, that -# person's next message there reaches the agent without its name. Who the -# agent is talking to comes from the message it handles, the question from -# the message it sends. See open_question.py. -try: - import time as _time_question - - import nanobot.channels.matrix.runtime as _matrix_question - from open_question import OpenQuestions as _OpenQuestions - from open_question import asks as _asks - - _open_questions = _OpenQuestions() - _talking_to: dict[str, str] = {} - _orig_should_process_q = _matrix_question.MatrixChannel._should_process_message - _orig_handle_q = _matrix_question.MatrixChannel._handle_message - _orig_send_q = _matrix_question.MatrixChannel.send - - def _should_process_or_answer(self, room, event): - if _orig_should_process_q(self, room, event): - return True - return _open_questions.answers(getattr(room, "room_id", ""), event.sender, - at=_time_question.monotonic()) - - async def _handle_and_remember(self, sender_id, chat_id, content, *args, **kwargs): - _talking_to[chat_id] = sender_id - return await _orig_handle_q(self, sender_id, chat_id, content, *args, **kwargs) - - async def _send_and_remember(self, msg): - if msg.content and not _is_progress(msg): - if _asks(msg.content): - _open_questions.asked(msg.chat_id, _talking_to.get(msg.chat_id, ""), - at=_time_question.monotonic()) - else: - _open_questions.closed(msg.chat_id) - return await _orig_send_q(self, msg) - - _matrix_question.MatrixChannel._should_process_message = _should_process_or_answer - _matrix_question.MatrixChannel._handle_message = _handle_and_remember - _matrix_question.MatrixChannel.send = _send_and_remember - _log.info("open-question shim active") -except Exception: - _log.exception("open-question shim could not attach (nanobot internals changed?)") - - # ── join_greeting: say something useful the moment you are invited ─────────── # Stock nanobot joins an invite silently. In a topic room that silence is the # family's first impression of the agent, so it takes one ordinary turn instead diff --git a/stacklets/agent/runtime/status_trace.py b/stacklets/agent/runtime/status_trace.py index 3a2750b6..55ef9e22 100644 --- a/stacklets/agent/runtime/status_trace.py +++ b/stacklets/agent/runtime/status_trace.py @@ -45,6 +45,10 @@ STILL_AFTER = 20.0 # A status longer than this stops growing; the answer is what matters. MAX_LINES = 5 +# Every turn ends in an answer, even a failed one ("Sorry, I encountered an +# error."), except one that is cancelled. Far longer than any turn takes, +# so the status of such a turn stops at some point instead of never. +GIVE_UP_AFTER = 600.0 LOOK, CHECK, WRITE, THINK, STILL, TIDY = "look", "check", "write", "think", "still", "tidy" @@ -90,7 +94,8 @@ class Trace: """The lines of one turn's status, and when they change. `tools` records what the agent started doing; `due` answers, at a given - time, the whole status to show if it should change now, or None. + time, the whole status to show if it should change now, or None; `over` + says when it will not change again. """ def __init__(self, *, started: float, lang: str, @@ -113,8 +118,12 @@ def tidying(self) -> None: """nanobot started compacting the conversation, which can take a while.""" self._pending = TIDY + def over(self, *, at: float) -> bool: + """True once the status is full, or the turn has run past all patience.""" + return len(self._lines) >= MAX_LINES or at - self._started >= GIVE_UP_AFTER + def due(self, *, at: float) -> list[str] | None: - if at - self._started < FIRST_AFTER or len(self._lines) >= MAX_LINES: + if at - self._started < FIRST_AFTER or self.over(at=at): return None if not self._lines: return self._add(self._pending, at) diff --git a/stacklets/agent/runtime/turn.py b/stacklets/agent/runtime/turn.py new file mode 100644 index 00000000..bd1c10fd --- /dev/null +++ b/stacklets/agent/runtime/turn.py @@ -0,0 +1,45 @@ +"""Who a tool call is for: the person, the conversation and the turn. + +A tool runs inside nanobot's handling of one message, and nanobot binds +that message's request context for exactly as long (a context variable, +so two rooms answered at the same time each see their own). This is the +one place the runtime reads it. + + now = current() + now.person # "homer", the vault slug of whoever sent the message + now.conversation # nanobot's session key: a room, or a thread in it + now.id # unique per turn +""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Turn: + """The message a tool call belongs to; every field is "" outside a turn.""" + + person: str = "" + conversation: str = "" + id: str = "" + + +def person_of(sender_id: str) -> str: + """The vault slug of a Matrix sender: "@homer:simpson" is "homer".""" + return str(sender_id or "").split(":")[0].lstrip("@") + + +def current() -> Turn: + """The turn running now, or an empty one when nanobot has none.""" + from nanobot.agent.tools.context import current_request_context + + request = current_request_context() + if request is None: + return Turn() + return Turn( + person=person_of(getattr(request, "sender_id", "") or ""), + conversation=str(getattr(request, "session_key", "") or getattr(request, "chat_id", "") + or ""), + id=str(getattr(request, "turn_id", "") or ""), + ) diff --git a/stacklets/agent/runtime/vault_write.py b/stacklets/agent/runtime/vault_write.py index 792085d0..464a2669 100644 --- a/stacklets/agent/runtime/vault_write.py +++ b/stacklets/agent/runtime/vault_write.py @@ -50,6 +50,8 @@ import subprocess from pathlib import Path +from turn import current as current_turn + _log = logging.getLogger("agent.runtime.vault_write") # The page the model addresses as `vault/...` is the same relative path the @@ -76,12 +78,8 @@ def vault_page(path: str | None) -> str: def _actor() -> str: - """Whoever the agent is speaking with this turn, for the commit.""" - try: - import brief - return getattr(brief, "speaking_with", "") or "someone" - except Exception: - return "someone" + """Whoever sent the message this write belongs to, for the commit.""" + return current_turn().person or "someone" def _run(page: str, payload: str, *flags: str) -> str: diff --git a/stacklets/memory/cli/list.py b/stacklets/memory/cli/list.py index 3ce1c4fa..f2828b9c 100644 --- a/stacklets/memory/cli/list.py +++ b/stacklets/memory/cli/list.py @@ -497,7 +497,7 @@ def move(vault: Path, config, slug: str, names: list[str], to: str, actor: str) def _add_all(prior: str) -> str: for item in items: - if not find(read_list(prior, path=target_path), item.text): + if not _same_on(read_list(prior, path=target_path), item): prior = insert_item(prior, item, path=target_path) return prior said = f"moved {'; '.join(i.text for i in items)} from {lst.title} to {target.title}" @@ -507,19 +507,27 @@ def _add_all(prior: str) -> str: def _remove_all(prior: str) -> str: for item in items: - current = read_list(prior, path=path) - match = [i for i in current.items if i.text == item.text] - if match: - prior = remove_item(prior, match[0]) + if match := _same_on(read_list(prior, path=path), item): + prior = remove_item(prior, match) return prior return _commit(config, path, _remove_all, actor, said) +def _same_on(lst: FamilyList, item: Item) -> Item | None: + """The item on `lst` that is `item` again: the same name, open or ticked alike. + + Exact and tick-aware, unlike `find`: "Eier" is not "Bio-Eier", and a + ticked "Eier" is not the open one a move or an add brings. + """ + return next((i for i in lst.items + if i.text.casefold() == item.text.casefold() and i.done == item.done), None) + + def _add(text: str, lst: FamilyList, entries: list[tuple[str, str]], links: tuple, due: str, path: str) -> tuple[str, str]: added, already = [], [] for name, section in entries: - if any(i.text.casefold() == name.casefold() and not i.done for i in lst.items): + if _same_on(lst, Item(name)): already.append(name) continue text = insert_item(text, Item(name, links=links, due=due, section=section), path=path) diff --git a/tests/unit/stacklets/conftest.py b/tests/unit/stacklets/conftest.py index 5fc6f79b..5d253689 100644 --- a/tests/unit/stacklets/conftest.py +++ b/tests/unit/stacklets/conftest.py @@ -232,7 +232,13 @@ class ContextCompactionEvent: mod("nanobot.events", ContextCompactionEvent=ContextCompactionEvent) mod("nanobot.bus") - mod("nanobot.bus.outbound_events", ProgressEvent=ProgressEvent) + @dataclasses.dataclass(frozen=True) + class StreamedResponseEvent: + # nanobot 0.3 bus/outbound_events.py: marks a final answer that was streamed. + pass + + mod("nanobot.bus.outbound_events", ProgressEvent=ProgressEvent, + StreamedResponseEvent=StreamedResponseEvent) mod("nanobot.runtime_context", RuntimeContextBlock=RuntimeContextBlock, wrap_runtime_context_lines=wrap_runtime_context_lines) mod("nanobot.agent.tools") diff --git a/tests/unit/stacklets/test_agent_config.py b/tests/unit/stacklets/test_agent_config.py index 5d93e419..506b90f0 100644 --- a/tests/unit/stacklets/test_agent_config.py +++ b/tests/unit/stacklets/test_agent_config.py @@ -29,3 +29,30 @@ def test_the_context_window_is_set_on_the_active_preset(): def test_no_context_window_where_nanobot_ignores_it(): """A second value under defaults would read as the one in effect.""" assert "context_window_tokens" not in _config()["agents"]["defaults"] + + +def _blocked_by_exec(command: str) -> bool: + """Whether nanobot's exec tool refuses `command` under this config. + + nanobot 0.3.5 searches each deny pattern in the lowercased command + (`ExecTool._guard_command`). + """ + import re + patterns = _config()["tools"]["exec"].get("deny_patterns", []) + return any(re.search(p, command.lower()) for p in patterns) + + +def test_the_shell_cannot_confirm_deleting_a_list(): + """Only `list_edit` may delete a list with open items, after the person said yes. + + The store takes `--yes` from whoever runs it, and the agent's shell can + run it too. Without this, the model could skip asking the person. + """ + assert _blocked_by_exec("stack memory list delete partyliste --yes --as homer") + assert _blocked_by_exec("stack memory list delete --yes partyliste") + assert _blocked_by_exec("stack memory list delete partyliste --ye") + + +def test_the_shell_still_shows_and_deletes_lists_the_store_allows(): + assert not _blocked_by_exec("stack memory list show partyliste") + assert not _blocked_by_exec("stack memory list delete partyliste --as homer") diff --git a/tests/unit/stacklets/test_agent_runtime_shims.py b/tests/unit/stacklets/test_agent_runtime_shims.py index 31b232fc..8537abe0 100644 --- a/tests/unit/stacklets/test_agent_runtime_shims.py +++ b/tests/unit/stacklets/test_agent_runtime_shims.py @@ -27,7 +27,7 @@ "memory_tool", "person_tool", "history_tool", "name_trigger", "thread_trigger", "join_greeting", "vault_write", "list_tool", "tool_trim", "voice_trigger", "open_question", - "status_trace") + "status_trace", "turn") # The stub nanobot itself lives in conftest as `nanobot_stub`, shared with @@ -420,36 +420,110 @@ class _Text: # ── the answer to the agent's own question ─────────────────────────────── +def _said(text, sender="@homer:simpson"): + class _Text: + body = text + pill_mention = False + source = {"content": {"msgtype": "m.text", "body": text}} + event = _Text() + event.sender = sender + return event + + +def _asks_homer(channel, *, event_id="$ask"): + """Homer asks the agent something, and the agent answers with a question.""" + import asyncio + asyncio.run(channel._handle_message( + sender_id="@homer:simpson", chat_id="!kitchen", + content="Stacky, lösch die Partyliste", metadata={"event_id": event_id})) + asyncio.run(channel.send(_outbound( + "Die Partyliste hat noch 2 offene Punkte. Sollen die auch weg?", + metadata={"event_id": event_id}))) + + def test_the_answer_to_the_agents_question_needs_no_name(nanobot, monkeypatch): """"Sollen die auch weg?" "Ja, die können weg." reaches the agent; once.""" + import types as _types + + monkeypatch.setenv("AGENT_NAME", "Stacky") + channel = nanobot()["nanobot.channels.matrix.runtime"].MatrixChannel() + _asks_homer(channel) + + kitchen = _types.SimpleNamespace(room_id="!kitchen") + assert channel._should_process_message(kitchen, _said("Ja, die können weg.")) + assert not channel._should_process_message(kitchen, _said("Ja, die können weg.")), \ + "only the first answer" + + +def test_a_notice_from_nanobot_leaves_the_question_open(nanobot, monkeypatch): + """nanobot's notices carry text ("Compressing context…") but are not the agent speaking.""" import asyncio - import dataclasses import types as _types - @dataclasses.dataclass - class _Outbound: - chat_id: str - content: str - metadata: dict - event: object = None + monkeypatch.setenv("AGENT_NAME", "Stacky") + mods = nanobot() + channel = mods["nanobot.channels.matrix.runtime"].MatrixChannel() + _asks_homer(channel) + compaction = mods["nanobot.events"].ContextCompactionEvent + asyncio.run(channel.send(_outbound("Context compacted.", + event=compaction(phase="succeeded")))) + + kitchen = _types.SimpleNamespace(room_id="!kitchen") + assert channel._should_process_message(kitchen, _said("Ja, die können weg.")) - class _Text: - sender = "@homer:simpson" - body = "Ja, die können weg." - pill_mention = False - source = {"content": {"msgtype": "m.text", "body": "Ja, die können weg."}} + +def test_a_spoken_answer_to_the_agents_question_needs_no_name(nanobot, monkeypatch): + import types as _types monkeypatch.setenv("AGENT_NAME", "Stacky") channel = nanobot()["nanobot.channels.matrix.runtime"].MatrixChannel() + _asks_homer(channel) + kitchen = _types.SimpleNamespace(room_id="!kitchen") + assert channel._should_process_message(kitchen, _voice()) + _hand_over(channel, "Ja, die können weg.") + + assert channel.handled[-1]["content"] == "Voice message\n[transcription: Ja, die können weg.]" + + +def test_the_question_is_for_the_person_whose_message_it_answers(nanobot, monkeypatch): + """Bart writing while Homer's answer is on its way does not make it Bart's question.""" + import asyncio + import types as _types + + monkeypatch.setenv("AGENT_NAME", "Stacky") + channel = nanobot()["nanobot.channels.matrix.runtime"].MatrixChannel() asyncio.run(channel._handle_message( sender_id="@homer:simpson", chat_id="!kitchen", - content="Stacky, lösch die Partyliste", metadata={"event_id": "$ask"})) - asyncio.run(channel.send(_Outbound( - "!kitchen", "Die Partyliste hat noch 2 offene Punkte. Sollen die auch weg?", {}))) + content="Stacky, lösch die Partyliste", metadata={"event_id": "$homer"})) + asyncio.run(channel._handle_message( + sender_id="@bart:simpson", chat_id="!kitchen", + content="Stacky, was gibt es heute?", metadata={"event_id": "$bart"})) + asyncio.run(channel.send(_outbound("Die Partyliste hat noch offene Punkte. Sollen die weg?", + metadata={"event_id": "$homer"}))) + + kitchen = _types.SimpleNamespace(room_id="!kitchen") + assert not channel._should_process_message(kitchen, _said("Ja", "@bart:simpson")) + assert channel._should_process_message(kitchen, _said("Ja")) + + +def test_a_notice_from_nanobot_does_not_take_the_quote(nanobot, monkeypatch): + import asyncio + + monkeypatch.setenv("AGENT_NAME", "Stacky") + mods = nanobot() + channel = mods["nanobot.channels.matrix.runtime"].MatrixChannel() + channel._should_process_message(None, _voice()) + _hand_over(channel, "Stacky, wir brauchen Milch.") - assert channel._should_process_message(kitchen, _Text()) - assert not channel._should_process_message(kitchen, _Text()), "only the first answer" + compaction = mods["nanobot.events"].ContextCompactionEvent + asyncio.run(channel.send(_outbound("Compressing context…", event=compaction(), + metadata={"event_id": "$voice"}))) + asyncio.run(channel.send(_outbound("Milch steht jetzt auf der Einkaufsliste.", + metadata={"event_id": "$voice"}))) + + assert channel.sent[-1].content == \ + "> Stacky, wir brauchen Milch.\n\nMilch steht jetzt auf der Einkaufsliste." # ── a slow turn shows what the agent is doing ──────────────────────────── @@ -463,7 +537,6 @@ class _Outbound: content: str metadata: dict event: object = None - event: object = None return _Outbound("!kitchen", content, metadata or {}, event) @@ -537,6 +610,44 @@ def test_the_status_stops_with_the_answer(nanobot, monkeypatch): assert len(channel.room_contents) == shown +def test_a_status_whose_answer_never_comes_stops_by_itself(nanobot, monkeypatch): + """A cancelled turn sends nothing; its status stops and lets go of the room.""" + import asyncio + import sys as _sys + + mods = nanobot() + trace, shim = _sys.modules["status_trace"], _sys.modules["sitecustomize"] + monkeypatch.setattr(trace, "FIRST_AFTER", 0.0) + monkeypatch.setattr(trace, "GIVE_UP_AFTER", 0.1) + monkeypatch.setattr(shim, "_TRACE_TICK", 0.01) + channel = mods["nanobot.channels.matrix.runtime"].MatrixChannel() + + async def _run(): + await channel._handle_message(sender_id="@homer:simpson", chat_id="!kitchen", + content="Stacky, was fehlt noch?", metadata={}) + await asyncio.sleep(0.2) + return dict(shim._traces) + + assert asyncio.run(_run()) == {} + assert len(channel.room_contents) == 1, "the status it showed stays, nothing more" + + +def test_a_message_that_fails_to_reach_the_agent_starts_no_status(nanobot, monkeypatch): + import asyncio + import sys as _sys + + mods = nanobot() + channel = mods["nanobot.channels.matrix.runtime"].MatrixChannel() + + async def _broken(*_args, **_kwargs): + raise RuntimeError("bus closed") + monkeypatch.setattr(_sys.modules["sitecustomize"], "_orig_handle_trace", _broken) + with pytest.raises(RuntimeError): + asyncio.run(channel._handle_message(sender_id="@homer:simpson", chat_id="!kitchen", + content="Stacky, was fehlt noch?", metadata={})) + assert _sys.modules["sitecustomize"]._traces == {} + + # ── failure is contained, and visible ──────────────────────────────────── def test_a_moved_symbol_does_not_take_the_others_down(nanobot): diff --git a/tests/unit/stacklets/test_agent_status_trace.py b/tests/unit/stacklets/test_agent_status_trace.py index 73f6d7c7..06c13aa1 100644 --- a/tests/unit/stacklets/test_agent_status_trace.py +++ b/tests/unit/stacklets/test_agent_status_trace.py @@ -16,7 +16,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[3] / "stacklets" / "agent" / "runtime")) from status_trace import ( # noqa: E402 - FIRST_AFTER, GAP, PHRASES, STILL_AFTER, Trace, phase_of, + FIRST_AFTER, GAP, GIVE_UP_AFTER, PHRASES, STILL_AFTER, Trace, phase_of, ) DE = PHRASES["de"] @@ -98,3 +98,22 @@ def test_compacting_the_conversation_is_a_line_in_the_familys_words(): trace = _trace() trace.tidying() assert trace.due(at=FIRST_AFTER) == [DE["tidy"][0]] + + + +def test_a_turn_that_never_answers_ends_its_status_by_itself(): + """A cancelled turn sends nothing, so the status cannot wait for an answer forever.""" + trace = _trace() + assert not trace.over(at=GIVE_UP_AFTER - 1) + assert trace.over(at=GIVE_UP_AFTER) + + +def test_a_full_status_is_over_before_the_turn_gives_up(): + trace = _trace() + at = FIRST_AFTER + for tools in (["memory_search"], ["list_edit"], ["memory_search"], ["list_edit"], + ["memory_search"]): + trace.tools(tools) + assert trace.due(at=at) + at += GAP + assert trace.over(at=at) diff --git a/tests/unit/stacklets/test_agent_vault_tools.py b/tests/unit/stacklets/test_agent_vault_tools.py index 0ccf86f7..42f73462 100644 --- a/tests/unit/stacklets/test_agent_vault_tools.py +++ b/tests/unit/stacklets/test_agent_vault_tools.py @@ -255,10 +255,17 @@ def test_search_sends_no_backend_flag(vault_tools): b"Delete it with them: add --yes\n") +def said(turn: str, *, by: str = "homer", room: str = "matrix:!kitchen"): + """nanobot's request context for a message `by` someone in `room`.""" + return types.SimpleNamespace(turn_id=turn, sender_id=f"@{by}:simpson", session_key=room, + chat_id=room.split(":", 1)[-1]) + + def deletes(tool_cls, turns, monkeypatch) -> list[bool]: """Run delete calls turn by turn; for each, whether `--yes` was sent. - `turns` holds (turn id, confirmed, what the store answers). The store's + `turns` holds (turn id or request, confirmed, what the store answers). + A bare turn id is a message from Homer in the kitchen. The store's answer is fed back to the tool as the CLI's output, as on the rig. """ import sys as _sys @@ -267,8 +274,8 @@ def deletes(tool_cls, turns, monkeypatch) -> list[bool]: sent: list[bool] = [] tool = tool_cls() for turn, confirmed, answer in turns: - monkeypatch.setattr(context, "current_request_context", - lambda turn=turn: types.SimpleNamespace(turn_id=turn)) + request = said(turn) if isinstance(turn, str) else turn + monkeypatch.setattr(context, "current_request_context", lambda request=request: request) class _Proc: returncode = 1 if answer == REFUSED else 0 @@ -304,7 +311,7 @@ def test_a_list_with_open_items_is_deleted_only_after_the_person_said_yes(vault_ def test_a_refused_delete_tells_the_model_to_ask_naming_what_is_open(vault_tools, monkeypatch): import sys as _sys monkeypatch.setattr(_sys.modules["nanobot.agent.tools.context"], "current_request_context", - lambda: types.SimpleNamespace(turn_id="turn-1")) + lambda: said("turn-1")) answer = result_of(vault_tools["list_edit"], returncode=1, stdout=REFUSED, op="delete", list="partyliste") assert answer.startswith("Not deleted: Partyliste still has 2 open items: Luftballons; " @@ -312,6 +319,35 @@ def test_a_refused_delete_tells_the_model_to_ask_naming_what_is_open(vault_tools assert "--yes" not in answer +def test_only_the_person_who_was_told_can_say_yes(vault_tools, monkeypatch): + sent = deletes(vault_tools["list_edit"], [ + (said("turn-1", by="homer"), None, REFUSED), + (said("turn-2", by="bart"), True, REFUSED), + ], monkeypatch) + assert sent == [False, False] + + +def test_a_yes_in_another_room_confirms_nothing(vault_tools, monkeypatch): + sent = deletes(vault_tools["list_edit"], [ + (said("turn-1", room="matrix:!kitchen"), None, REFUSED), + (said("turn-2", room="matrix:!garage"), True, REFUSED), + ], monkeypatch) + assert sent == [False, False] + + +def test_a_list_change_is_committed_as_whoever_sent_the_message(vault_tools, monkeypatch): + """Two rooms answered at once each commit as their own sender. + + The sender used to be a module global set by the latest briefing, so + a briefing for Bart's message could put his name on Homer's change. + """ + import sys as _sys + monkeypatch.setattr(_sys.modules["nanobot.agent.tools.context"], "current_request_context", + lambda: said("turn-1", by="marge")) + argv = argv_of(vault_tools["list_edit"], op="add", list="einkaufsliste", items=["Eier"]) + assert argv[argv.index("--as") + 1] == "marge" + + def test_a_yes_without_a_refusal_before_it_deletes_nothing_open(vault_tools, monkeypatch): sent = deletes(vault_tools["list_edit"], [("turn-1", True, REFUSED)], monkeypatch) assert sent == [False] diff --git a/tests/unit/stacklets/test_memory_list_cli.py b/tests/unit/stacklets/test_memory_list_cli.py index 16ff0bf4..d513edd9 100644 --- a/tests/unit/stacklets/test_memory_list_cli.py +++ b/tests/unit/stacklets/test_memory_list_cli.py @@ -324,6 +324,30 @@ def test_a_moved_item_keeps_its_people_and_leaves_the_old_list(self, run, store, assert store.text(groceries).endswith(" - Milchprodukte & Eier\n---\n"), \ "the old list is empty again, with no heading left behind" + def test_an_item_whose_name_is_part_of_another_still_arrives(self, run, store, groceries): + """A move removes the item from the old list, so the new one must hold it.""" + run("create", "markt", "--title", "Wochenmarkt", "--as", "marge") + run("add", "markt", "Bio-Eier", "--as", "marge") + run("add", "einkaufsliste", "Eier", "--section", "Milchprodukte & Eier", "--as", "marge") + run("move", "einkaufsliste", "Eier", "--to", "markt", "--as", "marge") + assert "- [ ] Eier\n" in store.text("family/lists/markt.md") + + def test_an_item_ticked_on_the_other_list_arrives_open(self, run, store, groceries): + run("create", "markt", "--title", "Wochenmarkt", "--as", "marge") + run("add", "markt", "Eier", "--as", "marge") + run("tick", "markt", "Eier", "--as", "marge") + run("add", "einkaufsliste", "Eier", "--section", "Milchprodukte & Eier", "--as", "marge") + run("move", "einkaufsliste", "Eier", "--to", "markt", "--as", "marge") + assert "- [ ] Eier\n" in store.text("family/lists/markt.md") + + def test_an_item_open_on_both_lists_is_kept_once(self, run, store, groceries): + run("create", "markt", "--title", "Wochenmarkt", "--as", "marge") + run("add", "markt", "Eier", "--as", "marge") + run("add", "einkaufsliste", "Eier", "--section", "Milchprodukte & Eier", "--as", "marge") + run("move", "einkaufsliste", "Eier", "--to", "markt", "--as", "marge") + assert store.text("family/lists/markt.md").count("Eier") == 1 + assert "Eier\n" not in store.text(groceries).split("---\n", 2)[2] + def test_a_list_that_does_not_exist_is_refused_with_the_lists(self, run, groceries): result = run("move", "einkaufsliste", "Eier", "--to", "baumarkt", "--as", "marge") assert result["error"] == "no list 'baumarkt'. Lists: einkaufsliste"