hermes-bot
8f08ef2f1d
review-bot re-reviewed konairius/ical-proxy#22 at an unchanged head (62cf054b) and wrote "I did not fetch the kaniko release notes, so I can't say what changed between v1.25.20 and v1.25.21", with no web call. Two causes: - prompt.md: a re-review with an unchanged head only answers the author's replies, and re-reviews never revisit code already judged, so the "Dependency updates" checks never ran on a PR first reviewed under the old prompt. Re-reviews now run those checks on every review of a dependency update, for each dependency the newest earlier review did not cover (upstream changes old -> new, or a digest-only rebuild), and carry over the ones it did. - No fetchable URL: the PR body has no link for the kaniko image (registry.gitlab.com/gitlab-ci-utils/container-images/kaniko has no source label), and Anthropic's web fetch only opens URLs already in the conversation (user message, client tool results, earlier fetch/search results; not the system prompt or MCP results). Following links from a fetched page is unreliable because page-to-text conversion drops relative links. The Anthropic backend now also offers the web search server tool (web_search_20250305, basic, no code execution) on the same WEB_FETCH_DOMAINS, max_uses WEB_SEARCH_MAX_USES (default 5); search results are a valid fetch source. The prompt tells the agent to search for a dependency without a link (package name or the upstream a wrapper image builds, plus the new version). Web search is enabled per org by default in the Claude Console; no manual step. - Tool guard: web_search is allowed only while web tools are on, like web_fetch; otherwise the review is not posted. The renovate-config sourceUrl rule for this image (renovate-config #4) puts notes into the PR body from the next scheduled Renovate run; this covers the PRs that already exist and any other package without a source.
101 lines
4.8 KiB
Go
101 lines
4.8 KiB
Go
package main
|
|
|
|
import (
|
|
"context"
|
|
"reflect"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
func TestFetchLinksKeepsOnlyAllowedHosts(t *testing.T) {
|
|
body := "| [kaniko](https://github.com/chainguard-forks/kaniko) | `v1.25.20` → `v1.25.21` |\n" +
|
|
"[Compare Source](https://github.com/chainguard-forks/kaniko/compare/v1.25.20...v1.25.21).\n" +
|
|
"see https://github.com/chainguard-forks/kaniko again, https://hub.docker.com/_/golang, " +
|
|
"https://evil.example/github.com/x, https://user:pw@github.com/x, http://github.com/plain " +
|
|
"https://gitlab.com/gitlab-ci-utils/container-images/kaniko/-/releases and https://sub.gitlab.com/a."
|
|
got := fetchLinks(body, []string{"github.com", "gitlab.com"})
|
|
want := []string{
|
|
"https://github.com/chainguard-forks/kaniko",
|
|
"https://github.com/chainguard-forks/kaniko/compare/v1.25.20...v1.25.21",
|
|
"https://gitlab.com/gitlab-ci-utils/container-images/kaniko/-/releases",
|
|
"https://sub.gitlab.com/a",
|
|
}
|
|
if !reflect.DeepEqual(got, want) {
|
|
t.Errorf("links\n got %q\nwant %q", got, want)
|
|
}
|
|
var many strings.Builder
|
|
for i := range 30 {
|
|
many.WriteString("https://github.com/o/r/pull/" + itoa(i) + " ")
|
|
}
|
|
if n := len(fetchLinks(many.String(), []string{"github.com"})); n != maxFetchLinks {
|
|
t.Errorf("links %d, want the cap %d", n, maxFetchLinks)
|
|
}
|
|
}
|
|
|
|
func TestReviewTaskListsLinksOnlyWithFetchOn(t *testing.T) {
|
|
pr := PR{Body: "notes: https://github.com/o/r/releases/tag/v2"}
|
|
pr.Head.SHA = "abc"
|
|
plain := "Review pull request #7 of konairius/homelab. Head commit: abc."
|
|
if got := reviewTask("konairius/homelab", 7, pr, nil); got != plain {
|
|
t.Errorf("fetch off: %q", got)
|
|
}
|
|
got := reviewTask("konairius/homelab", 7, pr, []string{"github.com"})
|
|
if !strings.HasPrefix(got, plain) || !strings.Contains(got, "\n- https://github.com/o/r/releases/tag/v2") {
|
|
t.Errorf("fetch on: %q", got)
|
|
}
|
|
if got := reviewTask("konairius/homelab", 7, PR{Body: "no links"}, []string{"github.com"}); strings.Contains(got, "Links") {
|
|
t.Errorf("no links, no list: %q", got)
|
|
}
|
|
}
|
|
|
|
const fetchUse = `{"type":"server_tool_use","id":"srvtoolu_1","name":"web_fetch","input":{"url":"https://github.com/o/r/releases"}}`
|
|
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, searchUse), block(1, searchResult),
|
|
block(2, fetchUse), block(3, fetchResult), textBlock(4, "## Summary\n\nok"))}
|
|
b := f.backend()
|
|
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 != 2 || len(a.Violations) != 0 {
|
|
t.Fatalf("answer %+v, err %v", a, err)
|
|
}
|
|
tools := f.bodies[0]["tools"].([]any)
|
|
if len(tools) != 3 {
|
|
t.Fatalf("tools %v", tools)
|
|
}
|
|
wf := tools[1].(map[string]any)
|
|
if wf["type"] != "web_fetch_20250910" || wf["name"] != "web_fetch" || wf["max_uses"] != float64(10) ||
|
|
!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)
|
|
}
|
|
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)
|
|
}
|
|
}
|