refactor(review): replace agentic reviews with /review guidance

[ko3n1g]

Background

The legacy review workflow still runs Claude Code. Reviews are moving to an explicit /review request handled by the central review capability.

What changed

• Preserve the existing trusted-author/label/dispatch gate and replace the Claude CLI job with one /review guidance comment.
• Keep the existing review-code skill and document the local legacy recipe’s publication boundary.
• Reuse .agents/skills/review-code/SKILL.md.

Details

CI-Events registration and publisher wiring are tracked in CI-Events-Workflows #2450. New skill sources must reach protected main, and their Ready Plugin snapshots must contain the configured rubric, before enabling the profile.

flowchart LR
    A[PR event or manual dispatch] --> B[Existing authorization gate]
    B -->|Allowed| C[Post /review guidance]
    C --> D[Maintainer requests /review]
    D --> E[Central review capability]

Loading

Tested

• actionlint .github/workflows/agentic-ci-pr-review.yml — passed.
• Executed the notice step with a mock gh and a multiline notice containing backticks and $() — exact arguments and stdin preserved; no shell expansion occurred.
• pre-commit run --files .agents/recipes/pr-review/recipe.md .github/workflows/agentic-ci-pr-review.yml — passed.
• git diff --check — passed.
• GitHub CI run 37075834985: the Python 3.14 end-to-end entry stopped during dependency installation because markupsafe==3.0.4 has no compatible CPython 3.14 distribution. This PR does not change dependencies.

[github-actions]

Thanks for putting this together, @ko3n1g. Cutting out the self-hosted Claude job also removes a lot of attack surface.

Summary

This PR swaps the review job in agentic-ci-pr-review.yml, which ran the Claude CLI on a self-hosted runner with secrets, for a small review-guidance job. On ubuntu-latest, that job posts one comment telling maintainers to use /review. The trusted-author, label and dispatch gate is kept. The local pr-review recipe gets a note saying it is now legacy and local-only. The change mostly does what the description says. One gate behavior changed without being mentioned, and the reformatting dropped the explanatory comments.

Findings

Warnings — Worth addressing

.github/workflows/agentic-ci-pr-review.yml (gate › Check permissions) — the permission lookup no longer has a fallback, so the gate can fail

• What: The old code was PERMISSION=$(gh api ... --jq '.permission' 2>/dev/null || echo "none"). The new code is PERMISSION=$(gh api "repos/${REPO}/collaborators/${USER}/permission" --jq '.permission'), with no fallback. GitHub's default run shell is bash -e. If the API call fails, the assignment exits non-zero and the step aborts before it writes allowed=false.
• Why: This API can return an error for some logins. Bot authors such as dependabot[bot] are a likely example, since dependabot opens PRs against this repo regularly. Rate limits and transient API errors also count. When that happens, the gate job goes red on the PR when it should quietly skip. The PR description says the gate was "preserved", so the change looks unintentional.
• Suggestion: Put the fallback back:

  PERMISSION=$(gh api "repos/${REPO}/collaborators/${USER}/permission" --jq '.permission' 2>/dev/null || echo "none")

.github/workflows/agentic-ci-pr-review.yml — the agent-review label is no longer removed, and every trigger posts a new comment

• What: The Remove agent-review label step was deleted. The workflow still triggers on labeled with agent-review, on opened, and on ready_for_review, and it posts a fresh guidance comment each time.
• Why: The label now stays on the PR forever, but it no longer does anything except post the same notice again. A PR that goes draft → ready → draft → ready collects one copy of the notice per transition. That is noise on every maintainer PR.
• Suggestion: Pick one of these:
  • Drop the labeled trigger, since the label no longer means "run a review".
  • Keep the label-removal step.
  • Before commenting, check for an existing notice and skip if one is found, e.g. gh pr view "$PR_NUMBER" --json comments --jq '.comments[].body' | grep -qF 'Automatic Agentic CI reviews have been retired'.

Suggestions — Take it or leave it

.github/workflows/agentic-ci-pr-review.yml — the reformatting removed the comments and makes the diff hard to read

• What: The whole file looks like it was rewritten by a YAML dumper. The quotes changed ('on':), flow lists became block lists, the step indentation moved, and the comments are gone. One lost comment explained why the gate uses the collaborator API instead of author_association (org membership can be private).
• Why: That comment records a non-obvious security decision. The re-indentation also turns the "gate preserved" part into a full delete-and-re-add, which is how the fallback change above slipped through unnoticed.
• Suggestion: Keep the original formatting and the gate comments, and only replace the review job.

.agents/recipes/pr-review/recipe.md:1-12 — the frontmatter still describes the old CI behavior

