Skip to content

fix(dispatch): route pull_request_target.labeled to review stage - #5015

Merged
ifireball merged 1 commit into
mainfrom
fix/2674-dispatch-review-bot-prs
Jul 16, 2026
Merged

fix(dispatch): route pull_request_target.labeled to review stage#5015
ifireball merged 1 commit into
mainfrom
fix/2674-dispatch-review-bot-prs

Conversation

@rh-hemartin

Copy link
Copy Markdown
Member

Closes #2674

Summary

  • Add labeled) case under pull_request_target) in both dispatch files so ready-for-review triggers review dispatch
  • Add Go test for bot-opened PR detection (ActorBot + RoleNone)
  • Add Go test documenting that the pre-CEL auth gate does not bypass write checks for bot-opened events

Problem

Bot-authored PRs (e.g. fullsend-ai-coder[bot]) never get automated review. The is_event_actor_authorized check on pull_request_target.opened calls has_write_permission, which queries the GitHub collaborator permission API. GitHub App bot accounts authenticate via installation tokens, not the collaborator model, so the API returns a non-write role and dispatch silently skips.

PR #2679 tried to fix this by having post-code.sh apply a ready-for-review label to use the ungated label dispatch path. But the shim subscribes to pull_request_target: [opened, synchronize, ready_for_review, closed] without labeled, and issues.labeled does not fire for PRs. The label was applied but never triggered dispatch.

Fix

The shim template already includes labeled in pull_request_target.types (it just hasn't been reconciled to all repos yet). This PR adds the missing routing: a labeled) case under pull_request_target) that dispatches to review when ready-for-review is applied. No explicit auth gate is needed because label application itself requires write access.

Test plan

  • go test ./internal/harnessdispatch/... passes
  • After merge + reconcile, next bot-authored PR gets a review agent comment
  • Dispatch logs show STAGE=review for pull_request_target.labeled events with ready-for-review

🤖 Generated with Claude Code

Bot-authored PRs (e.g. fullsend-ai-coder[bot]) fail the
is_event_actor_authorized check on pull_request_target.opened because
GitHub App accounts lack a collaborator role in the permission API.

PR #2679 added a ready-for-review label in post-code.sh to use the
label-based dispatch path, but pull_request_target.labeled was not
handled in the dispatch routing — only issues.labeled was. Since GitHub
does not fire issues.labeled for PRs, the label was silently ignored.

Add a labeled) case under pull_request_target) in both dispatch files
so ready-for-review triggers review dispatch. The label path needs no
explicit auth gate because label application already requires write
access.

Also add Go test coverage for bot-opened PR detection (ActorBot +
RoleNone) and document that the pre-CEL auth gate intentionally does
not bypass write checks for bot-opened events.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
@rh-hemartin
rh-hemartin requested a review from a team as a code owner July 15, 2026 13:57
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:57 PM UTC · Completed 2:11 PM UTC
Commit: 120bc50 · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix dispatch routing for pull_request_target.labeled → review stage

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Route pull_request_target.labeled ready-for-review events to the review stage.
• Keep bot PR auth behavior unchanged; label application remains the authorization boundary.
• Add Go tests for bot actor parsing and authorization fall-through.
Diagram

