Skip to content

ci(review): gate the on-open review on author_association, not the members API - #675

Closed
Eldad-Caura wants to merge 1 commit into
mainfrom
ci/review-gate-author-association
Closed

Eldad-Caura wants to merge 1 commit into
mainfrom
ci/review-gate-author-association

Conversation

@Eldad-Caura

Copy link
Copy Markdown
Member

The on-open review has been skipping every human pull request in this repo. Reviews only ever ran
when someone commented @claude.

The bug

Rule 1 called orgs/caura-ai/members/{user} at runtime. That endpoint answers for its caller, and
GITHUB_TOKEN sees only public members — it returns 404 for a private member exactly as for a
stranger. Every maintainer's caura-ai membership is private:

GET /orgs/caura-ai/public_members/Eldad-Caura  ->  404
GET /orgs/caura-ai/public_members/erni-a       ->  404

So the gate skipped, and said so on the PRs: #673 "PR author 'Eldad-Caura' is not a public member",
#524 the same for erni-a, #672 for caura-deploy-bot[bot].

This is easy to miss two ways. Checking the endpoint by hand returns 204, because a personal token
can always see your own membership. And the @claude path kept working throughout — it already read
author_association — so the pipeline never looked broken.

The comment that justified it was wrong

It claimed a private member's author_association surfaces as NONE. Live payloads from this repo:

PR author author_association
#673 Eldad-Caura (private member) MEMBER
#524 erni-a (private member) MEMBER
#577 Souptik96 (outsider) FIRST_TIME_CONTRIBUTOR

Membership visibility does not affect the field, and it still discriminates outsiders correctly. So
requiring public org membership to get a code review was never a deliberate policy — it fell out of
that wrong belief.

The fix

Rule 1 now reads author_association, accepting MEMBER and OWNER — the same field and the same
values claude-retrigger already uses. One mechanism for one policy; two ways of deciding "is this an
org member" is precisely the drift this pipeline exists to remove.

Behaviour otherwise preserved. COLLABORATOR stays excluded (the policy is "our org", stricter
than repo write access). Bots aren't MEMBER, so they stay skipped regardless of rule 4. An absent
field skips, so the gate remains fail-closed.

The trade, stated: author_association is computed when the event fires, not when the review runs,
so a member removed from the org in between is still honoured — reopening recomputes it. The
alternative reflected review time but didn't work at all, and retrigger already accepts this trade.

Verified

Gate behaviour across all nine values GitHub can send:

MEMBER, OWNER                                          -> review
COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR,
FIRST_TIMER, NONE, MANNEQUIN, unset                    -> skip

Full-repo actionlint output is byte-identical to main — five pre-existing findings, none
introduced. (The HTTP_CODE warning moves from line 266 to 292 only because my comment shifted the
line numbers.)

Also in here

Three comments repeated the NONE claim — the file header, rule 1, and the retrigger step. All
corrected, because the false claim is what produced the bug; leaving it in place would invite the same
"fix" again. Drops the now-unused ORG env var.

One thing this does not fix

caura-ai/.github's shared pipeline has the same API-based author-gate. It is dormant today —
auto disables it on private repos and all six consumers are private — but making any of them public
would reproduce this exact silent skip. Its input description is at least honest about the limitation
("an org member visible to GITHUB_TOKEN — in practice a PUBLIC member"), so it is a latent trap rather
than an active bug. Worth the same change, separately.

…mbers API

The on-open review has been skipping every human pull request in this repo.
Reviews only ever ran when someone commented @claude.

Rule 1 called `orgs/caura-ai/members/{user}` at runtime. That endpoint answers
for its CALLER, and GITHUB_TOKEN sees only PUBLIC members — it returns 404 for a
private member exactly as for a stranger. Every maintainer's caura-ai membership
is private:

  GET /orgs/caura-ai/public_members/Eldad-Caura -> 404
  GET /orgs/caura-ai/public_members/erni-a      -> 404

so the gate skipped, and said so: #673 "PR author 'Eldad-Caura' is not a public
member", #524 the same for erni-a, #672 for caura-deploy-bot[bot]. The @claude
path kept working the whole time because it already read author_association.
Checking the endpoint by hand hides this — a personal token returns 204, because
you can always see your own membership.

The comment that justified the API call was wrong. It claimed a private member's
author_association surfaces as NONE. Live payloads in this repo:

  #673 Eldad-Caura  MEMBER    (private member)
  #524 erni-a       MEMBER    (private member)
  #577 Souptik96    FIRST_TIME_CONTRIBUTOR

