feat(pr-review): run the review through a listed Mistral agent #1251

2026-10-11T13:53:53Z

claude-bot commented

2026-10-11 13:53:53 +00:00

The review prompt, model (mistral-large-4-0) and the read-only Gitea connector now live in a listed Mistral agent, pr-review (ag_01a12b392002750b8ad2b4692a9de03c). mistral-agent-sync.sh creates or updates it from ci/agents/pr-review/mistral-prompt.md and stamps the prompt hash into the agent description, so git stays the source of truth. mistral-review.sh starts a conversation with the agent id and a one-line task, warns if the agent was not built from the repo prompt, and writes the agent's model name for the review header.

The prompt gains mandatory checks (mergeable, base-branch state, recent commits on the base, linked issues, search before claiming that nothing else does X) and an instruction to batch independent reads. An earlier variant that capped tool calls was faster but missed that #1243 was already superseded by #1245; this one finds it.

Measured, same PRs:

#1250: old call about 6.7 min in the pipeline; agent 1.5 to 2.3 min.
#1243: old call about 3.5 min; agent 1.1 to 1.7 min, and it names the superseding PR and the revert of the newer -H fix.
shelly-manager #106: agent 4.5 min, 43 calls; it finds the unmigrated Mqtt.razor and the second unfiltered reader.

