From 354d2b18d7faa38118a6c17ec08f2ec62ad06634 Mon Sep 17 00:00:00 2001 From: hermes-bot Date: Mon, 9 Nov 2026 06:40:51 +0000 Subject: [PATCH] feat(review-bot,renovate): useful reviews of dependency updates The review-bot review of konairius/ical-proxy#22 (golang/alpine digests, kaniko v1.25.20 -> v1.25.21) restated the diff and listed "digests and CI not checked" as unverified. Three causes: - Renovate wrote no release notes: it has no github.com token ("No github.com token has been configured. Skipping release notes retrieval", job renovate-29903220). The ExternalSecret now renders RENOVATE_GITHUB_COM_TOKEN (the current name; GITHUB_COM_TOKEN is deprecated) from its own OpenBao key apps/renovate/github, field `token`. No other service reads that key. - defaultMCPTools still named per-method tools that gitea-mcp 1.13 folded into commit_read, branch_read, tag_read and release_read. tools/list on gitea-mcp.renner.uno has none of get_commit, list_commits, compare_commits, list_branches, get_tag, list_tags, get_release, get_latest_release, list_releases, so 9 of the 20 enabled names matched nothing and the prompt's compare_commits/list_commits steps had no tool. The list now has the 1.13 names; a test keeps it read-only. - prompt.md had no guidance for dependency updates. New "Dependency updates" section: release notes between old and new version, the repo's use of what they change (path:line), one line for digest-only rebuilds, no unverified boilerplate. New mandatory check: CI status of the head (pull_request_read get_status; Woodpecker reports commit statuses). --- kubernetes/apps/renovate/cronjob.yaml | 2 +- kubernetes/apps/renovate/externalsecret.yaml | 8 ++++ review-bot/config.go | 11 +++--- review-bot/config_test.go | 10 +++++ review-bot/prompt.md | 39 ++++++++++++++++---- 5 files changed, 57 insertions(+), 13 deletions(-) diff --git a/kubernetes/apps/renovate/cronjob.yaml b/kubernetes/apps/renovate/cronjob.yaml index b08a7e84..666d07dc 100644 --- a/kubernetes/apps/renovate/cronjob.yaml +++ b/kubernetes/apps/renovate/cronjob.yaml @@ -71,7 +71,7 @@ spec: # containerbase all write below $HOME. - name: HOME value: /tmp/home - # RENOVATE_TOKEN and RENOVATE_HOST_RULES. + # RENOVATE_TOKEN, RENOVATE_HOST_RULES and RENOVATE_GITHUB_COM_TOKEN. envFrom: - secretRef: name: renovate-token diff --git a/kubernetes/apps/renovate/externalsecret.yaml b/kubernetes/apps/renovate/externalsecret.yaml index b4ed4152..7690ae90 100644 --- a/kubernetes/apps/renovate/externalsecret.yaml +++ b/kubernetes/apps/renovate/externalsecret.yaml @@ -25,6 +25,9 @@ # RENOVATE_HOST_RULES the same token as registry credentials for # gitea.renner.uno, so Renovate can list the tags of the # in-house CI images (.woodpecker/*.yml). +# and a third from its own key: +# RENOVATE_GITHUB_COM_TOKEN apps/renovate/github `token`: github.com PAT, +# public read only; release notes and API rate limit. # # refreshInterval 1h, as for the other un-rotated keys. The CronJob reads the # Secret at pod start, so a new value is used from the next run. No Reloader @@ -46,8 +49,13 @@ spec: data: RENOVATE_TOKEN: "{{ .token }}" RENOVATE_HOST_RULES: '[{"matchHost":"gitea.renner.uno","hostType":"docker","username":"renovate-bot","password":"{{ .token }}"}]' + RENOVATE_GITHUB_COM_TOKEN: "{{ .githubToken }}" data: - secretKey: token remoteRef: key: infrastructure/gitea/renovate-bot property: token + - secretKey: githubToken + remoteRef: + key: apps/renovate/github + property: token diff --git a/review-bot/config.go b/review-bot/config.go index e2e9add3..50763722 100644 --- a/review-bot/config.go +++ b/review-bot/config.go @@ -8,12 +8,13 @@ import ( "time" ) -// defaultMCPTools are the read tools of the Gitea MCP server that the Anthropic backend enables. +// defaultMCPTools are the read tools of the Gitea MCP server (gitea-mcp 1.13) that the Anthropic +// backend enables. commit_read, branch_read, tag_read and release_read replace the per-method tools +// (list_commits, compare_commits, ...) that gitea-mcp 1.13 folded into them. var defaultMCPTools = []string{ - "get_commit", "get_dir_contents", "get_file_contents", "get_repository_tree", "get_tag", "get_release", - "get_latest_release", "issue_read", "label_read", "list_branches", "list_commits", "list_issues", - "list_pull_requests", "list_releases", "list_tags", "milestone_read", "pull_request_read", "search_issues", - "search_repos", "compare_commits", + "branch_read", "commit_read", "get_dir_contents", "get_file_contents", "get_repository_tree", "issue_read", + "label_read", "list_issues", "list_pull_requests", "milestone_read", "pull_request_read", "release_read", + "search_issues", "search_repos", "tag_read", } // Config is read from the environment. A secret comes from NAME, or from the file named by diff --git a/review-bot/config_test.go b/review-bot/config_test.go index d6b6e414..5ff5e716 100644 --- a/review-bot/config_test.go +++ b/review-bot/config_test.go @@ -2,6 +2,7 @@ package main import ( "errors" + "regexp" "strings" "testing" "time" @@ -104,3 +105,12 @@ func TestParseModelsErrors(t *testing.T) { } } } + +func TestDefaultMCPToolsOnlyRead(t *testing.T) { + read := regexp.MustCompile(`^(get|list|search)_|_read$`) + for _, name := range defaultMCPTools { + if !read.MatchString(name) { + t.Errorf("default MCP tool %q is not a read tool", name) + } + } +} diff --git a/review-bot/prompt.md b/review-bot/prompt.md index 43b31267..e7fccdf1 100644 --- a/review-bot/prompt.md +++ b/review-bot/prompt.md @@ -1,8 +1,9 @@ # PR Review (agent with read-only repository access) You review one pull request. You can read the repository through the Gitea tools attached to this -conversation: pull request details and diff, file contents at any ref, directory listings, the repository tree, -branches, commits, issues and comments. Use them. Do not guess what you can read. +conversation: pull request details and diff, the CI status of the head, file contents at any ref, directory +listings, the repository tree, branches, commits, tags, releases, issues and comments. Use them. Do not guess what +you can read. HARD RULES - READ ONLY. Use only tools that read or search. Never call a tool that creates, edits, deletes, merges, comments, @@ -28,14 +29,16 @@ HARD RULES only when a finding or the change's correctness depends on a convention you have not seen. 5. Check each suspicion against what you read before you write it down. A claim that depends on code you did not read, or on the current behaviour of an external product, goes under "Unverified" with what you checked. Your - training data is not evidence for how current releases behave. + training data is not evidence for how current releases behave. "Unverified" is for a concrete suspicion about + this change that you tried and failed to settle. Leave out what no tool of yours can check at all (that a + registry digest matches its tag, how a CI run will turn out); do not list it as unverified. 6. Before you write "no other code does X", search for it. ## Re-reviews You were asked again, so the author may have acted on your earlier review. Judge the pull request against it instead of starting over, and read only what changed. -- Read your newest earlier review and the author's replies after it. Then call `compare_commits` with 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 +- Read your newest earlier review and the author's replies after it. Then call `commit_read` with method `compare`, + 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. @@ -53,12 +56,34 @@ instead of starting over, and read only what changed. ## Mandatory checks Do these for every PR, in addition to reading the diff: -1. Check `mergeable`, and list the recent commits on the base branch that touch the changed files (`list_commits` - with `path`). Read those files at the base tip only when `mergeable` is false or one of those commits overlaps the +1. Check `mergeable`, and list the recent commits on the base branch that touch the changed files (`commit_read` + with method `list` and `path`). Read those files at the base tip only when `mergeable` is false or one of those commits overlaps the change. State plainly if the change is already on the base branch, superseded by another PR, or conflicts with it. That is a blocking finding. 2. Read every issue the PR links to. 3. Search the repository for other code that does the same thing before you write that nothing else does. +4. Read the CI status of the head (`pull_request_read` with method `get_status`) and state it in the Summary: the + combined state, and every failing or pending context by name. A failure caused by this change is blocking; say + so with the failing context. A failure that the change cannot have caused is not blocking, but name it. + +## Dependency updates +A pull request by `renovate-bot`, or one whose change is a version or digest bump of dependencies (images, Go +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 linked release or compare + pages for versions the body does not cover. +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 + head sha. A breaking change that hits a use you found is blocking. A change you checked and found unused needs + one line, not a finding. +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. +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. ## Efficiency Every tool result stays in the conversation and is paid for again on every later step, so each read costs more the -- 2.54.0