-
Notifications
You must be signed in to change notification settings - Fork 5
feat(web-ui): PR status panel with live CI checks, review, and merge state #579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| POST /api/v2/pr/{number}/close - Close a PR without merging | ||
| """ | ||
|
|
||
| import asyncio | ||
| import logging | ||
| from typing import Optional | ||
|
|
||
|
|
@@ -77,6 +78,24 @@ class MergeResponse(BaseModel): | |
| message: str | ||
|
|
||
|
|
||
| class CICheckResponse(BaseModel): | ||
| """A single CI check run result.""" | ||
|
|
||
| name: str | ||
| status: str | ||
| conclusion: Optional[str] | ||
|
|
||
|
|
||
| class PRStatusResponse(BaseModel): | ||
| """Live PR status: CI checks, review status, and merge state.""" | ||
|
|
||
| ci_checks: list[CICheckResponse] | ||
| review_status: str # "approved" | "changes_requested" | "pending" | ||
| merge_state: str # "open" | "merged" | "closed" | ||
| pr_url: str | ||
| pr_number: int | ||
|
|
||
|
|
||
| # ============================================================================ | ||
| # Helper Functions | ||
| # ============================================================================ | ||
|
|
@@ -124,6 +143,75 @@ def _get_github_client() -> GitHubIntegration: | |
| # ============================================================================ | ||
|
|
||
|
|
||
| @router.get("/status", response_model=PRStatusResponse) | ||
| @rate_limit_standard() | ||
| async def get_pr_status( | ||
| request: Request, | ||
| pr_number: int = Query(..., description="PR number to poll"), | ||
| workspace: Workspace = Depends(get_v2_workspace), | ||
| ) -> PRStatusResponse: | ||
| """Get live PR status: CI checks, review status, and merge state. | ||
|
|
||
| Polls the GitHub API for the given PR number and returns a snapshot | ||
| of all three status dimensions. The frontend polls this every 30 s | ||
| and stops when merge_state is merged or closed. | ||
|
|
||
| Args: | ||
| pr_number: PR number to inspect | ||
| workspace: v2 Workspace (for context) | ||
|
|
||
| Returns: | ||
| PRStatusResponse with CI checks, review status, and merge state | ||
| """ | ||
| try: | ||
| client = _get_github_client() | ||
|
|
||
| # Single call to get PR state, URL, and head SHA. | ||
| pr_raw = await client._make_request( | ||
| "GET", | ||
| f"/repos/{client.owner}/{client.repo_name}/pulls/{pr_number}", | ||
| ) | ||
| head_sha: str = pr_raw["head"]["sha"] | ||
| pr_url: str = pr_raw["html_url"] | ||
| merge_state: str = "merged" if pr_raw.get("merged_at") else pr_raw["state"] | ||
|
Comment on lines
+174
to
+176
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate required PR fields before deriving status fields. Line 174-Line 176 directly indexes Suggested fix- head_sha: str = pr_raw["head"]["sha"]
- pr_url: str = pr_raw["html_url"]
- merge_state: str = "merged" if pr_raw.get("merged_at") else pr_raw["state"]
+ head_sha = (pr_raw.get("head") or {}).get("sha") if isinstance(pr_raw, dict) else None
+ pr_url = pr_raw.get("html_url") if isinstance(pr_raw, dict) else None
+ pr_state = pr_raw.get("state") if isinstance(pr_raw, dict) else None
+ if not head_sha or not pr_url or pr_state not in {"open", "closed"}:
+ raise HTTPException(
+ status_code=502,
+ detail=api_error(
+ "Invalid GitHub PR payload",
+ ErrorCodes.EXECUTION_FAILED,
+ f"PR #{pr_number} missing required fields",
+ ),
+ )
+ merge_state: str = "merged" if pr_raw.get("merged_at") else pr_state🤖 Prompt for AI Agents |
||
|
|
||
| # Fetch CI checks and reviews in parallel (2 more GitHub API calls). | ||
| ci_checks, review_status = await asyncio.gather( | ||
| client.get_pr_ci_checks(pr_number, head_sha=head_sha), | ||
| client.get_pr_review_status(pr_number), | ||
| ) | ||
|
|
||
| return PRStatusResponse( | ||
| ci_checks=[ | ||
| CICheckResponse(name=c.name, status=c.status, conclusion=c.conclusion) | ||
| for c in ci_checks | ||
| ], | ||
| review_status=review_status, | ||
| merge_state=merge_state, | ||
| pr_url=pr_url, | ||
| pr_number=pr_number, | ||
| ) | ||
|
|
||
| except GitHubAPIError as e: | ||
| if e.status_code == 404: | ||
| raise HTTPException( | ||
| status_code=404, | ||
| detail=api_error("PR not found", ErrorCodes.NOT_FOUND, f"No PR #{pr_number}"), | ||
| ) | ||
| raise HTTPException( | ||
| status_code=e.status_code, | ||
| detail=api_error("GitHub API error", ErrorCodes.EXECUTION_FAILED, e.message), | ||
| ) | ||
| except HTTPException: | ||
| raise | ||
| except Exception as e: | ||
| logger.error(f"Failed to get PR #{pr_number} status: {e}", exc_info=True) | ||
| raise HTTPException( | ||
| status_code=500, | ||
| detail=api_error("Failed to get PR status", ErrorCodes.EXECUTION_FAILED, str(e)), | ||
| ) | ||
|
|
||
|
Comment on lines
+167
to
+213
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Close At Line 167, a new client is created but never closed. Since Suggested fix async def get_pr_status(
@@
) -> PRStatusResponse:
@@
- try:
- client = _get_github_client()
+ client: Optional[GitHubIntegration] = None
+ try:
+ client = _get_github_client()
@@
except Exception as e:
logger.error(f"Failed to get PR #{pr_number} status: {e}", exc_info=True)
raise HTTPException(
status_code=500,
detail=api_error("Failed to get PR status", ErrorCodes.EXECUTION_FAILED, str(e)),
)
+ finally:
+ if client is not None:
+ await client.close()🤖 Prompt for AI Agents |
||
|
|
||
| @router.get("", response_model=PRListResponse) | ||
| @rate_limit_standard() | ||
| async def list_pull_requests( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard review payload type before state aggregation.
At Line 430-Line 431,
r.get(...)assumes each review item is a dict. Ifreviewsis non-list or mixed, this can raise and mask the real upstream issue.Suggested hardening
🤖 Prompt for AI Agents