mirror of
https://github.com/hansjone/oclaw.git
synced 2026-10-09 03:30:48 +08:00
fix(attachments): parse channel uploads for LLM and stop echoing user files
WhatsApp/channel binary_ref uploads now expand into text_ref/tabular summaries for the model, and outbound replies only auto-attach generated image/video media instead of lookup tool text_ref results. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
fc10560052
commit
61d54b10fd
5 changed files with 215 additions and 10 deletions
|
|
@ -473,6 +473,7 @@ def _channel_attachments_for_gateway(raw: list[dict[str, Any]]) -> list[dict[str
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from svc.files.attachment_assets import AttachmentAssetStore
|
from svc.files.attachment_assets import AttachmentAssetStore
|
||||||
|
from svc.files.file_attachments import expand_attachment_ref, process_file_data
|
||||||
|
|
||||||
out: list[dict[str, Any]] = []
|
out: list[dict[str, Any]] = []
|
||||||
ast = AttachmentAssetStore()
|
ast = AttachmentAssetStore()
|
||||||
|
|
@ -481,8 +482,7 @@ def _channel_attachments_for_gateway(raw: list[dict[str, Any]]) -> list[dict[str
|
||||||
continue
|
continue
|
||||||
aid = str(a.get("attachment_id") or a.get("attachmentId") or "").strip().lower()
|
aid = str(a.get("attachment_id") or a.get("attachmentId") or "").strip().lower()
|
||||||
if aid:
|
if aid:
|
||||||
t = str(a.get("type") or "").strip().lower() or "binary_ref"
|
out.extend(expand_attachment_ref(a))
|
||||||
out.append({"type": t, "attachment_id": aid})
|
|
||||||
continue
|
continue
|
||||||
lp = str(a.get("local_path") or a.get("media_path") or "").strip()
|
lp = str(a.get("local_path") or a.get("media_path") or "").strip()
|
||||||
if not lp:
|
if not lp:
|
||||||
|
|
@ -495,7 +495,15 @@ def _channel_attachments_for_gateway(raw: list[dict[str, Any]]) -> list[dict[str
|
||||||
if not mime:
|
if not mime:
|
||||||
mime = "application/octet-stream"
|
mime = "application/octet-stream"
|
||||||
try:
|
try:
|
||||||
meta = ast.save_bytes(p.read_bytes(), filename=p.name, mime=mime)
|
data = p.read_bytes()
|
||||||
|
except Exception:
|
||||||
|
continue
|
||||||
|
got = process_file_data(p.name, data)
|
||||||
|
if got:
|
||||||
|
out.extend(got)
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
meta = ast.save_bytes(data, filename=p.name, mime=mime)
|
||||||
except Exception:
|
except Exception:
|
||||||
continue
|
continue
|
||||||
if kind == "image" or mime.startswith("image/"):
|
if kind == "image" or mime.startswith("image/"):
|
||||||
|
|
@ -503,7 +511,7 @@ def _channel_attachments_for_gateway(raw: list[dict[str, Any]]) -> list[dict[str
|
||||||
elif kind == "video" or mime.startswith("video/"):
|
elif kind == "video" or mime.startswith("video/"):
|
||||||
out.append({"type": "video_ref", "attachment_id": meta.attachment_id})
|
out.append({"type": "video_ref", "attachment_id": meta.attachment_id})
|
||||||
else:
|
else:
|
||||||
out.append({"type": "binary_ref", "attachment_id": meta.attachment_id})
|
out.append({"type": "binary_ref", "attachment_id": meta.attachment_id, "name": meta.name, "mime": meta.mime})
|
||||||
return out
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -548,10 +556,16 @@ def _rows_since_last_user_message(rows: list[Any]) -> list[Any]:
|
||||||
return list(rows[last_user_idx + 1 :])
|
return list(rows[last_user_idx + 1 :])
|
||||||
|
|
||||||
|
|
||||||
|
_CHANNEL_DELIVERABLE_ATTACHMENT_TYPES = frozenset(
|
||||||
|
{"image_ref", "video_ref", "image", "input_image", "image_url"}
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _collect_recent_tool_attachments(*, store: Any, session_id: str) -> list[dict[str, Any]]:
|
def _collect_recent_tool_attachments(*, store: Any, session_id: str) -> list[dict[str, Any]]:
|
||||||
"""Fallback for channel delivery: reuse tool media produced during the current user turn only.
|
"""Fallback for channel delivery: reuse tool media produced during the current user turn only.
|
||||||
|
|
||||||
Avoids re-sending images from earlier conversation turns when the latest assistant row has no attachments.
|
Avoids re-sending images from earlier conversation turns when the latest assistant row has no attachments.
|
||||||
|
Only visual outbound media is eligible — not text_ref/binary_ref from read/query tools.
|
||||||
"""
|
"""
|
||||||
sid = str(session_id or "").strip()
|
sid = str(session_id or "").strip()
|
||||||
if not sid:
|
if not sid:
|
||||||
|
|
@ -569,15 +583,16 @@ def _collect_recent_tool_attachments(*, store: Any, session_id: str) -> list[dic
|
||||||
if not atts:
|
if not atts:
|
||||||
continue
|
continue
|
||||||
ok = False
|
ok = False
|
||||||
|
deliverable: list[dict[str, Any]] = []
|
||||||
for a in atts:
|
for a in atts:
|
||||||
if not isinstance(a, dict):
|
if not isinstance(a, dict):
|
||||||
continue
|
continue
|
||||||
t = str(a.get("type") or "").strip().lower()
|
t = str(a.get("type") or "").strip().lower()
|
||||||
if t in {"image_ref", "video_ref", "binary_ref", "text_ref", "image", "input_image", "image_url"}:
|
if t in _CHANNEL_DELIVERABLE_ATTACHMENT_TYPES:
|
||||||
ok = True
|
ok = True
|
||||||
break
|
deliverable.append(a)
|
||||||
if ok:
|
if ok:
|
||||||
return atts
|
return deliverable
|
||||||
return []
|
return []
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,7 +17,8 @@ from svc.llm.chat_models import _normalize_image_b64_payload, gemini_openai_comp
|
||||||
from runtime.chat.media_redact import redact_embedded_image_blobs
|
from runtime.chat.media_redact import redact_embedded_image_blobs
|
||||||
from runtime.chat.tool_runtime import tool_llm_message_max_chars, truncate_tool_result_for_llm_messages
|
from runtime.chat.tool_runtime import tool_llm_message_max_chars, truncate_tool_result_for_llm_messages
|
||||||
from runtime.prompt_templates import render_prompt
|
from runtime.prompt_templates import render_prompt
|
||||||
from svc.files.attachment_assets import attachment_id_to_data_url
|
from svc.files.attachment_assets import attachment_id_to_data_url, AttachmentAssetStore
|
||||||
|
from svc.files.file_attachments import expand_attachment_ref
|
||||||
from runtime.relay_pointer import parse_pointer_uri
|
from runtime.relay_pointer import parse_pointer_uri
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
@ -316,11 +317,24 @@ def build_llm_messages(
|
||||||
except Exception:
|
except Exception:
|
||||||
attachments = []
|
attachments = []
|
||||||
|
|
||||||
|
expand_user_image_for_model = bool(i == last_user_msg_idx)
|
||||||
|
if expand_user_image_for_model:
|
||||||
|
expanded_atts: list[dict[str, Any]] = []
|
||||||
|
for att in attachments or []:
|
||||||
|
if not isinstance(att, dict):
|
||||||
|
continue
|
||||||
|
if str(att.get("type") or "").strip().lower() == "binary_ref":
|
||||||
|
subs = expand_attachment_ref(att)
|
||||||
|
if subs and any(str(s.get("type") or "") != "binary_ref" for s in subs):
|
||||||
|
expanded_atts.extend(subs)
|
||||||
|
continue
|
||||||
|
expanded_atts.append(att)
|
||||||
|
attachments = expanded_atts
|
||||||
|
|
||||||
for att in attachments or []:
|
for att in attachments or []:
|
||||||
if not isinstance(att, dict):
|
if not isinstance(att, dict):
|
||||||
continue
|
continue
|
||||||
att_type = att.get("type")
|
att_type = att.get("type")
|
||||||
expand_user_image_for_model = bool(i == last_user_msg_idx)
|
|
||||||
if att_type in ("image", "input_image"):
|
if att_type in ("image", "input_image"):
|
||||||
if expand_user_image_for_model:
|
if expand_user_image_for_model:
|
||||||
b64 = _normalize_image_b64_payload(att.get("image_base64") or att.get("data"))
|
b64 = _normalize_image_b64_payload(att.get("image_base64") or att.get("data"))
|
||||||
|
|
@ -459,6 +473,29 @@ def build_llm_messages(
|
||||||
meta_line += f"\n- bytes: {sz}"
|
meta_line += f"\n- bytes: {sz}"
|
||||||
meta_line += "\n- tools: query_video_attachment"
|
meta_line += "\n- tools: query_video_attachment"
|
||||||
content_list.append({"type": "text", "text": meta_line})
|
content_list.append({"type": "text", "text": meta_line})
|
||||||
|
elif att_type == "binary_ref":
|
||||||
|
aid = str(att.get("attachment_id") or "").strip()
|
||||||
|
name = str(att.get("name") or "file")
|
||||||
|
mime = str(att.get("mime") or "application/octet-stream")
|
||||||
|
sz = att.get("bytes")
|
||||||
|
if aid and (name == "file" or not mime or mime == "application/octet-stream"):
|
||||||
|
try:
|
||||||
|
meta = AttachmentAssetStore().get_meta(aid)
|
||||||
|
if meta:
|
||||||
|
if name == "file":
|
||||||
|
name = str(meta.name or name)
|
||||||
|
if not mime or mime == "application/octet-stream":
|
||||||
|
mime = str(meta.mime or mime)
|
||||||
|
if sz is None:
|
||||||
|
sz = meta.bytes
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
meta_line = f"[BinaryAttachment]\n- name: {name}\n- mime: {mime}\n- attachment_id: {aid}"
|
||||||
|
if sz:
|
||||||
|
meta_line += f"\n- bytes: {sz}"
|
||||||
|
meta_line += "\n- tools: attachment_local_url"
|
||||||
|
meta_line += "\n- note: user uploaded a file; resolve or analyze it via attachment_id."
|
||||||
|
content_list.append({"type": "text", "text": meta_line})
|
||||||
elif att_type == "relay_pointer":
|
elif att_type == "relay_pointer":
|
||||||
p_uri = str(att.get("pointer_uri") or "").strip()
|
p_uri = str(att.get("pointer_uri") or "").strip()
|
||||||
if not p_uri:
|
if not p_uri:
|
||||||
|
|
|
||||||
|
|
@ -373,6 +373,47 @@ def process_zip(zip_data: bytes, *, _depth: int = 0) -> list[dict[str, Any]]:
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def expand_attachment_ref(
|
||||||
|
att: dict[str, Any],
|
||||||
|
*,
|
||||||
|
data: bytes | None = None,
|
||||||
|
name: str | None = None,
|
||||||
|
) -> list[dict[str, Any]]:
|
||||||
|
"""Expand ``binary_ref`` (or raw bytes) into parsed attachment dicts for the agent."""
|
||||||
|
if not isinstance(att, dict):
|
||||||
|
return []
|
||||||
|
t = str(att.get("type") or "").strip().lower()
|
||||||
|
if t and t not in {"binary_ref"}:
|
||||||
|
return [att]
|
||||||
|
aid = str(att.get("attachment_id") or att.get("attachmentId") or "").strip().lower()
|
||||||
|
meta = None
|
||||||
|
if data is None and aid:
|
||||||
|
blob, meta = AttachmentAssetStore().load_bytes(aid)
|
||||||
|
if not blob:
|
||||||
|
return [att] if t == "binary_ref" else []
|
||||||
|
data = blob
|
||||||
|
name = name or str(getattr(meta, "name", "") or "file")
|
||||||
|
if data is None:
|
||||||
|
return [att] if t == "binary_ref" else []
|
||||||
|
fname = str(name or att.get("name") or getattr(meta, "name", "") or "file")
|
||||||
|
got = process_file_data(fname, data)
|
||||||
|
if got:
|
||||||
|
return got
|
||||||
|
if aid:
|
||||||
|
if meta is None:
|
||||||
|
meta = AttachmentAssetStore().get_meta(aid)
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
"type": "binary_ref",
|
||||||
|
"attachment_id": aid,
|
||||||
|
"name": fname,
|
||||||
|
"mime": str(att.get("mime") or getattr(meta, "mime", "") or "application/octet-stream"),
|
||||||
|
"bytes": att.get("bytes") if att.get("bytes") is not None else getattr(meta, "bytes", None),
|
||||||
|
}
|
||||||
|
]
|
||||||
|
return []
|
||||||
|
|
||||||
|
|
||||||
def process_file_data(name: str, data: bytes, *, _zip_depth: int = 0) -> list[dict[str, Any]]:
|
def process_file_data(name: str, data: bytes, *, _zip_depth: int = 0) -> list[dict[str, Any]]:
|
||||||
ext = name.split(".")[-1].lower() if "." in name else ""
|
ext = name.split(".")[-1].lower() if "." in name else ""
|
||||||
attachments: list[dict[str, Any]] = []
|
attachments: list[dict[str, Any]] = []
|
||||||
|
|
@ -773,4 +814,4 @@ def process_file_data(name: str, data: bytes, *, _zip_depth: int = 0) -> list[di
|
||||||
return attachments
|
return attachments
|
||||||
|
|
||||||
|
|
||||||
__all__ = ["process_zip", "process_file_data", "clear_attachment_limits_cache"]
|
__all__ = ["process_zip", "process_file_data", "expand_attachment_ref", "clear_attachment_limits_cache"]
|
||||||
|
|
|
||||||
98
tests/test_binary_ref_attachments.py
Normal file
98
tests/test_binary_ref_attachments.py
Normal file
|
|
@ -0,0 +1,98 @@
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from types import SimpleNamespace
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from runtime.application.gateway.inbound_service import _channel_attachments_for_gateway
|
||||||
|
from runtime.chat.agent_messages import build_llm_messages
|
||||||
|
from svc.files.attachment_assets import AttachmentAssetStore
|
||||||
|
from svc.files.file_attachments import expand_attachment_ref
|
||||||
|
from svc.llm.chat_models import RuleBasedChatModel
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def att_store(tmp_path, monkeypatch):
|
||||||
|
store = AttachmentAssetStore(root_dir=tmp_path / "att")
|
||||||
|
|
||||||
|
def _factory(root_dir=None):
|
||||||
|
if root_dir is None:
|
||||||
|
return store
|
||||||
|
return AttachmentAssetStore(root_dir=root_dir)
|
||||||
|
|
||||||
|
monkeypatch.setattr("svc.files.file_attachments.AttachmentAssetStore", _factory)
|
||||||
|
monkeypatch.setattr("runtime.chat.agent_messages.AttachmentAssetStore", _factory)
|
||||||
|
return store
|
||||||
|
|
||||||
|
|
||||||
|
def _msg_text(message: dict) -> str:
|
||||||
|
content = message.get("content")
|
||||||
|
if isinstance(content, list):
|
||||||
|
return "\n".join(str(x.get("text") or "") for x in content if isinstance(x, dict))
|
||||||
|
return str(content or "")
|
||||||
|
|
||||||
|
|
||||||
|
def test_expand_attachment_ref_large_txt_becomes_text_ref(att_store) -> None:
|
||||||
|
text = ("line\n" * 5000).strip()
|
||||||
|
data = text.encode("utf-8")
|
||||||
|
meta = att_store.save_bytes(data, filename="sites.txt", mime="text/plain")
|
||||||
|
got = expand_attachment_ref({"type": "binary_ref", "attachment_id": meta.attachment_id})
|
||||||
|
types = {str(x.get("type") or "") for x in got}
|
||||||
|
assert "text_ref" in types
|
||||||
|
assert "text" in types
|
||||||
|
|
||||||
|
|
||||||
|
def test_channel_attachments_for_gateway_parses_local_txt(tmp_path) -> None:
|
||||||
|
p = tmp_path / "Site_List.txt"
|
||||||
|
p.write_text("a\nb\nc\n", encoding="utf-8")
|
||||||
|
got = _channel_attachments_for_gateway([{"local_path": str(p), "kind": "document", "mime": "text/plain"}])
|
||||||
|
assert got
|
||||||
|
assert any(str(x.get("type") or "") == "text" for x in got)
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_llm_messages_binary_ref_meta_for_historical_turn(att_store) -> None:
|
||||||
|
data = b"hello attachment"
|
||||||
|
meta = att_store.save_bytes(data, filename="note.txt", mime="text/plain")
|
||||||
|
rows = [
|
||||||
|
SimpleNamespace(
|
||||||
|
role="user",
|
||||||
|
event_type="user_text",
|
||||||
|
content="first",
|
||||||
|
attachments=[{"type": "binary_ref", "attachment_id": meta.attachment_id}],
|
||||||
|
tool_calls=None,
|
||||||
|
turn_uuid="t1",
|
||||||
|
),
|
||||||
|
SimpleNamespace(
|
||||||
|
role="user",
|
||||||
|
event_type="user_text",
|
||||||
|
content="second",
|
||||||
|
attachments=None,
|
||||||
|
tool_calls=None,
|
||||||
|
turn_uuid="t2",
|
||||||
|
),
|
||||||
|
]
|
||||||
|
msgs = build_llm_messages(store_messages=rows, system_prompt="s", model=RuleBasedChatModel(), lang="zh")
|
||||||
|
first_user = next(m for m in msgs if m.get("role") == "user" and "first" in _msg_text(m))
|
||||||
|
content = _msg_text(first_user)
|
||||||
|
assert "BinaryAttachment" in content
|
||||||
|
assert meta.attachment_id in content
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_llm_messages_binary_ref_expands_on_last_turn(att_store) -> None:
|
||||||
|
text = ("row\n" * 4000).strip()
|
||||||
|
data = text.encode("utf-8")
|
||||||
|
meta = att_store.save_bytes(data, filename="Site_List.txt", mime="text/plain")
|
||||||
|
rows = [
|
||||||
|
SimpleNamespace(
|
||||||
|
role="user",
|
||||||
|
event_type="user_text",
|
||||||
|
content="帮看一下有多少数据",
|
||||||
|
attachments=[{"type": "binary_ref", "attachment_id": meta.attachment_id}],
|
||||||
|
tool_calls=None,
|
||||||
|
turn_uuid="t1",
|
||||||
|
),
|
||||||
|
]
|
||||||
|
msgs = build_llm_messages(store_messages=rows, system_prompt="s", model=RuleBasedChatModel(), lang="zh")
|
||||||
|
user = msgs[-1]
|
||||||
|
content = _msg_text(user)
|
||||||
|
assert "LongTextAttachment" in content or "text_id" in content
|
||||||
|
|
@ -174,6 +174,20 @@ def test_collect_recent_tool_attachments_ignores_media_from_prior_turn() -> None
|
||||||
assert out == []
|
assert out == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_collect_recent_tool_attachments_ignores_text_ref_from_lookup_tools() -> None:
|
||||||
|
rows = [
|
||||||
|
_Row(role="user", content="analyze file", attachments=None),
|
||||||
|
_Row(
|
||||||
|
role="tool",
|
||||||
|
content="{}",
|
||||||
|
attachments='[{"type":"text_ref","attachment_id":"user-upload","name":"Site_List.txt","mime":"text/plain"}]',
|
||||||
|
),
|
||||||
|
_Row(role="assistant", content="15,687 rows", attachments=None),
|
||||||
|
]
|
||||||
|
out = _collect_recent_tool_attachments(store=_FakeStore(rows), session_id="s1")
|
||||||
|
assert out == []
|
||||||
|
|
||||||
|
|
||||||
def test_maybe_add_media_path_for_wechat_reply_sets_media_path(monkeypatch) -> None:
|
def test_maybe_add_media_path_for_wechat_reply_sets_media_path(monkeypatch) -> None:
|
||||||
# Avoid touching disk: stub AttachmentAssetStore.get_local_path.
|
# Avoid touching disk: stub AttachmentAssetStore.get_local_path.
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue