Skip to content

Latest commit

 

History

21 Commits

Folders and files

NameName
Last commit message
Last commit date
 
 
 
 

Repository files navigation

pr-agent-settings

Table of Contents

Central PR-Agent configuration for the cuioss organization.

Purpose

PR-Agent is the third automated reviewer in this organization, running on Google Gemini. It stands beside — and does not replace — the existing two:

Reviewer Central config Role

CodeRabbit

cuioss/coderabbit (.coderabbit.yaml)

General review, nitpicks, committable suggestions

Sourcery

Dashboard only (app.sourcery.ai)

Refactoring and code-quality review

PR-Agent

This repository (.pr_agent.toml)

Security-weighted review — see Charter: why security-weighted

Adoption is opt-in per repository: a repo joins by adding a small caller workflow (Adding a repository). Repositories without that workflow are untouched.

What this repository is

PR-Agent reads organization-level settings from a repository named exactly pr-agent-settings, from the file .pr_agent.toml at the root of its default branch. The name and location are hard-coded in PR-Agent’s discovery — this repository exists solely to be found by that lookup, exactly as cuioss/coderabbit exists to be found by CodeRabbit’s.

There is no code here. Two files matter:

  • .pr_agent.toml — the active configuration, with the rationale for each setting inline.

  • README.adoc — this document: how the setup works, and the non-obvious mechanics that are easy to get wrong.

Which PR-Agent

Three different products share the name, and only one of them can run on our own Gemini key.

Product Bring-your-own model Infrastructure Verdict

Open-source PR-Agent, GitHub Action

yes

none

What we run. Upstream is The-PR-Agent/pr-agent — the community continuation of the project after the original vendor moved on. Its own description states it is not the Qodo free tier.

Open-source PR-Agent, self-hosted GitHub App

yes

always-on server + webhook endpoint

Rejected: no benefit over the Action for our usage, and it needs hosting.

Qodo Merge (hosted SaaS)

no on the free/basic tier

none

Rejected: the provider is not ours to choose, which fails the requirement outright.

The consequence of running the Action rather than the App is larger than it looks — see ignore_pr_* is dead config in GitHub Action mode.

Configuration precedence

Lowest to highest priority:

  1. Built-in defaults.

  2. extra_config_url (an external configuration URL; unused here).

  3. This repository — the organization-level global file.

  4. A repository-local .pr_agent.toml.

  5. A repository wiki .pr_agent.toml page.

  6. Environment variables.

Two properties are better than the CodeRabbit equivalent and worth relying on:

  • Merge, not override. The global file is merged beneath the repo-local one. A repository that overrides a single key keeps everything else from here. A repository-level .coderabbit.yaml, by contrast, fully replaces the central file unless it opts into inheritance.

  • No cache lag. A CI run is a short-lived process and fetches the global settings once per invocation, so a change here is live on the next pull request. (A long-running webhook deployment caches them for up to 15 minutes — not our case.)

A repository can opt out of this file entirely with use_global_settings_file = false in its own .pr_agent.toml.

Warning

Environment variables outrank this file. Every upstream example sets config.model in the workflow env: block — doing that in a caller workflow would silently decentralize the most important setting. Model and tool tuning belong here; the caller workflow passes only the credentials and the three tool toggles (see Tools).

Learnings

Each item below cost real investigation to establish, and several were established only after getting them wrong first. They are recorded here so nobody has to re-derive them.

A theme runs through most of them, and it is the most useful thing on this page: this setup’s characteristic failure is a confident signal that hides a caveat, not an error. A clean review indistinguishable from total failure; a charter that could only ever return empty; a clipped diff reviewed as though complete; a dead setting that appears in every run’s resolved-config dump; two reviewers agreeing on a clean result because neither could see the file that refutes the change. Each looked healthy from the outside. When something here seems fine, prefer checking the mechanism over trusting the appearance.

The reviewer token must be able to read two repositories

PR-Agent skips the global configuration — silently, with no error — if the token it runs under cannot read both the pull request’s repository and pr-agent-settings.

The reviewer runs under a token minted from the cuioss-review-bot GitHub App, and actions/create-github-app-token scopes its token to the current repository only by default. The reusable workflow therefore requests both explicitly:

with:
  app-id: ${{ secrets.REVIEW_APP_ID }}
  private-key: ${{ secrets.REVIEW_APP_PRIVATE_KEY }}
  repositories: ${{ github.event.repository.name }},pr-agent-settings
  owner: cuioss

The App must also be installed on pr-agent-settings; an org-wide installation covers this. Because the failure mode is silent, verify positively — change a visible key here and observe the review change — rather than by absence of an error in the log.

ignore_pr_* is dead config in GitHub Action mode

The most important thing in this document.

PR-Agent’s documented skip options — ignore_pr_labels, ignore_pr_authors, ignore_pr_title, ignore_pr_source_branches, ignore_pr_target_branches — are read by a single function, should_process_pr_logic(). That function exists in the webhook servers (servers/github_app.py, servers/gitlab_webhook.py, servers/gitea_app.py, servers/bitbucket_*). It does not exist in servers/github_action_runner.py, which goes straight from applying repository settings to running the tools.

So in GitHub Action mode those five keys do nothing. Putting the org skip rules in this file would look correct, pass review, and review every pull request anyway — a false green.

Where the skip rules actually live: job-level if: guards in cuioss/cuioss-organization/.github/workflows/reusable-pr-agent-review.yml. Because that is a reusable workflow, the rules remain centralized — one file to edit, same as this one.

jobs:
  review:
    if: >-
      github.event.pull_request.user.login != 'dependabot[bot]' &&
      github.event.pull_request.user.login != 'cuioss-release-bot[bot]' &&
      !contains(github.event.pull_request.labels.*.name, 'skip-bot-review')

This is also strictly cheaper than the webhook mechanism it replaces: a guarded-out pull request consumes no runner minutes and no tokens at all, whereas a webhook-mode ignore_* still starts the process before deciding to stop.

Pinning the action does not pin the code

PR-Agent’s action.yaml is a Docker action, and its Dockerfile is one line:

FROM pragent/pr-agent:github_action

That is a floating tag, in the former vendor’s Docker Hub namespace. Pinning uses: The-PR-Agent/pr-agent@<sha> pins the wrapper while the executed code remains whatever that tag points at today — which defeats the org’s pinned-dependency posture and its Scorecards score.

The reusable workflow therefore pins the image digest directly:

uses: docker://pragent/pr-agent@sha256:<digest>

Trade-off: the docker:// form does not accept the action’s artifact_path / artifact_instructions inputs. Those are only sugar for the ARTIFACT_PATH / ARTIFACT_INSTRUCTIONS environment variables and can be set directly if ever needed.

