fix(review-bot): dependency checks on re-reviews, web search for release notes #1612
@@ -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
|
||||
|
||||
+20
-8
@@ -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)
|
||||
|
||||
+14
-12
@@ -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),
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
+12
-4
@@ -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.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user