Repository navigation
feat: add youcom (You.com) provider to WebSearchTool - #358
valueadd-lgtm wants to merge 8 commits into
Conversation
Add a fourth pluggable search backend, "youcom", which calls the You.com you-search MCP tool over stateless JSON-RPC (streamable HTTP): - keyless by default via the free-profile endpoint (https://api.you.com/mcp?profile=free); YDC_API_KEY (or api_key) switches to the authenticated endpoint - maps web + news hits to SearchHit (description first, query-relevant highlights as snippet fallback) - lang maps to the inline lang: filter; blocked_domains map to server-side exclude_domains, allowed_domains filter client-side - JSON-RPC / tool errors surface as structured summaries - youcom_extra_params passthrough (freshness, safesearch, ...) - tests: SSE parsing, keyless/authenticated URLs, Bearer header, domain filtering, extra params, error paths, dedup - docs updated (en + zh): backend list, parameter tables, usage example Default provider and all existing behavior unchanged.
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
AI Code Review审查结论通过 审查范围：commit ed846d4（base 02509b2）为 WebSearchTool 新增 youcom（You.com you-search MCP）provider，共 4 个变更文件（trpc_agent_sdk/tools/_websearch_tool.py、tests/tools/test_websearch_tool.py、docs/mkdocs 英文与中文文档），对全部非生成变更文件完成了逐行、跨文件、协议与语言语义层面的审查。 计划符合性：计划为“add youcom (You.com) provider to WebSearchTool”。实现完整覆盖计划：youcom_extra_params 构造参数、YDC_API_KEY 环境变量回退、free/认证端点自动切换、_run_async_impl 分派分支、ProviderType Literal 扩展、SSE 解析（_parse_sse_payloads/_post_sse）与 15 个新增单元测试，中英文文档同步更新，无缺项。 测试充分性：正常路径、lang 内联过滤、域名过滤、extra params、count 限制、URL 去重、JSON-RPC 错误、isError、无结果与畸形 text 等路径均有覆盖，且所有新测试均正确关闭 mock client（aclose 无遗漏），测试内嵌 mock 与实现假设一致。缺口主要在与上游真实行为相关的畸形响应形态（highlights 非数组、isError 空文本、SSE 多行帧、results 形状漂移）上，已随各条评论给出。 主要风险：已证实 2 个 MODERATE 问题——(1) highlights[0] 未做类型校验，字符串→单字符 snippet、dict→KeyError 逃逸到 _run_async_impl 宽泛 except 使整次搜索变 SEARCH_ERROR；(2) isError=true 且 text 为空时落入误导性的 unexpected payload 摘要、且从不读取 MCP structuredContent。另有 3 个机制真实但触发依赖上游行为的 PLAUSIBLE 问题（error/result 帧优先级与 id 匹配、lang: 操作符拼接、SSE 跨行 data 帧/BOM）。经核实的排除项：域名 allow/block 列表有第 868 行 _is_blocked 客户端二次过滤、无绕过；JSON 对象键恒为字符串，"result" in item 不可能抛 TypeError；text 经 str() 强转后 json.loads 的类型异常已被捕获；_post_sse 客户端生命周期与既有 _get_json/_post_json 模式一致（注入 client 不 aclose、内部 client 在 finally 关闭），无泄漏。未发现安全、数据完整性或向后兼容性破坏。 门禁结论：无 SEVERE 级缺陷，state 为 PASSED。建议后续针对上述已证实问题（highlights 类型防御、isError 空文本路径）及其余 PLAUSIBLE 项补充畸形 payload 测试并修复。 发现的问题中等
问题： 触发条件： You.com 返回的命中项中 实际影响： 一条畸形命中会把整次成功搜索变成 修正方向： 与其它字段一致做类型防御： 中等
问题： 工具错误分支 触发条件： 上游服务端标记 实际影响： LLM 收到 "unexpected payload" 而非真实错误信息，无法区分“上游失败”与“响应格式异常”，文档承诺的“错误写入 summary”契约在该形态下失效，排障与降级都受到误导，并可能因此误报为“无结果”。 修正方向： 将 中等
问题： 对 SSE 帧的扫描 触发条件： 流内任一帧（如进度通知）携带 实际影响： 合法搜索结果被误报为错误摘要，或真实限流/配额错误被吞掉并表现为空结果成功，两种情况都与文档“错误写入 summary”的契约相反，结果不稳定且难以排障。 修正方向： 按 JSON-RPC 语义处理：只接受与请求 较低
问题： 触发条件： 上游把长 JSON 结果拆分为两行 实际影响： 修正方向： 按 SSE 规范支持跨行 data 帧（累计同一事件的多行 data 并检查本行是否为续行 较低
问题： 触发条件： 调用方固化 实际影响： 调用时的 query / 语言过滤被静默替换或失效（ 修正方向： 仅允许 extra params 覆盖白名单之外的键（对 较低
问题： 触发条件： You.com 响应结构漂移（如 实际影响： LLM 收到“搜索成功但 0 结果”，会如实向用户报告“未找到结果”而不会重试，与其它错误路径“结构化错误写入 summary”的契约不一致，掩盖了真实的协议兼容问题。 修正方向： 对非 dict 且非空的 |
| contents = item.get("contents") | ||
| highlights = contents.get("highlights") if isinstance(contents, dict) else None | ||
| if highlights: | ||
| description = str(highlights[0]).strip() |
There was a problem hiding this comment.
问题： _search_youcom 中 highlights 的类型校验缺失：代码只检查了 contents 是否为 dict，随后 if highlights: description = str(highlights[0]).strip() 直接对 highlights[0] 按下标取值，与其它 provider 对每个字段先做类型判断的防御风格不一致。
触发条件： You.com 返回的命中项中 contents.highlights 不是数组。若为字符串（如 "some text"），highlights[0] 取到单字符 "s"；若为 dict 或其它类型，highlights[0] 抛 KeyError/TypeError。该异常在 _search_youcom 内部未捕获，逃逸到 _run_async_impl 第 566 行的宽泛 except Exception（SEARCH_ERROR）。已实测复现两种行为。
实际影响： 一条畸形命中会把整次成功搜索变成 {"error": "SEARCH_ERROR: ..."}，所有已解析的合法命中与 query 一并丢失，LLM 无法降级使用部分结果；highlights 为字符串时用户看到 1 个字符的残缺 snippet。
修正方向： 与其它字段一致做类型防御：if isinstance(highlights, list) and highlights:，并对首元素做元素级校验后取值，或至少用 try/except 包裹该段，避免异常逃逸出 _search_youcom。
| if result.get("isError") and text: | ||
| return WebSearchResult( | ||
| query=query, | ||
| provider="youcom", | ||
| results=[], | ||
| summary=_truncate(f"You.com search error: {text}", self._snippet_len), | ||
| ) | ||
|
|
||
| data: Any = None | ||
| try: | ||
| data = json.loads(text) | ||
| except (json.JSONDecodeError, TypeError): |
There was a problem hiding this comment.
问题： 工具错误分支 if result.get("isError") and text: 要求 text 非空才返回错误摘要；若 isError=true 且 content[].text 为空（或信息只存在于 MCP 规范的 structuredContent 字段），代码落入后续 json.loads("")，被捕获后返回误导性的 "You.com search returned an unexpected payload." 成功形态摘要，且代码从未读取 structuredContent。
触发条件： 上游服务端标记 isError: true 但未携带 text 内容（MCP 层常如此），或把结构化结果放在 structuredContent 而 text 仅为人读摘要。两种情况在单元测试中均未覆盖（测试只覆盖了 isError 且 text 非空、以及 text 携带 JSON 两种形态）。
实际影响： LLM 收到 "unexpected payload" 而非真实错误信息，无法区分“上游失败”与“响应格式异常”，文档承诺的“错误写入 summary”契约在该形态下失效，排障与降级都受到误导，并可能因此误报为“无结果”。
修正方向： 将 isError 判定提前到内容提取之前：先检查 result.get("isError")，无论 text 是否为空都返回 You.com search error: ... 摘要；可同时读取 structuredContent 作为 text 的补充来源。
| result: Optional[dict[str, Any]] = None | ||
| error: Optional[dict[str, Any]] = None | ||
| for item in payloads: | ||
| if "result" in item: | ||
| result = item["result"] | ||
| elif "error" in item: | ||
| error = item["error"] | ||
| if error is not None: |
There was a problem hiding this comment.
问题： 对 SSE 帧的扫描 if "result" in item ... elif "error" in item 使“先 error 后 result”或“同帧双键”的流中 error 信息被错误取舍：错误分支在循环外 if error is not None 无条件优先于 result，且末帧胜出语义未按 JSON-RPC 的 id 匹配（请求固定 id: 1，响应未校验 id）。
触发条件： 流内任一帧（如进度通知）携带 error 键且其后跟有效 result 帧（已用模拟流实测：error 残留导致有效结果被丢弃）；或单帧同时含 result: null 与 error（elif 使 error 被丢弃，最终报 "no result payload"）。
实际影响： 合法搜索结果被误报为错误摘要，或真实限流/配额错误被吞掉并表现为空结果成功，两种情况都与文档“错误写入 summary”的契约相反，结果不稳定且难以排障。
修正方向： 按 JSON-RPC 语义处理：只接受与请求 id 匹配的帧；对同帧 result+error 共存时以 error 为准（先判 error 再判 result 或独立记录）；采用“最后一帧有效结果、显式指定优先级”的明确策略并补充对应测试。
| def _parse_sse_payloads(text: str) -> List[dict[str, Any]]: | ||
| """Parse the ``data:`` lines of a streamable-HTTP SSE body. | ||
|
|
||
| A streamable-HTTP server answers a JSON-RPC request with one SSE body | ||
| whose ``data:`` lines carry one JSON object each (progress notifications | ||
| followed by the final result). Lines that fail to parse are skipped so a | ||
| stray keep-alive or partial frame cannot break the search. | ||
| """ | ||
| payloads: List[dict[str, Any]] = [] | ||
| for line in (text or "").splitlines(): | ||
| if not line.startswith("data:"): | ||
| continue | ||
| raw = line[len("data:"):].strip() | ||
| if not raw: | ||
| continue | ||
| try: | ||
| decoded = json.loads(raw) | ||
| except json.JSONDecodeError: | ||
| logger.warning("WebSearchTool: skipping unparseable SSE line: %.80s", raw) | ||
| continue | ||
| if isinstance(decoded, dict): | ||
| payloads.append(decoded) | ||
| return payloads |
There was a problem hiding this comment.
问题： _parse_sse_payloads 把每个物理 data: 行当作独立 JSON 文档解析，未实现 SSE 规范允许的“单条 data 帧跨多行（后续行以空格开头）拼接”语义；第 297 行 line[len("data:"):] 只按行切片，且未处理首个 data: 行前的 UTF-8 BOM（Python 默认 utf-8 解码不剥离 BOM，line.startswith("data:") 对该行直接失配）。
触发条件： 上游把长 JSON 结果拆分为两行 data: 帧（两段各自无法被 json.loads 解析而被静默跳过，实测确认），或响应体以 BOM 起始；二者都会让一次 200 成功响应被解析为空帧列表。
实际影响： _search_youcom 返回 "You.com search returned no result payload." 空结果——成功搜索被误报为无结果，LLM 无法区分“真无结果”与“解析失败”，影响召回准确性。
修正方向： 按 SSE 规范支持跨行 data 帧（累计同一事件的多行 data 并检查本行是否为续行 data:），并在首行前剥离 BOM；或直接使用仓库已声明的 httpx-sse 依赖（pyproject 已含 httpx-sse>=0.4.0）解析。
| arguments: dict[str, Any] = {"query": query, "count": n} | ||
| if lang: | ||
| # You.com maps an inline ``lang:`` operator to a language filter. | ||
| arguments["query"] = f"{query} lang:{lang}" | ||
| if blocked: | ||
| arguments["exclude_domains"] = blocked | ||
| arguments.update(self._youcom_extra_params) |
There was a problem hiding this comment.
问题： arguments.update(self._youcom_extra_params) 在 query/count/lang: 重写之后执行，youcom_extra_params 中若包含 query、count 或 exclude_domains 键会静默覆盖本工具构造的核心参数；且 lang 值未做任何清洗，schema 描述（第 456 行）明确告诉模型“You.com maps it to the inline lang: filter”，模型可能直接传 lang="lang:ja"，拼接出 query lang:lang:ja。
触发条件： 调用方固化 youcom_extra_params={"query": "...", "count": 3} 等键，或 LLM 按 schema 字面解释传入已带 lang: 前缀的 lang 值；两种都无任何校验或警告。
实际影响： 调用时的 query / 语言过滤被静默替换或失效（lang:lang: 双操作符无法被服务端正确匹配），行为与文档“每次工具调用自动带上固定高级参数、不覆盖调用参数”的表述不符，且无日志可追踪。
修正方向： 仅允许 extra params 覆盖白名单之外的键（对 query/count/exclude_domains 做保护性合并，异常键记录 warning），并给 lang 增加防重前缀清洗（如去除已存在的 lang: 前缀）。
| sections = data.get("results") | ||
| if not isinstance(sections, dict): | ||
| sections = {} | ||
| items: List[dict[str, Any]] = [] | ||
| for section in ("web", "news"): | ||
| for item in sections.get(section) or []: | ||
| if isinstance(item, dict): | ||
| items.append(item) |
There was a problem hiding this comment.
问题： sections = data.get("results"); if not isinstance(sections, dict): sections = {} 对 dict 但形状非预期的响应（如 results 为数组、或嵌套层级与实现假设不同）静默降级为 {}，返回 summary="" 的空成功结果，未写入任何错误信号；测试仅覆盖了 results 为 dict 的正常形态。
触发条件： You.com 响应结构漂移（如 results 变更为扁平数组，或 free profile 命中位于其它嵌套层级）时发生。
实际影响： LLM 收到“搜索成功但 0 结果”，会如实向用户报告“未找到结果”而不会重试，与其它错误路径“结构化错误写入 summary”的契约不一致，掩盖了真实的协议兼容问题。
修正方向： 对非 dict 且非空的 results 返回显式提示（如 _truncate("You.com search returned an unexpected results shape.", ...) 写入 summary），并在映射循环中对 sections.get(section) 的非 list 值同样降级提示。
Address the openreview findings on the youcom provider: - _parse_sse_payloads: join multi-line data: frames per the SSE spec, strip a leading UTF-8 BOM, and fall back to per-line parsing for servers that omit blank-line event separators. - Frame selection: match the JSON-RPC request id (frames for other requests are ignored), give a same-frame error precedence over result: null, and let a later valid result supersede a stray earlier error frame. - isError: report the error whatever the text content is, using structuredContent as the detail source when no text is present, instead of falling through to payload parsing. - Read structuredContent as the results payload when the text content is not JSON (human-readable text + structured results). - Surface a non-dict non-empty results field as an explicit unexpected-shape summary instead of silently reporting 0 hits; skip non-list web/news sections without crashing. - Validate the first highlight is a string before using it as the description fallback. - Strip a pre-existing lang: prefix from the lang argument and ignore youcom_extra_params keys that would override per-call query/count/exclude_domains. Adds 13 tests covering each path; full websearch suite (104 tests), yapf and flake8 clean.
|
I have read the CLA Document and I hereby sign the CLA |
|
Thanks for the thorough review. Both verified MODERATE issues and the other findings are fixed in 04c3ea9, plus malformed-payload tests as suggested:
Added 13 targeted tests (malformed highlights, isError without text, structuredContent, cross-request ids, multi-line data, BOM, missing separators, shape drift). Full websearch suite: 104 tests passing; yapf and flake8 clean. The CLA is signed via the bot's comment format as well. |
|
I have read the CLA Document and I hereby sign the CLA |
AI Code Review审查结论通过 审查范围：base 计划符合性：实现与计划"add youcom (You.com) provider to WebSearchTool"一致——JSON-RPC 主要风险：1) 命中字段映射与官方 you-search 契约存在偏差：官方规范字段为 测试充分性：104 个 websearch 测试（新增 26 个 youcom + 4 个 SSE 解析测试），覆盖 keyless/认证端点、Bearer、域过滤、lang 过滤、JSON-RPC 错误、isError、structuredContent、去重、形状漂移等路径，质量高；缺口是 fixture 未按官方 门禁结论：未发现阻止合入的 SEVERE 级缺陷；报告 2 条 MODERATE、3 条 LOW 可操作问题，建议修复后合入。 发现的问题中等
问题： 新增的 触发条件： 真实端点的 实际影响： 在缺省配置（免密钥 free profile）下，凡是没有 修正方向： 按官方契约补充 中等
问题： 触发条件： 文档（ 实际影响： 未预期的凭据会被随请求发送到第三方端点 修正方向： 在 较低
问题： 帧选择循环中，一旦出现 error 帧随后又出现 result 帧（本提交新增的 触发条件： 服务端在返回最终有效结果前发出携带 实际影响： 相对加固前“无条件报错”的行为，现在真实发生过的失败在结果中完全不可见（ 修正方向： 在“后到 result 覆盖前序 error”的分支处补一条 较低
问题： 触发条件： 服务端把请求 id 以不同 JSON 类型回显（例如把 实际影响： 搜索结果被整体丢弃，返回 修正方向： 仿照官方客户端改为宽松比较（ 较低
问题： 触发条件： 服务端在事件流中周期性发送空 实际影响： 每个 keep-alive 行产生一条无意义告警（空内容告警尤其无信息量），日志噪音增大，掩盖真正值得关注的解析失败；旧实现（ 修正方向： 在 |
| description = str(item.get("description") or "").strip() | ||
| if not description: | ||
| contents = item.get("contents") | ||
| highlights = contents.get("highlights") if isinstance(contents, dict) else None | ||
| # Only a list of strings is a usable fallback; anything else | ||
| # (a bare string, a dict, ...) must not crash the whole search. | ||
| if isinstance(highlights, list) and highlights and isinstance(highlights[0], str): | ||
| description = highlights[0].strip() |
There was a problem hiding this comment.
问题： 新增的 _search_youcom 命中解析只读取 description，回退使用 contents.highlights[0]，完全没有读取 You.com you-search 的规范摘要字段 snippets（字符串数组）。
触发条件： 真实端点的 results.web[].snippets 是默认返回的规范字段（官方 quickstart 与官方 @youdotcom-oss/dsh-plugin 客户端均以 snippets 为第一摘要来源），而 description 并非所有命中都带（news 命中及部分 web 命中缺失）；contents.highlights 仅在开启 extraction 时存在，而本实现默认的 free-profile 端点恰好不提供 extraction。
实际影响： 在缺省配置（免密钥 free profile）下，凡是没有 description 的命中（尤以 news 区为主）其 snippet 会退化为空字符串，LLM 拿到的摘要信息与官方返回不一致，工具核心产出（标题/URL/摘要）质量下降；新增的 26 个测试 fixture 全部按 description+contents.highlights 建模，测试通过但不能发现该契约偏差。
修正方向： 按官方契约补充 snippets 读取：优先取 item.get("description") 或 item.get("snippets") 列表中第一个非空字符串，其次才回退 contents.highlights；同时为测试 fixture 增加带 snippets 字段的用例，与官方响应样例对齐。
| request_headers = { | ||
| "User-Agent": self._user_agent, | ||
| # The streamable-HTTP server rejects JSON-only Accept headers | ||
| # with 406 Not Acceptable, so SSE must be listed here. | ||
| "Accept": "application/json, text/event-stream", | ||
| } | ||
| if headers: | ||
| request_headers.update(headers) |
There was a problem hiding this comment.
问题： _post_sse（youcom 免密钥路径）的请求头固定为 User-Agent + Accept，仅当 self._api_key 非空时才在调用处追加 Authorization；httpx 对 client.post(..., headers=...) 的语义是请求头与客户端级头合并、同名覆盖，客户端级头不会被清除。
触发条件： 文档（docs/mkdocs/en/tool.md:2552/2665）明确推荐把预建的 httpx.AsyncClient 通过 http_client 参数跨 Agent/调用共享连接池；当该共享客户端带有客户端级敏感头（如为代理/网关/其它服务配置的 Authorization、Cookie、X-API-Key）时，youcom 免密钥请求不会覆盖这些头。
实际影响： 未预期的凭据会被随请求发送到第三方端点 https://api.you.com/mcp?profile=free，构成凭据泄露；现有测试 test_keyless_search_maps_web_and_news_hits 仅断言无头客户端下 authorization is None，该路径无测试覆盖。（DDG 免密钥路径有同款模式，但 youcom 是本变更新增的暴露面。）
修正方向： 在 _post_sse 内对 youcom 请求执行白名单化：显式构造仅含 User-Agent/Accept/（有 key 时的）Authorization 的请求头，避免继承客户端级头；或至少在 _search_youcom 免密钥分支记录警告，并为共享带头客户端场景补充测试。
| if "error" in item: | ||
| # A JSON-RPC response carries either ``result`` or ``error``; | ||
| # a frame carrying both is treated as an error. | ||
| error = item["error"] | ||
| error_wins = True | ||
| if "result" in item and "error" not in item: | ||
| # A later valid result supersedes an earlier stray error. | ||
| result = item["result"] | ||
| error_wins = False |
There was a problem hiding this comment.
问题： 帧选择循环中，一旦出现 error 帧随后又出现 result 帧（本提交新增的 test_stray_error_superseded_by_later_result 已证明服务端确实可能先发 error 帧再发 result 帧），error_wins 被置回 False，最终在 error is not None and (error_wins or result is None) 处走成功分支，被覆盖的 error 对象被静默丢弃、无任何日志。
触发条件： 服务端在返回最终有效结果前发出携带 id: 1 的 error 帧（如瞬时限流、配额提示），随后正常返回结果帧。
实际影响： 相对加固前“无条件报错”的行为，现在真实发生过的失败在结果中完全不可见（summary=""），运维无法区分“恢复后成功”与“从未失败”，限流类问题的可观测性丢失。
修正方向： 在“后到 result 覆盖前序 error”的分支处补一条 logger.warning（记录被覆盖的 error code/message），使瞬时失败可观测，同时保留“结果优先”的既有语义。
| if frame_id is not None and frame_id != payload["id"]: | ||
| continue |
There was a problem hiding this comment.
问题： frame_id is not None and frame_id != payload["id"] 使用严格的 Python 类型比较（int vs int），而 payload["id"] 恒为整数 1；官方 @youdotcom-oss/dsh-plugin 客户端对同一过滤使用 String(candidate.id) === String(id) 的宽松比较。
触发条件： 服务端把请求 id 以不同 JSON 类型回显（例如把 1 回显为字符串 "1"）时，所有帧都会被当作“其它请求的帧”过滤掉。
实际影响： 搜索结果被整体丢弃，返回 "You.com search returned no result payload."，且无任何日志提示原因，排障困难。
修正方向： 仿照官方客户端改为宽松比较（str(frame_id) != str(payload["id"]) 或对二者做类型归一后相等判断），并增加 id 为字符串形式的测试用例。
| for piece in buffer: | ||
| decoded = _try_decode(piece) | ||
| if decoded is None: | ||
| logger.warning("WebSearchTool: skipping unparseable SSE line: %.80s", piece) | ||
| continue | ||
| payloads.append(decoded) |
There was a problem hiding this comment.
问题： _parse_sse_payloads 逐行回退路径中，空 data: 行（keep-alive 常用）或内容为合法 JSON 但非 dict 的 data: 行（如 data: 123、data: [1,2]）现在会触发 logger.warning("WebSearchTool: skipping unparseable SSE line: ...")；同时 _try_decode 对空串/非 dict 不再静默跳过。
触发条件： 服务端在事件流中周期性发送空 data: keep-alive 行，或发送非对象 JSON 的 data 行；这些行导致所在块的整块 join 解析失败后进入逐行回退，每行各打一条警告。
实际影响： 每个 keep-alive 行产生一条无意义告警（空内容告警尤其无信息量），日志噪音增大，掩盖真正值得关注的解析失败；旧实现（if not raw: continue）对这些行是静默处理的。
修正方向： 在 _try_decode/回退循环中将空串与非 dict 的 JSON（列表、数字、字符串、true/null）与真正的 JSON 解析失败区分开：空串直接跳过不告警，非 dict 可降级为 debug 级别或仅在计数告警；为空 data 行场景补充测试。
…, harden SSE parsing - Send the JSON-RPC request as an explicitly built httpx.Request via client.send() so client-level headers on a shared http_client (gateway bearer tokens, cookie jars) are never forwarded to the You.com endpoint; the per-call timeout is preserved through request.extensions. - Map You.com's canonical snippets list into hit descriptions when description is empty or missing (notably news hits), guarded the same way as contents.highlights. - Compare JSON-RPC frame ids by string form so servers that echo the id back as "1" still match the outgoing request. - Log a stray error frame superseded by a later result frame instead of dropping it silently. - Quiet down SSE parsing: empty keep-alive data lines and valid non-object JSON no longer warn; only truly unparseable lines do. Tests: new cases for each fix; the full file passes (110 tests) on both httpx 0.27.2 and 0.28.1.
|
Thanks for the thorough review — all five points are addressed in 5874601: Header isolation (the big one): confirmed the leak —
Frame id comparison: compared by string form now — a server that echoes the id back as "1" still matches the outgoing integer id, so reply frames are no longer dropped. Superseded error frames: a stray error frame overridden by a later result frame is logged at WARNING (with the error payload included) instead of disappearing silently. SSE noise: empty keep-alive The full |
AI Code Review审查结论不通过 审查范围：base 02509b2..head 5874601，共 4 个文件（ 发现的问题严重
问题： 触发条件： 调用方复用共享 实际影响： 网关凭据（Basic 用户名密码、Bearer token）被发送到第三方 You.com 端点，凭据泄露；You.com 设置的 cookie 随后可能被发送到 Tavily/Google 等端点，造成跨供应商凭据污染，属于安全边界破坏，且新增测试（ 修正方向： 在 中等
问题： 触发条件： 默认客户端 实际影响： 搜索静默返回空结果，下游 LLM 会得到误导性“无结果”结论，且无法从日志区分是真实无结果还是端点重定向。 修正方向： 在 较低
问题： 当 触发条件： 上游把 实际影响： 现实命中被静默丢弃，LLM 得到 0 结果，无法区分“真无结果”与“上游形状漂移”，可用性下降。 修正方向： 在该分支把可解析的 list 命中映射为 |
| request = httpx.Request("POST", url, json=payload, headers=request_headers) | ||
| # send() falls back to the client timeout when the extension is | ||
| # absent; keep the per-call timeout semantics of the other providers. | ||
| request.extensions["timeout"] = httpx.Timeout(self._timeout).as_dict() | ||
| try: | ||
| resp = await client.send(request) |
There was a problem hiding this comment.
问题： _post_sse() 通过 client.send(request) 发送裸 httpx.Request，仅隔离了客户端级 headers/cookies，但客户端级的 auth（如 httpx.AsyncClient(auth=(user, pass))、BearerAuth、自定义 Auth 流程）仍会被应用：httpx 0.27–0.28 的 BaseClient.send() 内部调用 _build_request_auth(request, USE_CLIENT_DEFAULT) 返回 self._auth，随后 _send_handling_auth 在裸请求上执行 auth 流程（BasicAuth/BearerAuth 直接设置 Authorization 头，DigestAuth 甚至会响应 You.com 服务端发来的 WWW-Authenticate 质询），这与代码注释和英文文档承诺的“凭据不会转发给 You.com”（docs/mkdocs/en/tool.md:2540、docs/mkdocs/zh/tool.md:2592）相矛盾。另外 client.send() 仍会把 You.com 响应中的 Set-Cookie 写入共享客户端 cookie jar（self.cookies.extract_cookies(response)），该 cookie 会被后续经同一 http_client 发给其他供应商（如 Tavily）的请求附带。
触发条件： 调用方复用共享 http_client 且该客户端配置了 auth（网关 Basic/Bearer/自定义鉴权），或 You.com 端点为某次响应设置了 cookie 后再次复用同一客户端。
实际影响： 网关凭据（Basic 用户名密码、Bearer token）被发送到第三方 You.com 端点，凭据泄露；You.com 设置的 cookie 随后可能被发送到 Tavily/Google 等端点，造成跨供应商凭据污染，属于安全边界破坏，且新增测试（test_shared_client_credentials_not_forwarded 等）只覆盖了 headers= 场景，未覆盖 auth=，测试结论会误导后续维护者。
修正方向： 在 await client.send(request) 处显式传入 auth=None 禁止应用客户端 auth（已在 httpx 0.27/0.28 验证可抑制），并考虑在发送后不依赖共享客户端 cookie jar 的隐式写入（或使用独立的请求级 client/transport 发送该请求）；同时补充以 auth=("u", "p") 配置的共享客户端用例来锁定该行为。
| if self._api_key: | ||
| headers = {"Authorization": f"Bearer {self._api_key}"} | ||
|
|
||
| payloads = await self._post_sse(self._base_url, payload, headers=headers) |
There was a problem hiding this comment.
问题： _post_sse 的请求未经 follow_redirects 控制，且 You.com 端点在重定向场景下可能把裸请求重新发送到第三方地址；更直接的问题是：httpx.Request 未启用 follow_redirects 时，3xx 响应不会跟随，但 resp.raise_for_status() 对 3xx 不抛错，落到 _parse_sse_payloads(resp.text) 解析空 body，静默返回 0 结果，掩盖重定向/端点变更。
触发条件： 默认客户端 follow_redirects 为 False（httpx 默认）时 You.com 端点返回 301/302（如 base_url 变更）。
实际影响： 搜索静默返回空结果，下游 LLM 会得到误导性“无结果”结论，且无法从日志区分是真实无结果还是端点重定向。
修正方向： 在 client.send(request) 处显式传 follow_redirects=True（与其他 provider 的 client.post 一致），并在 raise_for_status 前对 3xx 记录警告。
| sections = data.get("results") | ||
| if not isinstance(sections, dict): | ||
| if sections: | ||
| # A non-empty non-dict ``results`` is a shape change upstream, | ||
| # not an empty search; surface it instead of reporting 0 hits. | ||
| return WebSearchResult( | ||
| query=query, | ||
| provider="youcom", | ||
| results=[], | ||
| summary="You.com search returned an unexpected results shape.", | ||
| ) | ||
| sections = {} |
There was a problem hiding this comment.
问题： 当 data["results"] 为非空非 dict（如列表 [{"url":...}, ...]，You.com 可能的形状变体）时，代码返回 results=[] + summary="You.com search returned an unexpected results shape."，直接丢弃全部命中且摘要不包含任何命中数量或提示可恢复的信息，与 Tavily/Google 路径对形状漂移的处理（保留可解析命中）不一致。
触发条件： 上游把 results 从 dict 改为 list 形状（测试 test_non_dict_results_shape_surfaced_as_summary 也证实该路径存在）。
实际影响： 现实命中被静默丢弃，LLM 得到 0 结果，无法区分“真无结果”与“上游形状漂移”，可用性下降。
修正方向： 在该分支把可解析的 list 命中映射为 SearchHit（复用下方 hit 构建逻辑），或至少在 summary 中带上命中数与命中标题样例，便于诊断。
… call - Pass auth=None to client.send so client-level Basic/Bearer/custom auth flows on a shared http_client are not applied to the bare You.com request (httpx applies them even to requests sent as-is). - Drop any Set-Cookie the You.com response wrote into the shared client's cookie jar afterwards, so it cannot be replayed to other providers (e.g. Tavily) through the same client. - Pass follow_redirects=True so an endpoint 301/302 does not degrade to parsing the empty 3xx body as a silent 0-hit search; log a warning if a residual 3xx still surfaces. - Map a flat list of hits in results to SearchHit entries instead of dropping them, and name the observed type in the unexpected-shape summary. - Add tests: shared-client basic-auth suppression (with and without the tool key), Set-Cookie jar isolation, redirect following, flat-list results shape.
|
Thanks again for the careful review — you were right that
Full |
AI Code Review审查结论不通过 审查范围：base 02509b2..head b5515a2，共 4 个提交（feat: add youcom provider + 三次加固），涉及 3 个文件： 发现的问题严重
问题： 触发条件： 配置了 实际影响： YDC API Key 或共享客户端的 cookie 会随重定向按原样发给重定向目标主机，第三方可借此获取凭据或身份信息，属于本变更引入的凭据泄露风险。 修正方向： 在 严重
问题： 触发条件： 上游在 SSE 事件内用多行 data 传输（例如 You.com 将 实际影响： 整次搜索静默降级为空结果（ 修正方向： 多行 data 拼接时应按 SSE 规范将各 中等
问题： 触发条件： You.com 服务端按 MCP 流式规范持续推送事件或 keep-alive 不结束正文，或响应正文较大导致客户端读取时间超过超时阈值。 实际影响： 搜索请求挂起 15 秒后报 HTTP_ERROR 或异常，且共享连接池内连接状态损坏，同进程后续其它 provider 的搜索也会失败；用户侧表现为偶发的『搜索超时 + 后续搜索全部报错』。 修正方向： 使用 中等
问题： 触发条件： 用户设置了 实际影响： 修正方向： 将 较低
问题： 触发条件： 服务端返回的 SSE 正文以 BOM 开头且仅含 BOM（或 BOM 后无任何 实际影响： 空 payload 导致 修正方向： 对 |
|
Thanks for the detailed review — pushed dc9f1f9 addressing the findings:
Full suite for this file: 131 tests pass, flake8 clean on both touched files. Happy to adjust the shape of any of these if something looks off. |
AI Code Review审查结论不通过 审查范围：base 02509b2..head dc9f1f9，4 个文件（_websearch_tool.py、test_websearch_tool.py、docs en/zh tool.md），新增 youcom provider（SSE JSON-RPC、keyless free profile、YDC_API_KEY 认证切换）。方法与证据：10 个角度并行发现候选，对照本地克隆 httpx 0.28.1 源码与 httpcore 0.14.7 逐条验证（send 的 auth=None/headers/超时扩展、extract_cookies 对未跟随 3xx 的调用、CookieJar.clear、raise_for_status 对 3xx 抛 HTTPStatusError 等）。计划符合性：功能完整实现，测试覆盖充分（约 1190 行新增），httpx>=0.27.0 兼容性核实通过。主要风险：SEVERE — 跨源重定向分支 return [] 位于 Cookie 清理循环之前，You.com Set-Cookie 残留共享 jar 并被后续 Tavily/Google/DDG 请求重放（docstring 承诺被破坏）；MODERATE — 重定向逐跳校验基准错误且 _same_origin 允许 http 明文降级链携带 Bearer 头；MODERATE — 相对跳转丢失 ?profile=free 致 keyless 请求落到认证端点；LOW — 扁平 list 形状分支的 sections 键名覆盖、非字符串字段 str() 静默转换、en 文档 summary 行未同步（锚定到同 hunk 已变更的 provider 表行 2594）。测试影响：新增测试设计良好但 test_flat_list_results_shape_parses_hits 的单键输入未暴露 1088-1094 行缺陷。门禁结论：FAILED（存在 1 条 SEVERE + 2 条 MODERATE）。 发现的问题严重
问题： 跨源重定向分支在响应里直接 触发条件： 传入共享 实际影响： You.com 种下的 修正方向： 将 Cookie 回滚移到 中等
问题： 重定向循环每跳都调用 触发条件： 服务器返回如 实际影响： 修正方向： 用 中等
问题： 相对 Location 通过 触发条件： 配置了 keyless free 端点 实际影响： 后续请求打到 修正方向： 跳转目标解析后，若原始 URL 带 较低
问题： 当 触发条件： 响应同时包含 实际影响： 真实 web 命中被静默丢弃，用户只看到扁平列表子集；依赖 web+news 合并语义的下游引用会缺漏来源。 修正方向： 改为保留真实键：如 较低
问题： 命中解析对非字符串字段用 触发条件： You.com 变体返回结构化 URL 对象（如 实际影响： 合法命中被静默丢弃（url 变 repr → 修正方向： 对 url/title/description 校验类型：非 str 时尝试取内部标准字段（如 较低问题： 本行把 触发条件： 阅读英文文档的用户按需依赖 summary 作为错误通道时无此信息。 实际影响： 文档误导——en 用户会以为 youcom 从不往 summary 写错误信息，错误处理与引用逻辑按文档假设实现时行为偏差。 修正方向： 将 zh 对应的 summary 行说明同步到 |
| return [] | ||
| current_url = next_url | ||
| for cookie in list(client.cookies.jar): | ||
| if (cookie.name, cookie.domain, cookie.path) not in known_cookies: | ||
| client.cookies.delete(cookie.name, domain=cookie.domain, path=cookie.path) |
There was a problem hiding this comment.
问题： 跨源重定向分支在响应里直接 return []，跳过了紧随其后的 Cookie 清理循环（806-808 行）。httpx 在 _send_single_request 中对每次响应（包括未跟随的 3xx）都会调用 extract_cookies，因此 You.com 3xx 响应携带的 Set-Cookie 已写入共享 http_client 的 cookie jar；早退使清理永不执行，与 _post_sse docstring 中"任何 Set-Cookie 都会被丢弃"的安全承诺直接冲突。
触发条件： 传入共享 http_client（其 jar 会被 client.post/client.get 合并进后续请求），且 You.com 端点返回 302/301 重定向到非同一源（如 https://collector.example.com/...）并带 Set-Cookie；或重定向链中的任意一跳 client.send 抛传输异常。
实际影响： You.com 种下的 you_session 等 cookie 残留在共享 jar 中，同一 client 后续发往 Tavily/Google/DDG 的请求会携带该 Cookie 头——把第三方端点写入的 Cookie 重放到其他提供方，正是本次变更要防止的泄漏场景。
修正方向： 将 Cookie 回滚移到 finally 块（或先于各 return 的公共出口），确保跨源早退、跳转预算耗尽、异常路径都执行清理；同时考虑对跳转中间响应（跳过的 3xx）的 cookie 也按快照回滚。
| if not _same_origin(url, next_url): | ||
| # The request carries this tool's Authorization header when | ||
| # a key is configured; following a redirect to another | ||
| # origin would forward it (and any cookies httpx re-attaches | ||
| # to redirect requests) to whatever host the 3xx names. | ||
| logger.warning( | ||
| "WebSearchTool: dropping redirect from the You.com endpoint to " | ||
| "non-same-origin URL %r; search may return no results.", | ||
| next_url, | ||
| ) | ||
| return [] | ||
| current_url = next_url |
There was a problem hiding this comment.
问题： 重定向循环每跳都调用 _same_origin(url, next_url)，把目标与最初 URL 比较而非与上一跳 current_url 比较。httpx 已确认 URL.join 对相对 Location 保留原 scheme/host，故同源跳转测试能过；但 _same_origin 允许同主机 https→http 升级被放行后，后续 http 明文跳转不再被拦（仍带 Bearer 头）；多跳链的校验基准也不一致。
触发条件： 服务器返回如 https://api.you.com/mcp → http://api.you.com/mcp → http://api.you.com/mcp2 的链（首次升级被策略放行），或第一跳同源、第二跳跨源但相对目标与原始 URL 同源的情形。
实际影响： YDC_API_KEY Bearer 头沿明文 http 跳转被发送，或在应拦截的跨源第二跳被放行；违反"Bearer 头绝不离开 you.com 源"的原始设计意图。
修正方向： 用 _same_origin(current_url, next_url) 逐跳比较，并额外要求每跳目标与原始 URL 同源或严格 https（禁止任何 http 明文目标），补多跳链测试。
| break | ||
| location = resp.headers.get("location", "") | ||
| try: | ||
| next_url = str(httpx.URL(current_url).join(location)) |
There was a problem hiding this comment.
问题： 相对 Location 通过 httpx.URL(current_url).join(location) 解析时，?profile=free 查询串不会随相对路径保留（httpx URL.join 委托 urljoin，仅当 Location 为绝对 URL 或空时才保留查询），导致 keyless 请求在跳转后落到认证端点。
触发条件： 配置了 keyless free 端点 https://api.you.com/mcp?profile=free，服务器对 /mcp 返回相对 Location（如 /mcp 或 /mcp-moved）。
实际影响： 后续请求打到 https://api.you.com/mcp（无 profile）且未配置 API key，认证端点拒绝请求产生错误摘要/0 结果，keyless 搜索在跳转泛化部署下静默失败。
修正方向： 跳转目标解析后，若原始 URL 带 profile 查询串且新 URL 无此参数，则将查询串合并到 next_url 再发送；或改用显式检查 next_url.query 保留原始查询。
| if isinstance(sections, list): | ||
| # Shape drift upstream: some You.com variants return a flat list | ||
| # of hits instead of ``{"web": [...], "news": [...]}`` sections. | ||
| # Route the parseable entries through the same section loop | ||
| # instead of dropping every hit. | ||
| sections = {"web": sections} | ||
| if not isinstance(sections, dict): |
There was a problem hiding this comment.
问题： 当 results 是扁平 list（上游形状漂移分支）时，sections = {"web": sections} 以 "web" 键覆盖 data 中已存在的真实 web 段：真实 web 段被整个丢弃，扁平列表被当作唯一 web 段解析。
触发条件： 响应同时包含 results.web 真实段与扁平列表形式（如 {"results": [{...}], "results.web": [...]}），或扁平列表形式恰好被测试用例 test_flat_list_results_shape_parses_hits 触发但现有测试仅有单键输入未察觉。
实际影响： 真实 web 命中被静默丢弃，用户只看到扁平列表子集；依赖 web+news 合并语义的下游引用会缺漏来源。
修正方向： 改为保留真实键：如 if isinstance(sections, list): sections = {"web": sections} 前先检查 sections.get("web") 是否已存在，不存在再赋值；并补充包含双键 web + 扁平列表的回归测试。
| url = str(item.get("url") or "").strip() | ||
| if not url or _is_blocked(url, allowed, blocked): | ||
| continue | ||
| if self._dedup_urls: | ||
| key = _dedup_key(url) | ||
| if key in seen: | ||
| continue | ||
| seen.add(key) | ||
| description = str(item.get("description") or "").strip() | ||
| if not description: | ||
| # ``snippets`` is You.com's canonical summary field (a list | ||
| # of strings returned by default); some hits — notably | ||
| # news — carry it while ``description`` is empty or absent. | ||
| snippets = item.get("snippets") | ||
| if isinstance(snippets, list): | ||
| for snippet in snippets: | ||
| if isinstance(snippet, str) and snippet.strip(): | ||
| description = snippet.strip() | ||
| break | ||
| if not description: | ||
| contents = item.get("contents") | ||
| highlights = contents.get("highlights") if isinstance(contents, dict) else None | ||
| # Only a list of strings is a usable fallback; anything else | ||
| # (a bare string, a dict, ...) must not crash the whole search. | ||
| if isinstance(highlights, list) and highlights and isinstance(highlights[0], str): | ||
| description = highlights[0].strip() | ||
| hits.append( |
There was a problem hiding this comment.
问题： 命中解析对非字符串字段用 str(item.get("url") or "") 等强制转字符串，dict/list 类型的 title、description、snippets 元素会被转成 Python repr 文本而非跳过或取内嵌字符串，且 _is_blocked 对这类假 URL 返回 True 会把合法命中静默丢弃（无日志）。
触发条件： You.com 变体返回结构化 URL 对象（如 {"url": {"kind": "web", "href": "https://..."}}）或 list 型 title/description。
实际影响： 合法命中被静默丢弃（url 变 repr → _is_blocked 返回 True）或模型在 Sources 段引用 {'kind': ...} 形式的杂散文本；列表型 description 被当作 snippet 输出污染结果。
修正方向： 对 url/title/description 校验类型：非 str 时尝试取内部标准字段（如 href）或直接跳过并告警，与 DDG/Tavily 的防御式 dict 处理保持一致。
| |------|------|------| | ||
| | `query` | `str` | Query term used for this search (echoed back verbatim) | | ||
| | `provider` | `"duckduckgo" \| "google"` | Provider actually used | | ||
| | `provider` | `"duckduckgo" \| "google" \| "tavily" \| "youcom"` | Provider actually used | |
There was a problem hiding this comment.
问题： 本行把 provider 返回值表更新为包含 youcom，但紧随其后的 summary 字段行未同步：zh 文档已加入"You.com 在 JSON-RPC / 工具错误时也会把错误信息写入此字段"，en 文档仍只写 DDG/Google/Tavily，中英文档对新行为的描述不一致。
触发条件： 阅读英文文档的用户按需依赖 summary 作为错误通道时无此信息。
实际影响： 文档误导——en 用户会以为 youcom 从不往 summary 写错误信息，错误处理与引用逻辑按文档假设实现时行为偏差。
修正方向： 将 zh 对应的 summary 行说明同步到 docs/mkdocs/en/tool.md（"You.com 在 JSON-RPC / 工具错误时也会把错误信息写入 summary"）。
…parsing - Roll the shared-jar cookie snapshot/rollback into _post_sse's finally block so it also runs on the cross-origin early return, the redirect-budget break, and exception paths (previously skipped by the early return, leaking You.com Set-Cookie into the shared jar). - Validate each redirect hop against the URL it redirects *from* (current_url, not the original), and forbid plaintext http targets unless the endpoint was configured that way (same-origin with the original URL) — stops the Authorization header from traveling over plaintext http after a same-host upgrade hop. - Re-attach the original ?profile=free query param on relative redirects (URL.join drops it), so keyless calls do not land on the authenticated endpoint. - Merge (not clobber) real web/news sections that sit next to a flat-list results payload. - Type-guard url/title/description instead of str()-coercing: unwrap structured url objects via href, skip unusable urls with a debug log, and stop Python repr text leaking into titles/snippets. - Sync the en WebSearchResult summary row with the zh docs (You.com JSON-RPC/tool errors also land in summary).
|
Thanks again for the sharp round — all six findings are addressed in b7cc7ae, with regression tests for each:
Local results on b7cc7ae: |
AI Code Review审查结论通过 审查范围：b7cc7ae（HEAD）对比 02509b2（base），6 个提交，变更 4 个文件： 发现的问题中等
问题： 触发条件： 服务端跨多行拆分 JSON 帧且拆分点位于 JSON 字符串内部（构造合法 JSON 但语义失真），同时事件间无空行分隔；两条 实际影响： 搜索结果的 snippet/标题文本被静默插入换行而失真（若恰好仍是合法 JSON），或被整帧丢弃退化为 0 命中的空搜索，且无任何告警； 修正方向： 调整拼接顺序为优先 中等
问题： 触发条件： （1）调用前 jar 里已存在同名 cookie 时，You.com 的 实际影响： （1）You.com 的会话/跟踪 cookie 经共享 jar 泄漏到后续发往其他域（如 修正方向： 回滚时连同值一并比对（快照存 较低
问题： 触发条件： 服务端返回 实际影响： 本可命中的 snippet 文本被丢弃，命中记录的 修正方向： 改为遍历 较低
问题： 触发条件： 输入形如 实际影响： 修复上述拼接顺序问题时若无对应回归测试，现有测试套件无法捕获帧错误合并/静默丢弃的回归，所有测试保持绿色但生产合成行为错误；当前实现本身已有测试覆盖盲区。 修正方向： 在 |
| def _flush() -> None: | ||
| if not buffer: | ||
| return | ||
| decoded = _try_decode("\n".join(buffer)) | ||
| if decoded is None and len(buffer) > 1: | ||
| # A server may chunk one JSON frame across several ``data:`` | ||
| # lines without regard for JSON token boundaries. Re-inserting | ||
| # newlines between the chunks (the SSE join above) puts raw | ||
| # control characters inside the serialized JSON, which strict | ||
| # ``json.loads`` always rejects; the whole event would then | ||
| # degrade to a silently dropped frame. Try the chunks joined | ||
| # with no separator before falling back to per-line parsing. | ||
| decoded = _try_decode("".join(buffer)) | ||
| if decoded is not None: | ||
| payloads.append(decoded) | ||
| else: | ||
| for piece in buffer: | ||
| raw = piece.strip() | ||
| if not raw: | ||
| # Empty ``data:`` lines are keep-alives; skip silently | ||
| # instead of logging noise for every one of them. | ||
| continue | ||
| decoded = _try_decode(piece) | ||
| if decoded is not None: | ||
| payloads.append(decoded) | ||
| continue | ||
| # At this point the line is either unparseable or valid JSON | ||
| # that is not an object. | ||
| if _is_json(raw): | ||
| # Valid JSON but not an object (``123``, ``[1]``) is | ||
| # not a JSON-RPC frame; skip quietly, not as a warning. | ||
| logger.debug("WebSearchTool: skipping non-object SSE data: %.80s", raw) | ||
| else: | ||
| logger.warning("WebSearchTool: skipping unparseable SSE line: %.80s", raw) | ||
| buffer.clear() |
There was a problem hiding this comment.
问题： _flush（_parse_sse_payloads）先尝试 "\n".join(buffer) 再回退到 "".join(buffer)；当服务端将单个 JSON 帧按字符串边界拆成多行 data:（如 data: {"snippet": "ab + data: c"}）且事件间省略空行分隔符时，\n 拼接先被判定为"有效 JSON"——字符串内的换行使整帧被解析成损坏的字符串值（如 snippet 变成 ab\nc），后续解码逻辑无法再用无分隔符拼接纠正，属于本变更新增逻辑引入的帧形变缺陷。
触发条件： 服务端跨多行拆分 JSON 帧且拆分点位于 JSON 字符串内部（构造合法 JSON 但语义失真），同时事件间无空行分隔；两条 if decoded is not None 分支使无分隔符重试成为实际不可达的死代码。
实际影响： 搜索结果的 snippet/标题文本被静默插入换行而失真（若恰好仍是合法 JSON），或被整帧丢弃退化为 0 命中的空搜索，且无任何告警；"".join 回退因永远落在 decoded is not None 之后而失去修复作用。
修正方向： 调整拼接顺序为优先 "".join(buffer)（这是 JSON 语义上正确的按数据块重拼接），\n 拼接仅在无分隔符拼接失败后才回退尝试；或者至少将二者结果都校验后再采纳，确保多行拆分的字符串帧得到正确还原。
| for cookie in list(client.cookies.jar): | ||
| if (cookie.name, cookie.domain, cookie.path) not in known_cookies: | ||
| client.cookies.delete(cookie.name, domain=cookie.domain, path=cookie.path) | ||
| # Release the response and its pooled connection even when the | ||
| # body read dies mid-stream (e.g. a read timeout): an abandoned |
There was a problem hiding this comment.
问题： _post_sse 的 cookie 回滚只按 (name, domain, path) 三元组快照比对，快照后新增的 cookie 被删除，但快照中已存在、值在本次调用期间被 Set-Cookie 覆盖的 cookie 不会回滚（如预置的 you_session=old 被响应覆盖为 you_session=new），回滚后 jar 里残留新的污染值；同时 with_httpx 的 AsyncClient 若被并发共享（仓库 example 即演示多工具共用一个持久 AsyncClient），互不相关的并发请求写进 jar 的 cookie 会被本调用的回滚误删，属于本变更新增逻辑引入的数据完整性竞态。
触发条件： （1）调用前 jar 里已存在同名 cookie 时，You.com 的 Set-Cookie 覆盖其值后回滚比对三元组命中快照而跳过删除；（2）同一共享客户端上其他提供商的请求（如 Tavily）在快照之后、回滚之前写入 cookie，回滚按三元组比对认为它是"新增"而误删。
实际影响： （1）You.com 的会话/跟踪 cookie 经共享 jar 泄漏到后续发往其他域（如 api.tavily.com）的请求中；（2）其他并发调用刚刚写入的有效 cookie 被静默删除，导致那些调用鉴权/会话失效，均无明显日志。
修正方向： 回滚时连同值一并比对（快照存 (name, domain, path, value)，只有完全一致才跳过删除），并在删除前复核其值仍与"本次新增"一致（比对值变化即视为本次响应写入，予以回滚）；或用不与 jar 共享的独立 cookie 存储（如请求级 cookies= 参数）从根上消除跨调用污染。
| contents = item.get("contents") | ||
| highlights = contents.get("highlights") if isinstance(contents, dict) else None | ||
| # Only a list of strings is a usable fallback; anything else | ||
| # (a bare string, a dict, ...) must not crash the whole search. | ||
| if isinstance(highlights, list) and highlights and isinstance(highlights[0], str): | ||
| description = highlights[0].strip() |
There was a problem hiding this comment.
问题： _search_youcom 的 highlights 兜底条件 isinstance(highlights, list) and highlights and isinstance(highlights[0], str) 只检查首元素类型；当首元素为非字符串（如 {"text": ...} 对象或数字）而后续元素是可用字符串时，整个兜底被跳过，后续元素永远不会扫描，属本变更新增兜底逻辑覆盖不全。
触发条件： 服务端返回 "contents": {"highlights": [{"text": "x"}, "Python is a high-level language"]} —— 首元素为 dict 而非 str，isinstance(highlights[0], str) 为 False，跳过整个列表。
实际影响： 本可命中的 snippet 文本被丢弃，命中记录的 snippet 为空，模型得到的上下文信息减少，搜索回答质量下降（部分 You.com 变体确实混合返回对象与字符串元素）。
修正方向： 改为遍历 highlights 取第一个 isinstance(x, str) 且非空白的元素（与上方 snippets 的遍历写法一致），而不是只检查 highlights[0]。
| for line in lines: | ||
| if line.startswith("data:"): | ||
| buffer.append(line[len("data:"):].strip()) | ||
| elif not line.strip(): | ||
| # A blank line dispatches the event per the SSE spec. | ||
| _flush() | ||
| _flush() |
There was a problem hiding this comment.
问题： _parse_sse_payloads 的 _flush 在解析失败时逐行回退，会把"无空行分隔的多个完整 JSON 帧"拆成多个 payload 而丢失事件边界（服务端省略空行分隔符时，期望的是把多行作为一个合并事件处理），这种帧级歧义正是本变更新增解析器引入的确定性行为，需要测试固定其取舍。
触发条件： 输入形如 'data: {"a": 1}\ndata: {"b": 2}\n'（无空行）或有分隔符的多行帧——现有测试 test_parse_sse_payloads_falls_back_to_single_lines_without_separators 只断言了逐行正确拆分的一个方向，未覆盖 data: {"... 字符串内换行帧在无空行分隔状态下的行为（现有 test_parse_sse_payloads_joins_chunked_frames_without_separator 带空行，不会触达该分支）。
实际影响： 修复上述拼接顺序问题时若无对应回归测试，现有测试套件无法捕获帧错误合并/静默丢弃的回归，所有测试保持绿色但生产合成行为错误；当前实现本身已有测试覆盖盲区。
修正方向： 在 TestParseSsePayloads 中增加无空行分隔且含字符串内换行帧的用例（断言能正确还原为单一 payload，或至少按已声明的取舍稳定输出），并补一个 \n 拼接已产出"有效 JSON"时无分隔符拼接应被采纳的用例，作为上述修正的回归保护。
…llback, highlights scan - _parse_sse_payloads now joins buffered data chunks with no separator first (the faithful reassembly of a frame split at any boundary, including inside a JSON string) and only then falls back to the newline join, instead of the reverse order where the newline join could be accepted first and corrupt string contents. - The _post_sse cookie rollback snapshots the jar with values and restores pre-call values for cookies the call overwrote (Set-Cookie on a name that already existed), not just deleting additions; and deletions are scoped to the endpoint host, so a cookie written by a concurrent call on the same shared client for an unrelated domain is not caught in the rollback. - The contents.highlights fallback scans for the first string element (mirroring the snippets traversal) instead of checking only highlights[0], which disabled the fallback behind a non-string first element. - Tests: string-boundary split without separators restored intact; mixed complete/multi-line frames without separators pin the per-line trade-off; overwritten cookie value restored; unrelated-domain cookie survives the rollback.
|
Thanks for another sharp round — all four findings are addressed in 62a652c, with regression tests for each:
136 websearch tests pass locally; flake8 (the repo's |
AI Code Review审查结论不通过 审查范围：02509b2..62a652c，共 4 个文件（trpc_agent_sdk/tools/_websearch_tool.py 新增 598 行、tests/tools/test_websearch_tool.py 新增 1448 行、docs 英文/中文两份），为 WebSearchTool 新增 youcom（You.com MCP streamable HTTP）provider。 计划符合性：基本符合。youcom 已接入 ProviderType 与构造函数（YDC_API_KEY 环境变量回退、youcom_extra_params、keyless 与认证双端点）、lang 内联过滤符、blocked_domains 服务端 exclude_domains、snippets/contents.highlights 摘要回退、SSE 解析、手动逐跳重定向 + 共享 client cookie 回滚等，并有约 60 个针对性测试用例。 主要风险（均经 httpx 0.27 源码逐行验证）：
安全评估：凭据隔离（显式 Request + auth=None、逐跳同源校验、请求级 headers）与 cookie 回滚意图正确、测试充分，未发现 YDC_API_KEY 泄漏到第三方 origin 或明文 http 的路径。 测试充分性：整体非常充分（SSE 解析各切分形态、BOM、重定向同源/降级、cookie 回滚三形态、类型防护、JSON-RPC 帧关联均有覆盖），但缺少：空 Location 循环测试、中间跳异常连接释放测试、并发同域 cookie 覆盖测试、result 先于 error 的帧序测试、query 内已含 lang: 的重复拼接测试。 结论：存在 6 个可操作问题（2 SEVERE / 3 MODERATE / 1 LOW），门禁 FAILED。 发现的问题严重
问题： 触发条件： 服务端（或中间 CDN/代理）对 POST 返回 302/301 且 实际影响： 每个此类响应会让工具连续发出最多 4 次完全相同的 POST（叠加评论#2 的 off-by-one 实际可发 5 个请求），放大带宽与上游负载；若可跟随的 3xx 走完预算后循环退出，随后的 修正方向： 在读取 严重
问题： cookie 回滚的"值恢复"路径会并发覆盖同一域名上其他调用写回的 cookie 值。快照在首个请求发出前对整本 jar 取 (name, domain, path)→value； 触发条件： 共享 实际影响： 并发调用写入的有效会话值被静默回滚为旧值，后续请求携带过期的会话 cookie，导致其他 provider 调用鉴权失败或状态丢失；同一根因也可能误删并发新增的同域 cookie（snapshot 中不存在的 key 且域名匹配 endpoint host 时）。 修正方向： 快照记录时同时记录原 cookie 的过期/版本等标识，恢复前校验其未被并发修改（如比较 中等
问题： 触发条件： 端点首先返回同源 3xx（如 302 到 实际影响： 半读的响应连接残留在共享 修正方向： 在循环内每次发送后记录当前 response 并保证其关闭：发送下一跳前先 中等
问题： 重定向预算 off-by-one。 触发条件： 端点连续返回 4 个可合法跟随、同源的 3xx（例如路径/负载均衡多次迁移）或叠加评论#1 的空 Location 重复 POST 场景。 实际影响： 每张凭证（ 修正方向： 将判断改为在发送前一跳时计数： 中等
问题： JSON-RPC 帧循环对"result 帧先到、error 帧后到"的顺序不对称。 触发条件： 服务端（或读取/缓冲顺序）在同一个 id 下先发送 result 帧、随后再发送一个 error 帧——JSON-RPC 2.0 规定同 id 只能有一个响应，但代码的注释和测试（ 实际影响： 完整且可解析的搜索结果被替换为 修正方向： 将最终判断改为"最后一个语义帧胜出"：例如只在 较低
问题： 内联 触发条件： LLM 在 query 文本中自行使用 实际影响： 发送给 You.com 的查询被改成含重复 修正方向： 拼接前检查 |
| location = resp.headers.get("location", "") | ||
| try: | ||
| next_obj = httpx.URL(current_url).join(location) |
There was a problem hiding this comment.
问题： _post_sse 的重定向循环在 Location 头存在但为空（''）或空白（' '）时会把同一请求重复发送到同一个 URL。resp.has_redirect_location 只要求 3xx 且 Location 头存在（httpx 源码 _models.py:704），空字符串会通过；随后 httpx.URL(current_url).join('') 按 urljoin 语义返回原 URL（含 query），_same_origin 检查必然通过，于是循环像正常重定向一样递增 redirects 并再次 POST 同一请求，直到预算耗尽。
触发条件： 服务端（或中间 CDN/代理）对 POST 返回 302/301 且 Location: 为空或仅为空白字符——这是生产中出现过的不规范响应形态，测试没有任何覆盖。
实际影响： 每个此类响应会让工具连续发出最多 4 次完全相同的 POST（叠加评论#2 的 off-by-one 实际可发 5 个请求），放大带宽与上游负载；若可跟随的 3xx 走完预算后循环退出，随后的 300 <= resp.status_code < 400 分支只打 warning，raise_for_status() 抛出 HTTPStatusError 被 _run_async_impl 包装为 HTTP_ERROR 返回给 LLM。
修正方向： 在读取 location 后先 strip()，为空时按"无法跟随的重定向"处理（打 warning 并 break，回到既有的 3xx 残余处理路径），不要进入 URL 拼接与再发送。
| for cookie in list(client.cookies.jar): | ||
| key = (cookie.name, cookie.domain, cookie.path) | ||
| if key in cookie_snapshot: | ||
| if cookie.value != cookie_snapshot[key]: | ||
| # The call overwrote a pre-existing cookie: restore | ||
| # the pre-call value in place so every other | ||
| # attribute (expiry, secure, ...) survives. | ||
| cookie.value = cookie_snapshot[key] | ||
| continue | ||
| domain = cookie.domain.lstrip(".") | ||
| if endpoint_host == domain or endpoint_host.endswith("." + domain): | ||
| client.cookies.delete(cookie.name, domain=cookie.domain, path=cookie.path) |
There was a problem hiding this comment.
问题： cookie 回滚的"值恢复"路径会并发覆盖同一域名上其他调用写回的 cookie 值。快照在首个请求发出前对整本 jar 取 (name, domain, path)→value；finally 中只要 key in cookie_snapshot 且值不同，就无条件 cookie.value = cookie_snapshot[key] 原地恢复。若同一个共享 http_client 上并发的另一 provider 调用（如 Tavily）在快照之后把同名同域 cookie 从旧值写成了新值，回滚会把被并发调用的新值覆盖回旧值。代码注释声称删除范围限定在 endpoint host 域、"并发调用写入的无关域名 cookie 不受影响"，但被覆盖的正是与 You.com 同域（该 host 上的每个 hop 都被验证保持同源）的 cookie——例如 gateway 域下多个云 API 共用的会话 cookie。
触发条件： 共享 http_client 的 jar 中预先存在 You.com endpoint host（或包含该 host 的域后缀、带初始点）上的 cookie；You.com 调用在快照之后、回滚之前，另一个 provider 调用或用户代码更新了该 cookie 的值；且 You.com 响应本身也 Set-Cookie 了这个名字（使其值偏离快照）。
实际影响： 并发调用写入的有效会话值被静默回滚为旧值，后续请求携带过期的会话 cookie，导致其他 provider 调用鉴权失败或状态丢失；同一根因也可能误删并发新增的同域 cookie（snapshot 中不存在的 key 且域名匹配 endpoint host 时）。
修正方向： 快照记录时同时记录原 cookie 的过期/版本等标识，恢复前校验其未被并发修改（如比较 client.cookies.jar 中该 cookie 的 expires/version 或使用局部锁）；或将"恢复值"改为"删除本次 Set-Cookie 带来的新值并重放并发修改"的合并策略，并明确并发场景的文档化保证。
| try: | ||
| current_url = url | ||
| redirects = 0 | ||
| while True: | ||
| request = httpx.Request("POST", current_url, json=payload, headers=request_headers) | ||
| # send() falls back to the client timeout when the extension is | ||
| # absent; keep the per-call timeout semantics of the other providers. | ||
| request.extensions["timeout"] = httpx.Timeout(self._timeout).as_dict() | ||
| resp = await client.send(request, auth=None, follow_redirects=False) | ||
| if not resp.has_redirect_location: | ||
| break | ||
| redirects += 1 | ||
| if redirects > _SSE_MAX_REDIRECTS: | ||
| logger.warning( | ||
| "WebSearchTool: You.com endpoint exceeded %d redirects; " | ||
| "search may return no results.", | ||
| _SSE_MAX_REDIRECTS, | ||
| ) | ||
| break | ||
| location = resp.headers.get("location", "") | ||
| try: | ||
| next_obj = httpx.URL(current_url).join(location) | ||
| # URL.join follows urljoin semantics, so a relative | ||
| # Location drops the original query string: a keyless | ||
| # "?profile=free" call would silently land on the | ||
| # authenticated endpoint after the redirect. Re-attach | ||
| # the profile param, keeping any the Location carries. | ||
| origin_profile = httpx.URL(url).params.get("profile") | ||
| if origin_profile is not None and next_obj.params.get("profile") is None: | ||
| next_obj = next_obj.copy_with(params=next_obj.params.merge({"profile": origin_profile})) | ||
| next_url = str(next_obj) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| logger.warning( | ||
| "WebSearchTool: You.com redirect target %r is not a valid URL (%s); " | ||
| "search may return no results.", | ||
| location, | ||
| ex, | ||
| ) | ||
| break | ||
| # Validate each hop on its own: the request (and the tool's | ||
| # Authorization header) travels to current_url's origin, so | ||
| # the hop must be same-origin with the *previous* URL — not | ||
| # merely with the first one — and it may not downgrade to | ||
| # plaintext http unless the endpoint was configured that way | ||
| # (same-origin with the original URL). Otherwise the bearer | ||
| # header (and any cookies httpx re-attaches to redirect | ||
| # requests) would travel to whatever host or plaintext hop | ||
| # the 3xx names. | ||
| if not (_same_origin(current_url, next_url) and | ||
| (_same_origin(url, next_url) or httpx.URL(next_url).scheme == "https")): | ||
| logger.warning( | ||
| "WebSearchTool: dropping redirect from the You.com endpoint to " | ||
| "URL %r (not same-origin with the previous hop or plaintext http); " | ||
| "search may return no results.", | ||
| next_url, | ||
| ) | ||
| return [] | ||
| current_url = next_url | ||
| if 300 <= resp.status_code < 400: | ||
| # Defensive: a 3xx that was not followed (missing/invalid | ||
| # location, or the redirect budget ran out) must not read | ||
| # downstream as a normal empty search. | ||
| logger.warning( | ||
| "WebSearchTool: You.com endpoint responded %d without " | ||
| "following the redirect; search may return no results.", | ||
| resp.status_code, | ||
| ) | ||
| resp.raise_for_status() | ||
| return _parse_sse_payloads(resp.text) |
There was a problem hiding this comment.
问题： _post_sse 中每次 hop 的 httpx.Response 都写入同一个 resp 变量，且 resp.aclose() 只在 finally 中对"最后一个"response 执行。当第 2 跳及以后的 client.send 抛出异常（读超时、连接重置、传输错误）时，之前跳的 response 流从未被关闭，连接不会被归还到共享连接池——这恰是该函数 docstring（第 757-760 行）声称要防止的"断连接残留"。
触发条件： 端点首先返回同源 3xx（如 302 到 /mcp-moved），随后对 follow-up POST 的响应体读取中途异常（keep-alive 密集体上的读超时、上游断连）。
实际影响： 半读的响应连接残留在共享 http_client 的连接池中，之后每个同类失败都泄漏一条连接，直至空闲超时/GC 回收；泄漏的连接可能被后续其他 provider（Tavily/Google）请求复用而失败，HTTP_ERROR 返回给 LLM。
修正方向： 在循环内每次发送后记录当前 response 并保证其关闭：发送下一跳前先 aclose() 上一跳的响应（3xx 响应体未读，直接关闭安全），并/或用列表收集所有 hop 的 response，finally 中逐个 aclose()。
| redirects += 1 | ||
| if redirects > _SSE_MAX_REDIRECTS: | ||
| logger.warning( | ||
| "WebSearchTool: You.com endpoint exceeded %d redirects; " | ||
| "search may return no results.", | ||
| _SSE_MAX_REDIRECTS, | ||
| ) | ||
| break |
There was a problem hiding this comment.
问题： 重定向预算 off-by-one。redirects 在每跳前先 += 1 再判断 redirects > _SSE_MAX_REDIRECTS，因此 _SSE_MAX_REDIRECTS = 3 时实际允许 4 次跳转、共 5 个请求，与常量注释"Maximum redirects the SSE POST follows"不一致。
触发条件： 端点连续返回 4 个可合法跟随、同源的 3xx（例如路径/负载均衡多次迁移）或叠加评论#1 的空 Location 重复 POST 场景。
实际影响： 每张凭证（YDC_API_KEY bearer）比承诺多发出一跳到目标外的路径，且放大评论#1 的重复请求次数（空 Location 时实际 5 个请求而不是注释承诺的 3 个），超出预算的循环以最后一次 3xx 的 warning 结束。
修正方向： 将判断改为在发送前一跳时计数：redirects >= _SSE_MAX_REDIRECTS 即拒绝跟随（在递增后判断 redirects > _SSE_MAX_REDIRECTS 之前先检查 redirects == _SSE_MAX_REDIRECTS），或把递增移到发送之后。
| result: Optional[dict[str, Any]] = None | ||
| error: Optional[dict[str, Any]] = None | ||
| error_wins = False | ||
| for item in payloads: | ||
| if not isinstance(item, dict): | ||
| continue | ||
| frame_id = item.get("id") | ||
| # Only frames replying to this request carry its id; frames | ||
| # without one are notifications and carry no result/error. | ||
| # Compare the string forms: some servers echo the request id | ||
| # back as a string ("1"), and a strict typed comparison would | ||
| # drop every reply frame. | ||
| if frame_id is not None and str(frame_id) != str(payload["id"]): | ||
| continue | ||
| if "error" in item: | ||
| # A JSON-RPC response carries either ``result`` or ``error``; | ||
| # a frame carrying both is treated as an error. | ||
| error = item["error"] | ||
| error_wins = True | ||
| if "result" in item and "error" not in item: | ||
| # A later valid result supersedes an earlier stray error. | ||
| if error is not None: | ||
| # Keep transient failures observable: a stray error frame | ||
| # (e.g. a rate-limit notice) was overridden by a later | ||
| # result frame; log it rather than dropping it silently. | ||
| logger.warning("WebSearchTool: You.com error frame superseded by a later result: %.200s", error) | ||
| result = item["result"] | ||
| error_wins = False | ||
| if error is not None and (error_wins or result is None): | ||
| message = error.get("message") if isinstance(error, dict) else str(error) | ||
| return WebSearchResult( | ||
| query=query, | ||
| provider="youcom", | ||
| results=[], | ||
| summary=_truncate(f"You.com search error: {message}", self._snippet_len), | ||
| ) |
There was a problem hiding this comment.
问题： JSON-RPC 帧循环对"result 帧先到、error 帧后到"的顺序不对称。error_wins 是一个锁存位：只有后续的 result 帧能把它复位，而后续的 error 帧会把 error_wins 置 True 并保留已先到的 result 值；最终 error is not None and (error_wins or result is None) 恒为 True，先到的有效 result 被丢弃并静默返回错误摘要（该路径没有任何 warning 日志，与"转瞬错误帧被后续 result 覆盖要打日志"的对称处理不一致）。
触发条件： 服务端（或读取/缓冲顺序）在同一个 id 下先发送 result 帧、随后再发送一个 error 帧——JSON-RPC 2.0 规定同 id 只能有一个响应，但代码的注释和测试（test_same_frame_result_and_error_treated_as_error、test_stray_error_superseded_by_later_result）明确把"服务器违规"当作要防御的输入类型，本分支却未防御。
实际影响： 完整且可解析的搜索结果被替换为 results=[] + "You.com search error: ..." 摘要，模型把一次正常搜索当成失败，且日志无提示，排障困难。
修正方向： 将最终判断改为"最后一个语义帧胜出"：例如只在 result is None 时才让 error 生效（error is not None and result is None），或在循环中记录"最后出现的帧类型"，用最后类型决定返回 result 还是 error，并对"error 覆盖 result"的方向同样打 warning。
| if lang: | ||
| # You.com maps an inline ``lang:`` operator to a language filter. | ||
| # Guard against the model passing the operator form verbatim | ||
| # (``lang:ja``, possibly with stray whitespace or casing) after | ||
| # reading the schema description. | ||
| lang_value = str(lang).strip() | ||
| while lang_value[:5].lower() == "lang:": | ||
| lang_value = lang_value[5:].strip() | ||
| if lang_value: | ||
| arguments["query"] = f"{query} lang:{lang_value}" |
There was a problem hiding this comment.
问题： 内联 lang: 过滤符拼接未检查 query 自身是否已包含 lang: 操作符。lang_value[:5].lower() == "lang:" 循环只剥离 lang 参数值的前缀，之后无条件执行 arguments["query"] = f"{query} lang:{lang_value}"；当模型按 schema 描述（"You.com maps it to the inline lang: filter"）在 query 里已写了 lang:ja、同时又传 lang="ja" 时，结果是 "python lang:ja lang:ja"。相应测试也只断言前缀剥离后的拼接，未覆盖 query 内已含 lang: 的重复场景。
触发条件： LLM 在 query 文本中自行使用 lang: 操作符（schema 描述鼓励该用法），同时调用参数带 lang，或工具级 lang 与 query 内操作符同值。
实际影响： 发送给 You.com 的查询被改成含重复 lang: 过滤符的复合串，可能导致该端拒绝或语义改变，搜索结果与预期不符；属于模型实际会走到的调用形态。
修正方向： 拼接前检查 query 中是否已存在 lang: 操作符（正则匹配词边界），若存在则不再追加；并把该场景补为测试用例。
…edirect budget, lang dedup Six findings from the latest review round (2 SEVERE / 3 MODERATE / 1 LOW): 1. SEVERE: Empty Location header caused repeated POST to the same URL. Strip the Location value and break if empty, instead of joining an empty string (which returns the same URL). 2. SEVERE: Cookie value restore could overwrite a concurrent call's update to the same cookie. Only delete cookies the call added (absent from the snapshot); leave pre-existing cookies whose values were overwritten as-is. 3. MODERATE: Intermediate 3xx hop responses were not closed before the next hop, leaking pooled connections. Close each intermediate response before issuing the next hop (aclose is idempotent, so the finally-block close on the final response is still safe). 4. MODERATE: Redirect budget was off-by-one — redirects > MAX allowed MAX+1 redirects. Check redirects >= MAX before incrementing to enforce exactly MAX redirect follows. 5. MODERATE: Frame order asymmetry — a result frame followed by an error frame silently discarded the valid result via the error_wins latch. Remove the latch: result always wins regardless of frame order; error only takes effect if no result arrived. 6. LOW: lang: operator duplication — if the query already contains 'lang:', don't append a second one from the lang parameter.
|
Thanks for another sharp round — all six findings are addressed in 7504580, with regression tests for each: 1. Empty Location (SEVERE): The redirect loop now strips the Location value and breaks immediately if it's empty or whitespace-only, instead of joining an empty string (which returns the same URL and causes repeated identical POSTs). 2. Cookie value restore overwriting concurrent calls (SEVERE): The rollback no longer restores pre-existing cookie values to their snapshot. Only cookies the call added (absent from the snapshot) are deleted; pre-existing cookies whose values were overwritten by a You.com Set-Cookie are left as-is. This avoids silently clobbering a concurrent call that updated the same cookie after our snapshot. 3. Intermediate hop responses not closed (MODERATE): Each 3xx response is now closed with 4. Redirect budget off-by-one (MODERATE): The check is now 5. Frame order asymmetry (MODERATE): Removed the 6. lang: operator duplication (LOW): The query is checked for an existing |
AI Code Review审查结论不通过 审查范围： 计划符合性：整体实现完整、测试覆盖充分（SSE 多行/BOM/分帧、重定向预算/跨源丢弃/明文降级、cookie 回滚、凭据隔离、lang 去重、JSON-RPC 帧序、shape drift 等 40+ 用例，且均围绕真实 httpx 行为编写）。经 httpx 0.27.2/0.28.1 源码核验与实测确认的关键机制均正确： 主要风险（3 项，均已建立证据链）：(1) SEVERE：非 SSE（application/json）错误响应被 测试影响：测试覆盖成功路径与大部分失败路径，但缺少「application/json 错误体」与「同 URL 重定向循环」两个用例（正是上述缺陷的触发条件）； 门禁结论：存在 1 条 SEVERE 缺陷（错误被静默吞掉的正确性问题），按标准判定为 FAILED。 发现的问题严重
问题： 触发条件： 服务端对 200 请求用 实际影响： 服务端带诊断信息的拒绝被静默降级为一次「成功但零结果」的搜索，LLM 会据此回答「未找到结果」而不是上报失败，属于数据完整性问题（错误被吞掉）。 修正方向： 在 中等
问题： 重定向循环对「Location 指向与当前 URL 完全相同的地址」的自我循环没有防护：预算检查 触发条件： 服务端（MCP 会话瞬态故障/负载均衡器）对 实际影响： 携带 修正方向： 在发送下一跳前比较解析后的 中等
问题： cookie 回滚只能识别「快照里不存在的同域名 cookie 就删除」，无法区分「本调用通过 Set-Cookie 新增的 cookie」与「并发调用在同域新增的 cookie」。注释（第 897 行前后）声称「unrelated domain 的并发 cookie 不会被误删」，但未覆盖同域并发写：快照在本次调用开始前拍摄，若共享同一 触发条件： 同域（ 实际影响： 并发请求的会话 cookie 被静默删除，导致共享 client 上同域请求的连接态丢失（如临时认证态失效、会话被取消），属于真实的并发数据竞争，尽管触发窗口较小。 修正方向： 将快照进一步细化为「域名 + cookie 名」两级集合，回滚时只删除 (1) 快照中不存在的、且 (2) 域名匹配的 cookie；或回滚前用带值快照做双向比对，仅当值确实等于本调用响应写入的值时才删除，避免误删并发写入的 cookie。 |
| resp.raise_for_status() | ||
| return _parse_sse_payloads(resp.text) |
There was a problem hiding this comment.
问题： _post_sse 的成功路径只有一条：无条件调用 _parse_sse_payloads(resp.text)，而该解析器只认 data: 前缀行。当 You.com 端点以 application/json 返回 JSON-RPC 错误帧（如参数校验失败 -32602 Invalid language filter、50x 限流等，HTTP 仍为 200）时，整个 ssse 体没有一行以 data: 开头，解析结果为 []，_search_youcom 里已有的 JSON-RPC error 分支（第 1084-1118 行）因此永远不可达，最终走到 "You.com search returned no result payload."（第 1119-1125 行）。已在受控环境实测：_parse_sse_payloads('{"jsonrpc":"2.0","id":1,"error":{...}}') 返回 []。
触发条件： 服务端对 200 请求用 application/json（而非 text/event-stream）返回错误帧——作者在 776-778 行的注释里已亲历过 406 非 SSE 响应，证明该端点确实存在非 SSE 响应；非法的 lang:/freshness 等参数值正是这类仍返回 200 的服务端校验错误。
实际影响： 服务端带诊断信息的拒绝被静默降级为一次「成功但零结果」的搜索，LLM 会据此回答「未找到结果」而不是上报失败，属于数据完整性问题（错误被吞掉）。
修正方向： 在 _post_sse 解析前检查 resp.headers.get("content-type", "")：若不含 text/event-stream，则直接 json.loads(resp.text)，把 JSON-RPC 错误帧放进返回列表（或直接走错误路径），让现有 error 分支可见。
| if not resp.has_redirect_location: | ||
| break | ||
| if redirects >= _SSE_MAX_REDIRECTS: | ||
| logger.warning( | ||
| "WebSearchTool: You.com endpoint exceeded %d redirects; " | ||
| "search may return no results.", | ||
| _SSE_MAX_REDIRECTS, | ||
| ) | ||
| break | ||
| redirects += 1 | ||
| location = resp.headers.get("location", "").strip() | ||
| if not location: | ||
| logger.warning( | ||
| "WebSearchTool: You.com endpoint returned a redirect " | ||
| "(%d) with an empty Location; search may return no " | ||
| "results.", | ||
| resp.status_code, | ||
| ) | ||
| break |
There was a problem hiding this comment.
问题： 重定向循环对「Location 指向与当前 URL 完全相同的地址」的自我循环没有防护：预算检查 if redirects >= _SSE_MAX_REDIRECTS 在首轮迭代时 redirects == 0 必然通过，且 redirects += 1 在读到 Location 之后才递增（第 809 行），所以同一 URL 的 3xx 会被连续 4 次重新 POST 才中断。空 Location 的分支（第 811-818 行）能立即中断，但服务端自我循环时发出的恰恰是「非空且指向自身」的 Location，恰好绕过同样的防护。306-655 行的本地测试 test_redirect_budget_limits_to_three 只断言 4 次请求，未覆盖同 URL 循环。
触发条件： 服务端（MCP 会话瞬态故障/负载均衡器）对 POST /mcp 持续返回 302 Location: https://api.you.com/mcp（与当前 URL 相同）。
实际影响： 携带 Authorization: Bearer <YDC_API_KEY> 与完整 JSON-RPC 体的同一个 POST 在 3 秒超时窗口内被重复发送 4 次（错误路径还有 raise_for_status，不会无限次），造成 4 倍凭据请求与无谓的上游负载；且 302/303 语义下 httpx 自身会自动转 GET（httpx _redirect_method 已实测确认），本实现却始终重发 POST，与 has_redirect_location 所建模的 307/308 之后行为的注释意图不符。
修正方向： 在发送下一跳前比较解析后的 httpx.URL(current_url) 与 next_url，若完全相同则按空 Location 同样处理（告警并 break）；同时按 RFC 9110 对 302/303 在下一跳改用 GET（或仅对 307/308 保持 POST）。
| for cookie in list(client.cookies.jar): | ||
| key = (cookie.name, cookie.domain, cookie.path) | ||
| if key in cookie_snapshot: | ||
| # A pre-existing cookie was in the jar before this call. | ||
| # Don't restore its snapshot value: a concurrent call on | ||
| # the same shared client may have updated it after our | ||
| # snapshot, and restoring the old value would silently | ||
| # overwrite that concurrent update. Only cookies this | ||
| # call added (absent from the snapshot) are cleaned up. | ||
| continue | ||
| domain = cookie.domain.lstrip(".") | ||
| if endpoint_host == domain or endpoint_host.endswith("." + domain): | ||
| client.cookies.delete(cookie.name, domain=cookie.domain, path=cookie.path) |
There was a problem hiding this comment.
问题： cookie 回滚只能识别「快照里不存在的同域名 cookie 就删除」，无法区分「本调用通过 Set-Cookie 新增的 cookie」与「并发调用在同域新增的 cookie」。注释（第 897 行前后）声称「unrelated domain 的并发 cookie 不会被误删」，但未覆盖同域并发写：快照在本次调用开始前拍摄，若共享同一 http_client 的另一协程（如另一次对 api.you.com 的请求，或 app 自身对 You.com 域名的调用）在本调用进行期间写入同域 cookie，本调用的 finally 回滚会把它一并删除。
触发条件： 同域（endpoint_host 或其父域）cookie 在本调用快照之后、finally 回滚之前，被并发协程写入共享 client 的 jar。
实际影响： 并发请求的会话 cookie 被静默删除，导致共享 client 上同域请求的连接态丢失（如临时认证态失效、会话被取消），属于真实的并发数据竞争，尽管触发窗口较小。
修正方向： 将快照进一步细化为「域名 + cookie 名」两级集合，回滚时只删除 (1) 快照中不存在的、且 (2) 域名匹配的 cookie；或回滚前用带值快照做双向比对，仅当值确实等于本调用响应写入的值时才删除，避免误删并发写入的 cookie。
This branch has not been deployed
2 participants
Why
The default
duckduckgobackend is keyless but returns DDG's curated instant-answer set rather than real web results, and thegoogle/tavilybackends both require an API key before they return anything. You.com exposes itsyou-searchMCP tool through a keyless free-profile endpoint (https://api.you.com/mcp?profile=free), so it fits the existing provider surface as a zero-setup backend that returns real web hits — the same property that makes the DDG default attractive.What changed
trpc_agent_sdk/tools/_websearch_tool.py: new"youcom"provider following the existing provider pattern —ProviderTypeliteral,_search_youcomdispatch,youcom_extra_paramspassthrough,YDC_API_KEYenv fallback. The endpoint is stateless JSON-RPC over streamable HTTP: onetools/callPOST answered by atext/event-streambody, parsed by a new_parse_sse_payloadshelper (progress notifications are skipped, unparseable lines ignored). Keyless by default; a configured key switches tohttps://api.you.com/mcpwith aBearerheader.SearchHit(description → snippet, first query-relevant highlight as fallback)langmaps to You.com's inlinelang:filter;blocked_domainsmap to server-sideexclude_domains,allowed_domainsfilter client-side (plus the usual_is_blockedpass)isErrorresults surface assummarytext instead of raisingtests/tools/test_websearch_tool.py:TestYoucomProvidermirroring the Tavily coverage — SSE parsing, keyless/authenticated base URLs, Bearer header, Accept header requirement (the endpoint returns 406 for JSON-only), inlinelang:mapping, server-sideexclude_domains, client-side allowlist, extra params, count clamping, URL dedup, JSON-RPC/isError/missing-result/non-JSON error paths. Plus keyless-by-default and env-fallback init tests and a base-props-only declaration check.docs/mkdocs/en/tool.md+docs/mkdocs/zh/tool.md: backend lists, parameter tables, return-fields table, and a usage example for the keyless configuration (the English page also catches up the missingtavilyrows it was lagging).Default behavior is unchanged:
provider="duckduckgo"stays the default and all existing provider paths are untouched — this is purely additive and opt-in viaprovider="youcom".Setup
No setup required — the free profile is keyless. Optional upgrades:
Validation
pytest tests/tools/test_websearch_tool.py→ 91 passed (75 pre-existing + 16 new)yapf --diffandflake8clean on both changed Python files (the CI checks)test_agent_tool.py,test_function_parameter_parse.py, langgraph/DSL collection errors) fail identically onmainin this environment — unrelated to this changeFor context, You.com also ships ready-made agent skills and MCP server configs (https://github.com/youdotcom-oss/agent-skills) for harnesses that want the full MCP server rather than a single tool.
Happy to adjust the shape if you'd rather land this differently — e.g. folding the SSE handling into the existing
_post_jsonhelper, or trimming the docs diff.Tracking: youdotcom-oss/integration-tracking#477