Digest bumps are a deliberate, reviewable step. Given that this is a community continuation of an abandoned upstream, distributed through the previous vendor’s registry namespace, digest pinning is what makes the dependency acceptable rather than merely convenient.

The reviewer needs its own identity

Run with the default GITHUB_TOKEN, the reviewer posts as github-actions[bot] — the same author as every other workflow comment in the repository. Downstream automation (plan-marshall’s automatic-review pipeline) attributes findings by author login, so that would mis-attribute unrelated workflow comments as review findings and make per-reviewer review metrics meaningless.

Hence the dedicated cuioss-review-bot GitHub App, which gives the reviewer a unique author login and keeps the downstream integration a pure data edit with no per-bot special-casing.

cuioss-release-bot was deliberately not reused: it is itself on the reviewer skip list, so sharing the identity would entangle "who authored this pull request" with "who reviewed it".

Required App permissions: Pull requests read/write, Issues read/write, Contents read-only, Metadata read-only. No ruleset bypass — the reviewer never pushes.

The upstream model documentation is stale

The upstream changing_a_model.md page lists only gemini/gemini-1.5-flash for Google AI Studio. The authoritative list is the MAX_TOKENS registry in pr_agent/algo/init.py, which registers gemini-2.5-flash, gemini-2.5-pro, and the gemini-3--preview, gemini-3.1- and gemini-3.5-* families under both the gemini/ and vertex_ai/ prefixes, all at 1M context.

The registry stops at the 3.5 pair. Anything newer is absent, and an unregistered model does not degrade gracefully: get_max_tokens() in algo/utils.py raises unless config.custom_model_max_tokens is positive. Check the id against that table before configuring it, and pair a newer model with custom_model_max_tokens. (An earlier revision of this document asserted gemini-3.6-flash was registered. It is not; the claim was never checked against the table.)

That correction is why the current ids were read out of the pinned image rather than assumed: neither gemini-3.7-flash nor gemini-3.6-flash is registered — the leader and the first fallback are both absent. One custom_model_max_tokens line therefore covers two rungs, and falling back does not make the ladder safe on its own.

Being registered in MAX_TOKENS does not mean the project can serve it, and this held on both backends:

  • AI Studio: gemini/gemini-3.5-pro404 models/gemini-3.5-pro is not found for API version v1alpha.

  • Vertex AI: vertex_ai/gemini-3.5-pro404 Publisher model … was not found or your project does not have access to it, while gemini-3.5-flash serves reviews normally.

That 404 is ambiguous by construction — one message covers "not entitled", "not offered in this region", and "wrong id". Rather than resolve it by hand, a self-selecting ladder was run once: lead with the strongest candidate, let LiteLLM fall through, and read which rung answered. The result, from plan-marshall#1031:

Model Outcome

gemini-3.5-pro

404 — not available to this project

gemini-3.1-pro

404 — not available to this project

gemini-2.5-pro

429 throttled, retried, then served"model": "gemini-2.5-pro" in the response

One run settled what three console visits had not. Both 404 rungs were then removed: they cost roughly eight wasted calls and a minute per review to re-learn a fixed fact.

The 3.x Pro tier is not available to this project at all. What the 3 series does offer here is the flash line, led by gemini-3.7-flash.