Membership visibility does not affect the field, and it still discriminates
outsiders. So requiring public org membership to get a code review was never a
deliberate policy — it was a consequence of that wrong belief.

Rule 1 now reads author_association and accepts MEMBER and OWNER: the same
field and the same values as claude-retrigger. One mechanism for one policy —
two ways of deciding "is this an org member" is the drift this pipeline exists
to remove.

Behaviour otherwise preserved. COLLABORATOR stays excluded, deliberately, since
the policy is "our org" rather than repo write access. Bots are not MEMBER, so
they stay skipped regardless of rule 4. An absent field skips, so the gate is
still fail-closed.

The trade, stated rather than buried: author_association is computed when the
event fires, not when the review runs, so a member removed from the org in
between is still honoured (reopening recomputes it). The alternative reflected
review time but did not work at all, and retrigger already accepts this trade.

Three comments repeated the NONE claim — the file header, rule 1, and the
retrigger step. All corrected, because the false claim is what produced the bug;
leaving it would invite the same "fix" again. Also drops the now-unused ORG env.

Verified: the gate across all nine association values GitHub can send (MEMBER
and OWNER review; COLLABORATOR, CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR,
FIRST_TIMER, NONE, MANNEQUIN and unset skip), and full-repo actionlint output
identical to main — five pre-existing findings, none introduced.

Signed-off-by: eldad-caura <eldad@caura.ai>
@github-actions

Copy link
Copy Markdown
Contributor

Claude Code Review — skipped: PR author 'Eldad-Caura' is not a caura-ai org member (author_association: CONTRIBUTOR)

@Eldad-Caura
Eldad-Caura marked this pull request as draft July 30, 2026 10:22
@Eldad-Caura

Copy link
Copy Markdown
Member Author

Converted to draft — this fix does not work, and CI caught it on its own PR.

The gate ran and skipped, with the new reason string:

Claude Code Review — skipped: PR author 'Eldad-Caura' is not a caura-ai org member
(author_association: CONTRIBUTOR)

Not MEMBER. So author_association does not report org membership for a private member on a
pull_request payload, and the premise of this PR is wrong.

Where I went wrong

I verified author_association by reading the REST API:

