Skip to content

Conversation

@derekmisler
Copy link
Contributor

@derekmisler derekmisler commented Feb 2, 2026

Add automatic PR review for Docker employees when PRs are marked as "ready for review".

Changes

  • Add pull_request_target trigger for ready_for_review and opened events
  • Add new auto-review job that:
    • Checks if the PR author is a Docker org member using the GitHub API
    • Only proceeds with the review if the author is an internal employee
    • Supports fork-based workflow

How It Works

Scenario Behavior
Docker employee opens/readies a PR ✅ Auto-review runs
External contributor opens a PR ⏭️ Skipped (not a Docker org member)
Trusted contributor comments /review ✅ Manual review runs
External contributor comments /review ❌ Blocked by cagent-action auth check

Security Considerations

  • Uses pull_request_target to access secrets for fork PRs
  • This is safe because review-pr only reads files for static analysis, it doesn't execute any code from the PR
  • Org membership check runs before any PR code is checked out
  • Requires ORG_MEMBERSHIP_TOKEN secret (classic PAT with read:org scope) (this is already set up)

@derekmisler derekmisler self-assigned this Feb 2, 2026
@derekmisler derekmisler force-pushed the auto-run-pr-reviewer-for-docker-employees branch from 86e113a to 37117ee Compare February 2, 2026 18:20
@derekmisler derekmisler marked this pull request as ready for review February 2, 2026 18:28
@derekmisler derekmisler requested a review from a team as a code owner February 2, 2026 18:28

- name: Run PR Review Team
if: steps.membership.outputs.is_member == 'true'
uses: docker/cagent-action/review-pr@latest
Copy link
Contributor

Choose a reason for hiding this comment

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

fairly minor, but should we consider pinning a sha here just so in case the upstream action gets compromised somehow we're not immediately affected? (same for anyone else using the action)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that, but it's our own action, so it seemed safe-ish?

@dgageot dgageot merged commit a41ac83 into docker:main Feb 3, 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.

3 participants