That leader was checked against this project before it was promoted, not inferred from the model being generally available — this whole section exists because ids the pinned image itself registers still 404’d here. A direct `generateContent call to projects/<this project>/locations/global/publishers/google/models/gemini-3.7-flash answered HTTP 200, with "modelVersion": "gemini-3.7-flash" in the response. The same probe served 3.6-flash and 3.5-flash, so the head of the ladder is entitlement-checked rather than assumed. Do this before promoting the next leader too; it costs one call.

The ladder is ordered newest generation first, and that takes two separate arguments — one for preferring a newer flash to an older one, one for preferring any flash to pro.

Newer flash first. Google’s own Gemini 3.7 Flash model evaluation puts 3.7 ahead of 3.6 on every coding and agentic benchmark it reports, including the long-context row that governs how much of a large diff survives the read:

Benchmark 3.7 Flash 3.6 Flash

FrontierCode 1.1 Main

43.6%

34.4%

DeepSWE v1.1

65.3%

48.6%

Code Arena (Elo)

1588

1536

Terminal-Bench 2.1

85.8%

78.0%

Terminal-Bench 3.0

14.9%

5.4%

GDM-MRCR v2 (8-needle), 128k average

97.0%

91.8%

It is not a clean sweep, and the exception is recorded rather than dropped: 3.6 keeps CharXiv Reasoning both ways (85.2 vs 84.5 without tools, 89.4 vs 88.7 with), which is chart comprehension, not this workload.

The promotion is elective and cost-neutral. gemini-3.6-flash is not deprecated and carries no retirement date, and the introductory price of $0.75 / $3.75 per 1M input / output tokens covers both models through 2026-12-31, after which both move to $1.50 / $7.50. Nothing forced this change; it is an upgrade taken on the table above.

So 3.6-flash is demoted, not dropped. It is the first fallback because it is what served every review before the promotion: a 404 or a 400 on the new leader costs one wasted call and lands on the model that worked yesterday. Leaving it out would have made the first fallback a two-generation drop.

Any flash before pro. This is not the intuitive "pro outranks flash" ordering. Google’s published head-to-head has 3.6-flash beating 3.1 Pro on every coding and agentic benchmark:

Benchmark 3.6 Flash 3.1 Pro

SWE-Bench Pro

58.7%

54.2%

DeepSWE v1.1

49%

12%

Terminal-Bench 2.1

78.0%

73.8%

MLE-Bench

63.9%

42.6%

Pro retains narrow leads only on pure reasoning (GPQA Diamond 94.3 vs 92.8, Humanity’s Last Exam 44.4 vs 38.3) — not this workload, and flash also avoids the 429 backoff that gemini-2.5-pro incurs. gemini-2.5-pro is a generation older still, so placing it ahead of a 3-series flash would have made a fallback a downgrade. It sits at the back.

Note

Do not "promote to pro" to fix thin reviews. It was proposed here once and was wrong twice over: it contradicts the benchmarks above, and review depth turned out to be governed by the charter, not the model — see A charter written against noise produces silence.

Do not reinstate a 3.x pro id without first confirming the Model Garden card offers it, the way the current leader was confirmed above. The 404 is ambiguous enough to read as a transient fault rather than a fixed fact.

The floor of the ladder is what served before it existed, so the worst case is the previous behaviour plus a few seconds. Before the gate below existed an unavailable model cost one silent, review-less run per pull request (see PR-Agent cannot report its own failure) — it now fails loudly instead, which is what makes leading with the optimistic choice safe.

The context window caps what the reviewer sees

max_model_tokens is set to 256000. Upstream defaults it to 32000, and with large_patch_policy = "clip" any diff above the limit is truncated before the model reads it.

This is the half of a model upgrade that is easy to miss: promoting to a larger-context model while leaving the cap at 32000 buys the stronger model and still feeds it a truncated diff.

The headroom is not theoretical. A 19-file pull request logged Tokens: 57449, total tokens under limit: 128000, returning full diff. — nearly twice the upstream default, and it would have been silently clipped under it. The two other reviewers both refused that same diff outright (one on a 150000-character limit, one on a rate limit), which is the case for keeping this reviewer’s own limits generous: it is sometimes the only one that reads the change.

256000 is sized on the largest diff actually observed (168188 tokens, see below) with a 2x margin, so a normal security change in this organization fits inside the window. It remains a quarter of the 1M ceiling, because an unbounded window turns one pathological pull request into a pathological bill.

Gemini 3 inverts the temperature advice, and reasoning_effort never reaches it

Two model-parameter facts, both of which contradict what a reader would reasonably assume.

Temperature must be 1.0, not lower. PR-Agent defaults config.temperature to 0.2, which was sound for the 2.5 generation: lower temperature bought determinism, and a reviewer wants reproducible output. Google’s Gemini 3 guidance reverses it —

For all Gemini 3 models, we strongly recommend keeping the temperature parameter at its default value of 1.0. […] lowering it may lead to unexpected behavior, such as looping or degraded performance, particularly in complex mathematical or reasoning tasks.

— and the migration section lists "remove explicit temperature parameters, especially low values" as an action item when moving off 2.5. Gemini 3’s reasoning is tuned for the default.

This was reaching the model, not sitting inert. In the pinned image configuration.toml:53 defaults temperature to 0.2, pr_reviewer.py:233 passes get_settings().config.temperature into chat_completion(), and litellm_ai_handler.py:531-533 sets kwargs["temperature"] for every model not in NO_SUPPORT_TEMPERATURE_MODELS — deepseek-reasoner, the o1/o3/o4 family, gpt-5.x-codex, gpt-5-mini and claude-opus-4-7, with no Gemini entry. Every review this organization ran before that line was added went out at 0.2.

Google’s two pages now disagree, and the newer one does not reach the older one. "What’s new in Gemini 3.7 Flash" lists temperature, top_p, top_k and candidate_count among deprecated parameters to remove on migration. The Gemini 3 developer guide quoted above — which does not mention 3.7 anywhere — still says to keep temperature at 1.0, and nothing on it marks it superseded. A reader arriving from either page has no way to learn that the other exists.

That was settled by asking the endpoint instead of choosing a page. gemini-3.7-flash accepts the parameter: generateContent returned HTTP 200 at temperature 1.0 and again at 0.2, and 200 once more with top_p, top_k and candidate_count all set alongside it. The parameter is still range-validated there — 3.0 and -1.0 each return HTTP 400, "the supported range is from 0 (inclusive) to 2.0001 (exclusive)". "Deprecated" on that page therefore means advice, not rejection.

That 200 is not a vacuous pass. The same endpoint on the same model does reject an unsupported generationConfig value — thinkingLevel: MINIMAL returns HTTP 400 Thinking level is unsupported: THINKING_LEVEL_MINIMAL — so accepting temperature is a real accept and not blanket tolerance of anything sent.

So the key stays, at 1.0. Deleting it would not remove the parameter; it would hand the model PR-Agent’s own 0.2, the exact value this line exists to prevent. The only suppression reachable from the configuration file, custom_reasoning_model = true, also disables system messages for the model, so it is not a temperature-only lever and is not worth its collateral. Revisit if a future model answers 400 rather than 200.

reasoning_effort is dead config for Gemini. An earlier revision of this document asserted it is mapped onto Gemini’s thinking budget by LiteLLM and special-cased for the 2.5 models. That is wrong, and it is corrected here rather than quietly deleted, because the setting appears in every run’s resolved-config dump and therefore looks live.

litellm_ai_handler.py adds kwargs["reasoning_effort"] only when the model is in SUPPORT_REASONING_EFFORT_MODELS, whose entire contents are o3-mini, o3-mini-2025-01-31, o3, o3-2025-04-16, o4-mini and o4-mini-2025-04-16. There is no Gemini-specific thinking path anywhere in that handler — the only Gemini references in the file are Vertex project and API key wiring. So reasoning_effort = "medium" is never transmitted, and Gemini 3’s thinking_level cannot be set through PR-Agent at all in this version.

The practical consequence: thinking depth is not a lever we currently have. If per-review cost or depth ever needs tuning, it takes an upstream change, not a configuration one — and a value showing up in the resolved-config log is not evidence that it was sent.

A clipped diff produces confident wrong findings, not fewer findings

The intuition is that clipping costs coverage — the reviewer sees less, so it reports less. The observed behaviour is worse: it reports things that are false, with full confidence, because a truncated diff is indistinguishable from a complete one from inside the model.

API-Sheriff#118 — 68 files, 6706 additions, a stateless cookie session sealed with AES-GCM — logged Tokens: 168188, total tokens over limit: 128000 and reported an Uncompleted Future Hang: that leader.complete(outcome) is bypassed when performRefresh throws, leaving every coalesced waiter blocked forever on existing.join().

The method it flagged:

try {
    RefreshOutcome outcome = performRefresh(cookieHeader, now);
    leader.complete(outcome);
    return outcome;
} finally {
    // Guarantee coalesced waiters never hang: a no-op when the leader already completed
    // above, but a deterministic FAILED when performRefresh threw before completing it
    leader.complete(RefreshOutcome.failed());
    inFlight.remove(sessionId, leader);
}

The finally block completes the future unconditionally, and CompletableFuture.complete() is a no-op once completed — so the happy path is untouched and the throw path yields a deterministic FAILED. Waiters cannot hang. The reviewer quoted the opening of that very finally block, clipped off immediately before the line that refutes its own claim.

The operational rule. A review published on a run whose log says total tokens over limit is inconclusive — neither a clearance nor a finding. Check the token line before reading the verdict, in both directions: a clean result is not evidence, and a finding is not either. Raising max_model_tokens makes clipping rarer; it does not make a clipped review detectable, and that log line remains the only signal.

The PR description is the only route for authored intent — and it was truncated

The reviewer judges code against generic correctness plus this organization’s documented rules. It has no knowledge of what a change was supposed to do. The one channel that can carry that is the pull request description, which pr_reviewer_prompts.toml places in the --PR Info-- block:

PR Description:
======
{{ description|trim }}
======

It is the only such channel, and the alternative does not work. repo_context_files looks like the natural place for a design document, but repo_context.py fetches those files with from_default_branch=True — a document committed on the PR branch is invisible to the reviewer, silently. Setting repo_context_from_default_branch = false to reach it would let pull request content rewrite the reviewer’s own instructions, which is exactly the injection this setup closes on purpose. A per-plan path also cannot be expressed in a fixed central list.

The defect. Upstream defaults max_description_tokens to 500, and git_provider.py:250-262 clips the description with clip_tokens() before it reaches the prompt. plan-marshall#1027’s body is 4503 characters — roughly 1100-1300 tokens — so well over half never reached the model. No marker, no log line. This is the description channel’s version of A clipped diff produces confident wrong findings, not fewer findings: the reviewer receives a fragment and treats it as the complete statement of intent. Now 2000.

Note

Feeding intent to a reviewer is not free. Stating what a change was meant to do invites the model to confirm the account rather than examine the code — the same pressure that produced five empty reviews, arriving as context instead of as instruction.

The charter therefore closes with a clause admitting stated intent only as a claim to be checked: report where the implementation diverges from it, and never treat agreement with it as evidence of correctness.

That clause is conditional by design — If the pull request description states what the change is intended to do…. Nothing guarantees a description states intent at all, so the conditional form is correct in both worlds: inert while none does, active the moment a generator starts emitting one. The guard is therefore in place before the thing it guards, and a tool that begins writing intent into pull request bodies cannot land ahead of its own safeguard — in either repository, in either order.

patch_extension_skip_types does not skip the file

The name reads as an exclusion list. It is not one, and reading it as one produces a confident wrong conclusion — that markdown changes go unreviewed.

git_patch_processing.py extend_patch() calls should_skip_patch(filename) and, on a match, returns the patch unchanged. The hunks still reach the model. What is skipped is only the patch extension: the patch_extra_lines_before / patch_extra_lines_after source lines that are otherwise spliced around each hunk to give the model surrounding context.

Nothing else filters markdown either. md and txt do appear in the bad_extensions table of language_extensions.toml, but under extra, not default — and that list is only appended when use_extra_bad_extensions is true, which it is not here. Both gates must be checked before concluding a file type is excluded; checking one is how the wrong conclusion above is reached.

.md is removed from the skip list because in this organization markdown is frequently not documentation about the code but the contract itself — plan-marshall’s skills, standards and workflow files are read by the agent at runtime. A diff hunk in a contract document is not reviewable without the rule it sits inside. .txt keeps the skip.

repo_context_max_lines is a shared budget that evicts silently

repo_context_files is the entire codebase-awareness surface this reviewer has (see This reviewer is diff-only, and that is a ceiling rather than a setting), which makes its one non-obvious mechanic worth knowing before it costs something.

repo_context_max_lines (default 500) is not a per-file limit. algo/repo_context.py walks the configured files in order, subtracting each file’s header, footer and content lines from one shared budget. When a file does not fit it is truncated with a …​(truncated)…​ marker — and then the loop `break`s. Every remaining file is dropped with no marker at all.

So the failure is ordered and asymmetric: the file that straddles the boundary shows evidence of truncation, and the files after it vanish leaving none. A reader checking whether the context was delivered would find the marker on file two and no sign that file three ever existed.

Current headroom. Not binding today — plan-marshall’s CLAUDE.md is 83 lines and API-Sheriff’s is 151, against a budget of 500. But the two ways to cross it are both ordinary maintenance: adding a third context file, or letting a CLAUDE.md grow. Neither produces an error.

If a repository’s project rules stop being honoured for no apparent reason, count the lines before suspecting the model.

repo_context_files is case-sensitive, and agents.md is not AGENTS.md

repo_context_files is ["CLAUDE.md", "AGENTS.md"]. That lookup resolves through GitHub’s contents API, which is case-sensitive. A repository that tracks a lowercase agents.md therefore hands this reviewer nothing from that file — and hands it nothing silently, because a context file that cannot be fetched is simply absent from the prompt. There is no error, and the review that follows looks exactly like a review that read the file and found it unremarkable.

This is not hypothetical. cuioss/TokenSheriff tracked a lowercase agents.md, so half of its intended reviewer context had never once been delivered. cuioss/plan-marshall tracks AGENTS.md and was unaffected — which is why the defect survived: the repository most likely to notice was the one repository it could not affect.

The upstream cause is worth naming, because it makes the failure reproducible rather than a one-off: plan-marshall’s own tools-sync-agents-file command wrote the lowercase name, so any repository whose agents file was produced by that tooling inherited the defect. Both the command and the steward check that mirrors it now emit and expect the uppercase name.

The rule. The file is AGENTS.md, never agents.md, and a rename between the two is a case change — on a case-insensitive filesystem a naive mv is a silent no-op. Use git mv, and verify against the index (git ls-files AGENTS.md agents.md) rather than against the working tree, which cannot tell you the truth here.

This reviewer is diff-only, and that is a ceiling rather than a setting

Worth recording so nobody re-runs the comparison expecting to find a configuration fix.

The strongest differentiator among current review agents is whole-codebase context — following the dependency graph instead of reading the diff in isolation. Vendors in that class attribute a 70–80% reduction in false positives to it, on the reasoning that most false positives come from not knowing a project’s conventions rather than from misreading the code.

PR-Agent does not do this. Its entire codebase-awareness surface is repo_context_files, which is a fixed list of instruction documents, not a code index. No configuration key closes that gap; it would take a different tool.

Two consequences that shape how this reviewer should be used:

  • Its findings are strongest where the defect is local to the diff. Every defect it has recovered here — a session-identity collision, a sequence-number reuse, a path predicate matched against the absolute rather than the repository-relative path, and a function returning a different key set on one branch than on the other — was visible within the changed code once the model could see all of it. Do not expect it to reason about a caller three files away that the pull request did not touch.

  • Diff completeness matters more for this tool than for an indexing one. A clipped diff removes the only context it has, which is why A clipped diff produces confident wrong findings, not fewer findings is a correctness issue here and not merely a coverage one.

  • A diff can be internally consistent and still be wrong. This is the sharpest form of the ceiling and it has its own case below — see The blind spot: claims contradicted by a file the diff does not touch.

The mitigation we have is the ensemble: three reviewers with different mandates, where the published benchmarks agree the tools disagree. API-Sheriff#118 is the concrete case — this reviewer found a session-collision defect that both of the other two missed. The blind spot: claims contradicted by a file the diff does not touch is the counter-case, where the ensemble did not help, so do not read the ensemble as coverage.

And The binding constraint is availability, not depth is the harder counter-case: an ensemble is only as wide as the number of its members that actually ran.

The blind spot: claims contradicted by a file the diff does not touch

The ceiling above has a specific shape that is worth naming, because it is invisible in exactly the way that makes it dangerous — the review comes back clean and the diff reads as coherent.

API-Sheriff#123 is the case. The pull request was titled "make the k6 suite run to completion". It documented two benchmark goals as deliberately skipped, gave a reason for each, and excluded both from the list of goals a new CI step required to produce a result. Read as a document, the change is consistent: it says twelve goals are wired, two are skipped, ten are expected.

Nothing in the change actually skipped them. Both goals remained unconditional executions in benchmarks/pom.xml — a file the pull request did not modify — so the suite still aborted partway through, and the CI step added to detect a truncated suite was written so that it could not fire in that case. Neither this reviewer nor CodeRabbit reported anything.

The general form:

  • The diff asserts a behaviour ("these are skipped", "this is now validated", "the guard covers X").

  • The mechanism that would implement the assertion lives in a file the diff does not touch.

  • Every statement inside the diff agrees with every other statement inside the diff.

There is no evidence of the defect anywhere in this reviewer’s input, so it is not a recall failure and no charter wording addresses it. It is the diff-only ceiling meeting a claim that can only be checked outside the diff.

Note
Why repo_context_files is not the fix

The obvious reflex is to add the contradicted file to repo_context_files. Resist it.

That key is a fixed list applied to every repository this configuration governs, so a build file from one project becomes dead weight in all the others. It also shares the repo_context_max_lines budget, which evicts silently and in order — see repo_context_max_lines is a shared budget that evicts silently — so paying for a rarely-relevant file makes the genuinely useful instruction documents likelier to be dropped. And the channel carries instruction documents rather than a code index: the file arrives as context to read, not as something the model can follow a reference into.

Reserve the list for documents that describe the rules of the project, which is what it is for.

The practical rule. When a change asserts that something is now skipped, guarded, validated or enforced, confirm by hand that the mechanism exists — and confirm it in the file that would have to implement it, not in the prose of the change. A clean review from either bot is not evidence on this class of claim, because neither one looked. That is also the honest limit on the ensemble: two independent reviewers sharing the same blind spot agree, and their agreement carries no information.

Recall beats precision for this reviewer, and that is deliberate

The charter (A charter written against noise produces silence) was rewritten to report more, not less, and the tuning pressure over time will be to narrow it again whenever a finding turns out to be wrong. That pressure should be resisted, for a reason worth writing down while the evidence is fresh.

The costs are asymmetric. A false positive costs one dismissal by a human who has the code in front of them. A missed defect ships. This reviewer’s measured failure mode was not noise — it was five consecutive pull requests of byte-identical silence, and a security-weighted charter that could not report a TOCTOU race it named in its own instructions.

There is one caveat on the other side, from teams who have published real numbers on this: a single agent accumulating rules in one growing prompt eventually degrades, and the remedy is several narrowly-scoped passes rather than one broader one. Google’s Gemini 3 guidance points the same way — the model "may over-analyze verbose or overly complex prompt engineering" and rewards direct, simplified instructions.

The practical rule. Treat charter growth as the warning sign, not finding volume. If the category list keeps expanding past roughly ten entries, the answer is a second focused pass — not an eleventh bullet.

A quieter review is not a worse one, and volume is not coverage. Two cases make the point from opposite directions, and both should be read before anyone concludes this reviewer under-reports.

On plan-marshall#1038 another reviewer posted three findings where this one posted none. That looked like a recall gap until the three were examined: all three were declined on their merits, and the reviewer that raised them then withdrew all three itself. The silence was the correct answer, and counting comments would have scored it as a miss.

On plan-marshall#1040 the direction reverses. Both reviewers examined the same predicate. This one posted first, on the opening commit, and its finding was the live defect — a path check matched against the absolute path, so any ancestor directory carrying the matched word made the predicate true for every document. The other reviewer arrived forty minutes later, after that defect had already been fixed, and raised a finding against the corrected code that was refuted and withdrawn. One finding each; one of them real.

The rule that survives both: a reviewer’s finding count carries no information about its recall. Compare what was substantiated, never how much was said.

The binding constraint is availability, not depth

The ensemble argument in This reviewer is diff-only, and that is a ceiling rather than a setting assumes the members run. That assumption is the weakest part of this setup, and it fails often enough to be the first thing to check.

On plan-marshall#1041 all three reviewers posted a comment on a 48-file pull request. Two of those comments were refusals — one declined on diff size, one on an exhausted review quota — so the number of reviewers that read the diff was one, and it was this one. A count of participating bots reads three. Coverage was a third of that.

Two properties make this worse than it first looks.

  • A refusal is shaped like a review. It is posted by the reviewer identity, in the same place, at the same point in the flow. Presence of a comment is therefore not evidence of a review; only its content is. Any consumer that counts comments — or check states, which are unreliable in both directions — will overstate coverage.

  • Quotas are account-wide, so the cost is paid by a different pull request than the one that incurred it. A quota consumed by re-reviews of intermediate commits on one repository’s pull request is unavailable to the next repository’s. The pull request that goes unreviewed is rarely the one that caused the exhaustion, which is what makes this hard to notice from any single pull request.

The practical rule. Before treating a clean pull request as reviewed, read each bot’s comment body and confirm it contains a review rather than a refusal. Do not infer coverage from the number of configured reviewers, the number of comments, or a green check.

This reviewer’s standing value in the ensemble is therefore not only that it reads the diff differently. It is that it has no per-account review quota and does not decline a pull request for being large, so it is frequently the member that is still available.

That availability has a sharp edge, and it is the reason this section does not simply recommend relying on it. This reviewer has a size limit too — max_model_tokens, see The context window caps what the reviewer sees — but it does not refuse when a diff exceeds it. It clips the diff and reviews the remainder, and A clipped diff produces confident wrong findings, not fewer findings is the case where that produced a confident finding refuted by the very lines the clip removed. So the two failure modes are not equivalent: a refusal is a truthful report of non-coverage, while a clipped review is indistinguishable from a complete one at the pull request. Read an oversized diff reviewed here as inconclusive, and prefer a bot that declines loudly over one that silently reviews part of the change.

PR-Agent cannot report its own failure

Its GitHub Action runner catches its own exceptions, logs them at WARNING/ERROR, and still exits 0. A run whose every model call failed — wrong model id, no entitlement to the model, exhausted credits, provider outage — reports a green check. Every failure mode described above was first observed exactly that way: a successful job with no review.

The reusable workflow therefore ends with a fail-closed gate. Its oracle is the reviewer step’s own structured output, not a published comment: pr_reviewer.py calls github_action_output(data, 'review') from _prepare_pr_review(), which runs before the publish decision, so steps.review.outputs.review exists whenever the model produced a review and is absent whenever every model call failed.

Gating on the comment instead is wrong in both directions, and the first version of this gate got it wrong in the first one:

  • A clean review may publish nothing, depending on publish_output_no_suggestions. A working reviewer that found no issues would fail the build.

  • The review comment is persistent and updated in place, so a stale comment from an earlier run satisfies a naive existence check forever.

Do not remove the gate on the grounds that "the action already fails on error". It does not.

Why a clean review is published

publish_output_no_suggestions is true, so the reviewer comments even when it found nothing. It was false at first, to suppress "No major issues detected" noise. That was a mistake, for a reason worth stating plainly: it made a clean review and a total failure indistinguishable at the pull request. Both show no comment. The difference existed only in a workflow log.

The cost was not hypothetical. plan-marshall#1024 was recorded as having had zero automated review and taken toward a merge decision on that basis, while this reviewer had read the diff and cleared it — {"relevant_tests": "yes", "key_issues_to_review": [], "security_concerns": "No"}. Two other reviewers had genuinely refused that pull request on rate limits, so "no comment" read as a third refusal.

It also starved the consumer pipeline. plan-marshall’s automatic-review step collects pr-comment findings as its evidence that a bot participated, so a reviewer that never speaks can never be counted: the configuration would claim three reviewers while the record could only ever show two.

The price is one short comment per clean pull request. Pay it. A reviewer whose agreement is unobservable is worth far less than one that says so, and silence that could mean either "approved" or "broken" is not a signal at all.

That price has to stay one comment, which is why final_update_message is false. Upstream defaults it to true, posting a second, separate comment every time the persistent review is refreshed:

**[Persistent review](<url>)** updated to latest commit <url>

It carries no review content, and it is redundant even as a pointer — the same code path already stamps (Review updated until commit …) into the persistent comment’s own header. It is not merely cosmetic either: the comment is authored by the reviewer identity, so a consumer that collects the bot’s comments as review findings files it as a finding requiring triage.

Note the interaction, because it runs the wrong way: publishing clean reviews increases this noise. Clean pull requests now carry a persistent comment, so there is something to "update" on every subsequent review, where previously there was nothing. The two settings belong together.

Vertex AI, keyless

The reviewer runs Gemini through Vertex AI (vertex_ai/… model ids), not AI Studio (gemini/…). Two reasons: Vertex consumes the GCP free-trial credits, which AI Studio does not; and it can be reached through Workload Identity Federation, so no Google credential is stored in GitHub at all.

pr-agent’s Vertex path is config-driven — litellm_ai_handler.py reads VERTEXAI.VERTEX_PROJECT / VERTEXAI.VERTEX_LOCATION and hands them to LiteLLM — while authentication itself is plain Application Default Credentials.

Project and location are deliberately not in this file. They are deployment identity rather than review tuning, so the reusable workflow passes them from organization variables (GCP_PROJECT_ID, GCP_VERTEX_LOCATION, default global), which also keeps the GCP project id out of a public repository. Everything that is review tuning stays here.

Two container boundaries have to be crossed by hand in the workflow, and getting either wrong produces an authentication failure inside the reviewer that looks like nothing at all:

  • Path. google-github-actions/auth writes its credentials file into $GITHUB_WORKSPACE (deliberately, so Docker actions can read it) and reports a host path. The reviewer is a Docker action with that workspace mounted at /github/workspace, so only the basename is reusable — the host path does not resolve inside the container.

  • Environment. A Docker action receives only the variables named in its own env: block, plus the standard GITHUB_* / RUNNER_* set. Exporting GOOGLE_APPLICATION_CREDENTIALS to $GITHUB_ENV, as the auth action does, does not forward it into the container; it has to be restated on the reviewer step.

There is no AI Studio fallback. The GEMINI_API_KEY secret is deleted and the workflow no longer passes it, so setting a gemini/… model id here would fail at the model call. Carrying the passthrough "just in case" was worse than removing it: it advertised a route that could not work, which is the same class of defect as the dead ignore_pr_* settings above. Restoring it means re-creating the secret and re-adding the env line — deliberately, both at once.

If AI Studio is ever revived, note that its free tier is rate-limited and its traffic may be used for training; use a billing-enabled key.

Charter: why security-weighted

The organization’s review docs record that the retired consumer tier of Gemini Code Assist was in practice the sharpest security reviewer of the three bots, and that its sunset left that gap to be covered by reading CodeRabbit’s security findings more carefully.

So this one is pointed at the gap: require_security_review = true plus a security-weighted extra_instructions block, and the low-value output (effort estimates, review labels, ticket analysis, help text) switched off. repo_context_files additionally feeds it this org’s CLAUDE.md / AGENTS.md, so it reviews against documented project rules rather than generic expectations — something neither other reviewer can do.

Warning

"Avoid duplicating the other reviewers" is not a charter — it is an off switch. The first version of this charter said so explicitly and produced a reviewer that never reported anything. See A charter written against noise produces silence. Overlap costs one duplicate comment; suppression costs the finding.

A charter written against noise produces silence

The first charter was tuned against a failure mode this reviewer never had. The result is the most expensive kind of bug in a review pipeline: a bot that looks healthy, runs green, publishes on schedule, and is worth nothing.

The measurement. Across five pull requests, four models (3.5-flash, 2.5-pro, 3.6-flash) and diffs from 5k to 57k tokens, every published review was byte-identical at 242 bytes — the bare table, zero findings. A result that does not move when the model changes is not a model result, and the instinct to fix it by promoting to a stronger model is a category error.

The controlled run. /review on plan-marshall#1027: gemini-3.6-flash, full 40748-token diff with no pruning, against the final commit. In that same diff CodeRabbit had already reported a TOCTOU race around shutil.move and an empty-string filename resolving to a directory — both squarely inside the charter. This reviewer reported neither. That single run eliminated model choice, diff pruning and the stale-commit trigger in one shot, leaving only the prompt.

The cause. Upstream’s field description for key_issues_to_review is already conservative ("Only include issues you are confident about… An empty list is acceptable if no clear issues are found"). The charter stacked three further suppressors on top, with nothing pushing back:

  • do not duplicate [the other reviewers] — an instruction to withhold, and the model has no way to know what the other two actually reported.

  • only when you can name the concrete input — redundant with the base confidence gate.

  • prefer one well-evidenced finding — resolves to zero when nothing reaches certainty.

The scope was also wrong for the codebase. Six classical AppSec bullets — injection, SSRF, authentication — that Python tooling repositories almost never contain, so the model correctly concluded its assignment was empty. The defect classes this organization actually ships (concurrency, resource lifecycle, fail-open error handling, state corruption) appeared only as a sub-clause of one bullet.

The rule that follows. Every clause in extra_instructions that narrows scope or raises a confidence bar is a suppressor, and they compose multiplicatively with a base prompt that is already conservative. Write the charter to say what to look for, never what to stay quiet about.

How to verify a charter change. plan-marshall#1027 is a known-answer regression case: comment /review and the two findings above must appear. A green run proves nothing — an empty review is exactly what the broken configuration produced.

A second known-answer case, for the severity clause specifically. #1027 only proves the reviewer reports a Major defect; it cannot distinguish a charter that reports everything substantiated from one that still applies a severity floor, because both report a TOCTOU race. The oracle for that is plan-marshall#1042, where another reviewer substantiated four findings on the same diff of which two were accepted as valid and then declined as minor — a test-fixture duplication and a narrow same-file scoping refinement. Those two are precisely what a severity floor suppresses and a floorless charter must surface.

The pass condition is therefore shaped, not counted: comment /review on #1042 and check whether anything narrow, cheap-to-fix, or confined to test code appears at all. A review returning only the two Major defects has NOT discharged the severity clause — it has reproduced the exact behaviour the clause was written against, while looking like a success.

This matters because "No major issues detected" is not a filter, and the real suppressor is not configurable makes the clause an argument against the pinned field description rather than a replacement of it, and the two can compose into competing instructions. Nothing in the configuration proves which one won. Only a known-answer run does, and this is that run.

"No major issues detected" is not a filter, and the real suppressor is not configurable

The natural reading of a review that says "No major issues detected" is that a severity threshold discarded the minor findings, and that some setting controls it. Both halves are wrong, and the mistake sends the next reader looking for a knob that does not exist.

There is no severity filter. That sentence is the text rendered when key_issues_to_review comes back as an empty list. Nothing was found and nothing was dropped.

The suppression lives in that field’s own description, inside the pinned image at pr_reviewer_prompts.toml:150:

A concise list (0-{num_max_findings} issues) of bugs, security vulnerabilities, or significant performance concerns introduced in this PR. Only include issues you are confident about. […​] An empty list is acceptable if no clear issues are found.

Four suppressors in one sentence:

  • a ceiling that reads as a target, reinforced by the word concise

  • a scope — bugs, security, significant performance — narrower than the categories the charter adds

  • a confidence gate

  • explicit permission to return nothing

The structural part. extra_instructions cannot reach this text. It is appended as its own block earlier in the system prompt, so the charter argues alongside the field description rather than replacing it. Every charter revision is therefore a rhetorical contest with a schema the model is also obeying, and the schema is closer to the answer it must produce.

That is why the charter now addresses the empty-list permission head-on rather than adding more categories. Adding categories was already tried, and Recall beats precision for this reviewer, and that is deliberate caps it at roughly ten. What had never been contested was the licence to report nothing.

num_max_findings matters more than a cap normally would for the same reason: it is interpolated into that sentence, so a low value is read as the expected shape of an answer. It was raised from five when plan-marshall#1042 produced six substantiated findings from another reviewer on the same diff — five was capable of binding.

Warning
Loosen the severity bar, never the substantiation bar. Pressure to report more is exactly what produces invented mechanisms, and this reviewer has done it once already — API-Sheriff#103, a confident finding about a branch that cannot be reached. The charter’s anti-fabrication clause is load-bearing and must survive any future tuning of this section.

The charter is composed per domain, and a repository carries exactly one pack

The charter in this file is the spine: the cross-cutting security and correctness categories that apply to any codebase. What it cannot carry is a language’s own defect vocabulary, and the cost of that was measured rather than suspected — a 58-pull-request sweep found findings on 4 of 13 Python and markdown pull requests and 0 of 19 Java ones. A single generic charter is shaped like whichever language its author had in mind.

The fix is a repository-local .pr_agent.toml carrying a [pr_reviewer].extra_instructions pack for that repository’s domains, merged above this file. Packs are generated — never hand-written — from cuioss/plan-marshall’s `marketplace/targets pr-agent target, which reads the same bundle sources that supply the organization’s own standards:

