Skip to content

Tell the reviewer which repository it is reading - #98

Merged
jschatz1 merged 1 commit into
mainfrom
fix/review-knows-its-repo
Sep 9, 2026
Merged

Tell the reviewer which repository it is reading#98
jschatz1 merged 1 commit into
mainfrom
fix/review-knows-its-repo

Conversation

@jschatz1

@jschatz1 jschatz1 commented Sep 9, 2026

Copy link
Copy Markdown
Member

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 that make the mistake inevitable

  1. The CI job clones into mktemp -d. The workspace the agent runs in is a path like /tmp/tmp.aBc123. The directory name carries nothing.
  2. The prompt never names the repo. rcRunReviewAgent builds AUTHOR CONTEXT, CHANGED SYMBOLS, WHERE THESE LIVE, INTENT, DIFF. The repository appears in none of them, and rcReviewSystem does not supply it either.
  3. rcReviewSystem requires one in the output:

Before writing a universal, name the boundary you actually searched and put that boundary in the sentence: "within this repo, the only caller is X" is honest.

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 ReadOnly with EnableBash: false, and finding out its own name should not cost a turn from a budget that already runs out.

The fix

rcRepoIdentity resolves the slug the way resolveGitHubClient already does (autofix_cmd.go:131), so this repo has one answer to "which GitHub repo am I in" rather than two:

GITHUB_REPOSITORY_FULLNAMEGITHUB_REPOSITORY → the checkout's own origin remote.

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-commit locally, with no GitHub environment at all.

REPOSITORY: kaicontext/kai-desktop
(The repository under review. Every path below is relative to its root. This is the
boundary to name when you write one — "within kaicontext/kai-desktop, the only caller
is X". Do not name a different repository as the one you are reading; sibling repos
you cannot see here are exactly the limit worth stating.)

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 origin remote path (real git 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_FULLNAME is 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:

// kailab-control/internal/runner/executor.go:2621
corev1.EnvVar{Name: "GITHUB_REPOSITORY_FULLNAME", Value: githubFullName},

So it is present in the environment rcRepoIdentity reads, alongside GITHUB_TOKEN, GITHUB_HEAD_REF and 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 the origin remote.

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>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jschatz1 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3a39279b-b599-463f-8a5b-37a6250d28cb


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kaicontext kaicontext Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:588rcRepoIdentity prefers GITHUB_REPOSITORY_FULLNAME over GITHUB_REPOSITORY, but GITHUB_REPOSITORY_FULLNAME is not a standard GitHub Actions variable (Actions sets GITHUB_REPOSITORY to owner/repo; there is no documented _FULLNAME counterpart). 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 to GITHUB_REPOSITORY (which Actions always sets) and then the origin remote, so the feature still works — but if the workflow does not set FULLNAME, 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:43TestRepoIdentityReadsTheOriginRemote skips with t.Skipf when git is 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".

Comment thread cmd/kai/review_commit.go
// 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"} {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@jschatz1
jschatz1 merged commit 1e4c17f into main Sep 9, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant