Skip to content

Reviewer: point the guide's diff instructions at the file the harness writes - #55

Merged
GianniCarlo merged 1 commit into
mainfrom
reviewer/guide-diff-from-file
Oct 6, 2026
Merged

GianniCarlo merged 1 commit into
mainfrom
reviewer/guide-diff-from-file

Conversation

@GianniCarlo

Copy link
Copy Markdown
Contributor

What

review-guide.md still carried two instructions from the first-generation reviewer that the hardened harness (#40) makes impossible:

  • "Diff against origin/main": the PR checkout is shallow (fetch-depth: 1), so there is no base ref.
  • Step 1, "Get the diff: gh pr diff <number>": gh is not on the sandbox's command allowlist, and the agent holds no GitHub token.

Each review round therefore started with a denied command before the agent found the diff file the task prompt names. The guide now says what the harness actually does, matching the android and support-pipeline guides (and the iOS port in TortugaPower/BookPlayer#1611, which carried the same stale lines).

Only review-guide.md changes. Under pull_request_target, this PR is reviewed with main's current guide; the new wording takes effect on PRs opened after the merge.

Verification

node --test test/ in .github/claude/reviewer/: 247/247. The harness-tests job runs on this PR because it touches .github/claude/.

🤖 Generated with Claude Code

… writes

The guide still told the agent to run `gh pr diff` and to diff against
`origin/main`, which the hardened harness made impossible: `gh` is not on the
sandbox's command allowlist, the agent holds no GitHub token, and the PR
checkout is shallow with no base ref. The harness prefetches the diff into a
file and names it in the task prompt, so the guide now says that, matching the
android and support-pipeline guides.
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

✅ Claude PR Review — PASS

This PR changes only .github/claude/review-guide.md. It removes two steps that can't work in the review sandbox: gh pr diff and diffing against origin/main. In their place the guide tells the reviewer to read the diff file the harness writes, and notes that the checkout is the PR head only, with no gh access. I checked those claims against .github/workflows/claude-review.yml: both checkouts use fetch-depth: 1 and the event is pull_request_target, so they are accurate. No harness test or code depends on the removed wording; a grep for gh pr diff, origin/main and Diff against finds only the new line. No application code changes, so authorization scoping, auth-middleware coverage and IAP/passkey checks don't apply here.

Findings: no findings

Converged: nothing new this round, and no earlier finding is open.

Model claude-opus-5-5 · run log · 0 new · 0 carried over · 0 resolved · advisory (a human should still review). Findings are de-duplicated across pushes; an earlier finding closes only when the verification pass judges it against the current code — fixed, no longer applicable, accepted by a maintainer, or a duplicate of a finding reported on this push.

@GianniCarlo
GianniCarlo merged commit a6495c1 into main Oct 6, 2026
4 checks passed

This branch was successfully deployed

1 active deployment
reviewer — 96970b53 Deployed Oct 6, 2026 by GianniCarlo via review #110
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