python3 marketplace/targets/generate.py --target pr-agent --output . --packs java

Three properties are load-bearing, and each is a way the mechanism could quietly degrade:

  • One pack per repository — swap, never accumulate. Regenerating REPLACES the pack rather than appending to it. A repository whose packs accumulated would drift back toward the generic charter this mechanism exists to replace, one merge at a time. A single pack may still compose several domains, because a repository is not always one language, and a composed pack still contributes exactly one review category.

  • The pack carries the domain rules only. Everything else — model, token budgets, output suppression, the spine categories — is inherited from this file. Restating a central key in a generated pack decentralizes the setting that this whole repository exists to centralize, and it does so invisibly, because the local file legitimately outranks this one.

  • The category ceiling is now enforced by a test, not by discipline. Recall beats precision for this reviewer, and that is deliberate caps the charter at roughly ten categories, because a single agent accumulating rules in one growing prompt degrades. That cap used to depend on whoever was editing noticing it. The generator now carries a regression test that fails when a generated pack exceeds the ceiling, or when the substantiation bar and anti-fabrication clause are diluted as packs are composed.

The last point is the one worth trusting least on appearance: the substantiation bar is carried into every generated pack verbatim, and the test exists because "the generator copied it" is exactly the kind of claim that stays true until a refactor makes it silently false.

