Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions scripts/dev-lead-intent.sh
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,14 @@ case "$EVENT_NAME" in
review_state=$(jq -r '.review.state // empty' "$EVENT_PATH" 2>/dev/null || true)
pr_number=$(jq -r '.pull_request.number // empty' "$EVENT_PATH" 2>/dev/null || true)
head_sha=$(jq -r '.pull_request.head.sha // empty' "$EVENT_PATH" 2>/dev/null || true)
author_assoc=$(jq -r '.pull_request.author_association // empty' "$EVENT_PATH" 2>/dev/null || true)
# Reviewer-trust gate (#1417): read the REVIEWER's association from .review,
# not the PR author's from .pull_request. On a dev-lead-authored PR the author
# is always dev-lead's own OWNER identity, so gating on the PR author made
# is_human_trusted always true and the untrusted-reviewer skip dead code.
# `.review?` + `// "" | tostring` keeps the gate fail-closed: a missing or
# null reviewer association produces an empty string that is_human_trusted
# rejects (returns false → skip).
reviewer_assoc=$(jq -r '(.review?.author_association // "" | tostring)' "$EVENT_PATH")

# Skip if actor is BOT_USER (self-review)
if [ "$reviewer" = "$BOT_USER" ]; then
Expand Down Expand Up @@ -296,7 +303,7 @@ case "$EVENT_NAME" in
else
emit_intent "fix-reviews" "bot-review-${review_state}" "$context"
fi
elif is_human_trusted "$author_assoc"; then
elif is_human_trusted "$reviewer_assoc"; then
emit_intent "review-changes" "human-review-${review_state}" "$context"
else
emit_skip "untrusted-reviewer"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"id": 1004,
"state": "CHANGES_REQUESTED",
"body": "Please refactor the authentication logic and add more tests.",
"author_association": "OWNER",
"user": {
"login": "donpetry",
"type": "User"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"_test_expected_intent": "skip",
"_test_note": "Reviewer-trust gate fail-closed case (#1417, AC #4): PR author held at OWNER, human reviewer with NO .review.author_association field. A missing/unparseable reviewer association must be treated as untrusted → skip untrusted-reviewer, never as trusted.",
"action": "submitted",
"review": {
"id": 5004,
"state": "CHANGES_REQUESTED",
"body": "Missing association on this review.",
"user": {
"login": "unknown-reviewer",
"type": "User"
}
},
"pull_request": {
"number": 1417,
"title": "fix: reviewer-trust gate",
"state": "open",
"author_association": "OWNER",
"user": {
"login": "donpetry-bot",
"type": "Bot"
},
"head": {
"sha": "aaa111bbb222",
"ref": "dev-lead/issue-1417-20260802-0956",
"repo": { "full_name": "petry-projects/.github-private" }
},
"base": {
"ref": "main",
"repo": { "full_name": "petry-projects/.github-private" }
}
},
"repository": { "full_name": "petry-projects/.github-private" },
"sender": { "login": "unknown-reviewer", "type": "User" }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"_test_expected_intent": "fix-reviews",
"_test_note": "Reviewer-trust gate (#1417): PR author held at OWNER, reviewer is a trusted bot whose review association is NONE. The bot path keys on the reviewer login, not the association, so it still routes fix-reviews — no regression from the association fix.",
"action": "submitted",
"review": {
"id": 5003,
"state": "CHANGES_REQUESTED",
"body": "Several issues need to be addressed.",
"author_association": "NONE",
"user": {
"login": "gemini-code-assist[bot]",
"type": "Bot"
}
},
"pull_request": {
"number": 1417,
"title": "fix: reviewer-trust gate",
"state": "open",
"author_association": "OWNER",
"user": {
"login": "donpetry-bot",
"type": "Bot"
},
"head": {
"sha": "aaa111bbb222",
"ref": "dev-lead/issue-1417-20260802-0956",
"repo": { "full_name": "petry-projects/.github-private" }
},
"base": {
"ref": "main",
"repo": { "full_name": "petry-projects/.github-private" }
}
},
"repository": { "full_name": "petry-projects/.github-private" },
"sender": { "login": "gemini-code-assist[bot]", "type": "Bot" }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"_test_expected_intent": "review-changes",
"_test_note": "Reviewer-trust gate (#1417): PR author held at OWNER, reviewer association COLLABORATOR → the reviewer is a trusted human, so route review-changes. Distinguishes the reviewer's association from the PR author's.",
"action": "submitted",
"review": {
"id": 5001,
"state": "CHANGES_REQUESTED",
"body": "Please tighten the error handling here.",
"author_association": "COLLABORATOR",
"user": {
"login": "trusted-maintainer",
"type": "User"
}
},
"pull_request": {
"number": 1417,
"title": "fix: reviewer-trust gate",
"state": "open",
"author_association": "OWNER",
"user": {
"login": "donpetry-bot",
"type": "Bot"
},
"head": {
"sha": "aaa111bbb222",
"ref": "dev-lead/issue-1417-20260802-0956",
"repo": { "full_name": "petry-projects/.github-private" }
},
"base": {
"ref": "main",
"repo": { "full_name": "petry-projects/.github-private" }
}
},
"repository": { "full_name": "petry-projects/.github-private" },
"sender": { "login": "trusted-maintainer", "type": "User" }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"_test_expected_intent": "skip",
"_test_note": "Reviewer-trust gate regression guard (#1417): PR author held at OWNER while reviewer association is CONTRIBUTOR (untrusted). Under the pre-fix code (which read .pull_request.author_association == OWNER) this routed review-changes; the fixed gate reads .review.author_association == CONTRIBUTOR and emits skip untrusted-reviewer.",
"action": "submitted",
"review": {
"id": 5002,
"state": "CHANGES_REQUESTED",
"body": "I think this needs work.",
"author_association": "CONTRIBUTOR",
"user": {
"login": "outside-contributor",
"type": "User"
}
},
"pull_request": {
"number": 1417,
"title": "fix: reviewer-trust gate",
"state": "open",
"author_association": "OWNER",
"user": {
"login": "donpetry-bot",
"type": "Bot"
},
"head": {
"sha": "aaa111bbb222",
"ref": "dev-lead/issue-1417-20260802-0956",
"repo": { "full_name": "petry-projects/.github-private" }
},
"base": {
"ref": "main",
"repo": { "full_name": "petry-projects/.github-private" }
}
},
"repository": { "full_name": "petry-projects/.github-private" },
"sender": { "login": "outside-contributor", "type": "User" }
}
52 changes: 52 additions & 0 deletions tests/dev-lead/unit/test_intent_reviews.bats
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,58 @@ EOF
[ "$(_get_env INTENT_TYPE)" = "fix-reviews" ]
}

