接入 Web Search Gateway 支持 BYOK - #141
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds web search configuration, Tavily and Brave provider clients, and a Messages gateway that executes server-side searches during Anthropic tool-use continuations with JSON and SSE response support. ChangesWeb Search Gateway and Providers
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant Gateway
participant Anthropic
participant Provider
Client->>Handler: POST /v1/messages
Handler->>Gateway: handle candidate request
Gateway->>Anthropic: send projected web_search request
Anthropic-->>Gateway: return tool_use
Gateway->>Provider: Search query and policy
Provider-->>Gateway: return SearchResponse
Gateway->>Anthropic: send continuation with tool_result
Anthropic-->>Gateway: return final response
Gateway-->>Handler: return JSON or SSE
Handler-->>Client: response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
internal/messages/gateway.go (1)
350-414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate result-mapping logic between
gatewayToolResultandprojectGatewayResponse.Both functions independently map
websearch.Resultfields (title/url/content, published_date/page_age) into output maps with slightly different key semantics (Line 364-369 uses separatepublished_date/page_agekeys; Line 401-406 collapses both intopage_age). Extracting a singleresultToContentItem(result)-style helper would avoid the two mappings drifting further apart as fields are added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway.go` around lines 350 - 414, Extract the shared websearch.Result-to-content mapping from gatewayToolResult and projectGatewayResponse into a helper such as resultToContentItem. Reuse it in both functions, preserving consistent title, URL, content, published-date, and page-age key behavior; remove the duplicated field-mapping logic and avoid collapsing published date into page age.internal/messages/handler.go (1)
99-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
PrincipalFromContextlookup.
principalis re-fetched at Line 99 even though the same value was already retrieved at Line 86 for the auth check. Reuse the earlier variable instead of callingauth.PrincipalFromContexttwice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/handler.go` around lines 99 - 100, Reuse the principal variable already retrieved for the auth check earlier in the handler instead of calling auth.PrincipalFromContext again before the h.gateway condition. Remove the redundant lookup while preserving the existing CredentialTypeCodeSessionOAuth check.internal/messages/gateway_test.go (1)
35-216: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest order interleaves failure and success scenarios.
Tests alternate: transparent-fallback (35-49) → loop-limit failure (51-63) → upstream-failure passthrough (65-79) → success transcript projection (81-134) → provider-failure/panic (136-193) → success SSE (195-216). Per path instructions,
**/*_test.goshould group all failure scenarios before success scenarios. Reordering soTestGatewayToolLoopProjectsTranscriptandTestGatewaySSEResponsecome afterTestGatewayProviderFailureBecomesToolError/TestGatewayProviderPanicBecomesToolErrorwould satisfy the convention.Coverage itself is solid — the tool-loop transcript, provider-error, and provider-panic assertions correctly exercise the gateway's continuation and error paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway_test.go` around lines 35 - 216, Reorder the tests in this section so all failure scenarios run before success scenarios: move TestGatewayToolLoopProjectsTranscript and TestGatewaySSEResponse after TestGatewayProviderFailureBecomesToolError and TestGatewayProviderPanicBecomesToolError. Do not change test logic or coverage.Source: Path instructions
tests/messages_api_test.go (1)
269-342: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGood end-to-end coverage of the happy path; consider adding a mixed-tool regression test.
This test correctly validates BYOK key isolation (Line 291-293), tool projection (Line 296-303), continuation message count (Line 307-310), and the reconstructed
server_tool_use/web_search_tool_resultcontent (Line 335-337) — solid coverage of the single-tool continuation flow.It does not cover the case where the request's
toolsarray includes a non-web_search tool alongsideweb_search_20250305and the model calls both in one turn — see the critical continuation bug flagged ininternal/messages/gateway.go(Lines 138-153). Once that's fixed, a regression test here (or ingateway_test.go) would help lock in the fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/messages_api_test.go` around lines 269 - 342, Add a regression test alongside TestMessagesWebSearchGateway for a request containing web_search_20250305 plus a non-web-search tool, with the model invoking both in the same turn. Assert the continuation request preserves the non-web-search tool call and includes the web-search result, then verify the final gateway response and expected upstream call count.internal/websearch/brave.go (1)
92-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant recomputation of merged options for pagination.
mergeSearchOptions(c.defaultOptions, request.Options)is computed once at line 67 to build the URL and recomputed again here (line 98) purely to readPageToken. Since it's a pure function the result is identical, but it clones slices twice and creates two places that must stay in sync if merge logic changes.♻️ Proposed fix: compute once and reuse
func (c *BraveClient) Search(ctx context.Context, request SearchRequest) (SearchResponse, error) { if c == nil || c.apiKey == "" { return SearchResponse{}, errors.New("web search provider is not configured") } query := strings.TrimSpace(request.Query) if query == "" { return SearchResponse{}, errors.New("web search query is required") } - endpoint, err := c.searchURL(query, mergeSearchOptions(c.defaultOptions, request.Options)) + options := mergeSearchOptions(c.defaultOptions, request.Options) + endpoint, err := c.searchURL(query, options) if err != nil { return SearchResponse{}, fmt.Errorf("build brave search endpoint: %w", err) } @@ if decoded.HasMore { offset := 0 - if options := mergeSearchOptions(c.defaultOptions, request.Options); options.PageToken != "" { + if options.PageToken != "" { offset, err = strconv.Atoi(options.PageToken) if err != nil { return SearchResponse{}, fmt.Errorf("parse brave search page token: %w", err) } } if offset < braveMaxOffset { decoded.NextPageToken = strconv.Itoa(offset + 1) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/websearch/brave.go` around lines 92 - 109, Reuse the merged search options already computed for URL construction instead of calling mergeSearchOptions again in the decoded.HasMore pagination block. Keep the existing PageToken parsing and NextPageToken behavior unchanged, using the previously computed options value throughout the surrounding search request flow.internal/websearch/tavily.go (1)
68-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShared HTTP response-limiting/status-check logic duplicated between
tavily.goandbrave.go. Both clients independently implement the same "bounded read → oversize check → 2xx status check" sequence with only naming/message differences; consolidating into one helper in thewebsearchpackage removes the duplication and keeps both providers' error handling in sync as they evolve. As per coding guidelines,**/*.goproduction code has a duplication-rate cap ("Go 生产代码重复率上限为 3.75%") and the check must not be bypassed by raising thresholds or excluding files.
internal/websearch/tavily.go#L68-L99: extract the "response, err := c.client.Do(...)" through "status code check" sequence (lines 74-88) into a shared helper (e.g.fetchLimitedBody(client, request, maxSize) ([]byte, error)plus acheckHTTPStatushelper) and call it here.internal/websearch/brave.go#L77-L91: replace the equivalent duplicated block with a call to the same shared helper(s).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/websearch/tavily.go` around lines 68 - 99, Consolidate the duplicated bounded-read and HTTP status handling into shared helpers in the websearch package, such as fetchLimitedBody and checkHTTPStatus. In internal/websearch/tavily.go lines 68-99, replace the response execution through status-check sequence with the shared helper while preserving request construction and response decoding. Apply the same replacement in internal/websearch/brave.go lines 77-91, keeping provider-specific error context through the shared helper’s returned errors.Source: Coding guidelines
internal/websearch/brave_test.go (1)
15-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the success-path test after failure coverage.
Declare
TestBraveClientSearchafterTestBraveClientFailuresso test organization matches the required failure-before-success order. As per coding guidelines, "**/*_test.go: 测试组织顺序应先覆盖失败场景，再覆盖成功场景。"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/websearch/brave_test.go` around lines 15 - 58, Move the TestBraveClientSearch function to appear after TestBraveClientFailures in the test file, preserving its implementation unchanged and placing failure coverage before success coverage.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/be/messages-proxy.md`:
- Line 26: Update the Messages proxy documentation to explicitly list the
supported Anthropic web search server-tool versions: web_search_20250305,
web_search_20260209, and web_search_20260318. Keep the documented behavior
aligned with the version checks in the gateway implementation rather than
implying that every web_search_* version is supported.
In `@internal/messages/gateway.go`:
- Around line 178-193: Replace the untyped map[string]any/[]any business-field
handling in extractGatewayToolCalls, assistantGatewayMessage, gatewayToolResult,
projectGatewayResponse, hasWebSearchTool, and projectGatewayFields with small
named structs for the fields read or written, including tool type/name/id,
input.query, and content. Decode boundary JSON into these DTOs before routing or
reconstructing messages, and retain json.RawMessage only for unknown or
passthrough payloads. Remove unchecked type assertions while preserving the
existing behavior for valid and malformed gateway data.
- Around line 195-240: Preserve caller-supplied web_search options through
projectGatewayFields and the g.search request path instead of replacing every
matching tool with the fixed searchToolDefinition or forcing
SearchOptions{MaxResults: 5}. Thread supported limits and filters, including
max_uses and domain restrictions, into the generated tool/request options; if an
option cannot be propagated, explicitly reject it rather than silently dropping
it.
- Around line 138-153: Update extractGatewayToolCalls and the assistant-turn
replay flow so every tool_use block receives a corresponding tool_result,
including non-web_search tools in mixed turns. Preserve web_search execution
through g.search, and create the established unsupported-tool/error result for
other tool uses before appending the user message, preventing partial
tool-result transcripts.
In `@internal/messages/handler.go`:
- Around line 67-70: Update NewHandler so the gateway field is nil when
websearch.NewProvider(cfg.WebSearch, client) returns nil, rather than always
storing the non-nil result of newGateway. Preserve the existing gateway
construction for configured providers, ensuring the h.gateway != nil check
prevents readGatewayCandidate from buffering requests when web search is
unavailable.
---
Nitpick comments:
In `@internal/messages/gateway_test.go`:
- Around line 35-216: Reorder the tests in this section so all failure scenarios
run before success scenarios: move TestGatewayToolLoopProjectsTranscript and
TestGatewaySSEResponse after TestGatewayProviderFailureBecomesToolError and
TestGatewayProviderPanicBecomesToolError. Do not change test logic or coverage.
In `@internal/messages/gateway.go`:
- Around line 350-414: Extract the shared websearch.Result-to-content mapping
from gatewayToolResult and projectGatewayResponse into a helper such as
resultToContentItem. Reuse it in both functions, preserving consistent title,
URL, content, published-date, and page-age key behavior; remove the duplicated
field-mapping logic and avoid collapsing published date into page age.
In `@internal/messages/handler.go`:
- Around line 99-100: Reuse the principal variable already retrieved for the
auth check earlier in the handler instead of calling auth.PrincipalFromContext
again before the h.gateway condition. Remove the redundant lookup while
preserving the existing CredentialTypeCodeSessionOAuth check.
In `@internal/websearch/brave_test.go`:
- Around line 15-58: Move the TestBraveClientSearch function to appear after
TestBraveClientFailures in the test file, preserving its implementation
unchanged and placing failure coverage before success coverage.
In `@internal/websearch/brave.go`:
- Around line 92-109: Reuse the merged search options already computed for URL
construction instead of calling mergeSearchOptions again in the decoded.HasMore
pagination block. Keep the existing PageToken parsing and NextPageToken behavior
unchanged, using the previously computed options value throughout the
surrounding search request flow.
In `@internal/websearch/tavily.go`:
- Around line 68-99: Consolidate the duplicated bounded-read and HTTP status
handling into shared helpers in the websearch package, such as fetchLimitedBody
and checkHTTPStatus. In internal/websearch/tavily.go lines 68-99, replace the
response execution through status-check sequence with the shared helper while
preserving request construction and response decoding. Apply the same
replacement in internal/websearch/brave.go lines 77-91, keeping
provider-specific error context through the shared helper’s returned errors.
In `@tests/messages_api_test.go`:
- Around line 269-342: Add a regression test alongside
TestMessagesWebSearchGateway for a request containing web_search_20250305 plus a
non-web-search tool, with the model invoking both in the same turn. Assert the
continuation request preserves the non-web-search tool call and includes the
web-search result, then verify the final gateway response and expected upstream
call count.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 659fbb3d-1741-4940-b915-ae8be886039b
📒 Files selected for processing (20)
.gitignoreconfig/config.example.yamldocs/configuration-reference.yamldocs/design/be/messages-proxy.mddocs/design/be/web-search-provider-research.mdinternal/config/config_test.gointernal/config/defaults.gointernal/config/reference_test.gointernal/config/types.gointernal/config/yaml_types.gointernal/messages/gateway.gointernal/messages/gateway_test.gointernal/messages/handler.gointernal/websearch/brave.gointernal/websearch/brave_test.gointernal/websearch/provider.gointernal/websearch/tavily.gointernal/websearch/tavily_test.gointernal/websearch/types.gotests/messages_api_test.go
f527903 to
e08a5ca
Compare
|
已处理 CodeRabbit review：
验证：全量 Go 测试、race 测试、定向 golangci-lint、dead-code、duplicates、complexity、large-files 均通过。仓库本地 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/be/web-search-provider-research.md`:
- Around line 3-7: Remove the blank line inside the introductory blockquote in
the research document, keeping the research scope, date, and implementation
status as one continuous blockquote without empty `>` lines.
- Around line 207-213: Update the date-mapping row in the comparison table to
distinguish Brave’s page_age from publication timestamps: map page_age to
PageAge, and state that PublishedDate is populated only from an actual
publication timestamp. Remove wording that implies page_age can partially
populate PublishedDate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a07cf381-2a13-4281-b8fd-e8501e4b7573
📒 Files selected for processing (21)
.gitignoreconfig/config.example.yamldocs/configuration-reference.yamldocs/design/be/messages-proxy.mddocs/design/be/web-search-provider-research.mdinternal/config/config_test.gointernal/config/defaults.gointernal/config/reference_test.gointernal/config/types.gointernal/config/yaml_types.gointernal/messages/gateway.gointernal/messages/gateway_test.gointernal/messages/handler.gointernal/websearch/brave.gointernal/websearch/brave_test.gointernal/websearch/http.gointernal/websearch/provider.gointernal/websearch/tavily.gointernal/websearch/tavily_test.gointernal/websearch/types.gotests/messages_api_test.go
🚧 Files skipped from review as they are similar to previous changes (19)
- internal/config/defaults.go
- internal/websearch/provider.go
- internal/config/yaml_types.go
- .gitignore
- config/config.example.yaml
- docs/configuration-reference.yaml
- internal/websearch/tavily_test.go
- internal/config/reference_test.go
- internal/websearch/types.go
- internal/config/types.go
- internal/websearch/tavily.go
- tests/messages_api_test.go
- internal/websearch/brave.go
- internal/messages/handler.go
- internal/config/config_test.go
- docs/design/be/messages-proxy.md
- internal/messages/gateway.go
- internal/websearch/brave_test.go
- internal/messages/gateway_test.go
e08a5ca to
ed313e1
Compare
|
补充处理了新一轮 review：
当前 PR 仍保持单一 commit： |
ed313e1 to
bbb6a1d
Compare
![duckpr[bot]](https://avatars.githubusercontent.com/in/4166808?s=60&v=4){kind=small}
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审无新问题。
bbb6a1d把调用方声明的搜索策略正确收敛到服务端强制路径。
Reviewed changes — 本次复审聚焦 bbb6a1d（"fix(messages): 保留 Web Search 请求策略"），即上一次评审后新增的唯一提交，核心是把 max_uses/allowed_domains/blocked_domains/user_location 从 model-controlled tool input 提升为调用方策略，由 OMA 在调用 BYOK 前解析并强制。
- 将搜索策略从模型输入剥离到服务端
gatewaySearchPolicy— 新增gatewaySearchTool与searchPolicy()，在projectGatewayFields阶段一次性解析max_uses、allowed_domains、blocked_domains、user_location；BYOK 只看到仅含query的searchToolDefinition()，避免模型改写或绕过调用方约束。 max_uses改由executeToolCalls统计并强制 — 超限的后续搜索转为web_search_tool_result_error，而不是依赖模型自控；searchUses跨轮累计，逻辑集中。user_location显式拒绝而非静默丢弃 — provider-neutral 合同无法表达该字段时，searchPolicy()返回明确错误，且在发往 BYOK 前即终止。- 拒绝重复 web search tool 与
max_uses非正值 —projectGatewayFields增加重复检测，searchPolicy()校验max_uses <= 0与allowed_domains/blocked_domains互斥。 - 同步更新设计文档与测试 —
messages-proxy.md补齐策略强制与user_location报错说明；新增TestGatewayEnforcesCallerMaxUses与TestGatewayRejectsUnsupportedCallerLocation覆盖两条新路径，既有 transcript 测试补充“策略字段未泄漏进模型输入”断言。
本次只复审了增量提交，未重复评审 CodeRabbit 已覆盖的初始实现。go test ./internal/messages/... 与 go vet 均通过。
anthropic/glm-5.2 ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
internal/messages/gateway_test.go (4)
140-177: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMixed-tool test doesn't assert
is_erroron the non-search tool result.Per the PR objectives, this test targets the fix that supplies an
is_errorresult for non-Web-Search tools in hybrid tool calls. The assertion only checks thattool_use_idvalues are present (lines 162-164); it never checks that thebashtool's synthesized result actually carries"is_error":true, so a regression that returns a plain (non-error) result for unsupported tools would still pass this test.✅ Proposed strengthening
if !strings.Contains(string(last.Content[0]), `"tool_use_id":"toolu_search"`) || !strings.Contains(string(last.Content[1]), `"tool_use_id":"toolu_other"`) { t.Fatalf("continuation results = %s", last.Content) } + if !strings.Contains(string(last.Content[1]), `"is_error":true`) { + t.Fatalf("expected is_error result for unsupported tool, got %s", last.Content[1]) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway_test.go` around lines 140 - 177, Strengthen TestGatewayMixedToolUseReturnsUnsupportedToolResult by inspecting the continuation content for the non-search tool result and asserting that the entry with tool_use_id "toolu_other" includes "is_error":true. Keep the existing assertions for both tool_use_id values and the request flow unchanged.
286-307: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSSE test never exercises the tool-search continuation loop.
TestGatewaySSEResponsesets"stream":truebut the mock upstream immediately returnsend_turnwith notool_use, so the gateway's search-and-continuation path is never invoked over SSE — only the trivial pass-through-to-SSE-encoding path is covered. Given SSE/JSON compatibility for the continuation loop is a stated goal of this PR, consider adding a variant modeled onTestGatewayToolLoopProjectsTranscriptbut with"stream":true, to confirm the reconstructedserver_tool_use/web_search_tool_resultcontent is correctly SSE-encoded after an actual provider search round-trip.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway_test.go` around lines 286 - 307, Extend TestGatewaySSEResponse to exercise the tool-search continuation loop, using the multi-round upstream behavior and assertions from TestGatewayToolLoopProjectsTranscript while keeping stream enabled. Verify the resulting SSE response includes correctly encoded server_tool_use and web_search_tool_result content after the provider search round-trip, in addition to the existing SSE headers and lifecycle events.
51-307: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated upstream-mock boilerplate across tests.
Nearly every test hand-rolls an
httptest.NewServer(http.HandlerFunc(...))with a request-count-based sequenced JSON response, followed by the samecfg/newGatewaywiring. A small helper (e.g.newSequencedUpstream(t, responses ...string) *httptest.Server) would remove this repetition across ~9 functions and make each test body focus on its distinguishing assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway_test.go` around lines 51 - 307, Add a shared test helper such as newSequencedUpstream(t, responses ...string) that creates and registers an httptest server, returns each JSON response in sequence, and fails on unexpected requests or exhausted responses. Refactor the repeated upstream setup and gateway wiring in the affected TestGateway* cases to use this helper while preserving tests that require custom request inspection or non-200 responses.
51-63: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLoop-limit boundary not verified by request count.
MaxToolLoops: 1should bound the exact number of upstream round-trips before the gateway errors out. The test only asserts the final outcome (handled,err, nil body) but never asserts how many times the upstream handler was actually invoked, so an off-by-one in the loop-bound check (e.g., allowing 2 iterations instead of 1) would not be caught.✅ Proposed addition
+ requestCount := 0 upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requestCount++ w.Header().Set("Content-Type", "application/json") _, _ = io.WriteString(w, "{\"type\":\"message\",\"content\":[{\"type\":\"tool_use\",\"id\":\"toolu_loop\",\"name\":\"web_search\",\"input\":{\"query\":\"query\"}}],\"stop_reason\":\"tool_use\"}") })) @@ if !handled || err == nil || response.body != nil { t.Fatalf("response = %#v, handled = %v, err = %v; want bounded loop error", response, handled, err) } + if requestCount != 1 { + t.Fatalf("upstream requests = %d, want exactly 1 for MaxToolLoops=1", requestCount) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway_test.go` around lines 51 - 63, Update TestGatewayToolLoopLimit to count invocations of the upstream httptest handler and assert that the request count is exactly one when MaxToolLoops is 1. Keep the existing bounded-loop outcome assertions, ensuring the test catches any off-by-one iteration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/be/web-search-provider-research.md`:
- Around line 14-15: Update the SearchOptions coverage statement in the document
to match the provider-neutral contract defined by internal/websearch/types.go:
either include SafeSearch, Spellcheck, ResultFilter, and Goggles only if they
are exposed there, or revise the text to identify them as Brave-specific
capabilities handled by the adapter.
In `@internal/messages/gateway_test.go`:
- Around line 196-218: Set WebSearchConfig.APIKey to "tavily-key" in the cfg
initialization for this test, while preserving the existing provider
configuration. Keep the strings.Contains assertion against the first BYOK
request so it verifies the configured Tavily key is not forwarded.
---
Nitpick comments:
In `@internal/messages/gateway_test.go`:
- Around line 140-177: Strengthen
TestGatewayMixedToolUseReturnsUnsupportedToolResult by inspecting the
continuation content for the non-search tool result and asserting that the entry
with tool_use_id "toolu_other" includes "is_error":true. Keep the existing
assertions for both tool_use_id values and the request flow unchanged.
- Around line 286-307: Extend TestGatewaySSEResponse to exercise the tool-search
continuation loop, using the multi-round upstream behavior and assertions from
TestGatewayToolLoopProjectsTranscript while keeping stream enabled. Verify the
resulting SSE response includes correctly encoded server_tool_use and
web_search_tool_result content after the provider search round-trip, in addition
to the existing SSE headers and lifecycle events.
- Around line 51-307: Add a shared test helper such as newSequencedUpstream(t,
responses ...string) that creates and registers an httptest server, returns each
JSON response in sequence, and fails on unexpected requests or exhausted
responses. Refactor the repeated upstream setup and gateway wiring in the
affected TestGateway* cases to use this helper while preserving tests that
require custom request inspection or non-200 responses.
- Around line 51-63: Update TestGatewayToolLoopLimit to count invocations of the
upstream httptest handler and assert that the request count is exactly one when
MaxToolLoops is 1. Keep the existing bounded-loop outcome assertions, ensuring
the test catches any off-by-one iteration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca56fccb-4abd-412f-a8bf-9847389c3657
📒 Files selected for processing (21)
.gitignoreconfig/config.example.yamldocs/configuration-reference.yamldocs/design/be/messages-proxy.mddocs/design/be/web-search-provider-research.mdinternal/config/config_test.gointernal/config/defaults.gointernal/config/reference_test.gointernal/config/types.gointernal/config/yaml_types.gointernal/messages/gateway.gointernal/messages/gateway_test.gointernal/messages/handler.gointernal/websearch/brave.gointernal/websearch/brave_test.gointernal/websearch/http.gointernal/websearch/provider.gointernal/websearch/tavily.gointernal/websearch/tavily_test.gointernal/websearch/types.gotests/messages_api_test.go
🚧 Files skipped from review as they are similar to previous changes (19)
- internal/config/defaults.go
- internal/config/reference_test.go
- internal/websearch/http.go
- internal/config/types.go
- docs/configuration-reference.yaml
- config/config.example.yaml
- .gitignore
- internal/websearch/provider.go
- internal/config/yaml_types.go
- internal/websearch/tavily_test.go
- internal/config/config_test.go
- docs/design/be/messages-proxy.md
- internal/websearch/brave.go
- internal/websearch/brave_test.go
- internal/websearch/types.go
- tests/messages_api_test.go
- internal/messages/handler.go
- internal/websearch/tavily.go
- internal/messages/gateway.go
|
这轮 review body 里的 4 个测试 nit 也一起处理了：
本地的 config/websearch/messages 窄测试、race test、lint、dead-code、duplicates、complexity 都已跑过。 |
|
补充一次协议语义核对（重新对照 Anthropic 官方文档，并用真实 Claude Code + DeepSeek-V4-Pro + Tavily 跑过请求链路）。 1.
|
|
再补两处实现级边界，避免把两个“次数”混成一个概念：
另外，合成的 server tool block 也应使用 Anthropic 约定的 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/messages/gateway_test.go (2)
165-183: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNon-atomic request counters mutated in
httptesthandler goroutines. Both files count upstream/provider requests with a plainintincremented inside the handler goroutine and read from the test goroutine, which is a data race under-race;newSequencedUpstreamalready provides an atomic counter for this purpose.
internal/messages/gateway_test.go#L165-L183: replace therequestCount intclosure with an atomic counter (ornewSequencedUpstream), and apply the same change to therequestCountinTestGatewayMixedToolUseDefersSearchUntilClientResultsat Lines 218-220.tests/messages_api_test.go#L343-L498: makesearchCallsandupstreamCallsatomic.Int64(or guard with a mutex) and read them viaLoad()at the assertion sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway_test.go` around lines 165 - 183, Replace the plain request counters in internal/messages/gateway_test.go lines 165-183, including the requestCount in TestGatewayMixedToolUseDefersSearchUntilClientResults at lines 218-220, with atomic counters or newSequencedUpstream and use synchronized reads in assertions. In tests/messages_api_test.go lines 343-498, make searchCalls and upstreamCalls atomic.Int64 (or protect them with a mutex) and use Load() when asserting their values.
345-419: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
t.Errorf+returnovert.Fatalfinside thehttptesthandler goroutine.
t.Fatalfis only valid on the goroutine running the test function; from the handler it callsruntime.Goexiton the server goroutine, so the request is aborted without the test necessarily failing at that point. Same applies toTestGatewayProjectsCompletedMixedHistoryBackToBYOK(Lines 395-418).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/messages/gateway_test.go` around lines 345 - 419, Replace each t.Fatalf call inside the httptest handlers of TestGatewayProjectsCompletedSearchHistoryBackToBYOK and TestGatewayProjectsCompletedMixedHistoryBackToBYOK with t.Errorf followed by return, including decode, length, role/content, and unmarshal validation failures. Leave assertions outside the handler goroutines unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/messages/gateway_protocol.go`:
- Around line 558-570: Update the projection flow around
projectClientSearchCallToServer so an empty execution.call.externalID falls back
to serverGatewayToolUseID(execution.call.id), matching
gatewayWebSearchResultBlock. Use the same resolved ID for both the emitted
server_tool_use and web_search_tool_result pair, while preserving existing
behavior when externalID is present.
- Around line 204-215: Update the mixed tool continuation validation around
gatewayProtocolBlock so non-tool_result content blocks are ignored rather than
rejected, allowing text and other supported blocks to remain in the user message
for transcript projection. Preserve the existing decode error handling and
tool_result-specific validation/checks for tool calls.
In `@tests/messages_api_test.go`:
- Around line 491-497: Replace the vacuous secondPayload secret check in the
mixed-response assertion with validation inside the upstream handler: inspect
the received BYOK request body, decoding from the captured raw body if
necessary, and assert it does not contain "tavily-mixed-key". Remove the
strings.Contains check on secondPayload while preserving the existing response
and call-count assertions.
---
Nitpick comments:
In `@internal/messages/gateway_test.go`:
- Around line 165-183: Replace the plain request counters in
internal/messages/gateway_test.go lines 165-183, including the requestCount in
TestGatewayMixedToolUseDefersSearchUntilClientResults at lines 218-220, with
atomic counters or newSequencedUpstream and use synchronized reads in
assertions. In tests/messages_api_test.go lines 343-498, make searchCalls and
upstreamCalls atomic.Int64 (or protect them with a mutex) and use Load() when
asserting their values.
- Around line 345-419: Replace each t.Fatalf call inside the httptest handlers
of TestGatewayProjectsCompletedSearchHistoryBackToBYOK and
TestGatewayProjectsCompletedMixedHistoryBackToBYOK with t.Errorf followed by
return, including decode, length, role/content, and unmarshal validation
failures. Leave assertions outside the handler goroutines unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 247e50af-79ec-4268-982d-d45457edc58a
📒 Files selected for processing (6)
docs/design/be/messages-proxy.mdinternal/messages/gateway.gointernal/messages/gateway_protocol.gointernal/messages/gateway_test.gointernal/messages/handler.gotests/messages_api_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/design/be/messages-proxy.md
- internal/messages/handler.go
- internal/messages/gateway.go
|
@arthur-zhang 这轮已经把你提出的配置和扩展性问题连同后续协议边界一起收尾，当前 head 是
我用最新 public 本地 targeted、race、handler integration、lint、dead-code、duplicate、complexity、large-files 均通过；当前 GitHub 7 项检查也全部通过。麻烦基于 |
|
关于 之前的 真实链路使用本地重编的最新公开 e2b-local、sandbox/rclone、Claude Code 2.1.120、DeepSeek-V4-Pro 和真实 Tavily：
边界也说明一下：Anthropic 公共 schema 仍把原生 验证结果： References: |
Remove defer/recover in search() method - panic recovery belongs at the HTTP boundary layer (recoverMiddleware), not in deep business methods. Also drop unused imports (runtime/debug, httpapi).
b5efbe2 to
ebb814e
Compare
…ability Three defects surfaced by reviewing this branch against the project Go skills and the "100 Go Mistakes" catalog. Forwarding the caller's Accept-Encoding to BYOK disabled the Transport's transparent decompression, so the gateway received raw gzip bytes and failed to parse every upstream response. The real Anthropic API does compress when asked, and Node/undici clients always ask, so this broke the whole gateway path in production while httptest servers hid it. Delete the header in send() and let the Transport negotiate. Gateway failures wrote a 502 without logging anything, leaving the decode failure above completely silent server-side. Log ERROR before the 502 and WARN before the 400, matching the surrounding handlers. Also log one WARN with request_id and error_code when a provider failure degrades into web_search_tool_result_error, which gives the previously unused gateway logger a real purpose. finalizeWebSearchResponse built the client response from the last BYOK body, so a multi-iteration search silently under-reported token usage (a two-iteration run charging 2200/130 reported 1200/80). Accumulate integer token counters by field name across samples and keep the last sample's non-numeric fields, so future usage counters aggregate without a field allowlist. Single-sample requests pass the upstream usage through unchanged. Replace the test that asserted the recover() removed in 07e751a with one asserting the opposite invariant: a provider panic must reach the HTTP boundary's recoverMiddleware rather than being swallowed. Update the design doc for the usage, encoding, and logging contracts, and fix its stale reference to the pre-0889133 gateway_test.go filename.
# Conflicts: # tests/messages_api_test.go
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审无新问题。
515e36b把 provider 能力校验和 replayed history 完整性检查前移到 BYOK 请求之前，逻辑正确且测试覆盖充分。
Reviewed changes — 本次复审聚焦 515e36b（"fix(messages): validate web search replay contracts"），即上一次 Pullfrog 评审（bbb6a1d）后新增的唯一提交，核心是在发往 BYOK 前校验 provider 选项能力，并在续传历史中拒绝重复 server_tool_use ID。
- 新增
Provider.ValidateOptions契约并前移校验 —websearch.Provider接口新增ValidateOptions(SearchOptions) error；BraveClient拒绝域名过滤、TavilyClient全部允许。gateway 在prepareRequest阶段（首次 BYOK 请求之前）调用，避免发起注定失败的搜索回合。 - 抽取
webSearchOptionshelper 统一选项构造 —search()与prepareRequest的校验路径复用同一份webSearchOptions(policy)，保证校验和执行看到一致的SearchOptions。 - 拒绝 replayed history 中的重复
server_tool_useID —findPendingWebSearchTurn与findPausedWebSearchTurn均增加serverToolUses去重，重复 ID 在转录投影前即报错，防止tool_use_id匹配歧义。 - 补充设计与测试 — mermaid 图修正
oma_web_search投影名；新增TestWebSearchGatewayRejectsUnsupportedProviderOptionsBeforeUpstream（断言零 BYOK 请求、零搜索）与TestWebSearchGatewayRejectsDuplicateServerToolUsesInReplayedHistory（mixed + paused 两条路径）。
go build / go test ./internal/messages/... ./internal/websearch/... 均通过。本次只复审了增量提交，未重复评审 CodeRabbit 已覆盖的初始实现。
anthropic/glm-5.2 ｜ 𝕏
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审无新问题。
b10f8f3把 provider factory 从运行时init()注册改为静态编译期 map，重复名称会在编译期失败，安全性更高。
Reviewed changes — 本次复审聚焦上次 Pullfrog 评审（515e36b）后新增的两个提交，核心是把 provider factory 从运行时注册收敛为静态声明。
- 用静态
builtInProviderFactories替换运行时 registry —providerFactory接口移除Name()方法，brave/tavily 各自删除init()注册逻辑，改为provider.go中可审查的map[string]providerFactory字面量；重复 provider 名称会在编译期失败而非运行时静默覆盖。 - 同步更新测试 — 新增
TestBuiltInProviderFactories断言内置表恰好包含 brave 与 tavily；原有 registry 测试重命名为TestNewProviderUsesBuiltInProviderFactory并保持验证路径不变。 - 对齐设计文档 —
messages-proxy.md、web-search-provider-research.md、docker-compose-deployment.md、配置参考与示例补充ValidateOptions合同说明、Brave 域名策略限制与 Compose Web Search 部署约定。
go build / go test ./internal/websearch/... ./internal/messages/... 均通过。本次只复审了增量提交，未重复评审 CodeRabbit 与早期 Pullfrog 已覆盖的初始实现。
｜ View workflow run ｜ Using anthropic/glm-5.2 ｜ 𝕏
anthropic/glm-5.2 ｜ 𝕏
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
ℹ️ 无新问题。重构与 opaque-ID 支持实现正确、测试充分；仅一处文档表述与新增编码路径不一致，已在内联标注。
Reviewed changes — 本次复审聚焦上次 Pullfrog 评审（dbe4ffa）后新增的三个提交：742d675（拆分 web search 文件职责）、32c1356（补齐文件边界设计文档）、42bf4a2（支持 BYOK provider-owned opaque tool ID）。
- 将 web search 实现按职责拆分为四个文件 —
web_search_request.go（入站 JSON 解析、tools[].type精确匹配与oma_web_search投影）、web_search_gateway.go（BYOK 采样循环、provider 执行与 mixed/pause continuation 编排）、web_search_protocol.go（历史双向投影与tool_use_id配对）、web_search_response.go（执行结果映射、usage 合成与 JSON/SSE 编码）。拆分是保持行为的机械迁移，依赖方向清晰，共享类型仍统一使用webSearch*前缀。 - 新增
## Web Search 代码职责与依赖边界章节 —messages-proxy.md用表格列出每个文件的职责与「不负责的事项」，并补充 Anthropic 官方协议依据（mixed turn 时序、tool_use_id配对、max_usesper-request 语义、web_search_requests计费规则）。 - 支持 provider-owned opaque tool-use ID — 非
toolu_*的上游 ID（例如call_00_*）经带版本标记的 URL-safe base64 编码为srvtoolu_oma_encoded_<payload>，而既有toolu_*保持前缀替换以兼容已保存历史；TestWebSearchToolUseIDMappingHasNoCollisions覆盖toolu_oma_*/toolu_srvtoolu_*等可能碰撞的边界形状，TestWebSearchGatewayExecutesOpaqueUpstreamSearchToolUse端到端验证 opaque ID 的搜索执行与响应投影。 - SSE 测试补齐 continuation loop 覆盖 —
TestWebSearchGatewaySSEResponse现在真实触发tool_use -> end_turn两轮 BYOK 采样，覆盖server_tool_use/web_search_tool_result/web_search_result与input_json_delta的 SSE 编码。
go build ./...、go test ./internal/messages/... ./internal/websearch/... 均通过。本次只复审了增量提交，未重复评审 CodeRabbit 与早期 Pullfrog 已覆盖的初始实现。
｜ View workflow run ｜ Using anthropic/glm-5.2 ｜ 𝕏
anthropic/glm-5.2 ｜ 𝕏
| end | ||
| ``` | ||
|
|
||
| 内部 transcript 与 Claude Code transcript 通过双向投影保持一致。gateway 声明给 BYOK 的是名为 `oma_web_search` 的 ordinary tool：调用方可以合法地声明自己的 `web_search` 工具，复用 Anthropic 协议名会让同一请求出现两个同名 tool，也无法再按名字判断 `tool_use` 归属。投影只在这两个名字之间双向替换；当顶层 `tool_choice` 强制 `web_search` 时，也只在发给 BYOK 的请求里改为 `oma_web_search`。调用方看到的响应、历史和续传请求始终使用协议名 `web_search`；调用方自有的同名工具按普通 client tool 交还调用方，不由 gateway 执行。BYOK 的 ordinary `toolu_*` 与面向调用方的 `srvtoolu_*` 之间只做前缀替换，因此映射是双射；不引入自定义编码可以避免同一上游 ID 存在多种外部表示、进而在同一条 assistant message 里产生重复 `tool_use` ID。反向解析只接受 gateway 自己铸造的 `srvtoolu_` 形状，其余形状一律拒绝，避免调用方伪造上游 ID。Claude Code 后续重放完整历史时，gateway 会把 `server_tool_use`/`web_search_tool_result` 展开回 BYOK 的 assistant `tool_use` 与下一条 user `tool_result`。mixed continuation 中 Claude Code 只返回自己拥有的 client results，每个 client `tool_use` 必须恰好对应一个 `tool_result`，该 user message 不得包含 text 或其他 block，并保留声明 pending search 的同一 Web Search tool；缺失、重复或未知 result、混入非 result content，以及缺少 pending search tool时返回 `400 invalid_request_error`，不会把 server block 透传给 BYOK。gateway 执行 pending search 后按原 tool call 顺序合并结果。该排序只为可读性：Anthropic 按 `tool_use_id` 匹配 result 而非按位置，也不区分 server/client tool 的 result 先后。真正的硬约束是配对完整性——每个 `tool_use` 必须恰好有一个 result，缺失会触发 `tool_use ids were found without tool_result blocks immediately after`，因此合并时即使出现重复 ID 也不丢弃任何一条 result。普通 client tool 不由 OMA 伪造 error result。已完成的 search 历史即使后续请求不再声明 Web Search，也仍会反向投影，但不会允许 BYOK 发起新的搜索。 |
There was a problem hiding this comment.
ℹ️ 这段投影说明仍写「toolu_* 与 srvtoolu_* 之间只做前缀替换」，但紧接着的 opaque-ID 段落已经引入了第二条编码路径（非 toolu_* ID 走 srvtoolu_oma_encoded_<base64>）。两者都确实以 srvtoolu_ 开头、反向解析也都只接受 gateway 铸造的形状，所以安全性结论不变；只是「只做前缀替换」这句与下方实现不一致，建议改成「toolu_* 走前缀替换、其他 opaque ID 走带版本标记的编码」之类的两句式描述，避免下一位维护者按旧描述推断映射是单一前缀交换。
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审无新问题。
515e36b之后的重构把 request/response/protocol 职责拆分到独立文件，opaque BYOK tool ID 编码正确且测试覆盖充分。
Reviewed changes — 本次复审聚焦 515e36b（上一次 Pullfrog 评审）之后的 6 个提交，核心是把 web search gateway 的职责按文件拆分、用静态 map 替换 init() 注册、并支持 provider-owned 的任意 opaque tool ID。
- 拆分 gateway 文件职责 — 把
web_search_gateway.go和web_search_protocol.go中的请求投影、响应编码、usage 累加等逻辑机械抽取到web_search_request.go和web_search_response.go；类型与方法签名不变，无行为改变。 - 支持 provider-owned opaque tool ID —
serverWebSearchToolUseID/upstreamWebSearchToolUseID新增 base64 URL-safe 编码路径，兼容既有toolu_*前缀替换的同时覆盖call_*等任意非空 opaque ID；encodedUpstreamToolUseIDPrefix命名空间隔离避免 legacy 与 encoded 路径碰撞。 - 用静态 map 替换
init()注册 —builtInProviderFactories改为编译期常量 map，消除init()副作用和Name()方法，重复键在编译期即冲突。 - 补充测试 — 新增
TestWebSearchGatewayExecutesOpaqueUpstreamSearchToolUse（端到端 opaque ID 流程）、TestWebSearchToolUseIDMappingSupportsOpaqueUpstreamIDs（三轮换回测 + 碰撞检测）和TestBuiltInProviderFactories。 - 同步设计文档 —
messages-proxy.md和web-search-provider-research.md补充 opaque ID 编码与 provider 契约说明。
go test ./internal/messages/... ./internal/websearch/... -count=1 与 -race 均通过。
｜ View workflow run ｜ Using anthropic/glm-5.2 ｜ 𝕏
anthropic/glm-5.2 ｜ 𝕏
There was a problem hiding this comment.
DuckPR reviewer: opencode
Model: anthropic/glm-5.2
✅ 增量复审无新问题。
7c89aaf把 tool ID 映射收敛为单一 opaque 编码路径，消除了 legacy 前缀替换分支，双射性和拒绝逻辑正确，测试覆盖充分。
Reviewed changes — 本次复审聚焦上一次 Pullfrog 评审（515e36b）后的 7 个增量提交（b10f8f3–7c89aaf），核心是把 tool ID 映射收敛为单一 opaque 编码、按职责拆分 web search 文件，以及显式声明 built-in provider。
- 将 tool ID 映射统一为单一 opaque 编码路径（
42bf4a2+7c89aaf）— 移除了toolu_*前缀替换的 legacy 分支；所有 provider-owned ID（包括 Anthropic 的toolu_*）统一走srvtoolu_oma_encoded_<base64>。upstreamWebSearchToolUseID只接受带版本标记的编码形状，拒绝其他srvtoolu_*，消除了"同一上游 ID 多种外部表示"的歧义。base64 编码保证双射，碰撞测试覆盖toolu_abc/toolu_oma_abc/toolu_srvtoolu_abc/toolu_1四种输入。 - 按职责拆分 web search 代码（
742d675+32c1356）— 将原gateway.go拆为web_search_gateway.go（主流程）、web_search_protocol.go（transcript 投影与 tool ID 映射）、web_search_request.go（请求解析与策略校验）、web_search_response.go（响应组装与 SSE 编码），符合 AGENTS.md 的垂直资源切片与关注点分离方向。 - 显式声明 built-in provider（
b10f8f3+dbe4ffa）— 用静态builtInProviderFactoriesmap 注册 brave/tavily，新增 provider 成为显式可评审改动；provider contract 文档同步对齐。 - 同步更新设计文档（
3b5b629）—messages-proxy.md明确了入口按tools[].type拦截、调用块按name=oma_web_search识别的两阶段区分，以及 opaque ID 映射不依赖 provider 格式的说明。
go build ./internal/messages/... ./internal/websearch/... 与 go test ./internal/messages/... ./internal/websearch/... 均通过；tool ID 映射、duplicate server tool use、mixed/paused continuation 相关测试全部通过。本次只复审增量提交，未重复评审 CodeRabbit 已覆盖的初始实现。
anthropic/glm-5.2 ｜ 𝕏
2 participants
背景与边界
Claude Code 2.1.120 的实际链路分成两层：
name=WebSearch；这一层由 Claude Code 自己执行，OMA 不接管 Bash、Edit、MCP 等普通 client tool。WebSearch后，Claude Code 的内置执行器会另发一条 Messages 请求，声明type=web_search_20250305的 Anthropic server tool。OMA 只拦截这一层的已知 Web Search server-tool type。OMA 将被拦截的 server tool 投影为 BYOK 可调用的普通
web_searchtool，但声明工具不等于执行搜索。只有 BYOK 响应真的返回tool_use(name=web_search)时，OMA 才调用 Tavily/Brave；模型不调用时不会访问搜索 provider。当前只支持
web_search_20250305、web_search_20260209、web_search_20260318。未登记的type=web_search和其他 Anthropic server tools 保持透明转发；后续统一 registry 见 #185。本次实现
max_uses、域名过滤、allowed_callers和response_inclusion作为调用方策略校验；不能可靠模拟的 caller/location 组合在 BYOK 请求前明确拒绝，不静默降级。tool_use返回 Claude Code 执行；OMA 暂缓搜索。Claude Code 下一条 user message 必须只包含全部 clienttool_result并保留 Web Search tool，OMA 才执行 pending search，并按tool_use_id将所有结果合并回同一 BYOK user message。OMA 不再为普通 client tool 伪造 unsupported result。pause_turn：完成本次已允许的搜索后返回配对的server_tool_use/web_search_tool_resultcheckpoint；续传时支持已完成结果回放、末尾未完成 server call 执行，以及连续多次暂停。max_tool_loops重命名为max_server_tool_iterations，默认值与 Anthropic server-side sampling loop 的每请求 10 次对齐。一次 iteration 是一次 BYOK Messages 请求（含初始采样），与限制实际搜索次数的tools[].max_uses无关。web_search_result只输出title、url和可选page_age等 OMA 支持的可见字段，不生成或伪造 Anthropic 原生encrypted_content、citation location 或 server-tool usage；原生 Anthropic provider 返回的encrypted_content是 opaque 数据，OMA-managed search 当前不实现其加密恢复合同。e2b_加小写十六进制格式。真实 E2E
使用最新公开
e2b-local源码、本地重建的 sandbox/rclone 镜像、Claude Code 2.1.120、DeepSeek-V4-Pro 和真实 Tavily 完成两轮端到端验证；未使用 CVTE 内网镜像或 package。最终代码在
max_server_tool_iterations=1下的抓包结果：tool_use -> pause_turn -> end_turntool_use -> end_turnserver_tool_use/web_search_tool_resultcheckpoint，续传后给出带来源的最终答案并回到 idle；OMA 没有伪造encrypted_content另有真实 pass-through 场景验证 Bash/WebFetch 由 sandbox 内 Claude Code 执行，Tavily 调用数为 0。
测试与门禁
go test ./internal/messages ./internal/config -count=1：104 passedgo test ./internal/messages -race -count=1：43 passedgo test ./tests -run 'TestMessagesWebSearchGateway' -count=1：2 passedjust lint、just dead-code、just duplicates、just complexity、just large-files：通过detect-private-key命中外，其余 hook 全部通过；本次 diff 不包含这两个 fixtureReferences