What was actually checked about /improve

Two beliefs about /improve were load-bearing for enabling it, and both had been assembled from circumstantial evidence rather than read. Both turned out to be wrong. They are recorded here with their sources so the next reader does not re-derive them — and, more to the point, does not act on the plausible version.

There is no pr_reviewer.inline_code_comments. The attractive theory was that /review could be made to publish its findings inline, which would be strictly cheaper than /improve — no second tool invocation and no doubled token cost. It does not exist. At upstream tag v0.39.0 the [pr_reviewer] section of pr_agent/settings/configuration.toml declares no such key, and pr_agent/tools/pr_reviewer.py publishes only through publish_comment and publish_persistent_comment — there is no publish_inline_comments or publish_code_suggestions call in that file. Setting the key would be an inert unknown key, which is the dead-config trap in its purest form: it would parse, appear in the resolved-config dump, and do nothing. Inline publishing belongs to /improve, through [pr_code_suggestions] commitable_code_suggestions (default false) or dual_publishing_score_threshold.

restricted_mode = true does not gate /improve. This was the live risk, since a reviewer that fails closed under the setting would have made the label gate useless. It does not. restricted_mode is declared in [config] defaulting to false, and its sole consumption is GithubProvider.is_supported, which returns False for exactly one capability string: push_code. The confirmed consumer of that capability is pr_agent/tools/pr_update_changelog.py, which degrades gracefully to a comment. pr_agent/tools/pr_code_suggestions.py contains no occurrence of restricted_mode at all, and its publish paths go through the pull request review API (pull-requests: write) rather than contents: write. So keeping every caller workflow free of contents: write costs nothing.