# ── reviewer-trust gate: gate on the REVIEWER's association (#1417) ───────────
# The pull_request_review branch must evaluate the reviewer's association
# (.review.author_association), NOT the PR author's (.pull_request.author_association).
# Every fixture below holds .pull_request.author_association at OWNER (as it always
# is on a dev-lead-authored PR) and varies only .review.author_association, so each
# assertion would fail against the pre-fix code that read the PR author's field.

@test "reviews: reviewer-trust — trusted-human reviewer (COLLABORATOR) → review-changes" {
export GITHUB_EVENT_NAME="pull_request_review"
export GITHUB_EVENT_PATH="$FIXTURES_DIR/pr_review_reviewer_trusted_human.json"

run bash "$INTENT_SCRIPT"

[ "$status" -eq 0 ]
[ "$(_get_env INTENT_TYPE)" = "review-changes" ]
}

@test "reviews: reviewer-trust — untrusted reviewer (CONTRIBUTOR) on OWNER-authored PR → skip untrusted-reviewer" {
# Regression guard (#1417): under the pre-fix code this read .pull_request.author_association
# (OWNER) and routed review-changes — the untrusted-reviewer skip was dead code. The fixed
# gate reads .review.author_association (CONTRIBUTOR) and reaches the skip.
export GITHUB_EVENT_NAME="pull_request_review"
export GITHUB_EVENT_PATH="$FIXTURES_DIR/pr_review_reviewer_untrusted.json"

run bash "$INTENT_SCRIPT"

[ "$status" -eq 0 ]
[ "$(_get_env INTENT_TYPE)" = "skip" ]
[ "$(_get_env INTENT_REASON)" = "untrusted-reviewer" ]
}

@test "reviews: reviewer-trust — trusted bot (assoc NONE) → fix-reviews (bot path ignores assoc)" {
export GITHUB_EVENT_NAME="pull_request_review"
export GITHUB_EVENT_PATH="$FIXTURES_DIR/pr_review_reviewer_trusted_bot.json"

run bash "$INTENT_SCRIPT"

[ "$status" -eq 0 ]
[ "$(_get_env INTENT_TYPE)" = "fix-reviews" ]
}

@test "reviews: reviewer-trust — missing reviewer association fails closed → skip untrusted-reviewer" {
export GITHUB_EVENT_NAME="pull_request_review"
export GITHUB_EVENT_PATH="$FIXTURES_DIR/pr_review_reviewer_missing_assoc.json"

run bash "$INTENT_SCRIPT"

[ "$status" -eq 0 ]
[ "$(_get_env INTENT_TYPE)" = "skip" ]
[ "$(_get_env INTENT_REASON)" = "untrusted-reviewer" ]
}

# ── pull_request_review_comment tests ────────────────────────────────────────

@test "reviews: pull_request_review_comment codex → fix-reviews" {
Expand Down
Loading