graph TD
  A{{"GitHub event"}} --> E["LoadGHAEvent"] --> F["IsAuthorized() gate"] --> B["Dispatch routing (yml)" ] --> C["STAGE=review"] --> D["Review stage"]
  T["Go tests"] --> E --> F
  subgraph Legend
    direction LR
    _ext{{"External"}} ~~~ _proc["Logic"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Move trusted-bot handling into Go IsAuthorized()
  • ➕ Single authorization policy surface (Go), less split-brain between bash/CEL/Go
  • ➕ Could explicitly model GitHub App installation tokens vs collaborator roles
  • ➖ Higher blast radius: changes global authorization semantics beyond label routing
  • ➖ Risk of accidentally widening privileges for bot-authored events
2. Trigger review via repository_dispatch/workflow_dispatch from post-code
  • ➕ Avoids relying on pull_request_target.labeled semantics across repos
  • ➕ Explicitly invokes the review pipeline without event-type routing gaps
  • ➖ Requires additional token/permissions management and careful validation
  • ➖ More complex operationally and harder to audit than label-based gating
3. Use pull_request (non-target) event for label routing
  • ➕ Simpler mental model; avoids some pull_request_target pitfalls
  • ➕ May reduce need for collaborator permission checks depending on usage
  • ➖ Often lacks required permissions/checkout context for privileged actions
  • ➖ May not work for forked PR scenarios depending on security constraints

Recommendation: Keep the current approach: route pull_request_target.labeled to review when ready-for-review is applied. It fixes the immediate regression with minimal privilege expansion, and it preserves the intended security boundary (only actors with write access can apply the label). The alternatives introduce broader auth policy changes or additional privileged dispatch mechanisms that are harder to reason about and audit.

Files changed (4) +61 / -0

Tests (2) +51 / -0
auth_test.goDocument bot-opened PR authorization fall-through +16/-0

Document bot-opened PR authorization fall-through

• Adds a test asserting bot-opened PR events (ActorBot + RoleNone) do not bypass the pre-CEL authorization gate. This codifies that trusted-bot handling is not implemented in 'IsAuthorized'.

internal/harnessdispatch/auth_test.go

ghaevent_test.goTest bot actor parsing for pull_request_target opened events +35/-0

Test bot actor parsing for pull_request_target opened events

• Adds a fixture-style test that loads a PR opened event with a bot sender/login. Verifies the normalized event sets 'Actor.Kind=ActorBot' and 'Actor.Role=RoleNone'.

internal/harnessdispatch/input/ghaevent_test.go

Other (2) +10 / -0
reusable-dispatch.ymlHandle pull_request_target.labeled for ready-for-review review routing +5/-0

Handle pull_request_target.labeled for ready-for-review review routing

• Adds a 'labeled)' branch under the 'pull_request_target)' action routing. When the triggering label is 'ready-for-review', the workflow sets 'STAGE=review' so review dispatch executes for PR label events.

.github/workflows/reusable-dispatch.yml

dispatch.ymlMirror labeled→review routing in scaffold dispatch workflow +5/-0

Mirror labeled→review routing in scaffold dispatch workflow

• Adds the same 'pull_request_target.labeled' routing behavior to the scaffolded repo workflow. Ensures generated repos dispatch 'ready-for-review' labeled events into the 'review' stage.

internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 61 rules

Grey Divider


Informational

1. Bot PRs still fail auth 📎 Requirement gap ☼ Reliability
Description
The PR adds/updates tests that explicitly assert bot-opened PR events are not authorized, and the
dispatch workflow still gates pull_request_target.opened|synchronize|ready_for_review on
collaborator write permission. This conflicts with the requirement to authorize fullsend GitHub App
bot actors so bot-authored PR events reliably dispatch the review stage.
Code

internal/harnessdispatch/auth_test.go[R102-116]

+func TestIsAuthorized_BotOpenedPRFallsThrough(t *testing.T) {
+	// Bot-opened PR events have no special bypass in the pre-CEL gate.
+	// Trusted-bot handling lives in bash dispatch (is_trusted_bot) and
+	// CEL trigger expressions, not here.
+	ev := &normevent.Event{
+		Transition: normevent.Transition{Kind: normevent.TransitionOpened},
+		Source:     normevent.Source{System: normevent.SystemGitHub},
+		Actor: normevent.Actor{
+			ID:   "myapp-coder[bot]",
+			Kind: normevent.ActorBot,
+			Role: normevent.RoleNone,
+		},
+	}
+	assert.False(t, IsAuthorized(ev))
+}
Relevance

⭐ Low

History (ADR0054) keeps PR opened/sync gated; bot handoffs handled via labeled implicit auth, not
bot bypass.

PR-#1688
PR-#2679
PR-#3820

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance IDs 1525850 and 1525852 require bot actors to be treated as authorized so bot-authored
pull_request_target events dispatch the review stage. The added test
TestIsAuthorized_BotOpenedPRFallsThrough asserts the opposite (bot-opened PR is not authorized),
and the dispatch routing continues to require is_event_actor_authorized for pull_request_target
opened|synchronize|ready_for_review, meaning bot PR events can still be skipped by the gate.

Authorize fullsend GitHub App bot actors for pull_request_target review dispatch
Review stage must dispatch (STAGE=review) for pull_request_target events from bot-authored PRs
internal/harnessdispatch/auth_test.go[102-116]
.github/workflows/reusable-dispatch.yml[268-279]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Bot-authored `pull_request_target` events are still treated as unauthorized (collaborator permission gate), which can prevent `STAGE=review` from being selected for bot PRs on `opened|synchronize|ready_for_review`.

## Issue Context
Compliance requires that fullsend GitHub App bot actors (e.g., `fullsend-ai-coder[bot]`) are recognized as authorized for review dispatch on `pull_request_target` events, instead of relying solely on collaborator permission checks that do not apply to GitHub App installation identities.

## Fix Focus Areas
- .github/workflows/reusable-dispatch.yml[160-163]
- .github/workflows/reusable-dispatch.yml[268-279]
- internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml[89-91]
- internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml[200-215]
- internal/harnessdispatch/auth_test.go[102-116]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

Low

  • [documentation-drift] internal/scaffold/fullsend-repo/scripts/post-code.sh:535 — The comment in post-code.sh states that the ready-for-review label triggers dispatch "via the issues.labeled path." After this PR, dispatch also handles pull_request_target.labeled for the same label. The comment is now incomplete and could mislead maintainers about which event path triggers review for bot-authored PRs.

  • [naming-inconsistency] .github/workflows/reusable-dispatch.yml — Pre-existing variable name inconsistency: reusable-dispatch.yml uses ISSUE_IS_PR (line 111) while internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml uses ISSUE_HAS_PR (line 39). Both compute the same expression and serve the same purpose. Not introduced by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dispatch: ADR 0054 authorization gate blocks review dispatch for coder-bot PRs

2 participants