Model comparison, same prompt and connector, four PRs with known main findings (#1243 superseded, #1250 rm -rf, shelly #106 Mqtt.razor, #105 missing @key). Crude keyword screen, one run each:

mistral-large-4-0: 4 of 4 found.
mistral-large-2512: 2 of 4. Small 4: 1 of 4. Medium 3.5: 1 of 4. Magistral Medium: 1 of 4. Ministral 14B: 0 of 4.
The others are faster (10 to 145 s) but three of them answered Looks reasonable on #106 or #105, which the run.sh rule would turn into APPROVE. Large 4 stays.

After merge, run mistral-agent-sync.sh whenever the prompt, model or connector changes; review runs warn when the agent has drifted.

🤖 Generated with Claude Code

The review prompt, model (mistral-large-4-0) and the read-only Gitea connector now live in a listed Mistral agent, pr-review (ag_01a12b392002750b8ad2b4692a9de03c). mistral-agent-sync.sh creates or updates it from ci/agents/pr-review/mistral-prompt.md and stamps the prompt hash into the agent description, so git stays the source of truth. mistral-review.sh starts a conversation with the agent id and a one-line task, warns if the agent was not built from the repo prompt, and writes the agent's model name for the review header. The prompt gains mandatory checks (mergeable, base-branch state, recent commits on the base, linked issues, search before claiming that nothing else does X) and an instruction to batch independent reads. An earlier variant that capped tool calls was faster but missed that #1243 was already superseded by #1245; this one finds it. Measured, same PRs: - #1250: old call about 6.7 min in the pipeline; agent 1.5 to 2.3 min. - #1243: old call about 3.5 min; agent 1.1 to 1.7 min, and it names the superseding PR and the revert of the newer -H fix. - shelly-manager #106: agent 4.5 min, 43 calls; it finds the unmigrated Mqtt.razor and the second unfiltered reader. Model comparison, same prompt and connector, four PRs with known main findings (#1243 superseded, #1250 rm -rf, shelly #106 Mqtt.razor, #105 missing @key). Crude keyword screen, one run each: - mistral-large-4-0: 4 of 4 found. - mistral-large-2512: 2 of 4. Small 4: 1 of 4. Medium 3.5: 1 of 4. Magistral Medium: 1 of 4. Ministral 14B: 0 of 4. - The others are faster (10 to 145 s) but three of them answered Looks reasonable on #106 or #105, which the run.sh rule would turn into APPROVE. Large 4 stays. After merge, run mistral-agent-sync.sh whenever the prompt, model or connector changes; review runs warn when the agent has drifted. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit 2026-10-11 13:53:54 +00:00

feat(pr-review): run the review through a listed Mistral agent

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

27b7ecda61

The prompt, model and read-only Gitea connector now live in the Mistral agent "pr-review", kept in sync from git by mistral-agent-sync.sh (which also stamps the prompt hash into the agent description). mistral-review.sh starts a conversation with the agent id and a one-line task instead of sending the whole prompt each time, warns when the agent was not built from this prompt, and passes the model name on for the review header. The prompt gains mandatory checks (mergeable, base-branch state, linked issues, search before claiming nothing else does X) and an instruction to batch independent reads.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

review-bot requested changes

2026-10-11 14:01:51 +00:00

review-bot left a comment

🤖 Automated review — model mistral-large-4-0, commit 27b7ecda61edd550732a968d2e5ce7013190ff41.

Automated review by the review-bot pipeline: it approves only when it found nothing red or yellow, and a human still merges. Push new commits to get a fresh review.

Summary

PR #1251 moves the pr-review Mistral review from a per-call inline prompt to a listed agent "pr-review" (ag_01a12b…): new ci/agents/pr-review/mistral-agent-sync.sh creates/updates the agent from mistral-prompt.md and stamps a prompt hash in its description; mistral-review.sh now starts a conversation with the agent id, warns on prompt drift, and records the model name for the review header; run.sh picks that model name up; the prompt gains mandatory checks and a batching instruction. Base is master @ a71da1e, which is still the branch tip, the PR is mergeable, and it is neither already on base nor superseded (the only other open PR, #1243, is an unrelated alert fix); it builds directly on the merged #1249. No secrets are added — the API key stays env-only and is passed to curl via a config file. The flow is coherent and keeps the read-only tool guard, but the new sync script swallows API failures and can create duplicate agents.

🔴 Blocking

ci/agents/pr-review/mistral-agent-sync.sh:41-47 — the create-or-update decision pipes api into jq, which masks any failure of the list call. api returns 1 on HTTP >= 300 (line 29) with nothing on stdout; jq on empty input exits 0, so the pipeline and the ID=$(…) assignment succeed, ID is empty, and the script falls into the POST branch and creates a second "pr-review" agent while printing created … and exiting 0. A curl network failure is equally swallowed (http_code 000 also passes the [ "$code" -ge 300 ] test at line 29). The same masking at line 46 means a failed POST yields an empty ID, still prints created …, and exits 0 with an empty agent id on stdout. Any transient failure of GET /agents (5xx, rate limit, network blip) therefore breaks the script's documented contract ("Create or update the listed agent … so the agent definition follows git"): duplicates accumulate, and later runs match the first same-named agent ([.[]? | select(.name == $n) | .id][0], line 41), which may be the stale duplicate, so PATCHes update the wrong agent while the pipeline reviews with the hardcoded id. Fix by not piping the fallible call into jq, e.g. if ! LIST=$(api GET "/agents?page_size=100"); then exit 1; fi; ID=$(printf '%s' "$LIST" | jq …), and add [ -n "$ID" ] || exit 1 after the POST.

⚪ Unverified

The Mistral API shapes the new code assumes: GET /v1/agents returning a top-level JSON array (mistral-agent-sync.sh:41), GET /v1/agents/{id} returning {model, description} (mistral-review.sh:24-27), and POST /v1/conversations accepting {agent_id, inputs, store} (mistral-review.sh:33-34). I cannot call the Mistral API from here; the PR body reports measured successful runs, which is consistent but not proof. If the list endpoint actually returns an envelope object (e.g. {data: [...]}), the name lookup never matches and every sync run would POST a duplicate — worth confirming against the live API.
The PR body's benchmark claims (review timings, the 4/4 vs 2/4 model comparison, the #1243/#1245 supersession anecdote) are not reproducible from the repository; treated as untrusted context.

🟡 Suggestions

The prompt-drift check (mistral-review.sh:27-28) only warns; a drifted agent still produces reviews, and run.sh:169-176 turns a "Looks reasonable" report into APPROVE. Since the agent's prompt is the rubric the pipeline approves on, consider failing (or at least blocking the APPROVE path) when the agent's prompt hash does not match mistral-prompt.md.
The pipeline relies on the hardcoded default agent id (mistral-review.sh:14; the .woodpecker/pr-review.yml stub sets only MISTRAL_API_KEY/REVIEW_TOKEN). If the agent is ever recreated in Mistral Studio the id changes and every review fails until the constant is updated; resolving the agent by name (as the sync script does) or documenting the coupling would be more robust.
mistral-review.sh:21 writes the bearer-key file .curl-auth and only removes it at line 43; any earlier set -e exit (e.g. a jq failure at line 25/33) leaves the key on disk in /tmp/pr-review/$NUM until the next run's rm -rf (run.sh:94). Pre-existing pattern and low risk (umask 077, temp dir), but a trap 'rm -f "$D/.curl-auth"' EXIT would harden it.

🟢 Nits

mistral-review.sh:25: jq -r '.model' writes the literal null into model.txt when the agent object lacks a model field; run.sh:157 then logs "Review model: null". jq -r '.model // empty' would let run.sh's "Mistral agent" fallback apply.
mistral-review.sh:38-39: on a curl network failure code becomes 000\n000 (curl's -w output plus || echo 000), so the retry log shows "HTTP 000 000". Pre-existing, cosmetic.

Tests

The repo gates pr-review helper scripts with an offline test on every PR (.woodpecker/ci-scripts-test.yml step pr-review-mistral-tests → scripts/ci/test-pr-review-mistral.sh). That test covers only clean-report.sh and parse-response.sh, which this PR does not change, so it still passes; none of the new logic is covered — the sync script's create-or-update decision (where the blocking bug lives), the prompt-hash drift check, and the model.txt extraction. Extend test-pr-review-mistral.sh with a stubbed api/curl: list-failure must not POST, list-hit must PATCH not POST, POST-failure must exit non-zero with no id, and a hash mismatch must print the drift warning.

Recommendation

Request changes — 1 blocking issue.

🤖 **Automated review** — model `mistral-large-4-0`, commit `27b7ecda61edd550732a968d2e5ce7013190ff41`. > Automated review by the `review-bot` pipeline: it approves only when it found nothing red or yellow, and a human still merges. Push new commits to get a fresh review. --- ## Summary PR #1251 moves the pr-review Mistral review from a per-call inline prompt to a listed agent "pr-review" (`ag_01a12b…`): new `ci/agents/pr-review/mistral-agent-sync.sh` creates/updates the agent from `mistral-prompt.md` and stamps a prompt hash in its description; `mistral-review.sh` now starts a conversation with the agent id, warns on prompt drift, and records the model name for the review header; `run.sh` picks that model name up; the prompt gains mandatory checks and a batching instruction. Base is `master` @ `a71da1e`, which is still the branch tip, the PR is mergeable, and it is neither already on base nor superseded (the only other open PR, #1243, is an unrelated alert fix); it builds directly on the merged #1249. No secrets are added — the API key stays env-only and is passed to curl via a config file. The flow is coherent and keeps the read-only tool guard, but the new sync script swallows API failures and can create duplicate agents. ## 🔴 Blocking - `ci/agents/pr-review/mistral-agent-sync.sh:41-47` — the create-or-update decision pipes `api` into `jq`, which masks any failure of the list call. `api` returns 1 on HTTP >= 300 (line 29) with nothing on stdout; `jq` on empty input exits 0, so the pipeline and the `ID=$(…)` assignment succeed, `ID` is empty, and the script falls into the POST branch and **creates a second "pr-review" agent** while printing `created …` and exiting 0. A curl network failure is equally swallowed (http_code `000` also passes the `[ "$code" -ge 300 ]` test at line 29). The same masking at line 46 means a failed POST yields an empty `ID`, still prints `created …`, and exits 0 with an empty agent id on stdout. Any transient failure of `GET /agents` (5xx, rate limit, network blip) therefore breaks the script's documented contract ("Create or update the listed agent … so the agent definition follows git"): duplicates accumulate, and later runs match the *first* same-named agent (`[.[]? | select(.name == $n) | .id][0]`, line 41), which may be the stale duplicate, so PATCHes update the wrong agent while the pipeline reviews with the hardcoded id. Fix by not piping the fallible call into `jq`, e.g. `if ! LIST=$(api GET "/agents?page_size=100"); then exit 1; fi; ID=$(printf '%s' "$LIST" | jq …)`, and add `[ -n "$ID" ] || exit 1` after the POST. ## ⚪ Unverified - The Mistral API shapes the new code assumes: `GET /v1/agents` returning a top-level JSON array (`mistral-agent-sync.sh:41`), `GET /v1/agents/{id}` returning `{model, description}` (`mistral-review.sh:24-27`), and `POST /v1/conversations` accepting `{agent_id, inputs, store}` (`mistral-review.sh:33-34`). I cannot call the Mistral API from here; the PR body reports measured successful runs, which is consistent but not proof. If the list endpoint actually returns an envelope object (e.g. `{data: [...]}`), the name lookup never matches and *every* sync run would POST a duplicate — worth confirming against the live API. - The PR body's benchmark claims (review timings, the 4/4 vs 2/4 model comparison, the #1243/#1245 supersession anecdote) are not reproducible from the repository; treated as untrusted context. ## 🟡 Suggestions - The prompt-drift check (`mistral-review.sh:27-28`) only warns; a drifted agent still produces reviews, and `run.sh:169-176` turns a "Looks reasonable" report into APPROVE. Since the agent's prompt *is* the rubric the pipeline approves on, consider failing (or at least blocking the APPROVE path) when the agent's prompt hash does not match `mistral-prompt.md`. - The pipeline relies on the hardcoded default agent id (`mistral-review.sh:14`; the `.woodpecker/pr-review.yml` stub sets only `MISTRAL_API_KEY`/`REVIEW_TOKEN`). If the agent is ever recreated in Mistral Studio the id changes and every review fails until the constant is updated; resolving the agent by name (as the sync script does) or documenting the coupling would be more robust. - `mistral-review.sh:21` writes the bearer-key file `.curl-auth` and only removes it at line 43; any earlier `set -e` exit (e.g. a jq failure at line 25/33) leaves the key on disk in `/tmp/pr-review/$NUM` until the next run's `rm -rf` (`run.sh:94`). Pre-existing pattern and low risk (umask 077, temp dir), but a `trap 'rm -f "$D/.curl-auth"' EXIT` would harden it. ## 🟢 Nits - `mistral-review.sh:25`: `jq -r '.model'` writes the literal `null` into `model.txt` when the agent object lacks a model field; `run.sh:157` then logs "Review model: null". `jq -r '.model // empty'` would let run.sh's "Mistral agent" fallback apply. - `mistral-review.sh:38-39`: on a curl network failure `code` becomes `000\n000` (curl's `-w` output plus `|| echo 000`), so the retry log shows "HTTP 000 000". Pre-existing, cosmetic. ## Tests The repo gates pr-review helper scripts with an offline test on every PR (`.woodpecker/ci-scripts-test.yml` step `pr-review-mistral-tests` → `scripts/ci/test-pr-review-mistral.sh`). That test covers only `clean-report.sh` and `parse-response.sh`, which this PR does not change, so it still passes; none of the new logic is covered — the sync script's create-or-update decision (where the blocking bug lives), the prompt-hash drift check, and the `model.txt` extraction. Extend `test-pr-review-mistral.sh` with a stubbed `api`/curl: list-failure must not POST, list-hit must PATCH not POST, POST-failure must exit non-zero with no id, and a hash mismatch must print the drift warning. ## Recommendation Request changes — 1 blocking issue.

claude-bot referenced this pull request

2026-10-11 16:39:52 +00:00

feat(pr-review): review by reconciliation from one cron, without per-repo stubs #1252

claude-bot commented

2026-10-11 18:57:40 +00:00