diff --git a/gateway/platforms/feishu.py b/gateway/platforms/feishu.py index 85cebe53815f..d89125dfc0c4 100644 --- a/gateway/platforms/feishu.py +++ b/gateway/platforms/feishu.py @@ -2039,14 +2039,22 @@ def _on_drive_comment_event(self, data: Any) -> None: logging, and reaction. Scheduling follows the same ``run_coroutine_threadsafe`` pattern used by ``_on_message_event``. """ - from gateway.platforms.feishu_comment import handle_drive_comment_event + from gateway.platforms.feishu_comment import ( + CommentContext, + handle_drive_comment_event, + ) loop = self._loop if not self._loop_accepts_callbacks(loop): logger.warning("[Feishu] Dropping drive comment event before adapter loop is ready") return + # Build the handler's dependency bundle here so the handler signature + # stays independent of the concrete ``FeishuAdapter`` type. All + # ``_client`` / ``_session_store`` reads are localized to + # ``CommentContext.from_adapter``. + ctx = CommentContext.from_adapter(self, self_open_id=self._bot_open_id) future = asyncio.run_coroutine_threadsafe( - handle_drive_comment_event(self._client, data, self_open_id=self._bot_open_id), + handle_drive_comment_event(ctx, data), loop, ) future.add_done_callback(self._log_background_failure) diff --git a/gateway/platforms/feishu_comment.py b/gateway/platforms/feishu_comment.py index 46807630ce3b..f376a9959cb7 100644 --- a/gateway/platforms/feishu_comment.py +++ b/gateway/platforms/feishu_comment.py @@ -14,7 +14,11 @@ Whole -> list whole comments timeline Local -> list comment thread replies 5. Build prompt (local or whole) - 6. Create AIAgent with feishu_doc + feishu_drive tools -> agent generates reply + 6. Run AIAgent with no feishu tools. If the agent decides it needs + document text to reply, it emits a ``{...}`` sentinel; + business code then fetches the requested docs (from a whitelist of + the source doc + comment-referenced docs) and re-invokes the agent + with the content appended. See ``_run_comment_agent``. 7. Route reply: Whole -> add_whole_comment Local -> reply_to_comment (fallback to add_whole_comment on 1069302) @@ -25,10 +29,50 @@ import asyncio import json import logging -from typing import Any, Dict, List, Optional, Tuple +import re +from dataclasses import dataclass +from typing import Any, Dict, List, Optional, Set, Tuple logger = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# External dependencies bundle +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True) +class CommentContext: + """External dependencies the comment handler needs from its adapter. + + Boundary dataclass: shields the handler from the concrete + ``FeishuAdapter`` type and its private-attribute layout. Everything + the handler reads off the adapter flows through this struct, which + keeps the handler signature stable across adapter refactors and makes + tests cheaper (construct a 3-field dataclass instead of mocking an + adapter). + """ + client: Any # lark_oapi client (for Feishu API calls) + session_store: Optional[Any] # gateway SessionStore; may be None in + # degraded runtimes or stateless tests + self_open_id: str # bot's own open_id — used to filter + # self-authored events and to strip + # routing @mentions from timeline text + + @classmethod + def from_adapter(cls, adapter: Any, *, self_open_id: str = "") -> "CommentContext": + """Build a ``CommentContext`` from a live ``FeishuAdapter``. + + Concentrates all ``adapter._xxx`` reads into this one method — if + the adapter later grows public getters, only this factory changes. + """ + return cls( + client=adapter._client, + session_store=getattr(adapter, "_session_store", None), + self_open_id=self_open_id, + ) + + # --------------------------------------------------------------------------- # Lark SDK helpers (lazy-imported) # --------------------------------------------------------------------------- @@ -59,9 +103,11 @@ def _build_request(method: str, uri: str, paths=None, queries=None, body=None): async def _exec_request(client, method, uri, paths=None, queries=None, body=None): """Execute a lark API request and return (code, msg, data_dict).""" - logger.info("[Feishu-Comment] API >>> %s %s paths=%s queries=%s body=%s", - method, uri, paths, queries, - json.dumps(body, ensure_ascii=False)[:500] if body else None) + # Log metadata only — request bodies may contain user content (reply text, + # comment text) which must not land in persistent logs. + body_bytes = len(json.dumps(body, ensure_ascii=False).encode("utf-8")) if body else 0 + logger.info("[Feishu-Comment] API >>> %s %s paths=%s queries=%s body_bytes=%d", + method, uri, paths, queries, body_bytes) request = _build_request(method, uri, paths, queries, body) response = await asyncio.to_thread(client.request, request) @@ -86,12 +132,10 @@ async def _exec_request(client, method, uri, paths=None, queries=None, body=None logger.info("[Feishu-Comment] API <<< %s %s code=%s msg=%s data_keys=%s", method, uri, code, msg, list(data.keys()) if data else "empty") if code != 0: - # Log raw response for debugging failed API calls - raw = getattr(response, "raw", None) - raw_content = "" - if raw and hasattr(raw, "content"): - raw_content = raw.content[:500] if isinstance(raw.content, (str, bytes)) else str(raw.content)[:500] - logger.warning("[Feishu-Comment] API FAIL raw response: %s", raw_content) + # Raw response bodies may echo user content back in error messages; + # log only the code + msg we've already extracted above. + logger.warning("[Feishu-Comment] API FAIL: %s %s code=%s msg=%s", + method, uri, code, msg) return code, msg, data @@ -275,8 +319,9 @@ async def query_document_meta( return {} metas = data.get("metas", []) - logger.debug("[Feishu-Comment] query_document_meta: raw metas type=%s value=%s", - type(metas).__name__, str(metas)[:300]) + # Don't dump metas value — entries include title and other business info. + logger.debug("[Feishu-Comment] query_document_meta: raw metas type=%s count=%s", + type(metas).__name__, len(metas) if hasattr(metas, "__len__") else "?") if not metas: # Try alternate response shape: metas may be a dict keyed by token if isinstance(data.get("metas"), dict): @@ -292,8 +337,9 @@ async def query_document_meta( "url": meta.get("url", ""), "doc_type": meta.get("doc_type", file_type), } - logger.info("[Feishu-Comment] query_document_meta: title=%s url=%s", - result["title"], result["url"][:80] if result["url"] else "") + # Title may contain business-sensitive info (e.g. project names); omit. + logger.info("[Feishu-Comment] query_document_meta: url=%s", + result["url"][:80] if result["url"] else "") return result @@ -343,9 +389,12 @@ async def batch_query_comment( logger.debug("[Feishu-Comment] batch_query_comment: got %d items", len(items) if isinstance(items, list) else 0) if items and isinstance(items, list): item = items[0] - logger.info("[Feishu-Comment] batch_query_comment: is_whole=%s quote=%s reply_count=%s", + # quote is user content — log length only so persistent logs don't + # expose the quoted snippet of the document to other operators. + quote = item.get("quote", "") or "" + logger.info("[Feishu-Comment] batch_query_comment: is_whole=%s quote_len=%d reply_count=%s", item.get("is_whole"), - (item.get("quote", "") or "")[:60], + len(quote), len(item.get("reply_list", {}).get("replies", [])) if isinstance(item.get("reply_list"), dict) else "?") return item logger.warning("[Feishu-Comment] batch_query_comment: empty items, raw data keys=%s", list(data.keys())) @@ -475,8 +524,9 @@ async def reply_to_comment( Returns ``(success, code)``. """ text = _sanitize_comment_text(text) - logger.info("[Feishu-Comment] reply_to_comment: comment_id=%s text=%s", - comment_id, text[:100]) + # Reply text is the agent's generated content — log length only. + logger.info("[Feishu-Comment] reply_to_comment: comment_id=%s text_len=%d", + comment_id, len(text)) body = { "content": { "elements": [ @@ -509,8 +559,9 @@ async def add_whole_comment( Returns ``True`` on success. """ text = _sanitize_comment_text(text) - logger.info("[Feishu-Comment] add_whole_comment: file_token=%s text=%s", - file_token, text[:100]) + # Agent-generated content — log length only. + logger.info("[Feishu-Comment] add_whole_comment: file_token=%s text_len=%d", + file_token, len(text)) body = { "file_type": file_type, "reply_elements": [ @@ -867,12 +918,23 @@ def _select_whole_timeline( _COMMON_INSTRUCTIONS = """ This is a Feishu document comment thread, not an IM chat. -Do NOT call feishu_drive_add_comment or feishu_drive_reply_comment yourself. Your reply will be posted automatically. Just output the reply text. Use the thread timeline above as the main context. -If the quoted content is not enough, use feishu_doc_read to read nearby context. The quoted content is your primary anchor — insert/summarize/explain requests are about it. Do not guess document content you haven't read. + +If the quote, timeline, and referenced-document metadata above are enough, +output the final reply directly. + +If you need the full text content of one or more documents to reply, output +exactly one line in this form (JSON object) and stop — do NOT include any +reply text in that response: + {"tokens": ["", ""]} +You may only request tokens that appear in the "Current commented document" +section or the "Referenced documents from current user comment" section +above. Non-docx or unknown tokens will be silently dropped. The contents +will be fetched and you will be asked to reply again. + Reply in the same language as the user's comment unless they request otherwise. Use plain text only. Do not use Markdown, headings, bullet lists, tables, or code blocks. Do not show your reasoning process. Do not start with "I will", "Let me", or "I'll first". @@ -896,14 +958,18 @@ def build_local_comment_prompt( target_index: int = -1, referenced_docs: str = "", ) -> str: - """Build the prompt for a local (quoted-text) comment.""" + """Build the prompt for a local (quoted-text) comment. + + All user-originated strings are passed through ``_strip_sentinel`` so a + malicious commenter can't inject a forged ```` marker. + """ selected = _select_local_timeline(timeline, target_index) lines = [ f'The user added a reply in "{doc_title}".', - f'Current user comment text: "{_truncate(target_reply_text)}"', - f'Original comment text: "{_truncate(root_comment_text)}"', - f'Quoted content: "{_truncate(quote_text, 500)}"', + f'Current user comment text: "{_truncate(_strip_sentinel(target_reply_text))}"', + f'Original comment text: "{_truncate(_strip_sentinel(root_comment_text))}"', + f'Quoted content: "{_truncate(_strip_sentinel(quote_text), 500)}"', "This comment mentioned you (@mention is for routing, not task content).", f"Document link: {doc_url}", "Current commented document:", @@ -916,7 +982,7 @@ def build_local_comment_prompt( for user_id, text, is_self in selected: marker = " <-- YOU" if is_self else "" - lines.append(f"[{user_id}] {_truncate(text)}{marker}") + lines.append(f"[{user_id}] {_truncate(_strip_sentinel(text))}{marker}") if referenced_docs: lines.append(referenced_docs) @@ -939,12 +1005,16 @@ def build_whole_comment_prompt( nearest_self_index: int = -1, referenced_docs: str = "", ) -> str: - """Build the prompt for a whole-document comment.""" + """Build the prompt for a whole-document comment. + + All user-originated strings are passed through ``_strip_sentinel`` so a + malicious commenter can't inject a forged ```` marker. + """ selected = _select_whole_timeline(timeline, current_index, nearest_self_index) lines = [ f'The user added a comment in "{doc_title}".', - f'Current user comment text: "{_truncate(comment_text)}"', + f'Current user comment text: "{_truncate(_strip_sentinel(comment_text))}"', "This is a whole-document comment.", "This comment mentioned you (@mention is for routing, not task content).", f"Document link: {doc_url}", @@ -957,7 +1027,7 @@ def build_whole_comment_prompt( for user_id, text, is_self in selected: marker = " <-- YOU" if is_self else "" - lines.append(f"[{user_id}] {_truncate(text)}{marker}") + lines.append(f"[{user_id}] {_truncate(_strip_sentinel(text))}{marker}") if referenced_docs: lines.append(referenced_docs) @@ -995,117 +1065,708 @@ def _resolve_model_and_runtime() -> Tuple[str, dict]: # --------------------------------------------------------------------------- -# Session cache for cross-card memory within the same document +# Session persistence (delegated to hermes's generic SessionStore) +# +# Comment sessions use ``chat_type="doc_comment"`` so they flow through the +# same SessionStore pipeline as IM — inheriting daily-reset, idle-reset, +# token tracking, and SQLite persistence automatically. +# +# Two scoping semantics, both keyed at the document level for ``chat_id``: +# +# Local comment thread_id = comment_id +# key: agent:main:feishu:doc_comment:{file_type}:{file_token}:{comment_id} +# Each comment thread (one root comment + its replies) is isolated. +# +# Whole-doc thread_id = _WHOLE_DOC_SENTINEL_THREAD_ID +# key: agent:main:feishu:doc_comment:{file_type}:{file_token}:__whole_doc__ +# All whole-document comments on the same doc share one session — +# matching the semantic that whole-doc comments form a document-level +# discussion rather than per-thread conversations. +# +# user_id is deliberately not in the key: ``build_session_key`` skips +# per-user isolation when ``thread_id`` is truthy (under the default +# ``thread_sessions_per_user=False``), so we always route through that +# thread-shared branch — the sentinel for whole-doc is also truthy. +# +# Why cross-user sharing is safe here (different from IM): +# +# IM's session-per-user model exists because DMs / threads in IM carry an +# access boundary — user A's DM with the bot is not visible to user B. +# Feishu document comments have the opposite property: whole-document +# comments are inherently public to everyone with document access, and any +# bot reply is likewise visible to every participant. Collapsing all +# whole-doc comments on a document onto one shared session therefore +# mirrors the document's native visibility — it does not leak anything +# across users, because nothing is private across users to begin with. +# +# Consequently: +# +# * No information-asymmetry risk. A cannot "leak" to B via this session +# because A's comments (and the bot's replies) are already visible to B +# in the document itself. +# +# * Cross-user context crosstalk (e.g. the agent treating A's "this part" +# as B's referent) is a quality concern, not a privacy concern — the +# prior context the agent reuses is the same content B can already see +# on the document. It also matches how whole-doc threads naturally +# unfold: later participants pick up where the discussion left off. +# +# * State churn on SessionEntry (token counts, memory_flushed, +# updated_at) is serialized by SessionStore._lock for a given key, so +# concurrent events from different users don't race on SessionEntry +# fields — their effect on state ordering is equivalent to interleaved +# turns from a single stream. +# +# Local-comment threads are a different story and keep their per- +# comment_id isolation above. # --------------------------------------------------------------------------- -import threading -import time as _time +# Sentinel used as thread_id for whole-document comments. Feishu comment_id +# values are alphanumeric strings (typically numeric), so a double-underscore +# literal never collides with a real thread. +_WHOLE_DOC_SENTINEL_THREAD_ID = "__whole_doc__" + + +def _build_comment_session_source( + *, + file_type: str, + file_token: str, + comment_id: str, + is_whole_comment: bool, + from_open_id: str, + doc_title: Optional[str], +) -> "SessionSource": + """Build the SessionSource identifying a comment-thread conversation. + + Whole-doc comments collapse to a single document-level session via the + ``_WHOLE_DOC_SENTINEL_THREAD_ID`` sentinel; local comments stay + isolated per-thread via their ``comment_id``. + """ + from gateway.config import Platform + from gateway.session import SessionSource + + # Prefer a readable display name; fall back to a short token-based stub + # so session listings / logs remain greppable even without title info. + display_name = doc_title or f"{file_type}:{file_token[:8]}" + + # Local: each comment card is its own session. + # Whole-doc: all whole comments on the doc share one session. Using a + # constant sentinel (not None) keeps build_session_key on the + # thread-shared branch, which is what prevents user_id from entering + # the key under the default ``thread_sessions_per_user=False``. + thread_id = ( + _WHOLE_DOC_SENTINEL_THREAD_ID if is_whole_comment else comment_id + ) + + return SessionSource( + platform=Platform.FEISHU, + chat_type="doc_comment", + chat_id=f"{file_type}:{file_token}", + chat_name=display_name, + thread_id=thread_id, + user_id=from_open_id, + ) -_SESSION_MAX_MESSAGES = 50 # keep last N messages per document session -_SESSION_TTL_S = 3600 # expire sessions after 1 hour of inactivity -_session_cache_lock = threading.Lock() -_session_cache: Dict[str, Dict] = {} # key -> {"messages": [...], "last_access": float} +def _load_comment_history( + session_store: Any, session_id: str, +) -> List[Dict[str, Any]]: + """Load this session's prior transcript via the SessionStore public API. + + Goes through ``SessionStore.load_transcript`` rather than poking + ``SessionStore._db`` directly. This keeps doc_comment sessions on + the same storage path as IM / other chat types — in particular: + * JSONL is written in lockstep with SQLite (``append_to_transcript``) + * reads are taken from whichever of JSONL / SQLite is longer, which + guards against silent truncation when a session straddles the + SessionDB introduction (see ``load_transcript`` in gateway/session.py) + * future SessionStore evolutions (encryption, hooks, format changes) + propagate automatically, instead of letting this path drift. + + Returns an empty list if the store raises for any reason — a flaky + transcript layer must not crash the comment handler. + """ + try: + return session_store.load_transcript(session_id) + except Exception as e: + logger.warning( + "[Feishu-Comment] failed to load history for session_id=%s: %s", + session_id, e, + ) + return [] -def _session_key(file_type: str, file_token: str) -> str: - return f"comment-doc:{file_type}:{file_token}" +def _persist_comment_turn( + session_store: Any, + session_id: str, + user_prompt: str, + assistant_reply: str, +) -> None: + """Persist one user→assistant turn via the SessionStore public API. + Same rationale as ``_load_comment_history``: route through + ``append_to_transcript`` so doc_comment sessions share the storage + semantics (SQLite + JSONL dual-write, future evolution, etc.) that + IM and other chat types already rely on. -def _load_session_history(key: str) -> List[Dict[str, Any]]: - """Load conversation history for a document session.""" - with _session_cache_lock: - entry = _session_cache.get(key) - if entry is None: - return [] - # Check TTL - if _time.time() - entry["last_access"] > _SESSION_TTL_S: - del _session_cache[key] - logger.info("[Feishu-Comment] Session expired: %s", key) - return [] - entry["last_access"] = _time.time() - return list(entry["messages"]) + Only final user-visible content is stored; the two-pass sentinel + protocol's intermediate ```` output and fetched-doc + payload are per-turn mechanics, not durable dialogue state, so they + are deliberately omitted to keep future prompts clean. + """ + try: + session_store.append_to_transcript( + session_id, {"role": "user", "content": user_prompt}, + ) + session_store.append_to_transcript( + session_id, {"role": "assistant", "content": assistant_reply}, + ) + except Exception as e: + logger.warning( + "[Feishu-Comment] failed to persist turn for session_id=%s: %s", + session_id, e, + ) -def _save_session_history(key: str, messages: List[Dict[str, Any]]) -> None: - """Save conversation history for a document session (keeps last N messages).""" - # Only keep user/assistant messages (strip system messages and tool internals) - cleaned = [ - m for m in messages - if m.get("role") in ("user", "assistant") and m.get("content") - ] - # Keep last N - if len(cleaned) > _SESSION_MAX_MESSAGES: - cleaned = cleaned[-_SESSION_MAX_MESSAGES:] - with _session_cache_lock: - _session_cache[key] = { - "messages": cleaned, - "last_access": _time.time(), - } - logger.info("[Feishu-Comment] Session saved: %s (%d messages)", key, len(cleaned)) +# Upper bound for persisted user-turn size. A single comment reply shouldn't +# be longer than this; abnormally long text is truncated with a marker so a +# runaway input can't bloat SessionDB rows indefinitely. +_MAX_PERSISTED_USER_TURN_CHARS = 2000 -def _run_comment_agent(prompt: str, client: Any, session_key: str = "") -> str: - """Create an AIAgent with feishu tools and run the prompt. +def _compact_user_turn_for_persistence( + *, + target_reply_text: str, + quote_text: str = "", +) -> str: + """Render a compact user-turn string suitable for SessionDB persistence. + + The live prompt built by ``build_local/whole_comment_prompt`` bundles + timeline, referenced-doc metadata, and instruction boilerplate — all + regenerated from fresh API data on each turn. Persisting that full + prompt would duplicate the evolving timeline into every historical row + (O(n·k) bytes) and drown the transcript in repeated rules. + + This helper keeps only: + * the user's actual comment text (``target_reply_text``) + * an optional quote marker for local comments, so future turns can + still resolve references like "this here" in history replay + + Both user-originated fields are passed through ``_strip_sentinel`` + before persistence. Without this, a commenter could stash a + ```` literal in one turn and have it replayed + unsanitized when the transcript becomes ``conversation_history`` on + later turns — reopening the protocol-injection surface that the + live-prompt path already closes. + + Result is clamped to ``_MAX_PERSISTED_USER_TURN_CHARS`` with a visible + truncation marker — long paste events can't bloat the DB unboundedly. + """ + parts: List[str] = [] + if quote_text: + parts.append(f"[Quoted] {_strip_sentinel(quote_text)}") + if target_reply_text: + parts.append(_strip_sentinel(target_reply_text)) + out = "\n".join(parts) + + if len(out) > _MAX_PERSISTED_USER_TURN_CHARS: + logger.warning( + "[Feishu-Comment] persisted user turn truncated: %d → %d chars", + len(out), _MAX_PERSISTED_USER_TURN_CHARS, + ) + out = ( + out[:_MAX_PERSISTED_USER_TURN_CHARS] + + f"\n[... truncated at {_MAX_PERSISTED_USER_TURN_CHARS} chars]" + ) + return out + + +# --------------------------------------------------------------------------- +# Document-content fetch helpers (business-code equivalents of the v1 +# ``feishu_doc_read`` tool). Callable only from this module's two-pass +# agent orchestration — never exposed as agent tools. +# --------------------------------------------------------------------------- + +_RAW_CONTENT_URI = "/open-apis/docx/v1/documents/:document_id/raw_content" + +# Per-document and aggregate caps. Feishu docs can run tens of thousands of +# characters; we truncate to keep the prompt within sane bounds. The agent +# is told when truncation happened (see ``_format_doc_content_block``). +_MAX_DOC_CHARS = 30_000 +_MAX_TOTAL_DOC_CHARS = 80_000 + + +async def _read_document_raw_content(client: Any, document_id: str) -> str: + """Fetch a Feishu docx's raw plain-text content. + + Raises ``RuntimeError`` on non-zero response codes so that the caller's + ``asyncio.gather(return_exceptions=True)`` converts failures into + exception objects that are rendered into prompt-visible error strings. + """ + code, msg, data = await _exec_request( + client, + "GET", + _RAW_CONTENT_URI, + paths={"document_id": document_id}, + ) + if code != 0: + raise RuntimeError(f"code={code} msg={msg}") + return data.get("content", "") or "" + + +def _truncate_doc_content(content: str, token: str) -> str: + """Truncate a single doc's content to ``_MAX_DOC_CHARS`` and annotate.""" + if len(content) <= _MAX_DOC_CHARS: + return content + logger.warning( + "[Feishu-Comment] truncating doc %s from %d to %d chars", + token, len(content), _MAX_DOC_CHARS, + ) + return ( + content[:_MAX_DOC_CHARS] + + f"\n\n[... truncated at {_MAX_DOC_CHARS} chars;" + f" original length was {len(content)} chars]" + ) + + +def _enforce_total_doc_budget( + contents: Dict[str, str], +) -> Dict[str, str]: + """Scale each doc proportionally if the aggregate exceeds the total cap. + + Preserves per-token ordering. Called after per-doc truncation, so this + only kicks in when many docs are requested at once. + """ + total = sum(len(c) for c in contents.values()) + if total <= _MAX_TOTAL_DOC_CHARS or total == 0: + return contents + ratio = _MAX_TOTAL_DOC_CHARS / total + logger.warning( + "[Feishu-Comment] aggregate doc content %d exceeds cap %d; scaling each by %.2f", + total, _MAX_TOTAL_DOC_CHARS, ratio, + ) + scaled: Dict[str, str] = {} + for token, content in contents.items(): + cut = int(len(content) * ratio) + scaled[token] = content[:cut] + "\n[... further truncated to fit aggregate cap]" + return scaled + + +# Cap on concurrent raw_content fetches. The whitelist already bounds N +# (docs must appear in the current comment context), so N is usually small; +# this semaphore is a cheap insurance against bursty fan-out triggering +# Feishu's per-app rate limit when a comment references many docs. +_DOC_FETCH_CONCURRENCY = 4 + + +async def _fetch_docs_for_agent( + client: Any, tokens: List[str], +) -> Dict[str, str]: + """Fetch raw content for each token in parallel. + + Failures are captured as prompt-ready error strings so the second + agent pass can still proceed (and the model knows which docs failed). + Results are truncated individually and then collectively. + + Concurrency is capped at ``_DOC_FETCH_CONCURRENCY`` to stay friendly + with Feishu's rate limits even if the whitelist admits many tokens. + """ + if not tokens: + return {} + + sem = asyncio.Semaphore(_DOC_FETCH_CONCURRENCY) + + async def _one(token: str) -> str: + async with sem: + try: + content = await _read_document_raw_content(client, token) + except Exception as e: + logger.warning( + "[Feishu-Comment] doc fetch failed token=%s: %s", + token, e, + ) + return f"[Failed to fetch this document: {type(e).__name__}: {e}]" + return _truncate_doc_content(content, token) + + raw = await asyncio.gather(*[_one(t) for t in tokens]) + return _enforce_total_doc_budget(dict(zip(tokens, raw))) + + +# --------------------------------------------------------------------------- +# sentinel protocol +# +# Protocol: the first agent pass either outputs the final reply directly, or +# outputs exactly one line: +# {"tokens": ["token_a", "token_b"]} +# Business code parses that line, fetches the requested (and whitelisted) +# docs, and runs a second pass with the content appended to the prompt. +# --------------------------------------------------------------------------- + +# Matches the sentinel tag followed by a JSON object. Uses DOTALL so embedded +# newlines inside the JSON (unlikely but legal) don't break the match. +_NEED_DOC_READ_PATTERN = re.compile(r"\s*(\{.*?\})", re.DOTALL) + +# Any occurrence of the bare sentinel literal — used to neutralize it inside +# user-originated or fetched-doc text so an attacker can't forge the protocol +# marker from within prompt-embedded content. +_NEED_DOC_READ_LITERAL = re.compile(r"", re.IGNORECASE) +_SENTINEL_PLACEHOLDER = "" + + +def _strip_sentinel(text: str) -> str: + """Replace any ```` literal inside untrusted text. + + Applied to every user-originated string (comment text, quotes, timeline + entries) and to fetched document content before they are interpolated + into a prompt. This prevents an attacker who controls a comment or a + whitelisted doc from forging the sentinel protocol marker. + + The replacement is a visible placeholder rather than an empty string so + that the substitution is greppable in logs if something goes wrong. + """ + if not text: + return text + return _NEED_DOC_READ_LITERAL.sub(_SENTINEL_PLACEHOLDER, text) + - If *session_key* is provided, loads/saves conversation history for - cross-card memory within the same document. +def _extract_effective_doc_token(link: Dict[str, Any]) -> Tuple[str, str]: + """Return the (effective_type, effective_token) for a referenced doc link. - Returns the agent's final response text, or empty string on failure. + After ``_resolve_wiki_nodes`` runs, wiki links carry ``resolved_type`` / + ``resolved_token`` pointing to the real underlying doc. Prefer those. + Returns empty strings for links we couldn't resolve. + """ + resolved_type = link.get("resolved_type") or "" + resolved_token = link.get("resolved_token") or "" + if resolved_type and resolved_token: + return resolved_type, resolved_token + return link.get("doc_type") or "", link.get("token") or "" + + +def _build_doc_token_whitelist( + source_file_type: str, + source_file_token: str, + referenced_links: List[Dict[str, Any]], +) -> Set[str]: + """Collect docx tokens that the agent is allowed to request for reading. + + Only docx is whitelisted — the raw-content API is docx-only. The source + document is whitelisted when it's docx; all referenced links that + resolve to docx are added as well. + """ + whitelist: Set[str] = set() + if source_file_type == "docx" and source_file_token: + whitelist.add(source_file_token) + for link in referenced_links or []: + eff_type, eff_token = _extract_effective_doc_token(link) + if eff_type == "docx" and eff_token: + whitelist.add(eff_token) + return whitelist + + +@dataclass +class SentinelParseResult: + """Structured outcome of ```` sentinel parsing. + + Three mutually-exclusive states for the first-pass response: + + - ``has_sentinel=False``: no sentinel literal in the response at all. + The response is the user-facing reply and should be delivered. + - ``has_sentinel=True``, ``accepted_tokens`` non-empty: the agent + asked for docs and the whitelist accepted at least one — the + caller runs the second pass. + - ``has_sentinel=True``, ``accepted_tokens`` empty: the agent emitted + a sentinel, but its payload was malformed JSON, had no ``tokens`` + list, or every requested token was dropped by the whitelist. The + caller MUST NOT return the raw first-pass response to the user — + it contains the sentinel literal, which is internal protocol + plumbing and must never be delivered. + """ + has_sentinel: bool + accepted_tokens: List[str] + + +def _parse_need_doc_read_sentinel( + response: str, whitelist: Set[str], +) -> SentinelParseResult: + """Parse the first-pass response for a ```` sentinel. + + Detection and extraction are deliberately split so that malformed + variants of the marker (bare ````, space-separated + token lists, marker embedded in natural-language hedging, etc.) + are still correctly identified as sentinel turns instead of being + silently treated as the final reply. + + Two-step algorithm: + + 1. **Detection** — ``_NEED_DOC_READ_LITERAL`` (broad, case-insensitive + literal search). If the marker appears anywhere in ``response``, + this *is* a sentinel turn, no matter what follows. Returning + ``has_sentinel=False`` when the literal is present was the bug + that allowed the marker to leak to the user as visible text. + + 2. **Extraction** — ``_NEED_DOC_READ_PATTERN`` (strict, JSON-gated). + Only used to pull out the ``{"tokens": [...]}`` payload. A missing + or malformed payload degrades to ``accepted_tokens=[]``, not to + ``has_sentinel=False``. + + See ``SentinelParseResult`` for the three return states. Non-whitelist + (including non-docx) tokens are logged and dropped so business code + only ever fetches docs the agent had advance knowledge of via the + prompt metadata. + """ + # Step 1: detection — literal present anywhere? + if not _NEED_DOC_READ_LITERAL.search(response): + return SentinelParseResult(has_sentinel=False, accepted_tokens=[]) + + # Step 2: extraction — try to pull the JSON payload. + match = _NEED_DOC_READ_PATTERN.search(response) + if not match: + logger.warning( + "[Feishu-Comment] literal present but no JSON " + "payload follows (len=%d); treating as no-token sentinel", + len(response), + ) + return SentinelParseResult(has_sentinel=True, accepted_tokens=[]) + + payload_raw = match.group(1) + try: + payload = json.loads(payload_raw) + except json.JSONDecodeError as e: + logger.warning( + "[Feishu-Comment] sentinel JSON parse failed: %s (payload=%r)", + e, payload_raw[:200], + ) + return SentinelParseResult(has_sentinel=True, accepted_tokens=[]) + tokens = payload.get("tokens") + if not isinstance(tokens, list): + return SentinelParseResult(has_sentinel=True, accepted_tokens=[]) + + accepted: List[str] = [] + rejected: List[str] = [] + for t in tokens: + if isinstance(t, str) and t in whitelist: + if t not in accepted: # preserve order, drop duplicates + accepted.append(t) + else: + rejected.append(t) + if rejected: + logger.warning( + "[Feishu-Comment] Dropping %d tokens not in whitelist: %s", + len(rejected), rejected, + ) + return SentinelParseResult(has_sentinel=True, accepted_tokens=accepted) + + +def _format_doc_content_block(contents: Dict[str, str]) -> str: + """Render fetched doc contents for injection into the second-pass prompt. + + Output deliberately forbids further ```` on the second + turn: we already gave the agent everything it asked for. Looping would + risk non-termination. + + Doc authors can edit doc content, so the fetched body is untrusted and + passed through ``_strip_sentinel`` — this neutralizes forged protocol + markers even though business code no longer parses the sentinel on the + second pass (defense in depth). + """ + lines = ["", "---", "Fetched document contents:"] + for token, content in contents.items(): + lines.append("") + lines.append(f"[Document token: {token}]") + lines.append(_strip_sentinel(content)) + lines.append("") + lines.append("---") + lines.append( + "Use the above content to generate the final reply. " + "You MUST produce the final user-facing reply now. " + " is no longer accepted in this turn." + ) + return "\n".join(lines) + + +# --------------------------------------------------------------------------- +# Two-pass comment agent orchestration +# --------------------------------------------------------------------------- + + +def _build_comment_agent(runtime_kwargs: Dict[str, Any], model: str) -> Any: + """Construct the AIAgent used for a single comment event. + + The agent is given **no feishu tools** — document content flows in via + the ```` sentinel protocol, not via tool calls. + + ``persist_session=False`` is critical: the durable transcript for a + comment thread goes through ``SessionStore.append_to_transcript`` in + *compact* form (user's actual reply text + optional quote anchor; + see ``_compact_user_turn_for_persistence``). With the default + ``persist_session=True``, ``AIAgent._persist_session`` would in + parallel write the helper agent's full in-memory message list to + ``~/.hermes/logs/session_{id}.json`` and to its own SessionDB — + re-leaking the first-pass rendered prompt (timeline, quote, doc URL) + and the second-pass fetched document bodies that this module is + explicitly designed to keep off durable storage. + + Known residual: ``AIAgent._save_session_log`` is also called directly + from a handful of edge paths in ``run_agent.py`` (length continuation, + certain retry failures) that bypass ``_persist_session`` and therefore + ignore this flag. Closing that gap requires a change in + ``run_agent.py`` itself (making ``_save_session_log`` honour + ``self.persist_session``) and is deliberately out of this module's + scope. """ from run_agent import AIAgent - logger.info("[Feishu-Comment] _run_comment_agent: injecting lark client into tool thread-locals") - from tools.feishu_doc_tool import set_client as set_doc_client - from tools.feishu_drive_tool import set_client as set_drive_client - set_doc_client(client) - set_drive_client(client) + return AIAgent( + model=model, + base_url=runtime_kwargs.get("base_url"), + api_key=runtime_kwargs.get("api_key"), + provider=runtime_kwargs.get("provider"), + api_mode=runtime_kwargs.get("api_mode"), + credential_pool=runtime_kwargs.get("credential_pool"), + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + # No tools are enabled; the two-pass sentinel protocol gets at most + # two model turns total (first pass + optional second pass after + # doc-content injection), so a tiny iteration budget is enough. + max_iterations=2, + enabled_toolsets=[], + persist_session=False, + ) + + +def _run_first_pass( + agent: Any, prompt: str, history: List[Dict[str, Any]], +) -> Tuple[str, Dict[str, Any]]: + """First agent turn — returns (response_text, raw_result).""" + logger.info( + "[Feishu-Comment] first pass: prompt=%d chars, history=%d", + len(prompt), len(history), + ) + result = agent.run_conversation( + prompt, conversation_history=history or None, + ) + response = (result.get("final_response") or "").strip() + # Response may contain user-visible reply text or the NEED_DOC_READ + # sentinel; don't log its body. + logger.info( + "[Feishu-Comment] first pass done: api_calls=%d response_len=%d", + result.get("api_calls", 0), len(response), + ) + return response, result + + +def _run_second_pass( + agent: Any, + client: Any, + requested_tokens: List[str], + *, + prior_history: List[Dict[str, Any]], + first_prompt: str, + first_response: str, +) -> Tuple[str, Dict[str, Any]]: + """Fetch requested docs and run the second agent turn. + + ``AIAgent.run_conversation`` reinitializes its message list from + ``conversation_history`` on every call, so reusing the same agent + instance does NOT retain the first-pass context. We explicitly + rebuild history as: + + prior_history + [first_pass_user, first_pass_assistant] + doc_block + + so turn 2 still sees the user's original question, quote, timeline, + and the agent's own prior ```` line — the doc content + block alone is not enough to reply coherently. + """ + logger.info( + "[Feishu-Comment] second pass: fetching %d docs: %s", + len(requested_tokens), requested_tokens, + ) + doc_contents = asyncio.run(_fetch_docs_for_agent(client, requested_tokens)) + doc_block = _format_doc_content_block(doc_contents) + + second_history: List[Dict[str, Any]] = list(prior_history) + [ + {"role": "user", "content": first_prompt}, + {"role": "assistant", "content": first_response}, + ] + result = agent.run_conversation( + doc_block, conversation_history=second_history, + ) + response = (result.get("final_response") or "").strip() + logger.info( + "[Feishu-Comment] second pass done: api_calls=%d response_len=%d", + result.get("api_calls", 0), len(response), + ) + return response, result + +def _run_comment_agent( + prompt: str, + client: Any, + doc_token_whitelist: Set[str], + history: List[Dict[str, Any]], +) -> str: + """Run the comment agent with the two-pass sentinel protocol. + + Step 1: invoke the agent with ``prompt`` and the caller-provided + ``history`` (already loaded from SessionStore). + Step 2: parse the response for ````. If absent, the + first-pass response is the final reply. + Step 3: if present, fetch the whitelisted tokens and run a second + agent turn with the doc contents appended. + + Persistence is the caller's responsibility — this function returns the + final reply text (or empty string on failure) and leaves history I/O + to ``handle_drive_comment_event`` which holds the ``SessionStore``. + """ try: model, runtime_kwargs = _resolve_model_and_runtime() - logger.info("[Feishu-Comment] _run_comment_agent: model=%s provider=%s base_url=%s", - model, runtime_kwargs.get("provider"), (runtime_kwargs.get("base_url") or "")[:50]) - - # Load session history for cross-card memory - history = _load_session_history(session_key) if session_key else [] - if history: - logger.info("[Feishu-Comment] _run_comment_agent: loaded %d history messages from session %s", - len(history), session_key) - - agent = AIAgent( - model=model, - base_url=runtime_kwargs.get("base_url"), - api_key=runtime_kwargs.get("api_key"), - provider=runtime_kwargs.get("provider"), - api_mode=runtime_kwargs.get("api_mode"), - credential_pool=runtime_kwargs.get("credential_pool"), - quiet_mode=True, - skip_context_files=True, - skip_memory=True, - max_iterations=15, - enabled_toolsets=["feishu_doc", "feishu_drive"], + logger.info( + "[Feishu-Comment] _run_comment_agent: model=%s provider=%s base_url=%s history=%d", + model, runtime_kwargs.get("provider"), + (runtime_kwargs.get("base_url") or "")[:50], len(history), ) - logger.info("[Feishu-Comment] _run_comment_agent: calling run_conversation (prompt=%d chars, history=%d)", - len(prompt), len(history)) - result = agent.run_conversation(prompt, conversation_history=history or None) - response = (result.get("final_response") or "").strip() - api_calls = result.get("api_calls", 0) - logger.info("[Feishu-Comment] _run_comment_agent: done api_calls=%d response_len=%d response=%s", - api_calls, len(response), response[:200]) - - # Save updated history - if session_key: - new_messages = result.get("messages", []) - if new_messages: - _save_session_history(session_key, new_messages) + agent = _build_comment_agent(runtime_kwargs, model) + + # First pass + first_response, _ = _run_first_pass(agent, prompt, history) + parse = _parse_need_doc_read_sentinel(first_response, doc_token_whitelist) + + if not parse.has_sentinel: + # No sentinel at all — first-pass response IS the final reply. + return first_response + + if not parse.accepted_tokens: + # Sentinel was emitted, but its payload was malformed / had no + # valid tokens / every token was dropped by the whitelist. The + # raw first-pass response contains the sentinel literal, which + # is internal protocol text — it must NEVER be delivered to the + # user. Coerce to the NO_REPLY path by returning "". + logger.warning( + "[Feishu-Comment] first-pass emitted but no " + "tokens survived parse/whitelist — dropping first-pass output " + "to prevent protocol leak (response_len=%d)", + len(first_response), + ) + return "" + + # Second pass: doc contents requested. Pass the first-pass prompt + # and response so ``_run_second_pass`` can explicitly reconstruct + # ``conversation_history`` — otherwise turn 2 loses the user's + # original question, quote, and timeline. + response, _ = _run_second_pass( + agent, client, parse.accepted_tokens, + prior_history=history, + first_prompt=prompt, + first_response=first_response, + ) return response + except Exception as e: - logger.exception("[Feishu-Comment] _run_comment_agent: agent failed: %s", e) + logger.exception("[Feishu-Comment] _run_comment_agent failed: %s", e) return "" - finally: - set_doc_client(None) - set_drive_client(None) # --------------------------------------------------------------------------- @@ -1115,21 +1776,75 @@ def _run_comment_agent(prompt: str, client: Any, session_key: str = "") -> str: _NO_REPLY_SENTINEL = "NO_REPLY" +def _gate_outbound_reply(response: Optional[str]) -> Optional[str]: + """Single gate for any text about to be posted back to Feishu. + + Invariant this function enforces: + *The returned string, if non-None, never contains the* + ```` *literal and is not the* ``NO_REPLY`` *signal.* + + Every delivery path must funnel its agent-produced text through this + gate. Adding a new delivery path without calling it re-opens the + leak window this module has closed repeatedly — the historical bugs + all followed the pattern "spot a new malformed-marker variant, add a + new regex check". Centralising the guarantee as a single boolean + invariant (literal-in-text → refuse) means every future variant is + covered without adding another regex. + + Returns: + The ``response`` unchanged when it is safe to deliver, or + ``None`` meaning "skip delivery". ``None`` is returned when: + + * the response is empty / whitespace only; + * the response contains the ``NO_REPLY`` sentinel (agent chose + not to reply); + * the response contains the ```` literal anywhere + (a malformed sentinel escaped parsing, or the second pass + ignored the "no more sentinel" instruction). Refusing to + deliver is strictly safer than shipping a half-formed marker + to the comment thread. + """ + if not response: + return None + stripped = response.strip() + if not stripped: + return None + if _NO_REPLY_SENTINEL in response: + return None + if _NEED_DOC_READ_LITERAL.search(response): + logger.error( + "[Feishu-Comment] Refusing delivery: response contains " + " literal (len=%d)", + len(response), + ) + return None + return response + + _ALLOWED_NOTICE_TYPES = {"add_comment", "add_reply"} async def handle_drive_comment_event( - client: Any, data: Any, *, self_open_id: str = "", + ctx: CommentContext, data: Any, ) -> None: """Full orchestration for a drive comment event. + *ctx* bundles the lark client, the gateway SessionStore, and the bot's + own open_id (see ``CommentContext``). The caller constructs it via + ``CommentContext.from_adapter(adapter, self_open_id=...)``; the handler + itself never touches adapter internals. + 1. Parse event + filter (self-reply, notice_type) 2. Add OK reaction 3. Fetch doc meta + comment details in parallel 4. Branch on is_whole: build timeline - 5. Build prompt, run agent - 6. Deliver reply + 5. Build prompt, run agent (history from SessionStore) + 6. Deliver reply + persist user/assistant turn to SessionStore """ + client = ctx.client + session_store = ctx.session_store + self_open_id = ctx.self_open_id + logger.info("[Feishu-Comment] ========== handle_drive_comment_event START ==========") parsed = parse_drive_comment_event(data) if parsed is None: @@ -1144,8 +1859,9 @@ async def handle_drive_comment_event( from_open_id = parsed["from_open_id"] to_open_id = parsed["to_open_id"] notice_type = parsed["notice_type"] + is_mentioned = parsed["is_mentioned"] - # Filter: self-reply, receiver check, notice_type + # Filter: self-reply, receiver check, notice_type, is_mentioned. if from_open_id and self_open_id and from_open_id == self_open_id: logger.debug("[Feishu-Comment] Skipping self-authored event: from=%s", from_open_id) return @@ -1155,6 +1871,17 @@ async def handle_drive_comment_event( if notice_type and notice_type not in _ALLOWED_NOTICE_TYPES: logger.debug("[Feishu-Comment] Skipping notice_type=%s", notice_type) return + # ``is_mentioned`` is the authoritative signal that the user explicitly + # @-ed the bot. Without this gate, any comment on a document the bot + # was previously invited to (and any reply in a thread the bot + # participated in) would route here — a noisy, privacy-adverse + # behavior. Drop events that lack an explicit mention. + if not is_mentioned: + logger.debug( + "[Feishu-Comment] Skipping unmentioned event: comment=%s from=%s", + comment_id, from_open_id, + ) + return if not file_token or not file_type or not comment_id: logger.warning("[Feishu-Comment] Missing required fields, skipping") return @@ -1252,9 +1979,10 @@ async def handle_drive_comment_event( current_index = i break - logger.info("[Feishu-Comment] Whole timeline: %d entries, current_idx=%d, self_idx=%d, text=%s", + # current_text is the user's comment; log indices and length only. + logger.info("[Feishu-Comment] Whole timeline: %d entries, current_idx=%d, self_idx=%d, current_len=%d", len(timeline), current_index, nearest_self_index, - current_text[:80] if current_text else "(empty)") + len(current_text) if current_text else 0) # Extract and resolve document links from all replies all_raw_replies = [] @@ -1283,6 +2011,11 @@ async def handle_drive_comment_event( nearest_self_index=nearest_self_index, referenced_docs=ref_docs_text, ) + # Persistence-only view: just the user's current whole-doc comment + # text. Whole-doc has no per-anchor quote, so the quote segment is + # empty. + persist_user_text = current_text + persist_quote_text = "" else: # Local comment: fetch the comment thread replies @@ -1317,11 +2050,13 @@ async def handle_drive_comment_event( target_index = i break - logger.info("[Feishu-Comment] Local timeline: %d entries, target_idx=%d, quote=%s root=%s target=%s", + # quote/root/target are user/agent content — log lengths only. + logger.info("[Feishu-Comment] Local timeline: %d entries, target_idx=%d, " + "quote_len=%d root_len=%d target_len=%d", len(timeline), target_index, - quote_text[:60] if quote_text else "(empty)", - root_text[:60] if root_text else "(empty)", - target_text[:60] if target_text else "(empty)") + len(quote_text) if quote_text else 0, + len(root_text) if root_text else 0, + len(target_text) if target_text else 0) # Extract and resolve document links from replies doc_links = _extract_docs_links(replies) @@ -1343,41 +2078,107 @@ async def handle_drive_comment_event( target_index=target_index, referenced_docs=ref_docs_text, ) - + # Persistence-only view: the user's actual reply text plus the + # quote they anchored to (if any). Everything else in ``prompt`` + # (timeline, referenced-doc metadata, instructions) is rebuilt + # from fresh API data on each turn and must not enter history. + persist_user_text = target_text + persist_quote_text = quote_text + + # Prompt contains the full quote + timeline — never log its content, even + # at DEBUG, because agent.log's per-level threshold is configurable and a + # misconfigured deployment would expose user comments to any log reader. logger.info("[Feishu-Comment] [Step 4/5] Prompt built (%d chars), running agent...", len(prompt)) - logger.debug("[Feishu-Comment] Full prompt:\n%s", prompt) - # Step 4: Run agent in a thread (run_conversation is synchronous) - # Session key groups all comment cards on the same document - sess_key = _session_key(file_type, file_token) + # Build the whitelist of document tokens the agent may request via the + # sentinel: the source document (if docx) plus any + # referenced docs resolved to docx. Non-docx tokens cannot be read via + # the raw_content API, so we never whitelist them. + doc_token_whitelist = _build_doc_token_whitelist( + file_type, file_token, doc_links, + ) + + # Resolve the per-comment-thread session via hermes's generic SessionStore. + # Falls back to a stateless turn (history=[]) if the gateway didn't wire + # a session_store into the adapter — this keeps tests and degraded + # runtimes working without crashing. + session_entry = None + history: List[Dict[str, Any]] = [] + if session_store is not None: + source = _build_comment_session_source( + file_type=file_type, + file_token=file_token, + comment_id=comment_id, + is_whole_comment=is_whole, + from_open_id=from_open_id, + doc_title=doc_title, + ) + session_entry = session_store.get_or_create_session(source) + history = _load_comment_history(session_store, session_entry.session_id) + logger.info( + "[Feishu-Comment] session resolved: key=%s id=%s history=%d", + session_entry.session_key, session_entry.session_id, len(history), + ) + else: + logger.info( + "[Feishu-Comment] no session_store on adapter — running stateless turn", + ) + + # Step 4: Run agent in a thread (run_conversation is synchronous). loop = asyncio.get_running_loop() response = await loop.run_in_executor( - None, _run_comment_agent, prompt, client, sess_key, + None, _run_comment_agent, prompt, client, doc_token_whitelist, history, ) - if not response or _NO_REPLY_SENTINEL in response: - logger.info("[Feishu-Comment] Agent returned NO_REPLY, skipping delivery") + # Funnel the agent's response through the single outbound gate. The + # gate enforces the invariant "delivered text never contains the + # literal and is not NO_REPLY" — see + # ``_gate_outbound_reply`` for why this is the one place to check. + delivery_text = _gate_outbound_reply(response) + + if delivery_text is None: + logger.info("[Feishu-Comment] No reply delivered (empty / NO_REPLY / gated)") else: - logger.info("[Feishu-Comment] Agent response (%d chars): %s", len(response), response[:200]) + # Agent response is the final user-visible reply — log length only. + logger.info("[Feishu-Comment] Agent response: %d chars", len(delivery_text)) # Step 5: Deliver reply logger.info("[Feishu-Comment] [Step 5/5] Delivering reply (is_whole=%s, comment_id=%s)", is_whole, comment_id) success = await deliver_comment_reply( - client, file_token, file_type, comment_id, response, is_whole, + client, file_token, file_type, comment_id, delivery_text, is_whole, ) if success: logger.info("[Feishu-Comment] Reply delivered successfully") + # Persist only on successful delivery: a failed delivery means the + # user never saw the reply, so treating it as "didn't happen" in + # the transcript avoids confusing future turns. + if session_entry is not None and session_store is not None: + # Persist only the semantic payload, not the full rendered + # prompt — see ``_compact_user_turn_for_persistence``. + user_turn = _compact_user_turn_for_persistence( + target_reply_text=persist_user_text, + quote_text=persist_quote_text, + ) + _persist_comment_turn( + session_store, session_entry.session_id, + user_prompt=user_turn, assistant_reply=delivery_text, + ) else: logger.error("[Feishu-Comment] Failed to deliver reply") - # Cleanup: remove OK reaction (best-effort, non-blocking) + # Cleanup: remove OK reaction (best-effort, fire-and-forget). + # Mirrors the add-reaction call at the start of the handler — we don't + # want this extra round-trip to Feishu to hold the event loop when the + # reply has already been delivered. if reply_id: - await delete_comment_reaction( - client, - file_token=file_token, - file_type=file_type, - reply_id=reply_id, - reaction_type="OK", + asyncio.ensure_future( + delete_comment_reaction( + client, + file_token=file_token, + file_type=file_type, + reply_id=reply_id, + reaction_type="OK", + ) ) logger.info("[Feishu-Comment] ========== handle_drive_comment_event END ==========") diff --git a/gateway/platforms/feishu_comment_rules.py b/gateway/platforms/feishu_comment_rules.py index 054ef9569898..0bf5fc54f5e0 100644 --- a/gateway/platforms/feishu_comment_rules.py +++ b/gateway/platforms/feishu_comment_rules.py @@ -11,6 +11,8 @@ import json import logging +import os +import stat import time from dataclasses import dataclass, field from pathlib import Path @@ -237,6 +239,19 @@ def _save_pairing(data: dict) -> None: tmp = PAIRING_FILE.with_suffix(".tmp") with open(tmp, "w", encoding="utf-8") as f: json.dump(data, f, indent=2, ensure_ascii=False) + # Restrict to owner-only rw BEFORE rename so there's never a window + # where the final file exists with the default umask permissions. + # The pairing file lists the open_ids allow-listed to @-mention the + # bot on documents — leaking it hands an attacker a targeting list. + try: + os.chmod(tmp, stat.S_IRUSR | stat.S_IWUSR) + except OSError as e: + # Best-effort: on filesystems / platforms where chmod is a no-op + # (e.g. certain Windows setups), log and continue. Don't block + # the write — correctness beats ideal permissions. + logger.warning( + "[Feishu-Rules] chmod 0600 on pairing tmp file failed: %s", e, + ) tmp.replace(PAIRING_FILE) # Invalidate cache so next load picks up change _pairing_cache._mtime = 0.0 diff --git a/tests/gateway/test_feishu_comment.py b/tests/gateway/test_feishu_comment.py index 0a09481ac8c2..b3eab50ab645 100644 --- a/tests/gateway/test_feishu_comment.py +++ b/tests/gateway/test_feishu_comment.py @@ -7,12 +7,22 @@ from unittest.mock import AsyncMock, Mock, patch from gateway.platforms.feishu_comment import ( + CommentContext, parse_drive_comment_event, _ALLOWED_NOTICE_TYPES, _sanitize_comment_text, ) +def _make_ctx(client=None, session_store=None, self_open_id="ou_bot") -> CommentContext: + """Build a CommentContext for tests — defaults to a bare Mock client.""" + return CommentContext( + client=client if client is not None else Mock(), + session_store=session_store, + self_open_id=self_open_id, + ) + + def _make_event( comment_id="c1", reply_id="r1", @@ -61,7 +71,7 @@ class TestEventFiltering(unittest.TestCase): """Test the filtering logic in handle_drive_comment_event.""" def _run(self, coro): - return asyncio.get_event_loop().run_until_complete(coro) + return asyncio.run(coro) @patch("gateway.platforms.feishu_comment_rules.load_config") @patch("gateway.platforms.feishu_comment_rules.resolve_rule") @@ -71,7 +81,7 @@ def test_self_reply_filtered(self, mock_allowed, mock_resolve, mock_load): from gateway.platforms.feishu_comment import handle_drive_comment_event evt = _make_event(from_open_id="ou_bot", to_open_id="ou_bot") - self._run(handle_drive_comment_event(Mock(), evt, self_open_id="ou_bot")) + self._run(handle_drive_comment_event(_make_ctx(), evt)) mock_load.assert_not_called() @patch("gateway.platforms.feishu_comment_rules.load_config") @@ -82,7 +92,7 @@ def test_wrong_receiver_filtered(self, mock_allowed, mock_resolve, mock_load): from gateway.platforms.feishu_comment import handle_drive_comment_event evt = _make_event(to_open_id="ou_other_bot") - self._run(handle_drive_comment_event(Mock(), evt, self_open_id="ou_bot")) + self._run(handle_drive_comment_event(_make_ctx(), evt)) mock_load.assert_not_called() @patch("gateway.platforms.feishu_comment_rules.load_config") @@ -93,7 +103,7 @@ def test_empty_to_open_id_filtered(self, mock_allowed, mock_resolve, mock_load): from gateway.platforms.feishu_comment import handle_drive_comment_event evt = _make_event(to_open_id="") - self._run(handle_drive_comment_event(Mock(), evt, self_open_id="ou_bot")) + self._run(handle_drive_comment_event(_make_ctx(), evt)) mock_load.assert_not_called() @patch("gateway.platforms.feishu_comment_rules.load_config") @@ -104,7 +114,23 @@ def test_invalid_notice_type_filtered(self, mock_allowed, mock_resolve, mock_loa from gateway.platforms.feishu_comment import handle_drive_comment_event evt = _make_event(notice_type="resolve_comment") - self._run(handle_drive_comment_event(Mock(), evt, self_open_id="ou_bot")) + self._run(handle_drive_comment_event(_make_ctx(), evt)) + mock_load.assert_not_called() + + @patch("gateway.platforms.feishu_comment_rules.load_config") + @patch("gateway.platforms.feishu_comment_rules.resolve_rule") + @patch("gateway.platforms.feishu_comment_rules.is_user_allowed") + def test_not_mentioned_filtered(self, mock_allowed, mock_resolve, mock_load): + """Events without an explicit @-mention of the bot must be dropped. + + Without this gate, any comment on a doc the bot was ever invited + to (and any reply in a thread the bot has touched) would route + to the handler — noisy and privacy-adverse. + """ + from gateway.platforms.feishu_comment import handle_drive_comment_event + + evt = _make_event(is_mentioned=False) + self._run(handle_drive_comment_event(_make_ctx(), evt)) mock_load.assert_not_called() def test_allowed_notice_types(self): @@ -115,7 +141,7 @@ def test_allowed_notice_types(self): class TestAccessControlIntegration(unittest.TestCase): def _run(self, coro): - return asyncio.get_event_loop().run_until_complete(coro) + return asyncio.run(coro) @patch("gateway.platforms.feishu_comment_rules.has_wiki_keys", return_value=False) @patch("gateway.platforms.feishu_comment_rules.is_user_allowed", return_value=False) @@ -129,11 +155,14 @@ def test_denied_user_no_side_effects(self, mock_load, mock_resolve, mock_allowed mock_resolve.return_value = ResolvedCommentRule(True, "allowlist", frozenset(), "top") mock_load.return_value = Mock() + # Build a ctx with an explicit client so we can assert no API calls + # were issued against it for the denied user. client = Mock() + ctx = _make_ctx(client=client) evt = _make_event() - self._run(handle_drive_comment_event(client, evt, self_open_id="ou_bot")) + self._run(handle_drive_comment_event(ctx, evt)) - # No API calls should be made for denied users + # No API calls should be made for denied users. client.request.assert_not_called() @patch("gateway.platforms.feishu_comment_rules.has_wiki_keys", return_value=False) @@ -149,7 +178,7 @@ def test_disabled_comment_skipped(self, mock_load, mock_resolve, mock_allowed, m mock_load.return_value = Mock() evt = _make_event() - self._run(handle_drive_comment_event(Mock(), evt, self_open_id="ou_bot")) + self._run(handle_drive_comment_event(_make_ctx(), evt)) mock_allowed.assert_not_called() @@ -183,7 +212,7 @@ def test_code_snippet(self): class TestWikiReverseLookup(unittest.TestCase): def _run(self, coro): - return asyncio.get_event_loop().run_until_complete(coro) + return asyncio.run(coro) @patch("gateway.platforms.feishu_comment._exec_request") def test_reverse_lookup_success(self, mock_exec): @@ -246,7 +275,7 @@ def test_wiki_lookup_triggered_when_no_exact_match( evt = _make_event() # Will proceed past access control but fail later — that's OK, we just test the lookup try: - self._run(handle_drive_comment_event(Mock(), evt, self_open_id="ou_bot")) + self._run(handle_drive_comment_event(_make_ctx(), evt)) except Exception: pass @@ -257,5 +286,865 @@ def test_wiki_lookup_triggered_when_no_exact_match( self.assertEqual(second_call_kwargs[1].get("wiki_token") or second_call_kwargs[0][3], "WIKI123") +class TestDocTokenWhitelist(unittest.TestCase): + """``_build_doc_token_whitelist`` — only docx tokens get fetch privilege.""" + + def test_source_docx_included(self): + from gateway.platforms.feishu_comment import _build_doc_token_whitelist + + wl = _build_doc_token_whitelist("docx", "src_token", []) + self.assertEqual(wl, {"src_token"}) + + def test_source_non_docx_excluded(self): + from gateway.platforms.feishu_comment import _build_doc_token_whitelist + + # raw_content API only supports docx; non-docx sources must not be + # whitelisted since we can't fetch them anyway. + wl = _build_doc_token_whitelist("sheet", "src_token", []) + self.assertEqual(wl, set()) + + def test_referenced_docx_links_included(self): + from gateway.platforms.feishu_comment import _build_doc_token_whitelist + + links = [ + {"url": "u1", "doc_type": "docx", "token": "doc_a"}, + {"url": "u2", "doc_type": "sheet", "token": "sheet_b"}, + ] + wl = _build_doc_token_whitelist("docx", "src", links) + self.assertEqual(wl, {"src", "doc_a"}) + + def test_resolved_wiki_link_uses_resolved_token(self): + from gateway.platforms.feishu_comment import _build_doc_token_whitelist + + # Wiki links get resolved_type/resolved_token after + # ``_resolve_wiki_nodes``; the resolved values win over the raw + # wiki token so we fetch the underlying doc, not the wiki wrapper. + links = [{ + "url": "w1", + "doc_type": "wiki", + "token": "wiki_tok", + "resolved_type": "docx", + "resolved_token": "real_docx", + }] + wl = _build_doc_token_whitelist("docx", "src", links) + self.assertEqual(wl, {"src", "real_docx"}) + + def test_resolved_wiki_non_docx_excluded(self): + from gateway.platforms.feishu_comment import _build_doc_token_whitelist + + links = [{ + "url": "w1", + "doc_type": "wiki", + "token": "wiki_tok", + "resolved_type": "sheet", + "resolved_token": "real_sheet", + }] + wl = _build_doc_token_whitelist("docx", "src", links) + self.assertEqual(wl, {"src"}) + + +class TestSentinelParsing(unittest.TestCase): + """``_parse_need_doc_read_sentinel`` — JSON sentinel + whitelist guard.""" + + def test_no_sentinel_returns_has_sentinel_false(self): + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + + result = _parse_need_doc_read_sentinel("just a regular reply", {"t1"}) + self.assertFalse(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_sentinel_with_valid_tokens(self): + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + + resp = '{"tokens": ["t1", "t2"]}' + result = _parse_need_doc_read_sentinel(resp, {"t1", "t2", "t3"}) + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, ["t1", "t2"]) + + def test_hallucinated_tokens_partially_dropped(self): + """Tokens not in the whitelist must be dropped; surviving ones kept.""" + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + + resp = '{"tokens": ["t1", "HALLUC", "t2"]}' + result = _parse_need_doc_read_sentinel(resp, {"t1", "t2"}) + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, ["t1", "t2"]) + + def test_all_tokens_hallucinated_keeps_has_sentinel_true(self): + """Sentinel present but every token dropped → has_sentinel stays True. + + This is the critical distinction that prevents the first-pass + response (which still contains the sentinel literal) from being + delivered to the user as if it were the final reply. + """ + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + + resp = '{"tokens": ["HAL1", "HAL2"]}' + result = _parse_need_doc_read_sentinel(resp, {"t1"}) + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_malformed_json_keeps_has_sentinel_true(self): + """Malformed sentinel payload → has_sentinel True, tokens empty. + + Same reasoning as the hallucination case: the sentinel literal is + in the response body, so the caller must NOT fall back to + delivering the raw response. + """ + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + + resp = '{not json}' + result = _parse_need_doc_read_sentinel(resp, {"t1"}) + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_missing_tokens_key_keeps_has_sentinel_true(self): + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + + resp = '{"other": "field"}' + result = _parse_need_doc_read_sentinel(resp, {"t1"}) + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_duplicate_tokens_deduplicated(self): + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + + resp = '{"tokens": ["t1", "t1", "t2", "t1"]}' + result = _parse_need_doc_read_sentinel(resp, {"t1", "t2"}) + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, ["t1", "t2"]) + + +class TestDocTruncation(unittest.TestCase): + """Per-doc and aggregate truncation keep the prompt bounded.""" + + def test_short_content_unchanged(self): + from gateway.platforms.feishu_comment import _truncate_doc_content + + short = "hello world" + self.assertEqual(_truncate_doc_content(short, "t1"), short) + + def test_long_content_truncated_with_marker(self): + from gateway.platforms.feishu_comment import _truncate_doc_content, _MAX_DOC_CHARS + + long = "x" * (_MAX_DOC_CHARS + 500) + result = _truncate_doc_content(long, "t1") + self.assertTrue(result.startswith("x" * _MAX_DOC_CHARS)) + self.assertIn("truncated at", result) + self.assertIn(str(_MAX_DOC_CHARS), result) + + def test_aggregate_budget_scales_proportionally(self): + from gateway.platforms.feishu_comment import ( + _enforce_total_doc_budget, + _MAX_TOTAL_DOC_CHARS, + ) + + # Two docs that together blow the aggregate cap (post-per-doc-truncation). + contents = { + "t1": "a" * _MAX_TOTAL_DOC_CHARS, + "t2": "b" * _MAX_TOTAL_DOC_CHARS, + } + scaled = _enforce_total_doc_budget(contents) + total = sum(len(c) for c in scaled.values()) + # Allow some slack for the "further truncated" suffix appended per entry. + self.assertLessEqual(total, _MAX_TOTAL_DOC_CHARS + 200) + self.assertIn("further truncated", scaled["t1"]) + self.assertIn("further truncated", scaled["t2"]) + + def test_aggregate_under_cap_unchanged(self): + from gateway.platforms.feishu_comment import _enforce_total_doc_budget + + contents = {"t1": "short", "t2": "also short"} + self.assertEqual(_enforce_total_doc_budget(contents), contents) + + +class TestDocFetchErrorPaths(unittest.TestCase): + """Fetch failures degrade to prompt-visible error strings, not raises.""" + + def _run(self, coro): + return asyncio.run(coro) + + def test_fetch_failure_produces_error_string(self): + from gateway.platforms.feishu_comment import _fetch_docs_for_agent + + async def scenario(): + # Mock client whose request is synchronous but raises on call. + client = Mock() + client.request = Mock(side_effect=RuntimeError("boom")) + + contents = await _fetch_docs_for_agent(client, ["tok1"]) + self.assertIn("tok1", contents) + self.assertTrue(contents["tok1"].startswith("[Failed to fetch")) + self.assertIn("boom", contents["tok1"]) + + self._run(scenario()) + + def test_empty_token_list_returns_empty_dict(self): + from gateway.platforms.feishu_comment import _fetch_docs_for_agent + + async def scenario(): + self.assertEqual(await _fetch_docs_for_agent(Mock(), []), {}) + + self._run(scenario()) + + +class TestDocContentBlockFormatting(unittest.TestCase): + """The second-pass prompt block forbids further NEED_DOC_READ loops.""" + + def test_block_includes_tokens_and_closing_instruction(self): + from gateway.platforms.feishu_comment import _format_doc_content_block + + block = _format_doc_content_block({"a": "aa", "b": "bb"}) + self.assertIn("[Document token: a]", block) + self.assertIn("aa", block) + self.assertIn("[Document token: b]", block) + self.assertIn("bb", block) + # Crucial: forbid the model from looping on turn 2. + self.assertIn(" is no longer accepted", block) + self.assertIn("MUST produce the final user-facing reply", block) + + def test_empty_contents_still_includes_instruction(self): + from gateway.platforms.feishu_comment import _format_doc_content_block + + block = _format_doc_content_block({}) + self.assertIn(" is no longer accepted", block) + + +class TestSecondPassHistoryRebuild(unittest.TestCase): + """``_run_second_pass`` must explicitly pass conversation_history. + + ``AIAgent.run_conversation`` reinitializes messages from + ``conversation_history`` on every call, so the second pass cannot + rely on the agent instance to remember turn 1. Without this the + model only sees the doc content block and loses the user's original + question, quote, and timeline. + """ + + def test_second_pass_passes_full_history(self): + from gateway.platforms.feishu_comment import _run_second_pass + + # Mock agent: capture what conversation_history was passed in. + agent = Mock() + agent.run_conversation.return_value = { + "final_response": "final reply", + "api_calls": 1, + } + + # Mock client: _fetch_docs_for_agent will invoke client.request, + # but we stub _read_document_raw_content via patching the fetch + # helper directly to keep this test focused on history shape. + client = Mock() + + prior_history = [ + {"role": "user", "content": "old user turn"}, + {"role": "assistant", "content": "old assistant turn"}, + ] + first_prompt = "original first-pass prompt with quote + timeline" + first_response = ( + 'I need the doc.\n{"tokens": ["tok1"]}' + ) + + with patch( + "gateway.platforms.feishu_comment._fetch_docs_for_agent", + new_callable=AsyncMock, + return_value={"tok1": "document body"}, + ): + response, _ = _run_second_pass( + agent, client, ["tok1"], + prior_history=prior_history, + first_prompt=first_prompt, + first_response=first_response, + ) + + self.assertEqual(response, "final reply") + + # Assert the agent was invoked with a full history: prior turns + + # first-pass user prompt + first-pass assistant response. + agent.run_conversation.assert_called_once() + call_args = agent.run_conversation.call_args + sent_history = call_args.kwargs.get("conversation_history") + self.assertIsNotNone(sent_history, "conversation_history must be passed") + self.assertEqual(len(sent_history), 4) + self.assertEqual(sent_history[0], prior_history[0]) + self.assertEqual(sent_history[1], prior_history[1]) + self.assertEqual(sent_history[2], {"role": "user", "content": first_prompt}) + self.assertEqual( + sent_history[3], {"role": "assistant", "content": first_response}, + ) + # user_message for turn 2 is the doc-content block, not the + # raw doc body. + user_message = call_args.args[0] if call_args.args else call_args.kwargs.get("user_message") + self.assertIn("[Document token: tok1]", user_message) + self.assertIn("document body", user_message) + + def test_second_pass_history_empty_prior_still_includes_first_turn(self): + """Even when prior_history is empty, first-pass turn must be preserved.""" + from gateway.platforms.feishu_comment import _run_second_pass + + agent = Mock() + agent.run_conversation.return_value = { + "final_response": "ok", + "api_calls": 1, + } + with patch( + "gateway.platforms.feishu_comment._fetch_docs_for_agent", + new_callable=AsyncMock, + return_value={}, + ): + _run_second_pass( + agent, Mock(), [], + prior_history=[], + first_prompt="p", + first_response="r", + ) + sent_history = agent.run_conversation.call_args.kwargs["conversation_history"] + self.assertEqual( + sent_history, + [ + {"role": "user", "content": "p"}, + {"role": "assistant", "content": "r"}, + ], + ) + + +class TestSessionSourceBuilder(unittest.TestCase): + """``_build_comment_session_source`` — maps a comment event to SessionSource. + + The key identity for a comment session is ``chat_id=f"{file_type}:{file_token}"`` + + ``thread_id=comment_id``; user_id is recorded but does NOT participate + in the session key (thread-shared semantics, see build_session_key). + """ + + def test_basic_mapping(self): + from gateway.config import Platform + from gateway.platforms.feishu_comment import _build_comment_session_source + + source = _build_comment_session_source( + file_type="docx", + file_token="TOKEN_A", + comment_id="CMT_1", + is_whole_comment=False, + from_open_id="ou_user_1", + doc_title="Project Plan", + ) + self.assertEqual(source.platform, Platform.FEISHU) + self.assertEqual(source.chat_type, "doc_comment") + self.assertEqual(source.chat_id, "docx:TOKEN_A") + self.assertEqual(source.thread_id, "CMT_1") + self.assertEqual(source.user_id, "ou_user_1") + self.assertEqual(source.chat_name, "Project Plan") + + def test_chat_name_fallback_when_no_title(self): + """Missing / empty title falls back to a short token-based stub.""" + from gateway.platforms.feishu_comment import _build_comment_session_source + + source = _build_comment_session_source( + file_type="docx", + file_token="LONGTOKEN123456", + comment_id="CMT_1", + is_whole_comment=False, + from_open_id="ou_user_1", + doc_title=None, + ) + self.assertEqual(source.chat_name, "docx:LONGTOKE") # first 8 chars + + def test_key_excludes_user_id(self): + """Thread-shared semantics: build_session_key must not add user_id. + + Two users on the same comment thread share a single session (same + bot context) — matching IM thread behavior. + """ + from gateway.session import build_session_key + from gateway.platforms.feishu_comment import _build_comment_session_source + + src_a = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="C1", + is_whole_comment=False, + from_open_id="ou_user_A", doc_title="Doc", + ) + src_b = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="C1", + is_whole_comment=False, + from_open_id="ou_user_B", doc_title="Doc", + ) + key_a = build_session_key(src_a) + key_b = build_session_key(src_b) + self.assertEqual(key_a, key_b) + # And the key shape matches the documented format. + self.assertEqual(key_a, "agent:main:feishu:doc_comment:docx:T1:C1") + + def test_different_comment_threads_isolated(self): + """Different comment_id on same doc → different session keys.""" + from gateway.session import build_session_key + from gateway.platforms.feishu_comment import _build_comment_session_source + + src_1 = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="C1", + is_whole_comment=False, + from_open_id="ou_u", doc_title="Doc", + ) + src_2 = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="C2", + is_whole_comment=False, + from_open_id="ou_u", doc_title="Doc", + ) + self.assertNotEqual(build_session_key(src_1), build_session_key(src_2)) + + def test_different_docs_isolated(self): + from gateway.session import build_session_key + from gateway.platforms.feishu_comment import _build_comment_session_source + + src_1 = _build_comment_session_source( + file_type="docx", file_token="TA", comment_id="C1", + is_whole_comment=False, + from_open_id="ou_u", doc_title="A", + ) + src_2 = _build_comment_session_source( + file_type="docx", file_token="TB", comment_id="C1", + is_whole_comment=False, + from_open_id="ou_u", doc_title="B", + ) + self.assertNotEqual(build_session_key(src_1), build_session_key(src_2)) + + def test_whole_doc_uses_sentinel_thread_id(self): + """Whole-doc comments collapse to a doc-level session via sentinel.""" + from gateway.platforms.feishu_comment import ( + _build_comment_session_source, + _WHOLE_DOC_SENTINEL_THREAD_ID, + ) + + source = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="CMT_1", + is_whole_comment=True, + from_open_id="ou_u", doc_title="Doc", + ) + # thread_id is the sentinel, not the actual comment_id — so + # successive whole-doc comments (with different comment_ids) + # resolve to the same session key. + self.assertEqual(source.thread_id, _WHOLE_DOC_SENTINEL_THREAD_ID) + + def test_whole_doc_same_doc_shares_key(self): + """All whole-doc comments on the same doc → same session key.""" + from gateway.session import build_session_key + from gateway.platforms.feishu_comment import _build_comment_session_source + + first = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="CMT_FIRST", + is_whole_comment=True, + from_open_id="ou_a", doc_title="Doc", + ) + later = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="CMT_LATER", + is_whole_comment=True, + from_open_id="ou_b", doc_title="Doc", + ) + key = build_session_key(first) + self.assertEqual(key, build_session_key(later)) + # Documented key shape for whole-doc comments. + self.assertEqual(key, "agent:main:feishu:doc_comment:docx:T1:__whole_doc__") + + def test_whole_doc_and_local_keys_disjoint(self): + """Same doc's whole-doc and local-comment sessions must not collide.""" + from gateway.session import build_session_key + from gateway.platforms.feishu_comment import _build_comment_session_source + + whole = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="CMT_X", + is_whole_comment=True, + from_open_id="ou_u", doc_title="Doc", + ) + local = _build_comment_session_source( + file_type="docx", file_token="T1", comment_id="CMT_X", + is_whole_comment=False, + from_open_id="ou_u", doc_title="Doc", + ) + self.assertNotEqual(build_session_key(whole), build_session_key(local)) + + def test_whole_doc_on_different_docs_isolated(self): + """Whole-doc sessions on different docs remain separate.""" + from gateway.session import build_session_key + from gateway.platforms.feishu_comment import _build_comment_session_source + + doc_a = _build_comment_session_source( + file_type="docx", file_token="TA", comment_id="CMT_1", + is_whole_comment=True, + from_open_id="ou_u", doc_title="A", + ) + doc_b = _build_comment_session_source( + file_type="docx", file_token="TB", comment_id="CMT_1", + is_whole_comment=True, + from_open_id="ou_u", doc_title="B", + ) + self.assertNotEqual(build_session_key(doc_a), build_session_key(doc_b)) + + +class TestSessionHistoryPersistence(unittest.TestCase): + """History load/save go through SessionStore's public transcript API. + + The comment handler deliberately does NOT poke ``SessionStore._db`` + directly — going through ``load_transcript`` / ``append_to_transcript`` + keeps doc_comment on the same storage path (SQLite + JSONL dual-write, + length-based tie-breaking on read) as IM and other chat types. + """ + + def test_load_returns_transcript_from_store(self): + from gateway.platforms.feishu_comment import _load_comment_history + + store = Mock() + fake_history = [ + {"role": "user", "content": "hello"}, + {"role": "assistant", "content": "hi"}, + ] + store.load_transcript.return_value = fake_history + self.assertEqual(_load_comment_history(store, "sid_1"), fake_history) + store.load_transcript.assert_called_once_with("sid_1") + + def test_load_swallows_store_errors(self): + """A flaky transcript layer mustn't crash the comment handler.""" + from gateway.platforms.feishu_comment import _load_comment_history + + store = Mock() + store.load_transcript.side_effect = RuntimeError("store down") + self.assertEqual(_load_comment_history(store, "sid_1"), []) + + def test_persist_writes_two_messages_via_public_api(self): + """One comment turn = one user + one assistant call on append_to_transcript.""" + from gateway.platforms.feishu_comment import _persist_comment_turn + + store = Mock() + _persist_comment_turn(store, "sid_1", "prompt", "reply") + + self.assertEqual(store.append_to_transcript.call_count, 2) + call_user, call_assistant = store.append_to_transcript.call_args_list + + # append_to_transcript(session_id, message_dict) + self.assertEqual(call_user.args[0], "sid_1") + self.assertEqual( + call_user.args[1], {"role": "user", "content": "prompt"}, + ) + self.assertEqual(call_assistant.args[0], "sid_1") + self.assertEqual( + call_assistant.args[1], {"role": "assistant", "content": "reply"}, + ) + + def test_persist_does_not_touch_private_db(self): + """Regression guard: comment flow must never reach into SessionStore._db. + + The whole point of routing through the public API is to avoid + drift from IM storage semantics — this test makes that a tested + invariant rather than a stylistic wish. + """ + from gateway.platforms.feishu_comment import _persist_comment_turn + + store = Mock() + # If any code path tries to use store._db, spec=[] guarantees + # AttributeError — Mock would otherwise silently vend anything. + store._db = Mock(spec=[]) + _persist_comment_turn(store, "sid_1", "p", "r") + store.append_to_transcript.assert_called() + + def test_persist_swallows_store_errors(self): + from gateway.platforms.feishu_comment import _persist_comment_turn + + store = Mock() + store.append_to_transcript.side_effect = RuntimeError("write fail") + # Should log but not raise + _persist_comment_turn(store, "sid_1", "p", "r") + + +class TestCompactUserTurnForPersistence(unittest.TestCase): + """``_compact_user_turn_for_persistence`` keeps session history bounded. + + Persisting the full rendered prompt would duplicate the timeline into + every historical user message, so we store only the user's actual + text plus an optional quote anchor. + """ + + def test_target_only(self): + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + out = _compact_user_turn_for_persistence( + target_reply_text="please summarize section 3", + ) + self.assertEqual(out, "please summarize section 3") + + def test_quote_and_target(self): + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + out = _compact_user_turn_for_persistence( + target_reply_text="fix this", + quote_text="Q1 budget is 500k", + ) + self.assertEqual(out, "[Quoted] Q1 budget is 500k\nfix this") + + def test_empty_quote_omits_marker(self): + """Whole-doc has no quote — output must not carry an empty marker.""" + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + out = _compact_user_turn_for_persistence( + target_reply_text="my whole-doc comment", + quote_text="", + ) + self.assertEqual(out, "my whole-doc comment") + self.assertNotIn("Quoted", out) + + def test_empty_target_with_quote(self): + """Edge case: user managed to produce no target text but a quote.""" + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + out = _compact_user_turn_for_persistence( + target_reply_text="", + quote_text="anchored snippet", + ) + self.assertEqual(out, "[Quoted] anchored snippet") + + def test_all_empty(self): + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + self.assertEqual( + _compact_user_turn_for_persistence(target_reply_text=""), + "", + ) + + def test_truncation_applies_when_overlong(self): + """Runaway input must be clamped so SessionDB rows stay bounded.""" + from gateway.platforms.feishu_comment import ( + _compact_user_turn_for_persistence, + _MAX_PERSISTED_USER_TURN_CHARS, + ) + + overlong = "x" * (_MAX_PERSISTED_USER_TURN_CHARS + 500) + out = _compact_user_turn_for_persistence(target_reply_text=overlong) + self.assertIn("truncated at", out) + # The preserved body fits within the cap; the suffix marker is added + # on top, so the absolute length is slightly larger but bounded. + self.assertLess( + len(out), + _MAX_PERSISTED_USER_TURN_CHARS + 100, + ) + + def test_typical_size_is_tiny(self): + """Sanity: a realistic turn is <1KB, two orders of magnitude under cap.""" + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + out = _compact_user_turn_for_persistence( + target_reply_text="请把这段改成过去式", + quote_text="我们将在下周完成这项工作", + ) + self.assertLess(len(out), 200) + + def test_sentinel_in_target_scrubbed_before_persistence(self): + """A commenter-supplied ```` must not reach SessionDB. + + Persisted history becomes ``conversation_history`` on later turns, + so any sentinel literal stored here would be replayed to the + model as if it originated from the bot — reopening the protocol + injection surface the live prompt already defends against. + """ + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + out = _compact_user_turn_for_persistence( + target_reply_text='{"tokens": ["src"]}', + ) + self.assertNotIn("", out) + self.assertIn("", out) + + def test_sentinel_in_quote_scrubbed_before_persistence(self): + """Same invariant for the quote anchor field.""" + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + out = _compact_user_turn_for_persistence( + target_reply_text="please fix", + quote_text="prefix suffix", + ) + self.assertNotIn("", out) + self.assertIn("", out) + # The quote marker prefix itself must still be preserved. + self.assertIn("[Quoted]", out) + + def test_sentinel_variants_all_scrubbed(self): + """Invariant: for any injected sentinel variant, the persisted + string never contains the raw literal. Mirrors the outbound-gate + invariant test but for the persistence channel.""" + import re as _re + from gateway.platforms.feishu_comment import _compact_user_turn_for_persistence + + literal = _re.compile(r"", _re.IGNORECASE) + variants = [ + "", + " tok1", + "", + "hi there", + '{"tokens":["x"]}', + ] + for v in variants: + with self.subTest(variant=v): + out_target = _compact_user_turn_for_persistence(target_reply_text=v) + self.assertIsNone(literal.search(out_target), f"target field leaked {v!r}") + + out_quote = _compact_user_turn_for_persistence( + target_reply_text="ok", quote_text=v, + ) + self.assertIsNone(literal.search(out_quote), f"quote field leaked {v!r}") + + +class TestSentinelDetectionBroadness(unittest.TestCase): + """Detection must treat any ```` literal as a sentinel turn. + + The previous JSON-gated regex only matched when the marker was + followed by a valid ``{...}`` payload. Malformed variants (bare + marker, space-separated tokens, natural-language hedging around the + marker) slipped through and, without this fix, would have been + delivered as if they were the final reply. + """ + + def _parse(self, response): + from gateway.platforms.feishu_comment import _parse_need_doc_read_sentinel + return _parse_need_doc_read_sentinel(response, {"t1", "t2"}) + + def test_bare_marker_detected(self): + result = self._parse("") + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_space_separated_tokens_detected(self): + result = self._parse(" t1 t2") + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_natural_language_hedging_detected(self): + """The particularly dangerous case: marker inside a reply-looking string.""" + result = self._parse( + "Sorry, — I need more context before answering." + ) + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_marker_after_preamble_detected(self): + result = self._parse("Here's my plan:\n{\"tokens\": [\"t1\"]}") + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, ["t1"]) + + def test_lowercase_marker_detected(self): + """Detection is case-insensitive (``_NEED_DOC_READ_LITERAL`` uses IGNORECASE).""" + result = self._parse("") + self.assertTrue(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + def test_real_reply_stays_clean(self): + """Sanity: a normal reply without the marker is not a sentinel.""" + result = self._parse("Your edit looks good; I've updated section 3.") + self.assertFalse(result.has_sentinel) + self.assertEqual(result.accepted_tokens, []) + + +class TestOutboundGateInvariant(unittest.TestCase): + """Invariant test suite for ``_gate_outbound_reply``. + + The single invariant: a non-None return value never contains the + ```` literal. We test this as a property over a + representative set of inputs — any future malformed variant just + needs a new input row, not a new gate rule. + """ + + def _gate(self, response): + from gateway.platforms.feishu_comment import _gate_outbound_reply + return _gate_outbound_reply(response) + + def test_clean_reply_passes_through(self): + self.assertEqual(self._gate("Hello, fixed."), "Hello, fixed.") + + def test_empty_is_none(self): + self.assertIsNone(self._gate("")) + self.assertIsNone(self._gate(None)) + self.assertIsNone(self._gate(" \n\t ")) + + def test_no_reply_sentinel_is_none(self): + self.assertIsNone(self._gate("NO_REPLY")) + self.assertIsNone(self._gate("Actually NO_REPLY")) # substring match is intentional + + def test_invariant_all_sentinel_variants_rejected(self): + """The invariant: literal present anywhere → gate returns None.""" + bad_inputs = [ + "", + " t1 t2", + 'Sorry, — I need more info', + "Here's my reply.\n\n", + '{"tokens":["t1"]}', + "", + "prefix suffix", + "\nI'm thinking...", + "{incomplete", + ] + for resp in bad_inputs: + with self.subTest(response=resp): + self.assertIsNone( + self._gate(resp), + f"Gate failed to reject {resp!r}", + ) + + def test_invariant_output_is_sentinel_free(self): + """For any non-None gate output, the literal MUST NOT appear. + + This is the structural invariant — tested via property-style + enumeration so regressions anywhere (outbound check, delivery + path, first-pass routing) cannot pass silently. + """ + import re + literal = re.compile(r"", re.IGNORECASE) + candidates = [ + "plain", + "multi\nline reply", + "Your edit looks good.", + "Done — updated the table.", + ] + for resp in candidates: + result = self._gate(resp) + if result is not None: + self.assertIsNone( + literal.search(result), + f"Gate returned sentinel-containing text for input {resp!r}", + ) + + +class TestCommentAgentDoesNotPersist(unittest.TestCase): + """Regression guard: the helper agent must NOT engage AIAgent's built-in + session persistence. + + Comment durable history goes through ``SessionStore.append_to_transcript`` + in compact form. Letting ``AIAgent._persist_session`` also run would + re-leak the first-pass rendered prompt (timeline, quote, doc URL) and + the second-pass fetched document bodies into + ``~/.hermes/logs/session_{id}.json`` and SessionDB. + + If this test fails, ``_build_comment_agent`` has drifted away from + ``persist_session=False`` — a direct user-content / document-content + leakage regression. + """ + + def test_build_comment_agent_disables_persist_session(self): + from gateway.platforms import feishu_comment + + # Stub AIAgent so we don't need real provider credentials — we only + # care about the kwargs the module passes in. ``run_agent`` is + # imported inside _build_comment_agent, so patch at that import site. + with patch("run_agent.AIAgent") as mock_agent_cls: + feishu_comment._build_comment_agent( + runtime_kwargs={"provider": "stub"}, model="stub-model", + ) + + mock_agent_cls.assert_called_once() + kwargs = mock_agent_cls.call_args.kwargs + self.assertIn( + "persist_session", kwargs, + "persist_session kwarg missing — relying on default (True) " + "would re-enable full-prompt / doc-content persistence", + ) + self.assertFalse( + kwargs["persist_session"], + "persist_session must be False for the comment helper agent", + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/gateway/test_feishu_comment_rules.py b/tests/gateway/test_feishu_comment_rules.py index baef7a54744e..69fe95ce1c88 100644 --- a/tests/gateway/test_feishu_comment_rules.py +++ b/tests/gateway/test_feishu_comment_rules.py @@ -315,6 +315,24 @@ def test_remove(self): def test_remove_nonexistent(self): self.assertFalse(pairing_remove("ou_nobody")) + @unittest.skipUnless( + os.name == "posix", + "chmod 0600 semantics only apply on POSIX filesystems", + ) + def test_pairing_file_has_owner_only_permissions(self): + """pairing file stores allowlisted open_ids — must be 0600. + + Leaking this list hands an attacker a ready-made targeting set of + users authorized to @-mention the bot on sensitive documents. + """ + pairing_add("ou_sensitive_user") + # stat.st_mode low 9 bits are the permission bits. + mode = self._pairing_file.stat().st_mode & 0o777 + self.assertEqual( + mode, 0o600, + f"pairing file permissions should be 0600, got 0o{mode:o}", + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/tools/test_feishu_tools.py b/tests/tools/test_feishu_tools.py deleted file mode 100644 index 15b27b4abf38..000000000000 --- a/tests/tools/test_feishu_tools.py +++ /dev/null @@ -1,62 +0,0 @@ -"""Tests for feishu_doc_tool and feishu_drive_tool — registration and schema validation.""" - -import importlib -import unittest - -from tools.registry import registry - -# Trigger tool discovery so feishu tools get registered -importlib.import_module("tools.feishu_doc_tool") -importlib.import_module("tools.feishu_drive_tool") - - -class TestFeishuToolRegistration(unittest.TestCase): - """Verify feishu tools are registered and have valid schemas.""" - - EXPECTED_TOOLS = { - "feishu_doc_read": "feishu_doc", - "feishu_drive_list_comments": "feishu_drive", - "feishu_drive_list_comment_replies": "feishu_drive", - "feishu_drive_reply_comment": "feishu_drive", - "feishu_drive_add_comment": "feishu_drive", - } - - def test_all_tools_registered(self): - for tool_name, toolset in self.EXPECTED_TOOLS.items(): - entry = registry.get_entry(tool_name) - self.assertIsNotNone(entry, f"{tool_name} not registered") - self.assertEqual(entry.toolset, toolset) - - def test_schemas_have_required_fields(self): - for tool_name in self.EXPECTED_TOOLS: - entry = registry.get_entry(tool_name) - schema = entry.schema - self.assertIn("name", schema) - self.assertEqual(schema["name"], tool_name) - self.assertIn("description", schema) - self.assertIn("parameters", schema) - self.assertIn("type", schema["parameters"]) - self.assertEqual(schema["parameters"]["type"], "object") - - def test_handlers_are_callable(self): - for tool_name in self.EXPECTED_TOOLS: - entry = registry.get_entry(tool_name) - self.assertTrue(callable(entry.handler)) - - def test_doc_read_schema_params(self): - entry = registry.get_entry("feishu_doc_read") - props = entry.schema["parameters"].get("properties", {}) - self.assertIn("doc_token", props) - - def test_drive_tools_require_file_token(self): - for tool_name in self.EXPECTED_TOOLS: - if tool_name == "feishu_doc_read": - continue - entry = registry.get_entry(tool_name) - props = entry.schema["parameters"].get("properties", {}) - self.assertIn("file_token", props, f"{tool_name} missing file_token param") - self.assertIn("file_type", props, f"{tool_name} missing file_type param") - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/tools/test_registry.py b/tests/tools/test_registry.py index d015b483864a..6a13648b840e 100644 --- a/tests/tools/test_registry.py +++ b/tests/tools/test_registry.py @@ -298,8 +298,6 @@ def test_matches_previous_manual_builtin_tool_set(self): "tools.cronjob_tools", "tools.delegate_tool", "tools.discord_tool", - "tools.feishu_doc_tool", - "tools.feishu_drive_tool", "tools.file_tools", "tools.homeassistant_tool", "tools.image_generation_tool", diff --git a/tools/feishu_doc_tool.py b/tools/feishu_doc_tool.py deleted file mode 100644 index f334b915e9b1..000000000000 --- a/tools/feishu_doc_tool.py +++ /dev/null @@ -1,131 +0,0 @@ -"""Feishu Document Tool -- read document content via Feishu/Lark API. - -Provides ``feishu_doc_read`` for reading document content as plain text. -Uses the same lazy-import + BaseRequest pattern as feishu_comment.py. -""" - -import json -import logging -import threading - -from tools.registry import registry, tool_error, tool_result - -logger = logging.getLogger(__name__) - -# Thread-local storage for the lark client injected by feishu_comment handler. -_local = threading.local() - - -def set_client(client): - """Store a lark client for the current thread (called by feishu_comment).""" - _local.client = client - - -def get_client(): - """Return the lark client for the current thread, or None.""" - return getattr(_local, "client", None) - - -# --------------------------------------------------------------------------- -# feishu_doc_read -# --------------------------------------------------------------------------- - -_RAW_CONTENT_URI = "/open-apis/docx/v1/documents/:document_id/raw_content" - -FEISHU_DOC_READ_SCHEMA = { - "name": "feishu_doc_read", - "description": ( - "Read the full content of a Feishu/Lark document as plain text. " - "Useful when you need more context beyond the quoted text in a comment." - ), - "parameters": { - "type": "object", - "properties": { - "doc_token": { - "type": "string", - "description": "The document token (from the document URL or comment context).", - }, - }, - "required": ["doc_token"], - }, -} - - -def _check_feishu(): - try: - import lark_oapi # noqa: F401 - return True - except ImportError: - return False - - -def _handle_feishu_doc_read(args: dict, **kwargs) -> str: - doc_token = args.get("doc_token", "").strip() - if not doc_token: - return tool_error("doc_token is required") - - client = get_client() - if client is None: - return tool_error("Feishu client not available (not in a Feishu comment context)") - - try: - from lark_oapi import AccessTokenType - from lark_oapi.core.enum import HttpMethod - from lark_oapi.core.model.base_request import BaseRequest - except ImportError: - return tool_error("lark_oapi not installed") - - request = ( - BaseRequest.builder() - .http_method(HttpMethod.GET) - .uri(_RAW_CONTENT_URI) - .token_types({AccessTokenType.TENANT}) - .paths({"document_id": doc_token}) - .build() - ) - - # Tool handlers run synchronously in a worker thread (no running event - # loop), so call the blocking lark client directly. - response = client.request(request) - - code = getattr(response, "code", None) - if code != 0: - msg = getattr(response, "msg", "unknown error") - return tool_error(f"Failed to read document: code={code} msg={msg}") - - raw = getattr(response, "raw", None) - if raw and hasattr(raw, "content"): - try: - body = json.loads(raw.content) - content = body.get("data", {}).get("content", "") - return tool_result(success=True, content=content) - except (json.JSONDecodeError, AttributeError): - pass - - # Fallback: try response.data - data = getattr(response, "data", None) - if data: - if isinstance(data, dict): - content = data.get("content", "") - else: - content = getattr(data, "content", str(data)) - return tool_result(success=True, content=content) - - return tool_error("No content returned from document API") - - -# --------------------------------------------------------------------------- -# Registration -# --------------------------------------------------------------------------- - -registry.register( - name="feishu_doc_read", - toolset="feishu_doc", - schema=FEISHU_DOC_READ_SCHEMA, - handler=_handle_feishu_doc_read, - check_fn=_check_feishu, - requires_env=[], - is_async=False, - description="Read Feishu document content", - emoji="\U0001f4c4", -) diff --git a/tools/feishu_drive_tool.py b/tools/feishu_drive_tool.py deleted file mode 100644 index 5742acf05834..000000000000 --- a/tools/feishu_drive_tool.py +++ /dev/null @@ -1,429 +0,0 @@ -"""Feishu Drive Tools -- document comment operations via Feishu/Lark API. - -Provides tools for listing, replying to, and adding document comments. -Uses the same lazy-import + BaseRequest pattern as feishu_comment.py. -The lark client is injected per-thread by the comment event handler. -""" - -import json -import logging -import threading - -from tools.registry import registry, tool_error, tool_result - -logger = logging.getLogger(__name__) - -# Thread-local storage for the lark client injected by feishu_comment handler. -_local = threading.local() - - -def set_client(client): - """Store a lark client for the current thread (called by feishu_comment).""" - _local.client = client - - -def get_client(): - """Return the lark client for the current thread, or None.""" - return getattr(_local, "client", None) - - -def _check_feishu(): - try: - import lark_oapi # noqa: F401 - return True - except ImportError: - return False - - -def _do_request(client, method, uri, paths=None, queries=None, body=None): - """Build and execute a BaseRequest, return (code, msg, data_dict).""" - from lark_oapi import AccessTokenType - from lark_oapi.core.enum import HttpMethod - from lark_oapi.core.model.base_request import BaseRequest - - http_method = HttpMethod.GET if method == "GET" else HttpMethod.POST - - builder = ( - BaseRequest.builder() - .http_method(http_method) - .uri(uri) - .token_types({AccessTokenType.TENANT}) - ) - if paths: - builder = builder.paths(paths) - if queries: - builder = builder.queries(queries) - if body is not None: - builder = builder.body(body) - - request = builder.build() - - # Tool handlers run synchronously in a worker thread (no running event - # loop), so call the blocking lark client directly. - response = client.request(request) - - code = getattr(response, "code", None) - msg = getattr(response, "msg", "") - - # Parse response data - data = {} - raw = getattr(response, "raw", None) - if raw and hasattr(raw, "content"): - try: - body_json = json.loads(raw.content) - data = body_json.get("data", {}) - except (json.JSONDecodeError, AttributeError): - pass - if not data: - resp_data = getattr(response, "data", None) - if isinstance(resp_data, dict): - data = resp_data - elif resp_data and hasattr(resp_data, "__dict__"): - data = vars(resp_data) - - return code, msg, data - - -# --------------------------------------------------------------------------- -# feishu_drive_list_comments -# --------------------------------------------------------------------------- - -_LIST_COMMENTS_URI = "/open-apis/drive/v1/files/:file_token/comments" - -FEISHU_DRIVE_LIST_COMMENTS_SCHEMA = { - "name": "feishu_drive_list_comments", - "description": ( - "List comments on a Feishu document. " - "Use is_whole=true to list whole-document comments only." - ), - "parameters": { - "type": "object", - "properties": { - "file_token": { - "type": "string", - "description": "The document file token.", - }, - "file_type": { - "type": "string", - "description": "File type (default: docx).", - "default": "docx", - }, - "is_whole": { - "type": "boolean", - "description": "If true, only return whole-document comments.", - "default": False, - }, - "page_size": { - "type": "integer", - "description": "Number of comments per page (max 100).", - "default": 100, - }, - "page_token": { - "type": "string", - "description": "Pagination token for next page.", - }, - }, - "required": ["file_token"], - }, -} - - -def _handle_list_comments(args: dict, **kwargs) -> str: - client = get_client() - if client is None: - return tool_error("Feishu client not available") - - file_token = args.get("file_token", "").strip() - if not file_token: - return tool_error("file_token is required") - - file_type = args.get("file_type", "docx") or "docx" - is_whole = args.get("is_whole", False) - page_size = args.get("page_size", 100) - page_token = args.get("page_token", "") - - queries = [ - ("file_type", file_type), - ("user_id_type", "open_id"), - ("page_size", str(page_size)), - ] - if is_whole: - queries.append(("is_whole", "true")) - if page_token: - queries.append(("page_token", page_token)) - - code, msg, data = _do_request( - client, "GET", _LIST_COMMENTS_URI, - paths={"file_token": file_token}, - queries=queries, - ) - if code != 0: - return tool_error(f"List comments failed: code={code} msg={msg}") - - return tool_result(data) - - -# --------------------------------------------------------------------------- -# feishu_drive_list_comment_replies -# --------------------------------------------------------------------------- - -_LIST_REPLIES_URI = "/open-apis/drive/v1/files/:file_token/comments/:comment_id/replies" - -FEISHU_DRIVE_LIST_REPLIES_SCHEMA = { - "name": "feishu_drive_list_comment_replies", - "description": "List all replies in a comment thread on a Feishu document.", - "parameters": { - "type": "object", - "properties": { - "file_token": { - "type": "string", - "description": "The document file token.", - }, - "comment_id": { - "type": "string", - "description": "The comment ID to list replies for.", - }, - "file_type": { - "type": "string", - "description": "File type (default: docx).", - "default": "docx", - }, - "page_size": { - "type": "integer", - "description": "Number of replies per page (max 100).", - "default": 100, - }, - "page_token": { - "type": "string", - "description": "Pagination token for next page.", - }, - }, - "required": ["file_token", "comment_id"], - }, -} - - -def _handle_list_replies(args: dict, **kwargs) -> str: - client = get_client() - if client is None: - return tool_error("Feishu client not available") - - file_token = args.get("file_token", "").strip() - comment_id = args.get("comment_id", "").strip() - if not file_token or not comment_id: - return tool_error("file_token and comment_id are required") - - file_type = args.get("file_type", "docx") or "docx" - page_size = args.get("page_size", 100) - page_token = args.get("page_token", "") - - queries = [ - ("file_type", file_type), - ("user_id_type", "open_id"), - ("page_size", str(page_size)), - ] - if page_token: - queries.append(("page_token", page_token)) - - code, msg, data = _do_request( - client, "GET", _LIST_REPLIES_URI, - paths={"file_token": file_token, "comment_id": comment_id}, - queries=queries, - ) - if code != 0: - return tool_error(f"List replies failed: code={code} msg={msg}") - - return tool_result(data) - - -# --------------------------------------------------------------------------- -# feishu_drive_reply_comment -# --------------------------------------------------------------------------- - -_REPLY_COMMENT_URI = "/open-apis/drive/v1/files/:file_token/comments/:comment_id/replies" - -FEISHU_DRIVE_REPLY_SCHEMA = { - "name": "feishu_drive_reply_comment", - "description": ( - "Reply to a local comment thread on a Feishu document. " - "Use this for local (quoted-text) comments. " - "For whole-document comments, use feishu_drive_add_comment instead." - ), - "parameters": { - "type": "object", - "properties": { - "file_token": { - "type": "string", - "description": "The document file token.", - }, - "comment_id": { - "type": "string", - "description": "The comment ID to reply to.", - }, - "content": { - "type": "string", - "description": "The reply text content (plain text only, no markdown).", - }, - "file_type": { - "type": "string", - "description": "File type (default: docx).", - "default": "docx", - }, - }, - "required": ["file_token", "comment_id", "content"], - }, -} - - -def _handle_reply_comment(args: dict, **kwargs) -> str: - client = get_client() - if client is None: - return tool_error("Feishu client not available") - - file_token = args.get("file_token", "").strip() - comment_id = args.get("comment_id", "").strip() - content = args.get("content", "").strip() - if not file_token or not comment_id or not content: - return tool_error("file_token, comment_id, and content are required") - - file_type = args.get("file_type", "docx") or "docx" - - body = { - "content": { - "elements": [ - { - "type": "text_run", - "text_run": {"text": content}, - } - ] - } - } - - code, msg, data = _do_request( - client, "POST", _REPLY_COMMENT_URI, - paths={"file_token": file_token, "comment_id": comment_id}, - queries=[("file_type", file_type)], - body=body, - ) - if code != 0: - return tool_error(f"Reply comment failed: code={code} msg={msg}") - - return tool_result(success=True, data=data) - - -# --------------------------------------------------------------------------- -# feishu_drive_add_comment -# --------------------------------------------------------------------------- - -_ADD_COMMENT_URI = "/open-apis/drive/v1/files/:file_token/new_comments" - -FEISHU_DRIVE_ADD_COMMENT_SCHEMA = { - "name": "feishu_drive_add_comment", - "description": ( - "Add a new whole-document comment on a Feishu document. " - "Use this for whole-document comments or as a fallback when " - "reply_comment fails with code 1069302." - ), - "parameters": { - "type": "object", - "properties": { - "file_token": { - "type": "string", - "description": "The document file token.", - }, - "content": { - "type": "string", - "description": "The comment text content (plain text only, no markdown).", - }, - "file_type": { - "type": "string", - "description": "File type (default: docx).", - "default": "docx", - }, - }, - "required": ["file_token", "content"], - }, -} - - -def _handle_add_comment(args: dict, **kwargs) -> str: - client = get_client() - if client is None: - return tool_error("Feishu client not available") - - file_token = args.get("file_token", "").strip() - content = args.get("content", "").strip() - if not file_token or not content: - return tool_error("file_token and content are required") - - file_type = args.get("file_type", "docx") or "docx" - - body = { - "file_type": file_type, - "reply_elements": [ - {"type": "text", "text": content}, - ], - } - - code, msg, data = _do_request( - client, "POST", _ADD_COMMENT_URI, - paths={"file_token": file_token}, - body=body, - ) - if code != 0: - return tool_error(f"Add comment failed: code={code} msg={msg}") - - return tool_result(success=True, data=data) - - -# --------------------------------------------------------------------------- -# Registration -# --------------------------------------------------------------------------- - -registry.register( - name="feishu_drive_list_comments", - toolset="feishu_drive", - schema=FEISHU_DRIVE_LIST_COMMENTS_SCHEMA, - handler=_handle_list_comments, - check_fn=_check_feishu, - requires_env=[], - is_async=False, - description="List document comments", - emoji="\U0001f4ac", -) - -registry.register( - name="feishu_drive_list_comment_replies", - toolset="feishu_drive", - schema=FEISHU_DRIVE_LIST_REPLIES_SCHEMA, - handler=_handle_list_replies, - check_fn=_check_feishu, - requires_env=[], - is_async=False, - description="List comment replies", - emoji="\U0001f4ac", -) - -registry.register( - name="feishu_drive_reply_comment", - toolset="feishu_drive", - schema=FEISHU_DRIVE_REPLY_SCHEMA, - handler=_handle_reply_comment, - check_fn=_check_feishu, - requires_env=[], - is_async=False, - description="Reply to a document comment", - emoji="\u2709\ufe0f", -) - -registry.register( - name="feishu_drive_add_comment", - toolset="feishu_drive", - schema=FEISHU_DRIVE_ADD_COMMENT_SCHEMA, - handler=_handle_add_comment, - check_fn=_check_feishu, - requires_env=[], - is_async=False, - description="Add a whole-document comment", - emoji="\u2709\ufe0f", -) diff --git a/toolsets.py b/toolsets.py index f1dc7fca1c1e..2cbf593ca94b 100644 --- a/toolsets.py +++ b/toolsets.py @@ -201,22 +201,6 @@ "includes": [] }, - "feishu_doc": { - "description": "Read Feishu/Lark document content", - "tools": ["feishu_doc_read"], - "includes": [] - }, - - "feishu_drive": { - "description": "Feishu/Lark document comment operations (list, reply, add)", - "tools": [ - "feishu_drive_list_comments", "feishu_drive_list_comment_replies", - "feishu_drive_reply_comment", "feishu_drive_add_comment", - ], - "includes": [] - }, - - # Scenario-specific toolsets "debugging": { diff --git a/website/docs/reference/tools-reference.md b/website/docs/reference/tools-reference.md index c255c8f6a41a..49a11d9ff3c2 100644 --- a/website/docs/reference/tools-reference.md +++ b/website/docs/reference/tools-reference.md @@ -54,25 +54,6 @@ In addition to built-in tools, Hermes can load tools dynamically from MCP server |------|-------------|----------------------| | `delegate_task` | Spawn one or more subagents to work on tasks in isolated contexts. Each subagent gets its own conversation, terminal session, and toolset. Only the final summary is returned -- intermediate tool results never enter your context window. TWO… | — | -## `feishu_doc` toolset - -Scoped to the Feishu document-comment intelligent-reply handler (`gateway/platforms/feishu_comment.py`). Not exposed on `hermes-cli` or the regular Feishu chat adapter. - -| Tool | Description | Requires environment | -|------|-------------|----------------------| -| `feishu_doc_read` | Read the full text content of a Feishu/Lark document (Docx, Doc, or Sheet) given its file_type and token. | Feishu app credentials | - -## `feishu_drive` toolset - -Scoped to the Feishu document-comment handler. Drives comment read/write operations on drive files. - -| Tool | Description | Requires environment | -|------|-------------|----------------------| -| `feishu_drive_add_comment` | Add a top-level comment on a Feishu/Lark document or file. | Feishu app credentials | -| `feishu_drive_list_comments` | List whole-document comments on a Feishu/Lark file, most recent first. | Feishu app credentials | -| `feishu_drive_list_comment_replies` | List replies on a specific Feishu comment thread (whole-doc or local-selection). | Feishu app credentials | -| `feishu_drive_reply_comment` | Post a reply on a Feishu comment thread, with optional `@`-mention. | Feishu app credentials | - ## `file` toolset | Tool | Description | Requires environment | diff --git a/website/docs/reference/toolsets-reference.md b/website/docs/reference/toolsets-reference.md index bb911004e192..1b15de7dfa49 100644 --- a/website/docs/reference/toolsets-reference.md +++ b/website/docs/reference/toolsets-reference.md @@ -57,8 +57,6 @@ Or in-session: | `code_execution` | `execute_code` | Run Python scripts that call Hermes tools programmatically. | | `cronjob` | `cronjob` | Schedule and manage recurring tasks. | | `delegation` | `delegate_task` | Spawn isolated subagent instances for parallel work. | -| `feishu_doc` | `feishu_doc_read` | Read Feishu/Lark document content. Used by the Feishu document-comment intelligent-reply handler. | -| `feishu_drive` | `feishu_drive_add_comment`, `feishu_drive_list_comments`, `feishu_drive_list_comment_replies`, `feishu_drive_reply_comment` | Feishu/Lark drive comment operations. Scoped to the comment agent; not exposed on `hermes-cli` or other messaging toolsets. | | `file` | `patch`, `read_file`, `search_files`, `write_file` | File reading, writing, searching, and editing. | | `homeassistant` | `ha_call_service`, `ha_get_state`, `ha_list_entities`, `ha_list_services` | Smart home control via Home Assistant. Only available when `HASS_TOKEN` is set. | | `image_gen` | `image_generate` | Text-to-image generation via FAL.ai. | @@ -104,7 +102,7 @@ Platform toolsets define the complete tool configuration for a deployment target | `hermes-sms` | Same as `hermes-cli`. | | `hermes-bluebubbles` | Same as `hermes-cli`. | | `hermes-dingtalk` | Same as `hermes-cli`. | -| `hermes-feishu` | Same as `hermes-cli`. Note: the `feishu_doc` / `feishu_drive` toolsets are used only by the document-comment handler, not by the regular Feishu chat adapter. | +| `hermes-feishu` | Same as `hermes-cli`. The document-comment handler (`feishu_comment.py`) runs its own agent with no feishu-specific tools, fetching document content via a business-code sentinel protocol. | | `hermes-qqbot` | Same as `hermes-cli`. | | `hermes-wecom` | Same as `hermes-cli`. | | `hermes-wecom-callback` | Same as `hermes-cli`. | diff --git a/website/docs/user-guide/messaging/feishu.md b/website/docs/user-guide/messaging/feishu.md index d2b52dff4bd7..7a6fd43a8d49 100644 --- a/website/docs/user-guide/messaging/feishu.md +++ b/website/docs/user-guide/messaging/feishu.md @@ -251,7 +251,7 @@ Beyond chat, the adapter can also answer `@`-mentions left on **Feishu/Lark docu Powered by the `drive.notice.comment_add_v1` event, the handler: - Fetches the document content and comment timeline in parallel (20 messages for whole-doc threads, 12 for local-selection threads). -- Runs the agent with the `feishu_doc` + `feishu_drive` toolsets scoped to that single comment session. +- Runs the agent with no feishu-specific tools; if the agent needs document content, it emits a `{"tokens": [...]}` sentinel and the business code fetches the requested docs (constrained to a whitelist of the source document and comment-referenced docs) before re-invoking the agent. - Chunks replies at 4000 chars and posts them back as threaded replies. - Caches per-document sessions for 1 hour with a 50-message cap so follow-up comments on the same doc keep context.