fix(keyword): retain distinct downloaded document replies

This commit is contained in:
Rudy Celekli committed 2026-10-11 09:07:41 -04:00
1 parent 5f54fc5beb
commit 5cc72cff23
4 files changed
+74 -2

No files matched your search

@@ -0,0 +1,19 @@
# ADR 028: fix(keyword): retain distinct downloaded document replies
Status: Accepted for this candidate.
## Context
Every keyword document download is saved as tmp/<remote-basename>. Two different URLs ending in report.pdf overwrite the same file before queued replies send it, so an earlier reply delivers the later document.
## Decision
Allocate a fresh managed temporary directory for each successful download, preserving the original basename used for outgoing display.
## Consequences
Sequential and concurrent replies keep their own bytes and filenames. The files remain inside the existing managed tmp area; failed downloads create no staging directory.
## Validation
The focused regression fails on the unmodified base and passes with this change. tests/test_keyword_unique_document_downloads.py tests/test_keyword_attachment_urls.py passed locally; changed Python files passed CI-equivalent Ruff F checks. These are local native-contract checks using controlled doubles, not live provider or channel acceptance.
+7 -1
View File
@@ -2,6 +2,8 @@
import json
import os
import tempfile
from pathlib import Path
import requests
from urllib.parse import urlparse
import plugins
@@ -72,7 +74,7 @@ class Keyword(Plugin):
# 下载到 Agent 受管的 tmp 目录。不要自己拼 "tmp":那是相对进程 CWD
# 解析的,打包后的桌面端控制不了 CWD,甚至可能没有写权限——
# 原来那句 makedirs 会直接抛错,被外层 except 吞掉后关键词静默不回复。
file_path = state_dir.tmp_dir() / file_name
tmp_dir = state_dir.tmp_dir()
# Bound the request. The reply is built on the thread handling
# this message, and requests with no timeout waits forever, so a
# file host that accepts the connection and then stalls would
@@ -102,6 +104,10 @@ class Keyword(Plugin):
reply.type = ReplyType.ERROR
reply.content = f"下载失败:{failure}"
else:
# Replies may wait in the send queue. A fresh directory
# keeps earlier bytes intact without changing the display name.
download_dir = Path(tempfile.mkdtemp(prefix="keyword_", dir=str(tmp_dir)))
file_path = download_dir / file_name
file_path.write_bytes(response.content)
reply = Reply()
reply.type = ReplyType.FILE
+2 -1
View File
@@ -68,7 +68,8 @@ def test_xlsx_url_uses_path_filename_without_query_string(tmp_path):
assert reply.type is ReplyType.FILE
# The query string must not leak into the saved name.
assert Path(reply.content).name == "report.XLSX"
assert Path(reply.content).parent == expected_dir
assert Path(reply.content).parent.parent == expected_dir
assert Path(reply.content).parent.name.startswith("keyword_")
assert Path(reply.content).read_bytes() == b"spreadsheet"
finally:
set_agent_registry(None)
@@ -0,0 +1,46 @@
"""Pending keyword file replies must not overwrite one another's bytes."""
import importlib
from concurrent.futures import ThreadPoolExecutor
from pathlib import Path
from types import SimpleNamespace
import plugins
from bridge.context import Context, ContextType
from bridge.reply import ReplyType
from plugins import Event, EventContext
plugins.instance.current_plugin_path = "./plugins/keyword"
keyword_module = importlib.import_module("plugins.keyword.keyword")
plugins.instance.current_plugin_path = None
Keyword = plugins.instance.plugins["KEYWORD"]
def handle(url):
plugin = Keyword.__new__(Keyword)
plugin.keyword = {"download": url}
event = EventContext(Event.ON_HANDLE_CONTEXT, {"context": Context(ContextType.TEXT, "download"), "reply": None})
plugin.on_handle_context(event)
return event["reply"]
def test_two_urls_with_same_basename_retain_each_document(monkeypatch, tmp_path):
monkeypatch.setattr(keyword_module.state_dir, "tmp_dir", lambda: tmp_path)
monkeypatch.setattr(keyword_module.requests, "get", lambda url, **kwargs: SimpleNamespace(status_code=200, content=url.encode()))
first = handle("https://one.invalid/report.pdf")
second = handle("https://two.invalid/report.pdf")
assert first.type is second.type is ReplyType.FILE
assert first.content != second.content
assert Path(first.content).read_bytes() == b"https://one.invalid/report.pdf"
assert Path(second.content).read_bytes() == b"https://two.invalid/report.pdf"
assert Path(first.content).name == Path(second.content).name == "report.pdf"
def test_simultaneous_downloads_keep_all_reply_paths(monkeypatch, tmp_path):
monkeypatch.setattr(keyword_module.state_dir, "tmp_dir", lambda: tmp_path)
monkeypatch.setattr(keyword_module.requests, "get", lambda url, **kwargs: SimpleNamespace(status_code=200, content=url.encode()))
urls = [f"https://host{i}.invalid/report.pdf" for i in range(8)]
with ThreadPoolExecutor(max_workers=8) as pool:
replies = list(pool.map(handle, urls))
assert len({reply.content for reply in replies}) == len(urls)
for url, reply in zip(urls, replies):
assert Path(reply.content).read_bytes() == url.encode()