diff --git a/AGENTS.md b/AGENTS.md index d0296b7..c8883ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,7 +38,7 @@ src/code_review_bot/ │ ├── models.py # ReviewTaskContext, ReviewOutcome │ └── publish/ │ ├── protocol.py # ReviewPublisher protocol -│ ├── platform.py # posts inline comments + summary note to the git platform +│ ├── platform.py # posts inline comments + summary to the git platform │ └── debug.py # writes Markdown report to disk (--debug mode) ├── skill/ │ ├── protocol.py # ReviewSkill protocol; Finding and SkillResult pydantic models @@ -63,22 +63,28 @@ src/code_review_bot/ prior review's stored metadata. All inline comment threads (resolved and open) are included in the prompt via ``; the agent applies the embedded rules to decide whether to re-report each one. -5. `ReviewPublisher.publish()` posts inline diff comments and a summary note, storing the new - fingerprint set in a hidden metadata comment for the next run. +5. `ReviewPublisher.publish()` posts inline diff comments and formats the summary, storing the new + fingerprint set in hidden metadata for the next run. GitLab and GitHub without automatic + approval post it as a note; GitHub with automatic approval defers it to the final review body. 6. When `AUTO_APPROVE_ON_CLEAN_REVIEW=true` (and not in `--debug` mode), the orchestrator approves the change request if no new findings were published, or revokes approval when new - findings exist (GitLab: approve/unapprove API; GitHub: `APPROVE` / `REQUEST_CHANGES` review). + findings exist (GitLab: approve/unapprove API; GitHub: `APPROVE` / `REQUEST_CHANGES` review + containing the full summary). If the GitHub review cannot be submitted, the summary falls back + to an issue comment. When `AUTO_APPROVE_IGNORE_LOW_SEVERITY=true`, low-severity findings are excluded from this decision: a review with only low-severity findings is still treated as clean. ## Key protocols -All three are `typing.Protocol` — swap an implementation by updating the corresponding factory. +The core interfaces are `typing.Protocol` types; swap an implementation by updating the +corresponding factory. `ReviewBodyApprovalAdapter` is an optional runtime-checkable capability used +to consolidate a GitHub summary into the final review decision. | Protocol | Module | Factory | |---|---|---| | `CodingAgent` | `agent/protocol.py` | `agent/factory.py` — `build_coding_agent()` | | `PlatformAdapter` | `platforms/protocol.py` | `platforms/factory.py` — `build_platform_adapter()` | +| `ReviewBodyApprovalAdapter` | `platforms/protocol.py` | implemented by the GitHub adapter | | `ReviewPublisher` | `review/publish/protocol.py` | instantiated in `orchestrator.py` | ## Extension points diff --git a/README.md b/README.md index 2b2805f..140a1b5 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,8 @@ adapters can be added behind the same interface. The bot clones the change-request branch, then delegates the entire review to a coding agent. The agent reads the diff, follows the review methodology defined in the `SKILL.md` of the configured review skill, and produces structured findings. Results are posted back to the platform as inline -review comments and a summary note on the change request. +review comments and a summary on the change request. On GitHub, when automatic approval is +enabled, the summary is used as the final `APPROVE` or `REQUEST_CHANGES` review body. Review skills live outside this repository. `REVIEW_SKILL` can be a local directory path or an `https` URL pointing at a skill containing `SKILL.md`; when empty the coding agent reviews using @@ -107,7 +108,7 @@ See `.env.example` for the full list with descriptions. | `REVIEW_EXCLUDE` | no | `[]` | JSON array of glob patterns for files to skip (e.g. `["dist/**", "*.pb.go"]`). Added on top of built-in defaults: `*.lock`, `*-lock.json`, `*.min.js`, `*.min.css`, `*.map`, `**/vendor/**`, `**/generated/**`. | | `REVIEW_INCLUDE` | no | `[]` | JSON array of glob patterns; when set, only matching files are reviewed. Empty means all files (subject to excludes). Example: `["src/**", "tests/**"]`. | | `OUTPUT_LANGUAGE` | no | `english` | Language for findings and the change-request summary (`english` or `chinese`). Code, configs, and identifiers stay in English. | -| `AUTO_APPROVE_ON_CLEAN_REVIEW` | no | `false` | When `true`, approve the MR/PR after publish when no new findings were posted; revoke approval when new findings exist. Requires token approval permissions. Skipped in `--debug` mode. | +| `AUTO_APPROVE_ON_CLEAN_REVIEW` | no | `false` | When `true`, approve the MR/PR after publish when no new findings were posted; revoke approval when new findings exist. On GitHub, the full summary is published as that review's body instead of a separate issue comment. Requires token approval permissions. Skipped in `--debug` mode. | | `AUTO_APPROVE_IGNORE_LOW_SEVERITY` | no | `false` | When `true`, low-severity findings are excluded from the approval decision: a review with only low-severity findings is treated as clean and approved. Has no effect when `AUTO_APPROVE_ON_CLEAN_REVIEW` is `false`. | ### Coding agent (ACP) diff --git a/src/code_review_bot/platforms/github/adapter.py b/src/code_review_bot/platforms/github/adapter.py index 9ba8b0f..995885f 100644 --- a/src/code_review_bot/platforms/github/adapter.py +++ b/src/code_review_bot/platforms/github/adapter.py @@ -27,7 +27,12 @@ async def fetch_change_request(self, project_ref: str, cr_id: str) -> ChangeRequ async def list_notes(self, project_ref: str, cr_id: str) -> list[dict[str, object]]: owner, repo = _split_project_ref(project_ref) - return await self._client.list_issue_comments(owner, repo, int(cr_id)) + issue_comments = await self._client.list_issue_comments(owner, repo, int(cr_id)) + reviews = await self._client.list_pull_reviews(owner, repo, int(cr_id)) + return sorted( + [*issue_comments, *reviews], + key=lambda note: str(note.get("submitted_at") or note.get("created_at") or ""), + ) async def list_inline_threads(self, project_ref: str, cr_id: str) -> list[InlineThread]: owner, repo = _split_project_ref(project_ref) @@ -82,8 +87,29 @@ async def approve_change_request( owner, repo = _split_project_ref(project_ref) return await self._client.create_pull_review(owner, repo, int(cr_id), "APPROVE", head_sha) + async def approve_change_request_with_body( + self, project_ref: str, cr_id: str, head_sha: str, body: str + ) -> dict[str, object]: + owner, repo = _split_project_ref(project_ref) + return await self._client.create_pull_review( + owner, repo, int(cr_id), "APPROVE", head_sha, body=body + ) + async def revoke_change_request_approval( - self, project_ref: str, cr_id: str, head_sha: str = "" + self, + project_ref: str, + cr_id: str, + head_sha: str = "", + ) -> dict[str, object]: + return await self.revoke_change_request_approval_with_body( + project_ref, + cr_id, + head_sha, + body="Code review found new issues.", + ) + + async def revoke_change_request_approval_with_body( + self, project_ref: str, cr_id: str, head_sha: str, body: str ) -> dict[str, object]: owner, repo = _split_project_ref(project_ref) if not head_sha: @@ -95,7 +121,7 @@ async def revoke_change_request_approval( int(cr_id), "REQUEST_CHANGES", head_sha, - body="Code review found new issues.", + body=body, ) diff --git a/src/code_review_bot/platforms/github/client.py b/src/code_review_bot/platforms/github/client.py index 6528645..2420d45 100644 --- a/src/code_review_bot/platforms/github/client.py +++ b/src/code_review_bot/platforms/github/client.py @@ -44,6 +44,24 @@ async def list_issue_comments( response.raise_for_status() return list(response.json()) + async def list_pull_reviews( + self, owner: str, repo: str, pr_number: int + ) -> list[dict[str, object]]: + items: list[dict[str, object]] = [] + page = 1 + while True: + response = await self._client.get( + f"/repos/{owner}/{repo}/pulls/{pr_number}/reviews", + params={"per_page": 100, "page": page}, + ) + response.raise_for_status() + batch = response.json() + items.extend(batch) + if len(batch) < 100: + break + page += 1 + return items + async def list_pull_review_comments( self, owner: str, repo: str, pr_number: int ) -> list[dict[str, object]]: diff --git a/src/code_review_bot/platforms/protocol.py b/src/code_review_bot/platforms/protocol.py index a824927..0e93966 100644 --- a/src/code_review_bot/platforms/protocol.py +++ b/src/code_review_bot/platforms/protocol.py @@ -1,4 +1,4 @@ -from typing import Protocol +from typing import Protocol, runtime_checkable from code_review_bot.platforms.models import ChangeRequest, InlinePosition, InlineThread @@ -39,3 +39,16 @@ async def revoke_change_request_approval( ) -> dict[str, object]: ... async def aclose(self) -> None: ... + + +@runtime_checkable +class ReviewBodyApprovalAdapter(Protocol): + """Optional capability for publishing approval decisions with a full review body.""" + + async def approve_change_request_with_body( + self, project_ref: str, cr_id: str, head_sha: str, body: str + ) -> dict[str, object]: ... + + async def revoke_change_request_approval_with_body( + self, project_ref: str, cr_id: str, head_sha: str, body: str + ) -> dict[str, object]: ... diff --git a/src/code_review_bot/review/models.py b/src/code_review_bot/review/models.py index 0b6cc5b..b6c4272 100644 --- a/src/code_review_bot/review/models.py +++ b/src/code_review_bot/review/models.py @@ -30,3 +30,4 @@ class ReviewOutcome(BaseModel): inline_comments: int = 0 report_path: str = "" approved: bool | None = None + review_body: str = "" diff --git a/src/code_review_bot/review/orchestrator.py b/src/code_review_bot/review/orchestrator.py index 9791931..4852111 100644 --- a/src/code_review_bot/review/orchestrator.py +++ b/src/code_review_bot/review/orchestrator.py @@ -1,5 +1,6 @@ import logging from pathlib import Path +from typing import cast from code_review_bot.agent.factory import build_coding_agent from code_review_bot.config import Settings @@ -9,7 +10,7 @@ detach_review_session_logging, ) from code_review_bot.platforms.models import ChangeRequest -from code_review_bot.platforms.protocol import PlatformAdapter +from code_review_bot.platforms.protocol import PlatformAdapter, ReviewBodyApprovalAdapter from code_review_bot.repo.manager import RepoManager from code_review_bot.review.context import ( compute_fingerprint, @@ -153,14 +154,34 @@ async def review_change_request(self, cr_id: str) -> ReviewOutcome: len(new_findings), ) - outcome = await self.publisher.publish( - cr, - result, - skill.name, - skill.version, - sorted(fingerprints), - existing_notes=notes, + consolidate_github_review = ( + self._platform_publish + and self.adapter.platform_name == "github" + and isinstance(self.adapter, ReviewBodyApprovalAdapter) + and self.settings.auto_approve_on_clean_review + and cr.is_open + and not cr.draft ) + if self._platform_publish: + platform_publisher = cast(PlatformPublisher, self.publisher) + outcome = await platform_publisher.publish( + cr, + result, + skill.name, + skill.version, + sorted(fingerprints), + existing_notes=notes, + publish_summary=not consolidate_github_review, + ) + else: + outcome = await self.publisher.publish( + cr, + result, + skill.name, + skill.version, + sorted(fingerprints), + existing_notes=notes, + ) approval_count = len(new_findings) if self.settings.auto_approve_ignore_low_severity: non_low = [f for f in new_findings if f.severity != "low"] @@ -171,7 +192,14 @@ async def review_change_request(self, cr_id: str) -> ReviewOutcome: len(non_low), ) approval_count = len(non_low) - approved = await self._maybe_update_approval(cr, resolved_ref, approval_count) + approved = await self._maybe_update_approval( + cr, + resolved_ref, + approval_count, + review_body=outcome.review_body if consolidate_github_review else "", + ) + if consolidate_github_review and approved is None: + await self.adapter.publish_summary(resolved_ref, cr.cr_id, outcome.review_body) outcome = outcome.model_copy(update={"approved": approved}) logger.info( "Published summary=%r published=%s inline_comments=%s approved=%s", @@ -199,6 +227,7 @@ async def _maybe_update_approval( cr: ChangeRequest, project_ref: str, new_findings_count: int, + review_body: str = "", ) -> bool | None: if not self.settings.auto_approve_on_clean_review: return None @@ -217,7 +246,12 @@ async def _maybe_update_approval( cr.cr_id, ) return None - await self.adapter.approve_change_request(project_ref, cr.cr_id, head_sha) + if review_body and isinstance(self.adapter, ReviewBodyApprovalAdapter): + await self.adapter.approve_change_request_with_body( + project_ref, cr.cr_id, head_sha, body=review_body + ) + else: + await self.adapter.approve_change_request(project_ref, cr.cr_id, head_sha) logger.info( "Approved change request project_ref=%s cr_id=%s head_sha=%s", project_ref, @@ -225,7 +259,12 @@ async def _maybe_update_approval( head_sha[:12] + "…" if len(head_sha) > 12 else head_sha, ) return True - await self.adapter.revoke_change_request_approval(project_ref, cr.cr_id, head_sha) + if review_body and isinstance(self.adapter, ReviewBodyApprovalAdapter): + await self.adapter.revoke_change_request_approval_with_body( + project_ref, cr.cr_id, head_sha, body=review_body + ) + else: + await self.adapter.revoke_change_request_approval(project_ref, cr.cr_id, head_sha) logger.info( "Revoked approval project_ref=%s cr_id=%s new_findings=%s", project_ref, diff --git a/src/code_review_bot/review/publish/formatter.py b/src/code_review_bot/review/publish/formatter.py index 405fb63..00ea59b 100644 --- a/src/code_review_bot/review/publish/formatter.py +++ b/src/code_review_bot/review/publish/formatter.py @@ -6,6 +6,7 @@ from code_review_bot.skill.protocol import Finding, count_findings_by_severity BOT_METADATA_PREFIX = "