Repository navigation
OSAC-1774: Add GitHub Action for automated EP review via agentic-ci - #89
Conversation
Adds a GitHub Action that triggers on PRs containing prd.md or design.md. Uses agentic-ci with Podman backend to run Claude Code review against the prd-review or ep-review skill from osac-workspace. Files: - .github/workflows/ep-review.yml — workflow triggered on PR events - .github/scripts/ep_review.py — entry point: detects skill, runs review - .github/scripts/ep_hooks.py — agentic-ci hooks (context, prompt, verdict, comment) - .github/scripts/ep_skill_config.py — SkillConfig builder Starts in shadow mode (EP_REVIEW_SHADOW=true) — reviews run but no comments are posted until verified. Requires secrets: GCP_SA_KEY, GCP_PROJECT, GCP_REGION Part of OSAC-1773 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughThis PR introduces an automated Enhancement Proposal (EP) review GitHub Action. It adds ChangesEP Review Automation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant Script as ep_review.py
participant Hooks as EPHooks
participant GH as gh CLI
GitHub->>Script: trigger on PR (prd.md/design.md change)
Script->>GH: fetch changed files, PR details
Script->>Script: detect_skill, build ticket
Script->>Hooks: check_pr_state
Hooks->>GH: check existing bot comment
Script->>Hooks: build_prompt / write_pr_context
Script->>Script: run_skill (agentic-ci) -> verdict.json
Script->>Hooks: validate_scores, apply_labels
Hooks->>GH: PATCH/POST comment, add reviewed label
Estimated code review effort: 3 (Moderate) | ~30 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ 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: 17
🤖 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/scripts/ep_hooks.py:
- Around line 27-34: The _gh helper currently swallows GitHub CLI failures by
printing stderr and returning an empty string, which lets callers of ep_hooks.py
continue as if comment/label operations succeeded. Update _gh to fail closed by
raising on nonzero subprocess.run return codes, or add an explicit opt-in
non-throwing mode only for probe-style callers; make sure the behavior is
enforced at the _gh method so side-effect operations do not silently succeed on
gh errors.
- Around line 93-138: The _prd_prompt and _design_prompt builders currently tell
the reviewer to read .context/pr-diff.txt, .context/template.md, and
.context/skill-prompt.md without explicitly separating instructions from
PR-authored content. Update these prompts to add a clear prompt-injection
boundary that says all diff/template/context contents are data only and any
instructions found inside them must be ignored, while keeping the existing
verdict.json schema and scoring guidance intact.
- Around line 200-203: The posted review comment built in ep_hooks.py is missing
the reviewed commit SHA, so the existing “already reviewed” check in
check_pr_state cannot match it reliably. Update the comment body construction
around the lines array in the comment-posting flow to include the current head
SHA (the same head[:8] value used by check_pr_state), preferably in the header
or metadata line alongside the verdict. Make sure the SHA is emitted every time
the review comment is created so the duplicate-review gate can detect it
consistently.
- Around line 154-173: The validate_scores method currently sums raw scores even
when invalid values or unexpected rubric keys are present, which can crash the
post-gate or produce a misleading verdict. Update validate_scores to first
verify the scores dict contains exactly the expected rubric keys for the ticket,
and only then validate each value is an int in range before computing the total.
Use the validate_scores function and its scores/verdict handling to keep the
existing error collection and only write verdict.json after the rubric key set
and values are confirmed valid.
In @.github/scripts/ep_review.py:
- Around line 39-47: The detect_skill() logic only returns the first matching
skill, so when both prd.md and design.md are present the ep-review path is
skipped. Update detect_skill() to detect both doc types and return all matching
skills, then adjust the caller to execute each returned skill (or explicitly
reject mixed inputs if that is the intended behavior). Use the detect_skill()
function and its current prd-review/ep-review selection logic as the main place
to fix this.
- Around line 24-36: The changed-files lookup in gh/get_changed_files is
swallowing api failures by returning an empty list, which makes main treat auth
or API errors as “No files changed in PR.” Update gh to fail loudly when
subprocess.run returns a nonzero code, and have get_changed_files propagate that
failure instead of converting it to [] so main can exit non-successfully; use
the existing gh and get_changed_files symbols to keep the error path consistent.
- Around line 135-142: The ImportError fallback in ep_review.py currently turns
a missing agentic-ci dependency into a silent dry-run, which should only happen
locally. Update the except ImportError path in the main review flow to check
GITHUB_ACTIONS (or equivalent CI detection): keep hooks.write_pr_context and the
dry-run print only when not running in GitHub Actions, and when GITHUB_ACTIONS
is set, fail the workflow immediately with a clear error instead of continuing.
Use the existing import/entrypoint around agentic-ci and hooks.write_pr_context
to place this guard.
- Around line 75-93: The ep_review.py flow currently builds the PR payload
without checking whether the workflow run is stale, so add a guard after
fetching PR data with gh and before creating the ticket: compare the live
pr["headRefOid"] against PR_HEAD_SHA (or the provided head_sha fallback) and
abort the review/posting path when they differ. Use the existing gh(),
pr_raw/json.loads, and ticket construction block to place the check so outdated
runs do not continue.
In @.github/scripts/ep_skill_config.py:
- Line 28: The container_image setting in ep_skill_config should not use the
mutable claude-runner:latest tag. Update the image reference to the pinned
digest form for the claude-runner entry so the execution environment stays fixed
and reproducible. Locate the container_image assignment in the script and
replace the current tag-based reference with the provided sha256 digest.
- Line 9: The build_skill_config helper has an unused skill_path parameter that
should be removed. Update build_skill_config() to accept only the values it
actually uses, then adjust its call site in EPHooks.write_pr_context to stop
passing the redundant path because the skill path is already handled via
ticket["_skill_path"]. Make sure any references to build_skill_config stay
consistent after the signature change.
In @.github/workflows/ep-review.yml:
- Around line 49-57: The ep-review workflow currently uploads retained
verdict/context artifacts in the Upload verdict artifacts step without any
signing or attestation. Add a Sigstore/cosign attestation/signing step for the
artifacts produced by this job, and wire it into the existing workflow around
the Upload verdict artifacts action so retained outputs are signed before being
published or consumed.
- Around line 30-31: The skills repository clone in the workflow is using the
default branch, which can change behavior outside this PR. Update the Clone
skills repo step in ep-review.yml to fetch a specific reviewed commit SHA for
/opt/skills instead of cloning HEAD, so the workflow uses a pinned revision.
Keep the change localized to the git clone step and ensure the selected commit
is explicit and reproducible.
- Around line 19-23: The workflow uses mutable action tags in the ep-review job,
so update each affected `uses:` entry in the GitHub Actions workflow to a full
40-character commit SHA instead of `@v4` or `@v5`. Locate the action references
for `actions/checkout` and `actions/setup-python`, and replace their version
tags with reviewed pinned SHAs while keeping the rest of the job unchanged.
- Around line 3-8: Add PR-scoped concurrency to the ep-review workflow so only
the latest run for a given pull request stays active and older synchronize runs
are cancelled. Update the workflow near the existing pull_request trigger in
ep-review.yml by adding a concurrency block keyed to the PR number or equivalent
unique PR identifier, and set cancel-in-progress so duplicate review runs do not
overlap.
- Around line 13-16: Reduce the GitHub token scope in the workflow permissions
block by changing the permissions used by the ep-review workflow so pull request
access is read-only while keeping issues as write. Update the permissions
section in the workflow definition that currently includes contents,
pull-requests, and issues so the review job still can read PR diffs but only
uses Issues APIs for comments and labels, with no pull-requests write access.
- Around line 19-47: The EP review job is executing
`.github/scripts/ep_review.py` from the checked-out PR/merge ref while sensitive
credentials are available. Update the workflow to run trusted automation code
from the base branch instead of the PR workspace, and have that script fetch PR
content via the API rather than reading potentially attacker-modified files. Use
the existing `Run EP review` step and `.github/scripts/ep_review.py` as the key
locations to harden.
- Around line 27-28: The Install dependencies step in the EP review workflow is
pulling unpinned packages at runtime, so replace the ad hoc pip install in the
workflow with installation from a locked requirements file that pins exact
versions and hashes. Update the workflow around the Install dependencies and Run
EP review steps to use the locked file, then add a dependency audit step before
Run EP review to verify the installed Python packages are safe and reproducible.
🪄 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: 0ff8cf15-5ed1-40af-a7ce-1aa0791d68a1
📒 Files selected for processing (4)
.github/scripts/ep_hooks.py.github/scripts/ep_review.py.github/scripts/ep_skill_config.py.github/workflows/ep-review.yml
| - name: Clone skills repo | ||
| run: git clone --depth 1 https://github.com/osac-project/osac-workspace /opt/skills |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Pin the skills repository revision.
Cloning the default branch makes prompt/skill behavior change outside this PR’s review. Fetch a reviewed commit SHA for /opt/skills.
🤖 Prompt for 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.
In @.github/workflows/ep-review.yml around lines 30 - 31, The skills repository
clone in the workflow is using the default branch, which can change behavior
outside this PR. Update the Clone skills repo step in ep-review.yml to fetch a
specific reviewed commit SHA for /opt/skills instead of cloning HEAD, so the
workflow uses a pinned revision. Keep the change localized to the git clone step
and ensure the selected commit is explicit and reproducible.
58e18b9 to
b4fe733
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
ada3962 to
426d4b4
Compare
1. SHA in comment body — include head SHA for dedup check 2. _gh error handling — raise on failure for write ops (check=True) 3. detect_skill returns all matches — PRs with both prd+design get both reviews 4. Stale run guard — abort if live headRefOid differs from trigger SHA 5. ImportError fails in CI — only dry-run locally 6. Concurrency group — cancel older runs on same PR 7. Prompt injection boundary — context files treated as data only 8. Pin action versions to commit SHAs 9. Pin dependencies via requirements.txt 10. validate_scores checks expected rubric keys 11. Remove unused skill_path param from build_skill_config Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
426d4b4 to
f684adf
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ItzikEzra-rh, maorfr 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 |
|
@ItzikEzra-rh: This pull request references OSAC-1774 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. |
Summary
Adds a GitHub Action that automatically reviews PRDs and design docs when PRs are opened or updated.
prd.mdordesign.mdprd-reviewskill (PRDs) orep-reviewskill (designs) from osac-workspacerfe-creator-auto-reviewedlabel after reviewFiles
.github/workflows/ep-review.yml— workflow definition.github/scripts/ep_review.py— entry point.github/scripts/ep_hooks.py— agentic-ci hooks (prompt, context, verdict, comment posting).github/scripts/ep_skill_config.py— SkillConfig builderShadow mode
Starts with
EP_REVIEW_SHADOW=true(set as repo variable). Reviews run but no comments are posted until verified.Required secrets
GCP_SA_KEY— GCP service account key (base64, for Vertex AI)GCP_PROJECT— Vertex AI project IDGCP_REGION— Vertex AI regionPart of OSAC-1773
Summary by CodeRabbit
New Features
Bug Fixes