Tell the reviewer which repository it is reading - #98
Conversation
Reviews on kai-desktop#288 and kai-desktop#300 (2026-09-08) each stated
the repository they were reading. Both were wrong — "the kai-engine repo"
and "the kai-server working tree" — and nothing in either run could have
told them otherwise.
Three facts, and together they make the mistake inevitable:
1. The CI job clones into `mktemp -d`, so the workspace the agent runs
in is a path like /tmp/tmp.aBc123. The directory name carries nothing.
2. The prompt is AUTHOR CONTEXT, CHANGED SYMBOLS, WHERE THESE LIVE,
INTENT, DIFF. The repository is named in none of them, and
rcReviewSystem does not name it either.
3. rcReviewSystem REQUIRES one in the output: "name the boundary you
actually searched and put that boundary in the sentence — 'within
this repo, the only caller is X' is honest".
So the instructions demand an answer the input withholds, from a process
running in an anonymous directory. The model did the only thing left and
supplied a plausible sibling from the same ecosystem. It could not have
recovered by looking, either: the run is ReadOnly with no bash, and
finding out its own name should not cost a turn from a budget that
already runs out.
rcRepoIdentity resolves the slug the way resolveGitHubClient already does
(autofix_cmd.go), so there is one answer to "which GitHub repo am I in"
rather than two: GITHUB_REPOSITORY_FULLNAME first — the review workflow
prefers it for exactly the case where the kai org name and the GitHub org
name differ — then GITHUB_REPOSITORY, then the checkout's own origin
remote, which is what makes this work for a human running review-commit
locally.
The header names the repo twice on purpose: once as a fact, and once
inside the boundary sentence the system prompt asks the model to write.
Naming it only at the top is how it kept getting ignored.
Empty in, empty out. When nothing resolves the header is omitted entirely
rather than guessed: an unnamed boundary is recoverable — the reviewer
says "this repo" and the reader knows which PR they are on — while a
confidently wrong one is not. Inventing a name here would rebuild the
defect.
This does not fix the anonymous clone directory; that is a kai-server
workflow change and a separate risk.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
jschatz1 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Kai review
Kai Summary
Read through this one. One thing worth your eyes before it merges. 👇
Scope: the kai-cli repo (this checkout, last commit 07c1a57), reviewing rcRepoIdentity/rcRepoHeader in cmd/kai/review_commit.go and the new tests. The one thing I could not read is the kai-server review workflow that actually invokes kai review-commit in CI — it is not in this repo, and its behavior is load-bearing for the precedence claim (below).
The rest of the read-through
What the change does: it resolves the repository under review as an owner/name slug and prepends it to the review agent's prompt as the named boundary rcReviewSystem asks for. The resolution order is GITHUB_REPOSITORY_FULLNAME, then GITHUB_REPOSITORY, then the checkout's origin remote via the existing gitio.RemoteURL + autofix.RepoSlugFromRemote. When nothing resolves, both functions return empty and the header is omitted entirely. This is a clean, well-scoped change that does exactly what it claims.
Concerns:
-
cmd/kai/review_commit.go:588—rcRepoIdentityprefersGITHUB_REPOSITORY_FULLNAMEoverGITHUB_REPOSITORY, butGITHUB_REPOSITORY_FULLNAMEis not a standard GitHub Actions variable (Actions setsGITHUB_REPOSITORYtoowner/repo; there is no documented_FULLNAMEcounterpart). The author's stated reason — "the CI workflow prefers it for exactly the case where the kai org name and the GitHub org name differ" — only holds if the kai-server review workflow explicitly exports that variable. I grepped every workflow in this repo (.github/workflows/*.yml) and none sets it; the review workflow is generated in kai-server and is outside my reach, so I could not confirm it sets the variable. This is not fatal: the change degrades gracefully toGITHUB_REPOSITORY(which Actions always sets) and then the origin remote, so the feature still works — but if the workflow does not setFULLNAME, the org-divergence case the author cites is not actually handled and the precedence is inert. The author should confirm against the kai-server workflow before trusting the precedence reasoning. -
cmd/kai/review_commit_repo_test.go:43—TestRepoIdentityReadsTheOriginRemoteskips witht.Skipfwhengitis unavailable. That is a reasonable guard for a git-shell-out test and git is present in CI, so it is not a coverage hole in practice; noting it only so the skip does not silently become the path the CI takes if the runner image changes.
Decision (not a defect): the change prefers a non-standard env var (GITHUB_REPOSITORY_FULLNAME) over the standard GITHUB_REPOSITORY, deliberately diverging from the existing resolveGitHubClient (autofix_cmd.go:131) it claims to mirror — that function checks only GITHUB_REPOSITORY. This affects the repo name printed in every CI review and depends on an external workflow the author must confirm populates it; if that workflow ever stops setting it, reviews silently fall back to GITHUB_REPOSITORY with no error, and the org-divergence case is lost quietly.
The tests are real and would fail on wrong code: TestRepoIdentityPrefersTheWorkflowsOwnAnswer asserts FULLNAME wins, FallsBackToGithubRepository asserts the fallback, StaysSilentWhenItCannotTell asserts empty-in/empty-out for both functions, and NamesTheBoundary asserts the header contains the boundary sentence and ends with \n\n. Each asserts behaviour, not mechanism.
Solid work; merge it once someone confirms the kai-server review workflow sets GITHUB_REPOSITORY_FULLNAME (or accepts that the precedence is inert without it).
Important files changed
| File | Change |
|---|---|
cmd/kai/review_commit.go |
modified · +58 −0 |
cmd/kai/review_commit_repo_test.go |
modified · +85 −0 |
+143 −0 · 2 files · reaches 10 · the full analysis
💬 Reply to any of my comments and I'll answer, or say @kaicontext anywhere on this PR — a question, or "take another look at the retry logic".
| // Returns "" when nothing resolves. The caller then says nothing rather than | ||
| // guessing, which is the whole point. | ||
| func rcRepoIdentity(dir string) string { | ||
| for _, env := range []string{"GITHUB_REPOSITORY_FULLNAME", "GITHUB_REPOSITORY"} { |
There was a problem hiding this comment.
prefers GITHUB_REPOSITORY_FULLNAME, a non-standard GitHub Actions variable; no workflow in this repo sets it, and the one that does is in kai-server and out of reach, so the precedence is unverified (though it degrades gracefully to GITHUB_REPOSITORY).
Reviews on kai-desktop#288 and kai-desktop#300 (2026-09-08) each stated the repository they were reading. Both were wrong — "the kai-engine repo" and "the
kai-serverworking tree" — and nothing in either run could have told them otherwise.Three facts that make the mistake inevitable
mktemp -d. The workspace the agent runs in is a path like/tmp/tmp.aBc123. The directory name carries nothing.rcRunReviewAgentbuildsAUTHOR CONTEXT,CHANGED SYMBOLS,WHERE THESE LIVE,INTENT,DIFF. The repository appears in none of them, andrcReviewSystemdoes not supply it either.rcReviewSystemrequires one in the output:The instructions demand an answer the input withholds, from a process running in an anonymous directory. The model did the only thing left and supplied a plausible sibling from the same ecosystem — on kai-desktop, a Go repo that imports kai-engine, it guessed kai-engine.
It could not have recovered by looking, either: the run is
ReadOnlywithEnableBash: false, and finding out its own name should not cost a turn from a budget that already runs out.The fix
rcRepoIdentityresolves the slug the wayresolveGitHubClientalready does (autofix_cmd.go:131), so this repo has one answer to "which GitHub repo am I in" rather than two:GITHUB_REPOSITORY_FULLNAME→GITHUB_REPOSITORY→ the checkout's ownoriginremote.The first is preferred because the review workflow itself prefers it, for exactly the case where the kai org name and the GitHub org name differ ("kai" vs "kaicontext"). The remote fallback is what makes this work for a human running
review-commitlocally, with no GitHub environment at all.The repo is named twice on purpose: once as a fact, and once inside the boundary sentence the system prompt asks the model to write. Naming it only at the top is how it kept getting ignored.
Empty in, empty out
When nothing resolves the header is omitted entirely rather than guessed. An unnamed boundary is recoverable — the reviewer says "this repo" and the reader knows which PR they are on — while a confidently wrong one is not. Inventing a name here would rebuild the exact defect.
Not in scope
This does not fix the anonymous clone directory. Cloning into
$(mktemp -d)/<repo>would give the workspace a meaningful basename as defense in depth, but that is a kai-server workflow change touching the review job's clone path, and it carries more risk than a prompt line.Verified
Five tests: both env vars and their precedence, the
originremote path (realgit init+git remote add, skipped if git is unavailable), the silent case in a non-repo with no environment, and the header naming the boundary.go test ./cmd/...green (exit 0).🤖 Generated with Claude Code
Answering the review's open question
Kai's review flagged that
GITHUB_REPOSITORY_FULLNAMEis not a standard GitHub Actions variable, that no workflow in this repo sets it, and that it could not verify the claim from here. All three are correct, and it was right to say so rather than assume.The answer is that it is not a workflow variable at all — the kai runner injects it into every job pod:
So it is present in the environment
rcRepoIdentityreads, alongsideGITHUB_TOKEN,GITHUB_HEAD_REFand the rest of the injected set. Both review and agent workflows already consume it the same way (CLONE_REPO="${GITHUB_REPOSITORY_FULLNAME:-$GITHUB_REPOSITORY}"), which is the precedent this follows.One correction to the wording above: the workflow consumes it, the runner sets it.
And the failure mode if it were ever unset is the benign one — it falls through to
GITHUB_REPOSITORY, then to theoriginremote.