feat(pr-review): 新增 PR 自動審查閘門 - #120
Conversation
|
Warning Review limit reached
More reviews will be available in 21 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR introduces a complete automated PR review agent gate. The agent validates every PR against OpenSpec alignment, secrets, repository boundaries, deterministic checks, and GitNexus impact, producing JSON and Markdown reports with risk classification and blocker/warning verdicts. The feature is implemented as a PowerShell script orchestrated by GitHub Actions and documented for local rerun and rollout guidance. ChangesPR Review Agent Gate Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a repository-level “PR review agent gate” capability (OpenSpec + docs) and a first implementation (PowerShell + GitHub Actions) that generates a JSON/Markdown review report per pull request, including path-based validation planning, guardrails, and a verdict.
Changes:
- Added an OpenSpec capability (
pull-request-review-agent) with proposal/design/tasks/spec artifacts defining gate status/risk semantics, evidence requirements, and guardrails. - Added a Windows GitHub Actions workflow to run the PR review agent on PR events and publish artifacts / comment a summary.
- Added PowerShell implementation + tests to generate the report, run minimal validations based on changed paths, and enforce guards (secrets paths, repo boundary, GitNexus).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/pr-review-agent.yml |
Runs the gate on PR events, uploads report artifacts, and posts a PR comment summary. |
scripts/pr-review-agent.ps1 |
CLI entrypoint wrapper for running the review agent locally/CI and exiting non-zero on blocked/failed. |
scripts/lib/pr-review-agent.ps1 |
Core implementation: changed-path detection, guards, validation planner/runner, GitNexus integration, report generation. |
scripts/tests/test-pr-review-agent.ps1 |
Script-level tests covering key fixtures (OpenSpec-only, missing OpenSpec, secret paths, retired runtime, GitNexus unavailable, planner). |
openspec/changes/add-pr-review-agent/.openspec.yaml |
Declares the OpenSpec change metadata. |
openspec/changes/add-pr-review-agent/proposal.md |
Motivation/scope for the PR review agent gate. |
openspec/changes/add-pr-review-agent/design.md |
Design decisions and rollout guidance for the gate/workflow/script split. |
openspec/changes/add-pr-review-agent/tasks.md |
Task checklist + validation notes for the change. |
openspec/changes/add-pr-review-agent/specs/pull-request-review-agent/spec.md |
Capability spec defining requirements/scenarios for the gate behavior and evidence. |
docs/PR_REVIEW_AGENT.md |
User-facing documentation for statuses, risk levels, report fields, rerun instructions, rollout. |
docs/PROJECT_DEVELOPMENT_WORKFLOW.md |
Integrates the new gate into the repo’s PR workflow/checklist. |
README.md |
Adds doc link + brief mention and local rerun snippet. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ($p -match '^(\.github/workflows|scripts)/') { | ||
| $fullPath = Join-Path $RepoRoot ($p -replace '/', [System.IO.Path]::DirectorySeparatorChar) | ||
| if (Test-Path -LiteralPath $fullPath -PathType Leaf) { | ||
| $content = Get-Content -LiteralPath $fullPath -Raw -ErrorAction SilentlyContinue | ||
| foreach ($name in $Script:RetiredRuntimeNames) { | ||
| if ($content -match [regex]::Escape($name)) { | ||
| [void]$blockers.Add((New-PrReviewIssue -Kind 'retired_runtime_reference' -Severity 'high' -Path $p -Message "Workflow/script references retired runtime '$name'; keep retired services out of current runtime gates.")) | ||
| break | ||
| } | ||
| } |
| $record.command = 'gitnexus detect-changes' | ||
| try { | ||
| $output = & gitnexus detect-changes 2>&1 | Out-String | ||
| $exitCode = if ($LASTEXITCODE -is [int]) { $LASTEXITCODE } else { 0 } | ||
| $record.status = if ($exitCode -eq 0) { 'passed' } else { 'failed' } |
| if ([string]::IsNullOrWhiteSpace($env:PR_REVIEW_AGENT_REQUIRE_AI)) { | ||
| [void]$warnings.Add((New-PrReviewIssue -Kind 'optional_ai_adapter_skipped' -Severity 'medium' -Message 'Optional AI adapter is not required for this gate and was skipped.')) | ||
| } elseif ([string]::IsNullOrWhiteSpace($env:OPENAI_API_KEY)) { |
| $paths = @(git -c "safe.directory=$safeRoot" status --porcelain=v1 -uall 2>$null | ForEach-Object { | ||
| if ($_.Length -gt 3) { $_.Substring(3) } | ||
| }) |
| $loaded3 = Get-Content -LiteralPath $result3.json_path -Raw | ConvertFrom-Json | ||
| $secretBlockers = @($loaded3.blockers | Where-Object { $_.kind -eq 'secret_path' }) | ||
| Assert-True ($secretBlockers.Count -ge 2) 'secret path blockers recorded' | ||
| Assert-True (($secretBlockers | ForEach-Object { $_.message }) -notmatch 'PASSWORD=|TOKEN=') 'secret values are not printed' |
| - name: Run PR review agent | ||
| id: review | ||
| shell: pwsh | ||
| continue-on-error: true | ||
| env: | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| PR_DRAFT: ${{ github.event.pull_request.draft }} | ||
| GITHUB_BASE_REF: ${{ github.event.pull_request.base.ref }} | ||
| GITHUB_HEAD_REF: ${{ github.event.pull_request.head.ref }} | ||
| run: | | ||
| $args = @( | ||
| '-NoProfile', | ||
| '-ExecutionPolicy', 'Bypass', | ||
| '-File', 'scripts/pr-review-agent.ps1', | ||
| '-BaseSha', '${{ github.event.pull_request.base.sha }}', | ||
| '-HeadSha', '${{ github.event.pull_request.head.sha }}', | ||
| '-PrNumber', '${{ github.event.pull_request.number }}', | ||
| '-RunId', '${{ github.run_id }}', | ||
| '-OutputDir', 'artifacts/pr-review-agent' | ||
| ) | ||
| if ($env:PR_DRAFT -eq 'true') { | ||
| $args += '-ReportOnly' | ||
| } | ||
| & powershell @args |
There was a problem hiding this comment.
Implemented in e1ed8f6. I added explicit runner setup/install steps in .github/workflows/pr-review-agent.yml (Node 20, Python 3.12, global @fission-ai/openspec + gitnexus, pip install pytest, and npm ci --prefix bim-review-coordinator) before running the PR review agent.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac0025d48f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (-not [string]::IsNullOrWhiteSpace($BaseSha) -and -not [string]::IsNullOrWhiteSpace($HeadSha)) { | ||
| $paths = @(git -c "safe.directory=$safeRoot" diff --name-only $BaseSha $HeadSha 2>$null) | ||
| } |
There was a problem hiding this comment.
Use a merge-base diff when collecting changed paths for PRs. git diff --name-only $BaseSha $HeadSha compares the two tip trees directly, so when main advances after the feature branch diverges, this includes unrelated base-branch edits and can trigger false blockers/extra validations for files the PR did not touch. This gate should mirror PR semantics (base...head or explicit merge-base) to avoid blocking valid PRs.
Useful? React with 👍 / 👎.
| foreach ($name in $Script:RetiredRuntimeNames) { | ||
| if ($content -match [regex]::Escape($name)) { | ||
| [void]$blockers.Add((New-PrReviewIssue -Kind 'retired_runtime_reference' -Severity 'high' -Path $p -Message "Workflow/script references retired runtime '$name'; keep retired services out of current runtime gates.")) | ||
| break |
There was a problem hiding this comment.
The workflow/script content scan flags any textual mention of retired runtime names as a blocker, which creates self-inflicted false positives (for example, guard lists or test fixtures that intentionally mention _worker). In that case the gate reports retired_runtime_reference even when no runtime dependency is being reintroduced, so legitimate maintenance PRs on review scripts/workflows can be blocked.
Useful? React with 👍 / 👎.
| run: | | ||
| $args = @( | ||
| '-NoProfile', | ||
| '-ExecutionPolicy', 'Bypass', | ||
| '-File', 'scripts/pr-review-agent.ps1', |
There was a problem hiding this comment.
This workflow invokes the gate without installing gitnexus or relaxing the unavailable check, but the script hard-blocks code/script PRs when gitnexus is missing. On GitHub-hosted runners this can turn most non-doc PRs into permanent blocked failures even when the underlying changes are valid, because the tool dependency is unmet rather than the PR violating policy.
Useful? React with 👍 / 👎.
| } | ||
|
|
There was a problem hiding this comment.
The library is dot-sourced before entering the try block, so load/parse errors bypass the fallback report writer entirely. In that scenario the workflow gets no pr-review-agent.json/md (and upload fails with if-no-files-found: error), which removes the diagnostic output the gate is supposed to provide when report generation fails.
Useful? React with 👍 / 👎.
| $paths = @(git -c "safe.directory=$safeRoot" status --porcelain=v1 -uall 2>$null | ForEach-Object { | ||
| if ($_.Length -gt 3) { $_.Substring(3) } | ||
| }) |
There was a problem hiding this comment.
When BaseSha/HeadSha are not provided, changed files come from git status --porcelain, but rename lines are returned as old -> new; taking Substring(3) leaves that combined token unchanged. That makes downstream path checks/planners miss the actual new path, so required validations or guardrails can be skipped during local reruns and other non-SHA executions.
Useful? React with 👍 / 👎.
| $record.command = 'gitnexus detect-changes' | ||
| try { | ||
| $output = & gitnexus detect-changes 2>&1 | Out-String |
There was a problem hiding this comment.
The gate runs gitnexus detect-changes without an explicit repo/worktree target, so environments with multiple indexed repos can return unrelated failures or unavailable even when GitNexus is installed. Because this status is treated as blocking for code/script changes, valid PRs can be rejected due to repo selection ambiguity rather than actual risk.
Useful? React with 👍 / 👎.
b3bbe3d to
980ae59
Compare
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
Agent-Logs-Url: https://github.com/monkey1sai/AI-BIM-governance/sessions/1693acf7-be4d-4911-a0c1-674ead2bceaf Co-authored-by: monkey1sai <26239865+monkey1sai@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 980ae5976f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ($needsGitNexus -and $AllowGitNexusUnavailable -and $gitnexus.status -in @('unavailable', 'failed')) { | ||
| $gitnexus.status = 'warning' |
There was a problem hiding this comment.
-AllowGitNexusUnavailable is meant to tolerate missing tooling during rollout, but this branch also rewrites a real GitNexus failed result to warning. In that case code/script PRs can pass the gate even when gitnexus detect-changes executed and reported an error, which suppresses a required risk signal instead of only handling tool absence. Restrict the downgrade to unavailable so actual detect-changes failures still block.
Useful? React with 👍 / 👎.
| $mergeBase = (git -c "safe.directory=$safeRoot" merge-base $BaseSha $HeadSha 2>$null | Select-Object -First 1) | ||
| if (-not [string]::IsNullOrWhiteSpace($mergeBase)) { | ||
| $paths = @(git -c "safe.directory=$safeRoot" diff --name-only $mergeBase $HeadSha 2>$null) | ||
| } else { | ||
| $paths = @(git -c "safe.directory=$safeRoot" diff --name-only "$BaseSha...$HeadSha" 2>$null) |
There was a problem hiding this comment.
When BaseSha/HeadSha are invalid or unavailable, both merge-base / diff failures are silently swallowed (2>$null) and the function falls back to local git status, which can yield an empty path list on clean checkouts. The agent then only emits a human note and can still return a non-blocking verdict, so required OpenSpec and validation checks may be skipped for the wrong commit range instead of failing fast on bad diff inputs.
Useful? React with 👍 / 👎.
| '-AllowGitNexusUnavailable', | ||
| '-AllowUnavailableCommands' |
There was a problem hiding this comment.
The workflow always passes -AllowUnavailableCommands, which converts every missing required validator (for example openspec, npm, python, or other planned commands returning unavailable) into a warning instead of a blocker. That makes the gate succeed even when deterministic checks never ran, so CI can report a healthy review verdict under runner/tooling drift rather than enforcing the required evidence.
Useful? React with 👍 / 👎.
| $report = [ordered]@{ | ||
| schema_version = 'pr-review-agent/v1' | ||
| status = 'failed' | ||
| risk_level = 'high' | ||
| generated_at = (Get-Date).ToUniversalTime().ToString('o') | ||
| blockers = @([ordered]@{ | ||
| kind = 'report_generation_failed' | ||
| severity = 'high' | ||
| path = '' | ||
| message = $message | ||
| }) | ||
| warnings = @() | ||
| checks = @() | ||
| } |
There was a problem hiding this comment.
The exception fallback writes a reduced JSON shape that omits documented core fields like changed_paths, openspec_changes, validation_commands, human_review_notes, and gitnexus. Any consumer that relies on the stable report schema can break exactly when the run fails, which is when diagnostics are most needed; the fallback should keep the same keys with empty/default values.
Useful? React with 👍 / 👎.
| $content = Get-Content -LiteralPath $fullPath -Raw -ErrorAction SilentlyContinue | ||
| foreach ($name in $Script:RetiredRuntimeNames) { | ||
| if (Test-PrReviewRetiredRuntimeWiringReference -Content $content -RuntimeName $name) { | ||
| [void]$blockers.Add((New-PrReviewIssue -Kind 'retired_runtime_reference' -Severity 'high' -Path $p -Message "Workflow/script references retired runtime '$name'; keep retired services out of current runtime gates.")) |
There was a problem hiding this comment.
Retired-runtime detection scans the full current file content for any matching wiring line whenever a workflow/script file is touched, instead of checking what this PR actually introduced. If a file already contains such a line from earlier history, unrelated edits to that file will still be blocked as a new violation, creating false blockers that are not attributable to the reviewed change.
Useful? React with 👍 / 👎.
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openspec/changes/add-pr-review-agent/design.md (1)
1-122:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftOpenSpec 設計文件仍有大量非例外英文章節標題。
Line 1、Line 9、Line 28、Line 101、Line 110、Line 118 等屬於一般文件敘述，不是 parser required headers，請改為繁體中文以符合規範。
As per coding guidelines, "
openspec/**/*.md: All OpenSpec artifacts must use Traditional Chinese (繁體中文); API paths, schema fields, CLI flags, status enums, logs/errors, external product names, and OpenSpec parser required headers must remain in original language".🤖 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 `@openspec/changes/add-pr-review-agent/design.md` around lines 1 - 122, The document contains several non-parser headers in English (e.g., "Context", "Goals / Non-Goals", "Decisions", "Risks / Trade-offs", "Migration Plan", "Open Questions") that must be converted to Traditional Chinese per OpenSpec rules; update those section titles and any other non-required-English subsection headings to 繁體中文 while preserving parser-required tokens (API paths, schema fields, CLI flags, status enums, logs/errors, external product names, and any OpenSpec parser required headers such as exact keywords) in their original language so the parser still recognizes them.openspec/changes/add-pr-review-agent/tasks.md (1)
1-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick wintasks 文件的非例外英文標題需改為繁體中文。
像 Line 1、Line 8、Line 18、Line 48 的英文標題/標示不屬於保留原文例外，請改成繁體中文。
As per coding guidelines, "
openspec/**/*.md: All OpenSpec artifacts must use Traditional Chinese (繁體中文); API paths, schema fields, CLI flags, status enums, logs/errors, external product names, and OpenSpec parser required headers must remain in original language".🤖 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 `@openspec/changes/add-pr-review-agent/tasks.md` around lines 1 - 66, Replace the non-exception English section titles in tasks.md with Traditional Chinese: change "1. Review Policy And Report Contract", "2. Local Review Agent Script", "3. Automated Workflow Integration", "4. Tests And Fixtures", "5. Documentation And Rollout", and "6. Validation" (and any other top-level/inline English headings like "Review Policy And Report Contract", "Local Review Agent Script", "Automated Workflow Integration", "Tests And Fixtures") into their 繁體中文 equivalents while preserving API paths, schema fields, CLI flags, status enums, logs/errors, external product names, and any OpenSpec parser-required headers in original language; update only the visible title strings (not code/flags) so the file conforms to the OpenSpec Traditional Chinese requirement.
🤖 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/pr-review-agent.yml:
- Line 18: The workflow uses mutable action tags (e.g., "uses:
actions/checkout@v4" and other "uses: ...@v5") which weakens supply-chain
security; replace each mutable tag occurrence (the "uses:" entries found in the
diff such as the actions/checkout and the other `@v4/`@v5 references) with the
corresponding pinned commit SHA for that action repository, updating each
"uses:" line to reference the exact full SHA of the released commit instead of
the floating tag so the workflow is immutable and verifiable.
- Around line 19-20: The checkout step currently sets fetch-depth: 0 but leaves
repo credentials persisted; update the actions/checkout configuration by adding
persist-credentials: false to disable storing workflow checkout credentials in
the workspace (keep fetch-depth as needed), i.e., modify the actions/checkout
step properties to include persist-credentials: false so later network calls
don't reuse repository credentials.
In `@openspec/changes/add-pr-review-agent/proposal.md`:
- Around line 1-31: Translate all non-reserved top-level and subsection headings
in this document from English to Traditional Chinese (e.g., "Why" → "為何", "What
Changes" → "變更內容", "Capabilities" → "能力", "New Capabilities" → "新增能力", "Modified
Capabilities" → "修改後的能力", "Impact" → "影響") while preserving any reserved terms
exactly as-is (for example keep `pull-request-review-agent`, file paths like
`.github/`, `openspec/**/*.md`, and other code/API names unchanged); update only
the header text, do not alter body content, bullet lists, code snippets, or any
product/API identifiers referenced in the diff such as "GitNexus", "CODEOWNERS",
or `.env`.
In
`@openspec/changes/add-pr-review-agent/specs/pull-request-review-agent/spec.md`:
- Around line 5-167: The document keeps parser-required headers in English but
the rest of the human-readable requirement and scenario descriptions must be
converted to Traditional Chinese; update every Scenario/Requirement paragraph
(e.g., the blocks beginning with "Scenario: PR is opened or updated",
"Requirement: PR review agent publishes reviewable evidence", "Scenario: Review
report is created", etc.) replacing the English prose with 繁體中文 while preserving
literal API paths, CLI flags, schema field names, enums, logs/errors, product
names, and the parser-required header lines (the "Requirement:" and "Scenario:"
headings and any inline code tokens like `openspec/changes/<change-id>/` or
`openspec validate <change-id>` must remain unchanged). Ensure translations keep
the same meaning and that markers such as `status`, `risk_level`,
`changed_paths`, `openspec_changes`, and other code tokens remain verbatim.
In `@scripts/lib/pr-review-agent.ps1`:
- Around line 362-397: The code currently uses "return ,$record" which forces
$record into a single-element array and causes downstream ConvertTo-Json to emit
an array (gitnexus: [{...}]); change each "return ,$record" to return a single
object instead (for example replace with "return [pscustomobject]$record" or at
minimum "return $record") so the function returns an object rather than an
array; update all occurrences around the early-exit branches that reference the
$record variable (the returns after SkipGitNexus, SimulateUnavailable, gitnexus
CLI missing, and the final return) to use the non-comma return with
[pscustomobject]$record to ensure JSON serializes as an object.
---
Outside diff comments:
In `@openspec/changes/add-pr-review-agent/design.md`:
- Around line 1-122: The document contains several non-parser headers in English
(e.g., "Context", "Goals / Non-Goals", "Decisions", "Risks / Trade-offs",
"Migration Plan", "Open Questions") that must be converted to Traditional
Chinese per OpenSpec rules; update those section titles and any other
non-required-English subsection headings to 繁體中文 while preserving
parser-required tokens (API paths, schema fields, CLI flags, status enums,
logs/errors, external product names, and any OpenSpec parser required headers
such as exact keywords) in their original language so the parser still
recognizes them.
In `@openspec/changes/add-pr-review-agent/tasks.md`:
- Around line 1-66: Replace the non-exception English section titles in tasks.md
with Traditional Chinese: change "1. Review Policy And Report Contract", "2.
Local Review Agent Script", "3. Automated Workflow Integration", "4. Tests And
Fixtures", "5. Documentation And Rollout", and "6. Validation" (and any other
top-level/inline English headings like "Review Policy And Report Contract",
"Local Review Agent Script", "Automated Workflow Integration", "Tests And
Fixtures") into their 繁體中文 equivalents while preserving API paths, schema
fields, CLI flags, status enums, logs/errors, external product names, and any
OpenSpec parser-required headers in original language; update only the visible
title strings (not code/flags) so the file conforms to the OpenSpec Traditional
Chinese requirement.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 20d5ea97-fcd8-4c1d-9c23-528170a24586
📒 Files selected for processing (12)
.github/workflows/pr-review-agent.ymlREADME.mddocs/PROJECT_DEVELOPMENT_WORKFLOW.mddocs/PR_REVIEW_AGENT.mdopenspec/changes/add-pr-review-agent/.openspec.yamlopenspec/changes/add-pr-review-agent/design.mdopenspec/changes/add-pr-review-agent/proposal.mdopenspec/changes/add-pr-review-agent/specs/pull-request-review-agent/spec.mdopenspec/changes/add-pr-review-agent/tasks.mdscripts/lib/pr-review-agent.ps1scripts/pr-review-agent.ps1scripts/tests/test-pr-review-agent.ps1
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66a4c668b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| npm install -g @fission-ai/openspec gitnexus | ||
| python -m pip install --upgrade pip | ||
| python -m pip install pytest | ||
| npm ci --prefix bim-review-coordinator |
There was a problem hiding this comment.
The workflow only provisions bim-review-coordinator dependencies (npm ci --prefix bim-review-coordinator) plus pytest, but the agent can plan npm run verify for web-viewer-sample and Python tests in bim-streaming-server for matching path changes (scripts/lib/pr-review-agent.ps1, validation plan). On those PRs, commands exist but fail due to missing project dependencies, producing validation_failed blockers unrelated to the actual patch and effectively turning the gate into a false blocker for viewer/streaming changes.
Useful? React with 👍 / 👎.
| if ($isEnvValue) { | ||
| [void]$blockers.Add((New-PrReviewIssue -Kind 'secret_path' -Severity 'critical' -Path $p -Message 'PR modifies a real environment file; do not change secret values in repo.')) | ||
| } elseif ($isEnvExample) { |
There was a problem hiding this comment.
The guard blocks any changed path named like .env/private-key patterns without checking change type, but Get-PrReviewChangedPathsFromGit only returns names (not A/M/D), so deletions are treated the same as additions/edits. This means a PR that removes an accidentally committed .env or key file is still hard-blocked as secret_path, which can prevent urgent remediation of leaked secrets.
Useful? React with 👍 / 👎.
| if ($p -match '^(\.codex/skills|\.claude/skills/generated|\.gitnexus)(/|$)') { | ||
| [void]$blockers.Add((New-PrReviewIssue -Kind 'generated_tooling_path' -Severity 'high' -Path $p -Message 'Generated local tooling state must not be committed as product source.')) |
There was a problem hiding this comment.
Generated-tooling paths (.codex/skills, .claude/skills/generated, .gitnexus) are always emitted as blocking findings based on path name alone, with no status check for deletion. Because changed paths are collected via --name-only, a PR that only removes mistakenly committed generated state is still blocked as generated_tooling_path, which discourages or prevents the intended repository cleanup.
Useful? React with 👍 / 👎.
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0dc3ef9e1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| foreach ($path in $ChangedPaths) { | ||
| $p = ConvertTo-PrReviewPath $path | ||
| if ($p -match '^(scripts|bim-review-coordinator|web-viewer-sample|bim-streaming-server|tests)/') { | ||
| if ($p -notmatch '\.(md|txt|json|png|jpg|jpeg|svg|html)$') { return $true } |
There was a problem hiding this comment.
Remove json from the non-code extension allowlist here, because this currently treats any *.json edit under scripts/, bim-review-coordinator/, web-viewer-sample/, bim-streaming-server/, and tests/ as not requiring GitNexus. That lets behavior-affecting changes (for example package.json, lock/config JSON, or runtime test fixtures) bypass the intended gitnexus detect-changes gate and merge without the required impact evidence.
Useful? React with 👍 / 👎.
| if ([string]::IsNullOrWhiteSpace($BaseSha) -or [string]::IsNullOrWhiteSpace($HeadSha)) { | ||
| return $false |
There was a problem hiding this comment.
Fresh evidence for the earlier deletion-guard concern: Test-PrReviewDeletedPath now returns false whenever BaseSha/HeadSha are omitted, while the agent still supports non-SHA executions via git status --porcelain fallback. In those local/non-CI runs, deleting .env, key-like files, or generated-tooling paths is misclassified as a modification and gets blocked instead of warned, which breaks the documented cleanup/remediation path outside SHA-based CI contexts.
Useful? React with 👍 / 👎.
| '-NoProfile', | ||
| '-ExecutionPolicy', 'Bypass', | ||
| '-File', 'scripts/pr-review-agent.ps1', | ||
| '-BaseSha', '${{ github.event.pull_request.base.sha }}', |
There was a problem hiding this comment.
Passing github.event.pull_request.base.sha into the gate can miscompute changed paths after the base branch advances, because GitHub keeps the PR base commit snapshot rather than updating it to the latest base-branch tip. In that scenario, merge-base is computed against an old base commit and the gate can include base-only files, causing unrelated blockers/validations on otherwise valid PR updates.
Useful? React with 👍 / 👎.
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cec310971
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| $needsGitNexus = Test-PrReviewNeedsGitNexus -ChangedPaths $ChangedPaths | ||
| $gitnexus = Invoke-PrReviewGitNexus -NeedsGitNexus:$needsGitNexus -SkipGitNexus:$SkipGitNexus -AllowUnavailable:$AllowGitNexusUnavailable -SimulateUnavailable:$SimulateGitNexusUnavailable -SimulateFailure:$SimulateGitNexusFailure -RepoName $RepoRoot |
There was a problem hiding this comment.
This call passes the filesystem root ($RepoRoot) as --repo, but the workflow indexes GitNexus with the alias AI-BIM-governance (gitnexus analyze --name AI-BIM-governance ...), so detect-changes can report repository-unavailable even when GitNexus is installed. Fresh evidence in this commit is the mismatch between alias creation in .github/workflows/pr-review-agent.yml and this invocation, which makes gate outcomes depend on repo-name mismatch instead of actual PR risk.
Useful? React with 👍 / 👎.
| if ($p -in @('README.md', 'AGENTS.md') -or $p -eq 'docs/PROJECT_DEVELOPMENT_WORKFLOW.md') { | ||
| return $true | ||
| } |
There was a problem hiding this comment.
These conditions force OpenSpec for README.md, AGENTS.md, and docs/PROJECT_DEVELOPMENT_WORKFLOW.md, so a pure documentation/refactor PR touching those files is blocked as missing_openspec even without behavior changes. That contradicts the documented checklist allowing docs/refactor exceptions, and will create false blockers for governance/doc maintenance PRs.
Useful? React with 👍 / 👎.
變更摘要
新增 PR review agent gate 的 OpenSpec change、GitHub Actions workflow、本機 PowerShell 審查工具、測試與文件，讓每個 PR 可產生自動審查 report。
修改原因
AI coding 產生的 diff 和驗證證據很大，人工審查容易漏掉 OpenSpec、repo 邊界、GitNexus、secrets 與必要驗證。
主要變更
pull-request-review-agentOpenSpec capability。.github/workflows/pr-review-agent.yml，在 PR opened / synchronize / reopened / ready_for_review 時跑 gate。scripts/pr-review-agent.ps1與scripts/lib/pr-review-agent.ps1，產生 JSON / Markdown report。scripts/tests/test-pr-review-agent.ps1，覆蓋 OpenSpec、service code、secret path、retired runtime、GitNexus unavailable、path planner fixtures。docs/PR_REVIEW_AGENT.md並更新 README / workflow v3。驗證方式
openspec validate add-pr-review-agent通過。openspec status --change add-pr-review-agent顯示 artifacts complete，tasks 已完成。powershell -NoProfile -ExecutionPolicy Bypass -File scripts\tests\test-pr-review-agent.ps1通過。powershell -NoProfile -ExecutionPolicy Bypass -Command "& .\scripts\pr-review-agent.ps1 ..."dry-run 成功，狀態為warning，原因是 optional AI adapter skipped。git diff --check通過。gitnexus detect-changes --repo AI-BIM-governance完成但回報 stale / sibling index；不視為乾淨 current-worktree pass。風險與影響
workflowscope 無法直接git push，已改用 GitHub connector 建立 workflow 後推送完整 commit。回滾方式
revert 本 PR；或停用 / 移除
.github/workflows/pr-review-agent.yml，產品 runtime 不受影響。後續建議
Summary by CodeRabbit
New Features
Documentation
Tests