Important

The limit of the second finding, stated rather than dropped. is_supported call sites were not exhaustively enumerated across the upstream repository, and only the GitHub provider was inspected. The result therefore does not transfer by proof to GitLab, Bitbucket or Azure DevOps. It is a checked fact about the provider this organization runs, not a general one — and both findings are pinned to tag v0.39.0, not to whatever the floating image tag points at later.

[skills] would be a strong lever and cannot be enabled as things stand

Version 0.39.0 adds a [skills] section that discovers */SKILL.md files from configured filesystem paths and inlines them into the review prompt. For an organization whose standards are literally written as SKILL.md files this looks like the highest-value setting in the file.

It is unusable here, for a reason worth recording so the idea is not rediscovered and quietly switched on.

skills.paths is a filesystem scan, not a repository reference. The reviewer workflow performs no checkout at all — deliberately, and the workflow says so at the step that fetches credentials — so there is nothing on disk to scan. Making the setting work means adding a checkout, and a checkout in this job is a checkout of the pull request head.

That is the one thing this configuration is built to prevent. repo_context_from_default_branch is true precisely so that pull request content cannot rewrite the reviewer’s own instructions; enabling skills against a head checkout would reintroduce that hole in a wider form, because a SKILL.md added by a pull request is read as organizational standards rather than as reviewed content.