• What: The new note says the recipe is local-only and has no GitHub publisher. The frontmatter still says description: Review a pull request and post findings as a PR comment, trigger: pull_request, and permissions: checks: write / pull-requests: write.
• Why: Anyone reading the frontmatter, or any tool that parses it, gets told the recipe still runs on PRs and posts comments.
• Suggestion: Change the description to something like "Review a pull request locally and save findings to /tmp", and drop or adjust trigger and permissions to match.

.agents/recipes/pr-review/recipe.md:41 — line length

• What: The rewritten constraint bullet is one long line, while the bullets around it wrap at about 80 columns.
• Suggestion: Wrap it to match the other bullets.

What Looks Good

• The new job is much safer. It needs no secrets, no environment, no self-hosted runner and no checkout of the PR head under pull_request_target. That removes the allow-unsafe-pr-checkout exposure noted in Harden Agentic CI PR reviews for forked pull requests #804. Permissions are narrowed to pull-requests: write.
• The notice goes in through env and --body-file - <<< "$NOTICE", so backticks and $() in the message are never shell-expanded. The PR description says this was tested with a mock gh.
• Short timeout-minutes: 5 limits on both jobs, and cancel-in-progress concurrency is kept.

Residual Risk

• The PR description says the CI-Events registration and publisher wiring for /review lands in a separate coordinated PR. If this PR merges first, trusted PRs will be told to comment /review (including mode=strict, model=claude and help) before that command is handled. It would help to merge after, or together with, the central capability being enabled.
• .agents/tools/structural_impact.py was the source of the ### Structural Impact section in PR reviews. It is now used only by the structure recipe and by local runs of this recipe. If the central /review should keep that signal, it needs its own wiring.
• plans/472/agentic-ci-plan.md still describes automatic PR reviews and the agent-review label. That is fine for a historical plan, but a short "superseded by /review" note would avoid confusion.
• The linter was skipped because the PR changes no Python files.

Verdict

Needs changes. Restore the || echo "none" fallback in the permission check. Then decide how the agent-review label and repeated notices should behave: remove the label, drop the trigger, or dedupe the comment. The rest are optional polish.

---

This review was generated by an AI assistant.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Replaces agentic review workflow with simpler guidance posting.

The PR should not merge until external contributors’ PRs cleanly skip the guidance workflow rather than producing a failed run.

Findings

1.  Permission lookup fails the gate ▶

Fix with agent prompt

### Issue 1
.github/workflows/agentic-ci-pr-review.yml:71
When a non-collaborator opens a PR, this lookup can return 404. Without the previous fallback, the runner’s default `bash -e` stops the gate before it writes `allowed=false`. External contributors then get a failed workflow run instead of a clean skip.

```suggestion
        PERMISSION=$(gh api "repos/${REPO}/collaborators/${USER}/permission" --jq '.permission' 2>/dev/null || echo "none")
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR replaces the legacy automated review job with gated guidance to request /review and marks the old recipe as local-only.

• The permission gate can fail rather than skip for non-collaborator PR authors.
• Repeated eligible events can post duplicate guidance comments.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[PR event or manual dispatch] --> B[Permission gate]
  B -->|Allowed| C[Post /review guidance]
  B -->|Not allowed| D[Skip]

Loading

_{Reviews (1) · Last reviewed commit: "refactor(review): migrate legacy reviews..."}

[greptile-apps]

Permission lookup fails the gate

When a non-collaborator opens a PR, this lookup can return 404. Without the previous fallback, the runner’s default bash -e stops the gate before it writes allowed=false. External contributors then get a failed workflow run instead of a clean skip.

Suggested change

	PERMISSION=$(gh api "repos/${REPO}/collaborators/${USER}/permission" --jq '.permission')
	PERMISSION=$(gh api "repos/${REPO}/collaborators/${USER}/permission" --jq '.permission' 2>/dev/null || echo "none")

Prompt To Fix With AI

This is a comment left during a code review.
Path: .github/workflows/agentic-ci-pr-review.yml
Line: 71

Comment:
**Permission lookup fails the gate**

When a non-collaborator opens a PR, this lookup can return 404. Without the previous fallback, the runner’s default `bash -e` stops the gate before it writes `allowed=false`. External contributors then get a failed workflow run instead of a clean skip.

```suggestion
        PERMISSION=$(gh api "repos/${REPO}/collaborators/${USER}/permission" --jq '.permission' 2>/dev/null || echo "none")
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[chtruong814]

We should probably close this for now. I can start a conversation with the maintainer of this repo and you so we can share our thoughts but another team owns this.