gh api repos/caura-ai/caura-memclaw/pulls/673 --jq .author_association   ->  MEMBER
github.event.pull_request.author_association (in CI, #675)               ->  CONTRIBUTOR

Two different sources for the same field, same user, same repo — and the workflow reads the second.
The REST call is evaluated for the authenticated caller, and my token can see my own private
membership; the webhook payload is not. I verified against a context that isn't the one that runs,
which is the same error as testing shell behaviour on bash 3.2 when CI runs 5.2.

The comment I deleted was substantively right. It said the payload field doesn't reflect private
membership; it was wrong only about the exact value (CONTRIBUTOR, not NONE). I removed a correct
warning because I'd "disproved" it with the wrong instrument. That's worse than leaving it alone, and
the reason it was there is now obvious.

What does hold is that the @claude path works for private members — confirmed by the maintainer's
own experience, and that path gates on the issue_comment payload, not the pull_request one. So the
two event types genuinely differ here. I'm not going to assert why without evidence.

Where that leaves the actual problem

The on-open review is still skipped for every human pull request, and neither the old gate nor this
one fixes it. Two viable routes:

  1. Make org membership public (Organization → People → your row → Public). The original API gate
    then works as designed, with no code change at all. I argued against this earlier on the strength of
    the verification that just fell over — I withdraw that. The disclosure is a public members list and
    an org badge on your profile; the real risk is spear-phishing target-listing, and it is modest here
    because this repo is public and both maintainers already author and merge in it, so the affiliation
    is already in public commit history.
  2. Gate on repo permission insteadGET /repos/{owner}/{repo}/collaborators/{user}/permission,
    which resolves access granted via org membership regardless of visibility, and which GITHUB_TOKEN
    can read for its own repo. Two caveats: it widens the policy from "org member" to "has write access
    here", which pulls in outside collaborators that this workflow deliberately excludes; and I would
    have to verify what GITHUB_TOKEN actually returns in CI, not locally, since that is exactly
    the distinction I just got wrong.

Accepting CONTRIBUTOR is not an option — on a public repo that would let anyone with a merged commit
trigger paid reviews.

I'd take (1): no code, no new failure mode, and it restores the gate the way it was designed. If you'd
rather not, say so and I'll build (2) with a CI probe first and no guessing.

The spend ceiling in #673 is unaffected by any of this and still stands on its own.

@github-actions

Copy link
Copy Markdown
Contributor

The workflow was blocked at the permission gate. I'll deliver the review directly from my analysis of the diff.


Summary

This PR fixes a broken org-membership gate by replacing a runtime gh api orgs/.../members/{user} call with a check on github.event.pull_request.author_association. The fix is correct: the old API call used GITHUB_TOKEN, which can only see public org members, so every private member was silently treated as a non-member and skipped. The new approach is simpler, consistent with the retrigger path, and matches actual GitHub behavior for private members.

Medium/Low Issues

Misleading actor name in skip reason on reopened events

Severity: Low
File: .github/workflows/claude_code_review.yml:140 (the new reason= echo line)
Problem: On a pull_request reopened event triggered by an admin, PR_ACTOR (github.actor) is the admin who reopened the PR, while PR_AUTHOR_ASSOC is always the original PR author's association — the reason message logs the admin's name as "PR author … is not a caura-ai org member", which is factually wrong.

🤖 Claude Code Prompt
In .github/workflows/claude_code_review.yml, in the `check` job's step that sets PR_AUTHOR_ASSOC,
the skip reason message reads:

  echo "reason=PR author '${PR_ACTOR}' is not a caura-ai org member (author_association: ${PR_AUTHOR_ASSOC:-unset})"

PR_ACTOR is github.actor — whoever triggered the event. On a `reopened` action triggered by an
admin on a non-member's PR, github.actor is the admin, not the PR author. The author_association
check is correctly on the PR author's field, but the logged name is the admin's.

Fix: capture the PR author username separately from github.actor and use it in the reason message.
Add a new env var:
  PR_AUTHOR: ${{ github.event.pull_request.user.login }}

Then change the reason line to:
  echo "reason=PR author '${PR_AUTHOR}' is not a caura-ai org member (author_association: ${PR_AUTHOR_ASSOC:-unset})"

This makes the logged name and the checked association consistent regardless of who triggered the event.

🚀 Fix All Prompts

🟡 Medium/Low

In .github/workflows/claude_code_review.yml, in the `check` job's step that sets PR_AUTHOR_ASSOC,
the skip reason message reads:

  echo "reason=PR author '${PR_ACTOR}' is not a caura-ai org member (author_association: ${PR_AUTHOR_ASSOC:-unset})"

PR_ACTOR is github.actor — whoever triggered the event. On a `reopened` action triggered by an
admin on a non-member's PR, github.actor is the admin, not the PR author. The author_association
check is correctly on the PR author's field, but the logged name is the admin's.

Fix: capture the PR author username separately from github.actor and use it in the reason message.
Add a new env var:
  PR_AUTHOR: ${{ github.event.pull_request.user.login }}

Then change the reason line to:
  echo "reason=PR author '${PR_AUTHOR}' is not a caura-ai org member (author_association: ${PR_AUTHOR_ASSOC:-unset})"

This makes the logged name and the checked association consistent regardless of who triggered the event.

Reviewed by claude-sonnet-4-6 · cost $0.24273035

@Eldad-Caura

Copy link
Copy Markdown
Member Author

Closing. The premise was wrong and the actual fix has landed elsewhere — org membership visibility.
orgs/caura-ai/members/{user} now resolves for the maintainer who changed it, so the original gate
works as designed and no code change is needed.

Keeping this closed rather than reworked, because what it proposed was worse than what it replaced: it
deleted a warning that was substantively correct.

The record, so this is not rediscovered:

pull_request payload   github.event.pull_request.author_association  ->  CONTRIBUTOR  (private member)
issue_comment payload  github.event.comment.author_association       ->  MEMBER       (private member)
REST API               GET /repos/../pulls/N  .author_association    ->  MEMBER       (caller-dependent)
members API            GET /orgs/../members/U with GITHUB_TOKEN      ->  404          (private member)

Two traps in there. The pull_request and issue_comment payloads disagree for the same person, which
is why @claude kept working while the on-open review did not. And the REST API's value is computed
for the authenticated caller, so a personal token reports MEMBER where CI sees CONTRIBUTOR — it
cannot be used to validate a gate that reads the payload. That is the mistake that produced this PR.

The operational requirement is real and worth stating plainly: this gate requires public org
membership. A member whose org membership is private is skipped exactly like a stranger, silently, with
a green check.

@github-actions

Copy link
Copy Markdown
Contributor

Claude Code Review ✅ No issues found.


Reviewed by claude-sonnet-4-6 · cost $0.08639300000000001

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