Revisit only if the loader gains a default-branch reference, or if a checkout can be pinned to the base branch and proven not to follow the head.

Skip rules

Three rules, matching cuioss/coderabbit exactly in effect, but not in mechanism:

Rule Enforced by Notes

Skip dependabot[bot] pull requests

Reusable-workflow if: guard

Dependency bumps are gated by CI and auto-merge.

Skip cuioss-release-bot[bot] pull requests

Reusable-workflow if: guard

Consumer-propagation and workflow-SHA bumps.

Skip any pull request labelled skip-bot-review

Reusable-workflow if: guard

The shared org-wide opt-out label. Create it per repository where needed.

Skip fork pull requests

Reusable-workflow if: guard

Secrets are unavailable to them, so the token step could only fail. Reviewing forks needs pull_request_target and a separate security review of that decision.

The guard is evaluated when a review would be triggered — on opened, reopened and ready_for_review. Label the pull request at creation time (gh pr create --label skip-bot-review) or before marking a draft ready; labelling it after a review has been posted does not retract that review, exactly as with CodeRabbit.

An explicit /review comment from a human overrides the skip label: someone is asking on purpose. Bot senders are excluded, so the reviewer cannot retrigger itself off its own comment.

Important

Do not "move these into the central config for tidiness". That is precisely the dead-config trap: the settings exist, they parse, and they are ignored.

