diff --git a/kubernetes/apps/review-bot/deployment.yaml b/kubernetes/apps/review-bot/deployment.yaml index c64ccfbb..1304ecf7 100644 --- a/kubernetes/apps/review-bot/deployment.yaml +++ b/kubernetes/apps/review-bot/deployment.yaml @@ -48,7 +48,7 @@ spec: mistral:mistral-large-4-0 = Mistral Large 4. Separate budget; for routine changes of small to medium size. - name: SELECTOR value: typesafe - # Anthropic-side web fetch for release notes; only links from the PR body and fetched pages. + # Anthropic-side web fetch and web search for release notes, on these domains only. - name: WEB_FETCH_DOMAINS value: github.com,gitlab.com - name: ANTHROPIC_API_KEY diff --git a/review-bot/anthropic.go b/review-bot/anthropic.go index f950c6e8..4db5c8dc 100644 --- a/review-bot/anthropic.go +++ b/review-bot/anthropic.go @@ -32,13 +32,18 @@ type Anthropic struct { mcpToken string tools []string effort string - // fetchDomains enables the web fetch server tool for these domains; empty leaves it off. - fetchDomains []string - fetchMaxUses int + // fetchDomains enables the web fetch and web search server tools for these domains; empty + // leaves both off. + fetchDomains []string + fetchMaxUses int + searchMaxUses int } -// webFetch is the name of Anthropic's web fetch server tool. -const webFetch = "web_fetch" +// webFetch and webSearch are the names of Anthropic's web fetch and web search server tools. +const ( + webFetch = "web_fetch" + webSearch = "web_search" +) // fetchMaxContentTokens caps the text one fetched page adds to the conversation. const fetchMaxContentTokens = 20000 @@ -49,7 +54,8 @@ func newAnthropic(cfg Config) *Anthropic { opts = append(opts, option.WithBaseURL(cfg.AnthropicURL)) } return &Anthropic{client: anthropic.NewClient(opts...), mcpURL: cfg.MCPURL, mcpToken: cfg.MCPToken, - tools: cfg.MCPTools, effort: cfg.AnthropicEffort, fetchDomains: cfg.WebFetchDomains, fetchMaxUses: cfg.WebFetchMaxUses} + tools: cfg.MCPTools, effort: cfg.AnthropicEffort, fetchDomains: cfg.WebFetchDomains, fetchMaxUses: cfg.WebFetchMaxUses, + searchMaxUses: cfg.WebSearchMaxUses} } // Prepare has nothing to set up: the prompt and tools travel with every request. @@ -83,6 +89,12 @@ func (a *Anthropic) params(model, task string) anthropic.BetaMessageNewParams { MaxUses: param.NewOpt(int64(a.fetchMaxUses)), MaxContentTokens: param.NewOpt(int64(fetchMaxContentTokens)), }}) + // Web fetch opens only URLs already in the conversation; a search result makes the release + // page of a package whose PR body has no link fetchable. + p.Tools = append(p.Tools, anthropic.BetaToolUnionParam{OfWebSearchTool20250305: &anthropic.BetaWebSearchTool20250305Param{ + AllowedDomains: a.fetchDomains, + MaxUses: param.NewOpt(int64(a.searchMaxUses)), + }}) } if slices.Contains(fallbackModels, model) { p.Fallbacks = anthropic.BetaFallbacksParamUnion{OfDefault: constant.ValueOf[constant.Default]()} @@ -122,11 +134,11 @@ func (a *Anthropic) Ask(ctx context.Context, model, task string) (Answer, error) texts = nil case anthropic.BetaServerToolUseBlock: ans.ToolCalls++ - if string(b.Name) != webFetch || len(a.fetchDomains) == 0 { + if name := string(b.Name); (name != webFetch && name != webSearch) || len(a.fetchDomains) == 0 { ans.Violations = append(ans.Violations, string(b.Name)) } texts = nil - case anthropic.BetaMCPToolResultBlock, anthropic.BetaWebFetchToolResultBlock: + case anthropic.BetaMCPToolResultBlock, anthropic.BetaWebFetchToolResultBlock, anthropic.BetaWebSearchToolResultBlock: texts = nil case anthropic.BetaTextBlock: texts = append(texts, b.Text) diff --git a/review-bot/config.go b/review-bot/config.go index 610722ac..5f33c720 100644 --- a/review-bot/config.go +++ b/review-bot/config.go @@ -40,10 +40,11 @@ type Config struct { MCPURL string MCPToken string // read-only Gitea token Anthropic uses on the MCP server MCPTools []string - // WebFetchDomains turns on Anthropic's server-side web fetch, limited to these domains; empty - // leaves it off. - WebFetchDomains []string - WebFetchMaxUses int + // WebFetchDomains turns on Anthropic's server-side web fetch and web search, limited to these + // domains; empty leaves both off. + WebFetchDomains []string + WebFetchMaxUses int + WebSearchMaxUses int // Selector "typesafe" picks the model per pull request; empty uses the default model. A pick // other than the default below SelectorMinConfidence falls back to the default. @@ -151,14 +152,15 @@ func loadConfig(getenv func(string) string, readFile func(string) ([]byte, error AgentName: text("AGENT_NAME", "pr-review"), ConnectorID: text("CONNECTOR_ID", ""), - AnthropicURL: strings.TrimRight(text("ANTHROPIC_URL", ""), "/"), - AnthropicKey: optional("ANTHROPIC_API_KEY"), - AnthropicEffort: text("ANTHROPIC_EFFORT", "medium"), - MCPURL: text("MCP_URL", "https://gitea-mcp.renner.uno/mcp"), - MCPToken: optional("MCP_TOKEN"), - MCPTools: list("MCP_TOOLS", defaultMCPTools), - WebFetchDomains: list("WEB_FETCH_DOMAINS", nil), - WebFetchMaxUses: number("WEB_FETCH_MAX_USES", 10), + AnthropicURL: strings.TrimRight(text("ANTHROPIC_URL", ""), "/"), + AnthropicKey: optional("ANTHROPIC_API_KEY"), + AnthropicEffort: text("ANTHROPIC_EFFORT", "medium"), + MCPURL: text("MCP_URL", "https://gitea-mcp.renner.uno/mcp"), + MCPToken: optional("MCP_TOKEN"), + MCPTools: list("MCP_TOOLS", defaultMCPTools), + WebFetchDomains: list("WEB_FETCH_DOMAINS", nil), + WebFetchMaxUses: number("WEB_FETCH_MAX_USES", 10), + WebSearchMaxUses: number("WEB_SEARCH_MAX_USES", 5), Selector: text("SELECTOR", ""), SelectorMinConfidence: fraction("SELECTOR_MIN_CONFIDENCE", 0.6), diff --git a/review-bot/fetch_test.go b/review-bot/fetch_test.go index 6f5359ca..2b8b83fe 100644 --- a/review-bot/fetch_test.go +++ b/review-bot/fetch_test.go @@ -52,18 +52,22 @@ const fetchUse = `{"type":"server_tool_use","id":"srvtoolu_1","name":"web_fetch" const fetchResult = `{"type":"web_fetch_tool_result","tool_use_id":"srvtoolu_1","content":{"type":"web_fetch_result",` + `"url":"https://github.com/o/r/releases","content":{"type":"document","source":{"type":"text","media_type":"text/plain","data":"v2: x"}}}}` +const searchUse = `{"type":"server_tool_use","id":"srvtoolu_0","name":"web_search","input":{"query":"chainguard kaniko v1.25.21"}}` +const searchResult = `{"type":"web_search_tool_result","tool_use_id":"srvtoolu_0","content":[{"type":"web_search_result",` + + `"url":"https://github.com/o/r/releases","title":"Releases","encrypted_content":"x","page_age":null}]}` + func TestAnthropicWebFetch(t *testing.T) { f := newFakeAnthropic(t) - f.streams = []string{messageStream("claude-sonnet-5-5", "end_turn", block(0, fetchUse), block(1, fetchResult), - textBlock(2, "## Summary\n\nok"))} + f.streams = []string{messageStream("claude-sonnet-5-5", "end_turn", block(0, searchUse), block(1, searchResult), + block(2, fetchUse), block(3, fetchResult), textBlock(4, "## Summary\n\nok"))} b := f.backend() - b.fetchDomains, b.fetchMaxUses = []string{"github.com", "gitlab.com"}, 10 + b.fetchDomains, b.fetchMaxUses, b.searchMaxUses = []string{"github.com", "gitlab.com"}, 10, 5 a, err := b.Ask(context.Background(), "claude-sonnet-5-5", "task") - if err != nil || a.Text != "## Summary\n\nok" || a.ToolCalls != 1 || len(a.Violations) != 0 { + if err != nil || a.Text != "## Summary\n\nok" || a.ToolCalls != 2 || len(a.Violations) != 0 { t.Fatalf("answer %+v, err %v", a, err) } tools := f.bodies[0]["tools"].([]any) - if len(tools) != 2 { + if len(tools) != 3 { t.Fatalf("tools %v", tools) } wf := tools[1].(map[string]any) @@ -71,10 +75,17 @@ func TestAnthropicWebFetch(t *testing.T) { !reflect.DeepEqual(wf["allowed_domains"], []any{"github.com", "gitlab.com"}) { t.Errorf("web fetch tool: %v", wf) } + ws := tools[2].(map[string]any) + if ws["type"] != "web_search_20250305" || ws["name"] != "web_search" || ws["max_uses"] != float64(5) || + !reflect.DeepEqual(ws["allowed_domains"], []any{"github.com", "gitlab.com"}) { + t.Errorf("web search tool: %v", ws) + } // Off: no web fetch tool, and a server tool call is a violation. f.streams = []string{messageStream("claude-sonnet-5-5", "end_turn", block(0, fetchUse), block(1, fetchResult), textBlock(2, "## Summary\n\nok"))} + f.streams = append(f.streams, messageStream("claude-sonnet-5-5", "end_turn", block(0, searchUse), + block(1, searchResult), textBlock(2, "## Summary\n\nok"))) a, err = f.backend().Ask(context.Background(), "claude-sonnet-5-5", "task") if err != nil || len(a.Violations) != 1 || a.Violations[0] != "web_fetch" { t.Errorf("fetch off: answer %+v, err %v", a, err) @@ -82,4 +93,8 @@ func TestAnthropicWebFetch(t *testing.T) { if n := len(f.bodies[1]["tools"].([]any)); n != 1 { t.Errorf("fetch off sends %d tools, want 1", n) } + a, err = f.backend().Ask(context.Background(), "claude-sonnet-5-5", "task") + if err != nil || len(a.Violations) != 1 || a.Violations[0] != "web_search" { + t.Errorf("search off: answer %+v, err %v", a, err) + } } diff --git a/review-bot/prompt.md b/review-bot/prompt.md index 39368c43..7a51c0c1 100644 --- a/review-bot/prompt.md +++ b/review-bot/prompt.md @@ -41,7 +41,12 @@ instead of starting over, and read only what changed. the sha in your newest marker as base and the head sha as head, and read the diffs of those commits. Do not read the whole pull request diff again, and do not re-read files the new commits do not touch. - If the head sha equals the sha in your newest marker, nothing new was pushed: answer the author's replies and - settle the points they dispute, reading only what that needs. + settle the points they dispute, reading only what that needs. A dependency update is the exception below. +- A dependency update (see "Dependency updates") gets those checks on every review, also when nothing new was + pushed. Your newest earlier review counts for a dependency only if it names that dependency's upstream changes + between the old and the new version, or calls it a digest-only rebuild. Do the checks for every dependency it does + not cover this way, and carry over what it does cover. These checks are not new points on unchanged code: a + breaking change that hits this repository is a 🔴 you missed; say so. - Open the Summary with what changed since that commit and the state of every earlier 🔴 and 🟡 point: fixed, still open, or withdrawn with the reason. An earlier 🔴 that is still open stays in the Blocking section, and the verdict stays `request-changes` until it is fixed. @@ -71,8 +76,11 @@ A pull request by `renovate-bot`, or one whose change is a version or digest bum modules, Helm charts, Actions, packages), gets this review instead of a code-style one. The diff only says that a version changed; the review is about what changed upstream and whether it touches this repository. 1. For each updated dependency, take the release notes between the old and the new version from the PR body - (Renovate puts them under "Release Notes"). If a web fetch tool is available, read the release or compare pages - for versions the body does not cover. It opens only the links listed in the task and links on pages it fetched. + (Renovate puts them under "Release Notes"). If web tools are available, read the release or compare pages for + versions the body does not cover. Web fetch opens only the links listed in the task and links in earlier fetch or + search results. For a dependency without a link, search for its release page first: the package name, or the + upstream project a wrapper image builds, with the new version. Read the notes of every version after the old one + up to the new one. 2. List what in those notes can affect this repository: breaking changes, removals and deprecations, changed defaults, new required settings, fixed CVEs. Then check how this repository uses the dependency: search for the flags, options, APIs, config keys or image entrypoint the notes name, and cite each use with `path:line` at the @@ -81,7 +89,7 @@ version changed; the review is about what changed upstream and whether it touche 3. A digest-only update of an unchanged tag is an upstream rebuild of the same version. Say so in one line per image; there are no release notes to read and nothing to check beyond CI. 4. When no release notes are available for a version change and you cannot read them upstream, say that in one - line in the Summary, with the old and new version. Do not list it as unverified. + line in the Summary, with the old and new version and the pages you tried. Do not list it as unverified. 5. The Summary names every updated dependency with old → new version and the upstream changes that matter, and the CI result of the head. Do not restate the diff ("the bump is mechanical and consistent"): that adds nothing.