Code review

A table, one sentence, then a block per finding. Nothing else: no preamble, no summary of the diff, no praise.

.claude/agents/code-review.md, in the repository being reviewed. Short on purpose: Claude Code loads CLAUDE.md itself, so the house style is not repeated here. The agent is told to review against it.

HEAD^1, not the base branch. For a pull_request event the checked-out ref is refs/pull/N/merge, and its first parent is the commit the branch was merged onto. Diffing against origin/main instead means anything that lands on main while the job waits for a runner appears in the review as if this pull request had written it.

persist-credentials: false, because the agent runs with permissions skipped and has Bash. The default leaves a header carrying the job's GITHUB_TOKEN (including its pull-requests: write) in .git/config inside the workspace the agent reads.

shell: bash for the pipefail it adds. Under the default bash -e, a failed curl -f in curl … | sh leaves sh reading EOF and exiting 0: the step passes with nothing installed, and the run ends green with no comment and no cause on screen.

This is the version that runs, not a translation of the workflow above. Three things differ, and each one was a red pipeline first.

The agent cannot run as root. selan agent-run adds --dangerously-skip-permissions unconditionally, and Claude Code refuses that flag at uid 0: --dangerously-skip-permissions cannot be used with root/sudo privileges for security reasons. A self-hosted GitHub runner is already an unprivileged user, which is why the workflow above never mentions this. A container image is root, and GitLab has no image:user, so the job drops privileges itself. IS_SANDBOX=1 is the escape the guard offers; it asserts the container is a sandbox while handing an agent Bash inside it.

HOME must be set inside the script, not in the job's variables:. A job variable is already in force during get_sources, which runs git before your first script line, and the clone dies with could not lock config file /tmp/agent/.gitconfig.

No merge ref, so no HEAD^1. GitLab checks out the source branch itself. HEAD^1 is the previous commit on that branch, so it would review the last push instead of the change. CI_MERGE_REQUEST_DIFF_BASE_SHA is the merge base, which also keeps anything that lands on main while the job queues out of the diff. GIT_DEPTH: 0 is what puts it in the clone.

On the Free plan, nothing can post a comment. CI_JOB_TOKEN cannot create a note, and on GitLab.com Free neither a project nor a group access token can be issued at all: both endpoints answer 400 User does not have permission to a group Owner. POST /user/personal_access_tokens accepts only k8s_proxy and self_rotate, so api scope is web-UI only. What is left is a personal token: authored by a person, not a bot, and carrying everything that person can do, in a variable any job can read.

So the findings ride artifacts:reports:codequality, which GitLab reads itself (no API call, no bot account, no credential). The cost is where they render: the merge request's Reports tab, because the inline markers in the Changes view are Ultimate and the pipeline's Code Quality tab is Premium. review.md goes up as an artifact too, since that is where a finding's fix actually reads.

The agent writes markdown, so a small script converts it: heading to description, the C/M/L prefix to critical/major/minor, and the backticked path:line to location. Derive fingerprint from the id, path and claim and not the line number: GitLab matches findings across pushes by fingerprint, and a fingerprint that ignores the line keeps a finding the same one after its code moves down a file. Check the parsed count against the counts table the agent wrote. A parser that matches nothing produces an empty report, and an empty report is exactly what GitLab renders for a clean review.

One pipeline per push, and do not swallow tags. Asking for merge request pipelines makes GitLab willing to create one alongside the branch pipeline it already made, so every other job runs twice per push. A workflow: block fixes that. The tag rule in it is load-bearing, not tidiness: without it no pipeline is created for a tag at all, which silently stops a tag-triggered release from ever shipping.

The follow-up base is a cache keyed on the merge request, not the last comment read back through the API. It is less code, and it drops the author filter the GitHub version needs: nobody but the job can write a cache, and the agent reads that file.

An unconstrained reviewer writes an essay: nobody reads the fourth paragraph, and the finding that matters sits under one that does not. Four rules fix that.

• Define each severity in the agent file, in your terms. Critical is "merging this ships something wrong". Medium is a defect in a narrower state. Low is the wording nits. Undefined labels drift upward until everything is important and the labels mean nothing.
• Keep the zero rows. Critical | 0 is the most useful cell in the table, and a table listing only what was found cannot say it.
• Cap the low ones (three is plenty) and say what gets dropped when a review runs long: low first, then medium, and never a critical. Without a drop order a length limit is met by dropping whatever is last.
• Demand a fix, not a concern. "Consider refactoring this" costs the reader more than it saves. If the agent cannot write the fix, what it has is a question. Give it somewhere to put one so it does not dress it up as a finding.

This is the one that bites. Every push triggers a fresh run, and a fresh run rescans from nothing, so it finds a different set of true things than last time. You fix what it said, it comes back with three more, and the list never empties. The review is correct every time and impossible to finish.

The fix is to hand the agent what it already said. Fetch the last review your pipeline posted, write it into the workspace, and tell the agent that file is the base:

Then four rules in the agent file, which are the whole point:

• A finding keeps the id it was given. M2 is M2 across three pushes, so one finding can be followed to its end.
• The table counts what is still open, and one line under it names what was fixed: Fixed since the last review: M1, L2. A fixed finding gets no block. Its id on that line is the whole report.
• A new finding is added only if it is critical, or if it is in code this push changed. The agent is not re-reviewing the branch; it is reporting on a list it already wrote.
• Nothing is re-worded, re-ranked or re-scoped. Medium last time is medium now.

Read every page, not just the first: the comments endpoint returns oldest-first and ignores sort and direction, so a single page of 100 silently starts following up on a stale review, and it does so on exactly the long-running merge request that needed this most.