Repository navigation
OSAC-2001: add /review-ep comment trigger for EP review workflow - #94
Conversation
|
@ItzikEzra-rh: This pull request references OSAC-2001 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe ep-review.yml workflow adds workflow_dispatch and issue_comment triggers alongside pull_request_target, introduces a resolve-pr job that computes pr_number and head_sha per event type, updates concurrency grouping, changes checkout ref to main, and sources review job env vars from resolve-pr outputs. ChangesEP Review Workflow Trigger Expansion
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Trigger as GitHub Event
participant Workflow as ep-review.yml
participant ResolvePR as resolve-pr job
participant GH as gh CLI / GitHub API
participant Review as review job
Trigger->>Workflow: pull_request_target / workflow_dispatch / issue_comment
Workflow->>ResolvePR: run resolve-pr job
alt event is pull_request_target
ResolvePR->>ResolvePR: read pr_number and head_sha from event fields
else event is workflow_dispatch or issue_comment
ResolvePR->>GH: gh pr view --json headRefOid
GH-->>ResolvePR: pr_number, head_sha
end
ResolvePR-->>Review: outputs pr_number, head_sha
Review->>Review: checkout ref main
Review->>Review: run ep_review.py with PR_NUMBER, PR_HEAD_SHA
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ep-review.yml:
- Around line 35-47: The Acknowledge comment step in the resolve-pr job is
missing the permission required to create an issue-comment reaction. Update the
job-level permissions in ep-review.yml to keep least privilege while adding only
the narrow write access needed for the gh api call in Acknowledge comment, and
leave the existing pull-requests permission unchanged.
- Around line 20-22: Move the concurrency control from the top-level workflow
into the review job so only actual EP review runs share the cancellation group.
Update the review job in the workflow to use the same group key currently built
from github.event.pull_request.number, github.event.inputs.pr_number, or
github.event.issue.number, and remove the workflow-level concurrency so a plain
issue_comment event cannot cancel an in-progress review before resolve-pr is
skipped.
- Line 82: The checkout step is hard-coded to main, so release-branch PRs will
use the wrong base. Update the workflow to use the PR base branch resolved by
resolve-pr, and thread that base ref into the checkout step instead of the
static ref. Make sure the existing resolve-pr output is used consistently
wherever the base branch is needed, especially around the checkout
configuration.
- Around line 49-69: The Resolve PR context step is interpolating
github.event.inputs.pr_number directly inside the run script, which can allow
shell breakout in the workflow_dispatch branch. Move the PR input into an env
variable for the Resolve PR context step, quote the value when assigning PR, and
validate that it is a numeric pull request number before calling gh pr view;
keep the existing pr_number and head_sha outputs in the same resolve step.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: db7ead85-1f7b-4ec1-89ca-afafc47f39fe
📒 Files selected for processing (1)
.github/workflows/ep-review.yml
7794eba to
329f8c4
Compare
Add workflow_dispatch (PR number input) and issue_comment (/review-ep) triggers so the EP review agent can run on existing PRs, not just new ones. Extracts PR resolution into a dedicated job that all triggers feed into. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Itzik Ezra <iezra@redhat.com>
329f8c4 to
9211474
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eranco74, ItzikEzra-rh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
issue_commenttrigger on/review-ep— comment on any PR to run the EP review agent on demandresolve-prjob that normalizes PR number/SHA/base-ref across both trigger types/review-epcomments for UX feedbackreviewjob to avoid comment-triggered cancellation racesenv:vars to prevent shell injectionep_review.py— it already takesPR_NUMBERfrom envHow to use
From a PR comment: Comment
/review-epon any PRAutomatic (unchanged): PRs touching
prd.mdordesign.mdstill trigger automaticallyPart of OSAC-2001
Assisted-by: Claude Code noreply@anthropic.com