Tools

PR-Agent has three automatic tools:

Tool Enabled Reason

/review

yes

The reason this reviewer exists.

/describe

no

It rewrites the pull request description, which collides with generated pull request bodies and the machine-readable blocks our automation reads out of them.

/improve

per pull request

Off by default, on for any pull request labelled pr-agent-improve. The label is read by the reusable workflow itself, so no adopting repository edits anything to get the gate. Tuned centrally in [pr_code_suggestions], including the substantiation bar — see The charter is composed per domain, and a repository carries exactly one pack for why duplicating CodeRabbit is no longer the objection it was.

The old objection to /improve — that committable suggestions are already CodeRabbit’s job — was retired with the per-domain packs. The suggestions now carry this organization’s domain rules rather than a second copy of a generic reviewer’s, so the overlap argument no longer holds; and Recall beats precision for this reviewer, and that is deliberate already settled that overlap costs one duplicate comment while suppression costs the finding. What remains true is the cost, which is why the gate is per pull request rather than per repository: a reviewer decides, on a pull request they are already reading, that inline suggestions are worth paying for on that one.

The three toggles are the one deliberate exception to "everything is configured here": they are inputs of the reusable workflow, so a single repository can opt into /improve unconditionally without editing the org-wide file. The pr-agent-improve label is the finer-grained version of the same escape hatch, and it needs no caller change at all.

On-demand commands (/review, /ask, /improve, /help) still work as pull request comments in any adopting repository, regardless of the automatic toggles.

Adding a repository

Add one caller workflow to the repository. No configuration is copied — it inherits this file.

name: PR Agent Review

on:
  pull_request:
    types: [opened, reopened, ready_for_review]
  # On-demand commands, and how a re-review is requested after pushing fixes — there is
  # deliberately no automatic re-review on every push.
  issue_comment:
    types: [created]

permissions:
  contents: read
  pull-requests: write
  issues: write
  id-token: write   # required — Workload Identity Federation for Vertex AI

jobs:
  review:
    uses: cuioss/cuioss-organization/.github/workflows/reusable-pr-agent-review.yml@<sha>
    secrets:
      REVIEW_APP_ID: ${{ secrets.REVIEW_APP_ID }}
      REVIEW_APP_PRIVATE_KEY: ${{ secrets.REVIEW_APP_PRIVATE_KEY }}

id-token: write is what lets the reusable workflow mint the OIDC token Workload Identity Federation exchanges for GCP credentials.

Map the two secrets explicitly rather than using secrets: inherit: those two are the whole requirement now that Vertex AI is keyless, and inherit would hand the reviewer every secret the calling repository can see.

Prerequisites, all organization-wide and already in place for adopting repositories:

  • the cuioss-review-bot App installed org-wide;

  • organization secrets REVIEW_APP_ID and REVIEW_APP_PRIVATE_KEY;

  • organization variables GCP_WIF_PROVIDER, GCP_REVIEW_SERVICE_ACCOUNT, GCP_PROJECT_ID (and optionally GCP_VERTEX_LOCATION, default global) — variables, not secrets, because none of them is a credential.

Create the skip-bot-review label in the repository if it does not exist yet, and pr-agent-improve alongside it if inline suggestions are wanted there — a label that does not exist cannot be applied, which is the one way the gate in Tools can appear not to work while the workflow is entirely correct.

See also

About

Central PR-Agent configuration for the cuioss organization (third automated PR reviewer, on Google Gemini)

Resources

Code of conduct

Contributing

Security policy

Stars

0 stars

Watchers

0 watching

Forks

Releases

Packages

Contributors