diff --git a/emrg/server/feishu_prompt.md b/emrg/server/feishu_prompt.md index d7d97166..eb6ee11d 100644 --- a/emrg/server/feishu_prompt.md +++ b/emrg/server/feishu_prompt.md @@ -58,6 +58,11 @@ in the usual install locations before calling it blocked. replied", because it returns the conversation in the order it was written. `im +messages-search` answers out of an index that lags a send by seconds, so it can never establish that nothing came back. +- **A message's `thread_replies` are part of that conversation.** A listing answers a thread as + its parent carrying the replies, so an @ to the host written as a reply arrives in that key + and in no other list. Read the replies as you read the top level — the same sender test, the + same judgement — and never treat a thread as read because its parent was: a thread nobody + opened is a message nobody saw. - The conversation the host answers in is the one your own last message landed in (`oc_…`), so keep that id beside the message id: §3 reads a reply by position, and a position means nothing in a conversation you are not looking at. diff --git a/emrg/server/wake.py b/emrg/server/wake.py index d268470e..f828fd60 100644 --- a/emrg/server/wake.py +++ b/emrg/server/wake.py @@ -34,7 +34,7 @@ from dataclasses import dataclass from datetime import datetime, timedelta, timezone from pathlib import Path -from typing import Callable, Sequence +from typing import Callable, Iterator, Sequence from emrg.server.rants import default_rants_log, list_rants from emrg.tools.base import non_regular_kind @@ -328,12 +328,13 @@ def _condition_feishu(*, state: dict, now: datetime, runner, cli: str) -> Verdic # that matters may be in exactly the one that could not be read. return Verdict(UNKNOWN, f"conversation {chat_id}: {exc}") for message in _message_items(reply): - message_id = _message_id(message) - if message_id is None or message_id in seen: - continue - if _needs_the_host(message, host=host, direct=_is_direct(chat)): - hits += 1 - seen.add(message_id) + for item in _messages_with_replies(message): + message_id = _message_id(item) + if message_id is None or message_id in seen: + continue + if _needs_the_host(item, host=host, direct=_is_direct(chat)): + hits += 1 + seen.add(message_id) state["seen"] = sorted(seen)[-FEISHU_SEEN_CAP:] state["since"] = ( @@ -505,6 +506,24 @@ def _message_items(data: dict) -> list[dict]: raise ConditionUnavailable(f"the message list carried no messages: {sorted(payload)}") +def _messages_with_replies(message: dict) -> Iterator[dict]: + """A message and the replies hanging under it, the message first. + + `+chat-messages-list` answers a thread as its parent carrying the replies in + `thread_replies`, so an @ the host was handed inside a thread is in that list and in + no other one. A reader that stops at the top level does not see it, and the condition + then answers `absent` for a host who was in fact asked something — the failure is + silent, because the answer it gives is the same one a quiet inbox gives. + """ + yield message + replies = message.get("thread_replies") + if not isinstance(replies, list): + return + for reply in replies: + if isinstance(reply, dict): + yield reply + + def _chat_id(chat: dict) -> str | None: for key in ("chat_id", "chatId", "id"): value = chat.get(key) diff --git a/tests/test_feishu_prompt.py b/tests/test_feishu_prompt.py index 8b4b02ab..acd40ab6 100644 --- a/tests/test_feishu_prompt.py +++ b/tests/test_feishu_prompt.py @@ -254,6 +254,34 @@ def test_both_templates_that_forbid_a_slowdown_give_the_same_answer(): ) +#: The rule that makes a thread part of the conversation it hangs under: the key the +#: listing answers a thread in, so a round that reads it sees the replies. +THREAD_MARK = "`thread_replies`" + + +def test_the_template_says_a_thread_is_part_of_its_conversation(): + """Without this the round reads the parent and reports the thread as covered.""" + text = PROMPT.read_text(encoding="utf-8") + section = text[text.index(FIRST_STEP) :] + assert THREAD_MARK in section, ( + "§1 no longer tells a round to read a message's thread replies — an @ to the host " + "written inside a thread arrives in that key and in no other list, so a round that " + "stops at the parent reports a conversation it never saw" + ) + assert "never treat a thread as read because its parent was" in section, ( + "§1 lost the sentence that stops a thread being closed by having read its parent — " + "the replies are the part that can carry the @" + ) + + +def test_the_thread_rule_reaches_the_round_and_not_merely_the_file(rendered): + """The same requirement on the prompt a round is sent, where a Jinja construct hides.""" + assert THREAD_MARK in rendered, ( + "the rendered Feishu prompt lost the thread-reply rule — a round is told to read the " + "conversation and not that a thread is part of it" + ) + + #: The rule that tells a round how to read an empty answer: one conversation known to hold #: messages, read before "nothing arrived" is reported. POSITIVE_CONTROL_MARK = "known to hold messages" @@ -286,6 +314,7 @@ def test_the_positive_control_reaches_the_round_and_not_merely_the_file(rendered "the rendered Feishu prompt lost the positive-control sentence — a round is told an " "empty round is a correct round and given no way to know the round was empty rather " "than blind" + ) diff --git a/tests/test_wake.py b/tests/test_wake.py index 1688f3c8..638037d7 100644 --- a/tests/test_wake.py +++ b/tests/test_wake.py @@ -78,17 +78,23 @@ def runner(argv): return runner -def _message(message_id, *, sender="ou_stranger", sender_type="user", mentions=()): +def _message(message_id, *, sender="ou_stranger", sender_type="user", mentions=(), replies=()): """A message in the shape `+chat-messages-list` answers. The sender defaults to a stranger: the host's own messages are excluded by design, so a host-sender default would make every case below read as noise for the wrong reason. + + `replies` are the messages hanging under this one in a thread. The listing answers them + nested, so they are attached rather than listed beside their parent. """ - return { + message = { "message_id": message_id, "sender": {"id": sender, "sender_type": sender_type}, "mentions": list(mentions), } + if replies: + message["thread_replies"] = list(replies) + return message def _mention(open_id=None, *, name=None, key=None): @@ -314,6 +320,70 @@ def test_the_feishu_condition_answers_the_three_cases_that_need_the_host(tmp_pat assert held is True, f"{label} should wake the task: {notes}" +def test_the_feishu_condition_reads_a_thread_reply_that_mentions_the_host(tmp_path): + """An @ inside a thread holds the condition, exactly as a top-level one does. + + `+chat-messages-list` answers a thread as its parent carrying the replies, so the @ + arrives under `thread_replies` and in no other list. A condition that walks the top + level alone answers `absent` — the same answer a quiet inbox gives, which is why the + miss is silent and the task then sleeps for its whole interval. + """ + runner = _feishu_runner( + chats=[{"chat_id": "oc_group", "chat_mode": "group"}], + messages={"oc_group": [ + _message("om_parent", replies=[_message("om_reply", mentions=[_mention(HOST)])]), + ]}, + ) + held, notes = wakes(_record(feishu=300), task_name="t", agent="t", + state_dir=tmp_path / "wake-state", runner=runner) + assert held is True, notes + + +def test_the_feishu_condition_does_not_wake_on_a_thread_that_asks_nothing(tmp_path): + """The other direction: a reply that mentions nobody, and one the task itself sent. + + A wake that fires on every threaded conversation costs a round every interval, which + is worse than the miss it buys — so a reply passes the same sender test the top level + does, and the answer stays `absent`. + """ + runner = _feishu_runner( + chats=[{"chat_id": "oc_group", "chat_mode": "group"}], + messages={"oc_group": [ + _message("om_parent", sender="ou_stranger", replies=[ + _message("om_reply_noise", sender="ou_other"), + _message("om_reply_own", sender="ou_bot", sender_type="app"), + ]), + ]}, + ) + held, notes = wakes(_record(feishu=300), task_name="t", agent="t", + state_dir=tmp_path / "wake-state", runner=runner) + assert held is False, notes + assert any("feishu: absent" in note for note in notes), notes + + +def test_a_thread_reply_is_de_duplicated_by_its_own_id(tmp_path): + """A reply read twice in one lookback window is one message, not a second wake. + + The reply's id is not its parent's: a de-duplication that keyed a thread by its parent + would re-wake the task for as long as the parent stayed in the window. + """ + runner = _feishu_runner( + chats=[{"chat_id": "oc_group", "chat_mode": "group"}], + messages={"oc_group": [ + _message("om_parent", replies=[_message("om_reply", mentions=[_mention(HOST)])]), + ]}, + ) + state_dir = tmp_path / "wake-state" + start = datetime(2026, 10, 26, 12, 0, 0) + first, _ = wakes(_record(feishu=300), task_name="t", agent="t", + state_dir=state_dir, runner=runner, now=start) + second, notes = wakes(_record(feishu=300), task_name="t", agent="t", + state_dir=state_dir, runner=runner, + now=start + timedelta(minutes=6)) + assert first is True + assert second is False, f"the same reply must not wake the task twice: {notes}" + + def test_the_feishu_condition_ignores_noise_and_the_tasks_own_messages(tmp_path): """A group message that asks for nothing, and the bot's own reply, both leave it asleep.