feat: local git checkout for PR code review with real diff
- Add git_local.py: fetch, checkout PR branch, generate git diff against target, restore branch after review - Update fetch_pr_details to use local git diff when REPOS_BASE_DIR is set, with fallback to AzDo iteration API - Update run_code_review to restore repo to target branch after review - Refine Claude review prompt to only comment on diff changes, not pre-existing code - Update README: WSL venv gotcha, local git checkout flow, flow diagram
This commit is contained in:
71
README.md
71
README.md
@@ -51,7 +51,7 @@ interactive notifications.
|
|||||||
|---------|-------------|
|
|---------|-------------|
|
||||||
| **PR Discovery** | Webhook-based (push) or polling-based (pull) — or both |
|
| **PR Discovery** | Webhook-based (push) or polling-based (pull) — or both |
|
||||||
| **Auto-Create Jira Ticket** | When PR branch has no ticket ID, Claude generates summary + description and creates a Jira Story |
|
| **Auto-Create Jira Ticket** | When PR branch has no ticket ID, Claude generates summary + description and creates a Jira Story |
|
||||||
| **AI Code Review** | Claude Code CLI reviews PRs with full repo context (Read/Glob/Grep), using your subscription |
|
| **AI Code Review** | Claude Code CLI reviews PRs on the checked-out PR branch with full repo context (Read/Glob/Grep), using real `git diff` |
|
||||||
| **CI/CD Integration** | Triggers CI builds after merge, polls for completion, handles CD release approval gates |
|
| **CI/CD Integration** | Triggers CI builds after merge, polls for completion, handles CD release approval gates |
|
||||||
| **Slack Interactive** | Approval requests with [Approve]/[Cancel] buttons, CI/CD status notifications |
|
| **Slack Interactive** | Approval requests with [Approve]/[Cancel] buttons, CI/CD status notifications |
|
||||||
| **Human-in-the-loop** | 5 interrupt points where operator confirmation is required before destructive actions |
|
| **Human-in-the-loop** | 5 interrupt points where operator confirmation is required before destructive actions |
|
||||||
@@ -120,7 +120,7 @@ All configuration is via environment variables. See `.env.example` for the full
|
|||||||
|
|
||||||
| Variable | Default | Description |
|
| Variable | Default | Description |
|
||||||
|----------|---------|-------------|
|
|----------|---------|-------------|
|
||||||
| `REPOS_BASE_DIR` | `""` | Base dir with Billo repos (e.g., `/c/Users/yaoji/git/Billo`) |
|
| `REPOS_BASE_DIR` | `""` | Base dir with Billo repos (e.g., `/home/kai/git/billo`). Each repo must be cloned with its AzDo name as the directory name. |
|
||||||
| `WATCHED_REPOS` | `""` | Comma-separated repos to poll (e.g., `Billo.Platform.Payment,Billo.Platform.Document.DocumentAnalyser`) |
|
| `WATCHED_REPOS` | `""` | Comma-separated repos to poll (e.g., `Billo.Platform.Payment,Billo.Platform.Document.DocumentAnalyser`) |
|
||||||
| `PR_POLL_ENABLED` | `False` | Enable periodic PR polling |
|
| `PR_POLL_ENABLED` | `False` | Enable periodic PR polling |
|
||||||
| `PR_POLL_INTERVAL_SECONDS` | `300` | Polling interval (5 min) |
|
| `PR_POLL_INTERVAL_SECONDS` | `300` | Polling interval (5 min) |
|
||||||
@@ -177,6 +177,11 @@ All configuration is via environment variables. See `.env.example` for the full
|
|||||||
|
|
||||||
```
|
```
|
||||||
parse_webhook -> fetch_pr_details -> route_after_fetch
|
parse_webhook -> fetch_pr_details -> route_after_fetch
|
||||||
|
| |
|
||||||
|
| +-- (local repo available?)
|
||||||
|
| yes -> git fetch + checkout PR branch + git diff
|
||||||
|
| no -> AzDo iteration API (file list only)
|
||||||
|
|
|
||||||
|-- merged -----------------> calculate_version -> update_staging -> CI build -> END
|
|-- merged -----------------> calculate_version -> update_staging -> CI build -> END
|
||||||
|-- active_with_ticket -----> move_jira_code_review -+
|
|-- active_with_ticket -----> move_jira_code_review -+
|
||||||
|-- active_no_ticket -------> auto_create_ticket ----+
|
|-- active_no_ticket -------> auto_create_ticket ----+
|
||||||
@@ -184,6 +189,7 @@ parse_webhook -> fetch_pr_details -> route_after_fetch
|
|||||||
run_code_review -> evaluate_review
|
run_code_review -> evaluate_review
|
||||||
|-- approve -> [Slack: Merge?] -> merge_pr
|
|-- approve -> [Slack: Merge?] -> merge_pr
|
||||||
|-- request_changes -> notify -> END
|
|-- request_changes -> notify -> END
|
||||||
|
-> restore branch to develop
|
||||||
-> Jira transitions -> calculate_version
|
-> Jira transitions -> calculate_version
|
||||||
-> update_staging -> CI build -> notify -> END
|
-> update_staging -> CI build -> notify -> END
|
||||||
```
|
```
|
||||||
@@ -311,6 +317,7 @@ src/release_agent/
|
|||||||
jira.py # Jira REST client (transitions + create_issue)
|
jira.py # Jira REST client (transitions + create_issue)
|
||||||
slack.py # Slack dual-mode (webhook + Web API)
|
slack.py # Slack dual-mode (webhook + Web API)
|
||||||
claude_review.py # Claude Code CLI (review + ticket generation)
|
claude_review.py # Claude Code CLI (review + ticket generation)
|
||||||
|
git_local.py # Local git ops (fetch, checkout PR branch, diff)
|
||||||
_http.py, _retry.py # Shared helpers
|
_http.py, _retry.py # Shared helpers
|
||||||
services/
|
services/
|
||||||
pr_poller.py # Background PR polling loop
|
pr_poller.py # Background PR polling loop
|
||||||
@@ -340,32 +347,47 @@ The app runs best on **WSL (Ubuntu)** because:
|
|||||||
### Setup
|
### Setup
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# 1. Start PostgreSQL (from Windows or WSL)
|
# 1. Clone the project to WSL native filesystem (NOT /mnt/c/)
|
||||||
cd /mnt/c/Users/yaoji/git/Billo/billo-release-agent
|
cd ~/git/billo
|
||||||
|
git clone <repo-url> billo-release-agent
|
||||||
|
cd billo-release-agent
|
||||||
|
|
||||||
|
# 2. Start PostgreSQL (from Windows or WSL)
|
||||||
docker compose up -d db
|
docker compose up -d db
|
||||||
|
|
||||||
# 2. Install uv in WSL (if needed)
|
# 3. Install uv in WSL (if needed)
|
||||||
curl -LsSf https://astral.sh/uv/install.sh | sh
|
curl -LsSf https://astral.sh/uv/install.sh | sh
|
||||||
export PATH="$HOME/.local/bin:$PATH"
|
export PATH="$HOME/.local/bin:$PATH"
|
||||||
|
|
||||||
# 3. Install dependencies
|
# 4. Create venv and install dependencies
|
||||||
|
# IMPORTANT: Run from the WSL-native path, not /mnt/c/.
|
||||||
|
# If .venv was created from /mnt/c/, delete and recreate:
|
||||||
|
# rm -rf .venv && uv venv --python python3.12
|
||||||
uv sync --all-extras
|
uv sync --all-extras
|
||||||
|
|
||||||
# 4. Configure .env
|
# 5. Configure .env
|
||||||
# Key settings:
|
# Key settings:
|
||||||
# CLAUDE_CMD=claude (not claude.cmd — WSL finds it via PATH)
|
# CLAUDE_CMD=claude
|
||||||
# REPOS_BASE_DIR=/mnt/c/Users/yaoji/git/Billo
|
# REPOS_BASE_DIR=~/git/billo (WSL native path, not /mnt/c/)
|
||||||
# PR_POLL_ENABLED=False (disable during dev to avoid noise)
|
# PR_POLL_ENABLED=False (disable during dev to avoid noise)
|
||||||
# SLACK_WEBHOOK_URL= (leave empty during dev)
|
# SLACK_WEBHOOK_URL= (leave empty during dev)
|
||||||
|
|
||||||
# 5. Start the server
|
# 6. Start the server
|
||||||
uv run uvicorn release_agent.main:app --host 0.0.0.0 --port 8080
|
uv run uvicorn release_agent.main:app --host 0.0.0.0 --port 8080
|
||||||
|
|
||||||
# 6. Test
|
# 7. Test
|
||||||
curl http://localhost:8080/status
|
curl http://localhost:8080/status
|
||||||
curl -X POST http://localhost:8080/manual/pr/10443
|
curl -X POST http://localhost:8080/manual/pr/10443
|
||||||
```
|
```
|
||||||
|
|
||||||
|
### Important: venv Must Be Created from WSL Path
|
||||||
|
|
||||||
|
If the `.venv` was created while the working directory was `/mnt/c/...`, the
|
||||||
|
uvicorn shebang will point to the Windows-mounted path. This causes the server
|
||||||
|
to load stale code from the Windows filesystem instead of your WSL edits.
|
||||||
|
|
||||||
|
To fix: `rm -rf .venv && uv venv --python python3.12 && uv sync --all-extras`
|
||||||
|
|
||||||
### Windows-only (Fallback)
|
### Windows-only (Fallback)
|
||||||
|
|
||||||
If you must run on Windows directly, use the provided `run.py` script which sets
|
If you must run on Windows directly, use the provided `run.py` script which sets
|
||||||
@@ -378,13 +400,22 @@ uv run python run.py
|
|||||||
Note: Claude CLI subprocess may return empty stdout on Windows due to event loop
|
Note: Claude CLI subprocess may return empty stdout on Windows due to event loop
|
||||||
incompatibility. WSL is the recommended approach.
|
incompatibility. WSL is the recommended approach.
|
||||||
|
|
||||||
### Performance Note: WSL + /mnt/c
|
### Local Git Checkout for Code Review
|
||||||
|
|
||||||
Claude Code CLI with `--allowedTools Read,Glob,Grep` on `/mnt/c` (Windows filesystem
|
When `REPOS_BASE_DIR` is set and the repo is cloned locally, the agent will:
|
||||||
mounted in WSL) is very slow. For faster code reviews, either:
|
|
||||||
|
|
||||||
1. **Clone repos to WSL native filesystem** (`~/git/Billo/`) and set `REPOS_BASE_DIR=~/git/Billo`
|
1. `git fetch origin` to get the latest remote state
|
||||||
2. **Remove `--allowedTools`** so Claude only reviews the diff text (faster but less thorough)
|
2. `git checkout` the PR source branch
|
||||||
|
3. `git diff origin/develop...HEAD` to generate a real diff (not just file names)
|
||||||
|
4. Run Claude Code CLI with `cwd` set to the repo on the PR branch
|
||||||
|
5. `git checkout develop` to restore the branch after review
|
||||||
|
|
||||||
|
This gives Claude full codebase context on the actual PR branch, producing much
|
||||||
|
more thorough reviews than the AzDo iteration API (which only returns file paths).
|
||||||
|
|
||||||
|
**Performance**: Clone repos to WSL native filesystem (`~/git/billo/`), not
|
||||||
|
`/mnt/c/`. Claude Code CLI with `--allowedTools Read,Glob,Grep` on `/mnt/c`
|
||||||
|
is very slow (10+ minutes per review vs seconds on native fs).
|
||||||
|
|
||||||
## Slack App Setup
|
## Slack App Setup
|
||||||
|
|
||||||
@@ -409,6 +440,8 @@ To use interactive buttons (optional — REST API approvals still work without i
|
|||||||
- Claude CLI ticket generation (tested: returns structured JSON)
|
- Claude CLI ticket generation (tested: returns structured JSON)
|
||||||
- Claude CLI code review (tested: returns structured JSON with verdict + issues)
|
- Claude CLI code review (tested: returns structured JSON with verdict + issues)
|
||||||
- PR review comments posted to Azure DevOps (inline + summary)
|
- PR review comments posted to Azure DevOps (inline + summary)
|
||||||
|
- Local git checkout + real `git diff` for PR code review (with fallback to AzDo API)
|
||||||
|
- Branch restore to develop after review completes
|
||||||
- Node type annotations fixed (`RunnableConfig` instead of `dict`)
|
- Node type annotations fixed (`RunnableConfig` instead of `dict`)
|
||||||
|
|
||||||
### Known Issues
|
### Known Issues
|
||||||
@@ -416,11 +449,13 @@ To use interactive buttons (optional — REST API approvals still work without i
|
|||||||
| Issue | Severity | Workaround |
|
| Issue | Severity | Workaround |
|
||||||
|-------|----------|------------|
|
|-------|----------|------------|
|
||||||
| Windows: Claude CLI subprocess returns empty stdout | HIGH | Run in WSL |
|
| Windows: Claude CLI subprocess returns empty stdout | HIGH | Run in WSL |
|
||||||
|
| WSL: venv created from /mnt/c/ loads stale code | HIGH | Delete .venv, recreate from WSL native path |
|
||||||
| WSL + /mnt/c: Claude CLI Read/Glob very slow (10+ min) | MEDIUM | Clone repos to WSL native fs |
|
| WSL + /mnt/c: Claude CLI Read/Glob very slow (10+ min) | MEDIUM | Clone repos to WSL native fs |
|
||||||
| Graph has no LangGraph checkpointer (interrupt not persistent) | MEDIUM | Graphs run to completion or fail; no resume |
|
| Graph has no LangGraph checkpointer (interrupt not persistent) | MEDIUM | Graphs run to completion or fail; no resume |
|
||||||
| `_upsert_thread` only writes final state (no intermediate updates) | LOW | Query DB only after graph completes |
|
| `_upsert_thread` only writes final state (no intermediate updates) | LOW | Query DB only after graph completes |
|
||||||
| CI poll may run indefinitely (no build to poll in dev) | LOW | Leave `PR_POLL_ENABLED=False` |
|
| CI poll may run indefinitely (no build to poll in dev) | LOW | Leave `PR_POLL_ENABLED=False` |
|
||||||
| Config test failures (env var leakage from .env) | LOW | Run with `-k "not test_config"` |
|
| Config test failures (env var leakage from .env) | LOW | Run with `-k "not test_config"` |
|
||||||
|
| Local git checkout mutates repo working tree | LOW | Review runs sequentially; branch restored after |
|
||||||
|
|
||||||
### TODO (Not Yet Implemented)
|
### TODO (Not Yet Implemented)
|
||||||
|
|
||||||
|
|||||||
114
docs/flow.mermaid
Normal file
114
docs/flow.mermaid
Normal file
@@ -0,0 +1,114 @@
|
|||||||
|
%% Billo Release Agent - Full Flow
|
||||||
|
%% Two main graphs: PR Completed and Release
|
||||||
|
|
||||||
|
graph TD
|
||||||
|
subgraph ENTRY["Entry Points"]
|
||||||
|
WEBHOOK["POST /webhooks/azdo"]
|
||||||
|
POLLER["PR Poller (every 5 min)"]
|
||||||
|
MANUAL["POST /manual/pr/{id}"]
|
||||||
|
end
|
||||||
|
|
||||||
|
WEBHOOK --> PARSE
|
||||||
|
POLLER --> PARSE
|
||||||
|
MANUAL --> PARSE
|
||||||
|
|
||||||
|
subgraph PR_GRAPH["PR Completed Graph"]
|
||||||
|
PARSE["parse_webhook<br/>Extract PR info, ticket ID"]
|
||||||
|
FETCH["fetch_pr_details<br/>AzDo API: get PR status"]
|
||||||
|
|
||||||
|
PARSE --> FETCH
|
||||||
|
|
||||||
|
subgraph LOCAL_GIT["Local Git (when REPOS_BASE_DIR set)"]
|
||||||
|
GIT_FETCH["git fetch origin"]
|
||||||
|
GIT_CHECKOUT["git checkout PR-branch"]
|
||||||
|
GIT_PULL["git pull origin PR-branch"]
|
||||||
|
GIT_DIFF["git diff origin/develop...HEAD"]
|
||||||
|
GIT_FETCH --> GIT_CHECKOUT --> GIT_PULL --> GIT_DIFF
|
||||||
|
end
|
||||||
|
|
||||||
|
FETCH -- "active PR + local repo" --> GIT_FETCH
|
||||||
|
GIT_DIFF --> ROUTE
|
||||||
|
FETCH -- "no local repo" --> AZDO_DIFF["AzDo iteration API<br/>(file list only)"]
|
||||||
|
AZDO_DIFF --> ROUTE
|
||||||
|
|
||||||
|
ROUTE{"route_after_fetch"}
|
||||||
|
|
||||||
|
FETCH -- "already merged" --> CALC_VER
|
||||||
|
|
||||||
|
ROUTE -- "active_no_ticket" --> AUTO_TICKET["auto_create_ticket<br/>Claude CLI generates<br/>Jira ticket content"]
|
||||||
|
AUTO_TICKET --> JIRA_CR
|
||||||
|
|
||||||
|
ROUTE -- "active_with_ticket" --> JIRA_CR["move_jira_code_review<br/>Jira: Code Review status"]
|
||||||
|
|
||||||
|
JIRA_CR --> CODE_REVIEW["run_code_review<br/>Claude Code CLI<br/>(cwd = PR branch)"]
|
||||||
|
CODE_REVIEW --> RESTORE["restore_branch<br/>git checkout develop"]
|
||||||
|
RESTORE --> EVAL{"evaluate_review"}
|
||||||
|
|
||||||
|
EVAL -- "approve" --> INTERRUPT_MERGE["interrupt_confirm_merge<br/>Slack: Merge? / Cancel?"]
|
||||||
|
EVAL -- "request_changes" --> NOTIFY_RC["notify_request_changes<br/>Slack notification"]
|
||||||
|
NOTIFY_RC --> END_RC((END))
|
||||||
|
|
||||||
|
INTERRUPT_MERGE --> MERGE["merge_pr_node<br/>AzDo: complete PR"]
|
||||||
|
MERGE --> JIRA_STAGE["move_jira_ready_for_stage<br/>Jira: Ready for Stage"]
|
||||||
|
JIRA_STAGE --> JIRA_LINK["add_jira_pr_link<br/>Jira: add PR link"]
|
||||||
|
JIRA_LINK --> CALC_VER
|
||||||
|
|
||||||
|
CALC_VER["calculate_version<br/>Semantic versioning"]
|
||||||
|
CALC_VER --> UPDATE_STAGING["update_staging<br/>PostgreSQL: staging_releases"]
|
||||||
|
UPDATE_STAGING --> CI_TRIGGER["trigger_ci_build<br/>AzDo: queue build"]
|
||||||
|
CI_TRIGGER --> CI_POLL["poll_ci_build<br/>Poll until complete"]
|
||||||
|
CI_POLL --> CI_NOTIFY["notify_ci_result<br/>Slack notification"]
|
||||||
|
CI_NOTIFY --> END_PR((END))
|
||||||
|
end
|
||||||
|
|
||||||
|
subgraph RELEASE_GRAPH["Release Graph"]
|
||||||
|
LOAD["load_staging<br/>Load current staging release"]
|
||||||
|
LOAD --> INT_REL["interrupt_confirm_release<br/>Slack: Create release? / Cancel?"]
|
||||||
|
INT_REL --> CREATE_PR["create_release_pr<br/>AzDo: create release PR<br/>develop -> main"]
|
||||||
|
CREATE_PR --> INT_MERGE_REL["interrupt_confirm_merge_release<br/>Slack: Merge release? / Cancel?"]
|
||||||
|
INT_MERGE_REL --> MERGE_REL["merge_release_pr<br/>AzDo: complete release PR"]
|
||||||
|
|
||||||
|
MERGE_REL --> CI_MAIN["trigger_ci_build_main<br/>AzDo: queue build on main"]
|
||||||
|
CI_MAIN --> CI_POLL_MAIN["poll_ci_build_main<br/>Poll until complete"]
|
||||||
|
|
||||||
|
CI_POLL_MAIN --> CI_ROUTE{"route_ci_result"}
|
||||||
|
|
||||||
|
CI_ROUTE -- "ci_failed" --> CI_FAIL["notify_ci_failure<br/>Slack notification"]
|
||||||
|
CI_FAIL --> END_FAIL((END))
|
||||||
|
|
||||||
|
CI_ROUTE -- "ci_passed" --> CD_WAIT["wait_for_cd_release<br/>Wait for CD pipeline"]
|
||||||
|
CD_WAIT --> POLL_APPROVALS["poll_release_approvals"]
|
||||||
|
|
||||||
|
POLL_APPROVALS --> APPROVAL_ROUTE{"route_approval_stage"}
|
||||||
|
|
||||||
|
APPROVAL_ROUTE -- "sandbox_pending" --> INT_SANDBOX["interrupt_sandbox_approval<br/>Slack: Approve Sandbox? / Skip?"]
|
||||||
|
INT_SANDBOX --> EXEC_SANDBOX["execute_sandbox_approval<br/>AzDo: approve stage"]
|
||||||
|
EXEC_SANDBOX --> POLL_APPROVALS
|
||||||
|
|
||||||
|
APPROVAL_ROUTE -- "prod_pending" --> INT_PROD["interrupt_prod_approval<br/>Slack: Approve Prod? / Skip?"]
|
||||||
|
INT_PROD --> EXEC_PROD["execute_prod_approval<br/>AzDo: approve stage"]
|
||||||
|
EXEC_PROD --> POLL_APPROVALS
|
||||||
|
|
||||||
|
APPROVAL_ROUTE -- "all_deployed" --> DONE["move_tickets_to_done<br/>Jira: Done status"]
|
||||||
|
DONE --> SLACK_NOTIFY["send_release_notification<br/>Slack: release notes"]
|
||||||
|
SLACK_NOTIFY --> ARCHIVE["archive_release<br/>PostgreSQL: archived_releases"]
|
||||||
|
ARCHIVE --> END_REL((END))
|
||||||
|
end
|
||||||
|
|
||||||
|
MANUAL_REL["POST /manual/release"] --> LOAD
|
||||||
|
|
||||||
|
style ENTRY fill:#e8f4fd,stroke:#2196F3
|
||||||
|
style PR_GRAPH fill:#f9f9f9,stroke:#666
|
||||||
|
style RELEASE_GRAPH fill:#f9f9f9,stroke:#666
|
||||||
|
style LOCAL_GIT fill:#e8f5e9,stroke:#4CAF50
|
||||||
|
style INTERRUPT_MERGE fill:#fff3e0,stroke:#FF9800
|
||||||
|
style INT_REL fill:#fff3e0,stroke:#FF9800
|
||||||
|
style INT_MERGE_REL fill:#fff3e0,stroke:#FF9800
|
||||||
|
style INT_SANDBOX fill:#fff3e0,stroke:#FF9800
|
||||||
|
style INT_PROD fill:#fff3e0,stroke:#FF9800
|
||||||
|
style NOTIFY_RC fill:#ffebee,stroke:#f44336
|
||||||
|
style CI_FAIL fill:#ffebee,stroke:#f44336
|
||||||
|
style END_RC fill:#ccc,stroke:#666
|
||||||
|
style END_PR fill:#ccc,stroke:#666
|
||||||
|
style END_FAIL fill:#ccc,stroke:#666
|
||||||
|
style END_REL fill:#c8e6c9,stroke:#4CAF50
|
||||||
@@ -13,6 +13,7 @@ from langgraph.types import RunnableConfig, interrupt
|
|||||||
|
|
||||||
from release_agent.branch_parser import parse_branch
|
from release_agent.branch_parser import parse_branch
|
||||||
from release_agent.exceptions import ReleaseAgentError
|
from release_agent.exceptions import ReleaseAgentError
|
||||||
|
from release_agent.tools.git_local import prepare_pr_branch, restore_branch
|
||||||
from release_agent.graph.ci_nodes import notify_ci_result, poll_ci_build, trigger_ci_build
|
from release_agent.graph.ci_nodes import notify_ci_result, poll_ci_build, trigger_ci_build
|
||||||
from release_agent.graph.dependencies import JsonFileStagingStore, ToolClients
|
from release_agent.graph.dependencies import JsonFileStagingStore, ToolClients
|
||||||
from release_agent.graph.routing import has_ticket, is_pr_already_merged, is_review_approved, route_after_fetch
|
from release_agent.graph.routing import has_ticket, is_pr_already_merged, is_review_approved, route_after_fetch
|
||||||
@@ -83,6 +84,11 @@ async def parse_webhook(state: dict[str, Any], config: RunnableConfig) -> dict:
|
|||||||
async def fetch_pr_details(state: dict[str, Any], config: RunnableConfig) -> dict:
|
async def fetch_pr_details(state: dict[str, Any], config: RunnableConfig) -> dict:
|
||||||
"""Fetch full PR details from AzDo and check if already merged.
|
"""Fetch full PR details from AzDo and check if already merged.
|
||||||
|
|
||||||
|
When a local repo is available (repos_base_dir configured), fetches and
|
||||||
|
checks out the PR branch locally, then generates a real git diff against
|
||||||
|
the target branch. Falls back to AzDo iteration API if local repo is
|
||||||
|
unavailable.
|
||||||
|
|
||||||
Sets pr_already_merged, pr_diff, and last_merge_source_commit.
|
Sets pr_already_merged, pr_diff, and last_merge_source_commit.
|
||||||
On ReleaseAgentError, appends to errors (non-critical).
|
On ReleaseAgentError, appends to errors (non-critical).
|
||||||
"""
|
"""
|
||||||
@@ -94,9 +100,36 @@ async def fetch_pr_details(state: dict[str, Any], config: RunnableConfig) -> dic
|
|||||||
already_merged = pr.pr_status == "completed"
|
already_merged = pr.pr_status == "completed"
|
||||||
diff = ""
|
diff = ""
|
||||||
if not already_merged:
|
if not already_merged:
|
||||||
diff = await clients.azdo.get_pr_diff(pr_id)
|
# Try local git diff first (real diff content)
|
||||||
|
repos_base_dir = config.get("configurable", {}).get("repos_base_dir", "")
|
||||||
|
repo_name = pr.repo_name
|
||||||
|
source_branch = pr.branch
|
||||||
|
webhook_resource = (state.get("webhook_payload") or {}).get("resource", {})
|
||||||
|
target_branch = webhook_resource.get("target_ref_name", "refs/heads/develop")
|
||||||
|
|
||||||
|
import logging
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
local_diff_ok = False
|
||||||
|
if repos_base_dir and repo_name and source_branch:
|
||||||
|
from pathlib import Path
|
||||||
|
repo_path = Path(repos_base_dir) / repo_name
|
||||||
|
if repo_path.is_dir():
|
||||||
|
logger.info("Using local git diff for PR %d in %s", pr_id, repo_path)
|
||||||
|
diff, err = await prepare_pr_branch(
|
||||||
|
repo_path=repo_path,
|
||||||
|
source_branch=source_branch,
|
||||||
|
target_branch=target_branch,
|
||||||
|
)
|
||||||
|
if err:
|
||||||
|
logger.warning("Local git diff failed: %s, falling back to AzDo API", err)
|
||||||
|
else:
|
||||||
|
local_diff_ok = True
|
||||||
|
|
||||||
|
# Fall back to AzDo API if local diff not available
|
||||||
|
if not local_diff_ok:
|
||||||
|
diff = await clients.azdo.get_pr_diff(pr_id)
|
||||||
|
|
||||||
# last_merge_source_commit is not directly on PRInfo; pass None if unavailable
|
|
||||||
return {
|
return {
|
||||||
"pr_already_merged": already_merged,
|
"pr_already_merged": already_merged,
|
||||||
"pr_diff": diff,
|
"pr_diff": diff,
|
||||||
@@ -243,6 +276,12 @@ async def _post_review_to_pr(
|
|||||||
async def run_code_review(state: dict[str, Any], config: RunnableConfig) -> dict:
|
async def run_code_review(state: dict[str, Any], config: RunnableConfig) -> dict:
|
||||||
"""Run Claude code review on the PR diff.
|
"""Run Claude code review on the PR diff.
|
||||||
|
|
||||||
|
The local repo should already be on the PR branch (checked out by
|
||||||
|
fetch_pr_details). Claude Code CLI runs with cwd set to the repo
|
||||||
|
so it can Read/Glob/Grep the full codebase on the PR branch.
|
||||||
|
|
||||||
|
After the review completes, restores the repo to the target branch.
|
||||||
|
|
||||||
Returns review_result as a serialisable dict.
|
Returns review_result as a serialisable dict.
|
||||||
On error, appends to errors.
|
On error, appends to errors.
|
||||||
"""
|
"""
|
||||||
@@ -255,6 +294,7 @@ async def run_code_review(state: dict[str, Any], config: RunnableConfig) -> dict
|
|||||||
# Build cwd from repos_base_dir + repo_name so Claude Code can read the codebase
|
# Build cwd from repos_base_dir + repo_name so Claude Code can read the codebase
|
||||||
repos_base_dir = config.get("configurable", {}).get("repos_base_dir", "")
|
repos_base_dir = config.get("configurable", {}).get("repos_base_dir", "")
|
||||||
cwd = None
|
cwd = None
|
||||||
|
repo_path = None
|
||||||
if repos_base_dir and repo_name:
|
if repos_base_dir and repo_name:
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
repo_path = Path(repos_base_dir) / repo_name
|
repo_path = Path(repos_base_dir) / repo_name
|
||||||
@@ -272,6 +312,15 @@ async def run_code_review(state: dict[str, Any], config: RunnableConfig) -> dict
|
|||||||
return {"review_result": review.model_dump(mode="json")}
|
return {"review_result": review.model_dump(mode="json")}
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
return {"errors": [f"run_code_review failed: {exc}"]}
|
return {"errors": [f"run_code_review failed: {exc}"]}
|
||||||
|
finally:
|
||||||
|
# Restore repo to target branch after review
|
||||||
|
if repo_path and repo_path.is_dir():
|
||||||
|
target = pr_info.get("branch", "develop")
|
||||||
|
# target_ref_name is the merge target, not the source
|
||||||
|
target_branch = (state.get("webhook_payload") or {}).get(
|
||||||
|
"resource", {}
|
||||||
|
).get("target_ref_name", "refs/heads/develop")
|
||||||
|
await restore_branch(repo_path=repo_path, branch=target_branch)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -53,11 +53,16 @@ _REVIEW_JSON_SCHEMA = json.dumps({
|
|||||||
|
|
||||||
_SYSTEM_PROMPT = (
|
_SYSTEM_PROMPT = (
|
||||||
"You are a senior code reviewer for .NET C# projects. "
|
"You are a senior code reviewer for .NET C# projects. "
|
||||||
"Review the PR for: backward compatibility (DB migration, serialization, integration events), "
|
"IMPORTANT: Only review code that appears in the diff. Do NOT comment on "
|
||||||
"null handling, business logic correctness, test coverage, hardcoded values, and security. "
|
"pre-existing code that is not part of this PR's changes. "
|
||||||
"You have access to the full codebase via Read, Glob, and Grep tools. "
|
"You may use Read, Glob, and Grep tools to understand surrounding context, "
|
||||||
"Use them to understand the context around the changed files. "
|
"but every issue you report MUST reference a line that was added or modified "
|
||||||
"For each issue, include the file_path and line_start/line_end where the issue occurs."
|
"in the diff (lines starting with '+' in the unified diff). "
|
||||||
|
"Review the changed code for: backward compatibility (DB migration, serialization, "
|
||||||
|
"integration events), null handling, business logic correctness, test coverage, "
|
||||||
|
"hardcoded values, and security. "
|
||||||
|
"For each issue, include the file_path and line_start/line_end where the issue occurs "
|
||||||
|
"in the current version of the file."
|
||||||
)
|
)
|
||||||
|
|
||||||
# JSON schema for structured ticket content output
|
# JSON schema for structured ticket content output
|
||||||
@@ -228,7 +233,8 @@ def _build_prompt(*, diff: str, pr_title: str, repo_name: str) -> str:
|
|||||||
f"Review this pull request from repository '{repo_name}'.\n\n"
|
f"Review this pull request from repository '{repo_name}'.\n\n"
|
||||||
f"PR Title: {pr_title}\n\n"
|
f"PR Title: {pr_title}\n\n"
|
||||||
f"Diff:\n```\n{diff}\n```\n\n"
|
f"Diff:\n```\n{diff}\n```\n\n"
|
||||||
"Read the related source files to understand context. "
|
"You may read related source files to understand context, but only "
|
||||||
|
"report issues on lines that were added or changed in this diff. "
|
||||||
"Provide your review as structured JSON output."
|
"Provide your review as structured JSON output."
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
124
src/release_agent/tools/git_local.py
Normal file
124
src/release_agent/tools/git_local.py
Normal file
@@ -0,0 +1,124 @@
|
|||||||
|
"""Local git operations for PR branch checkout and diff generation.
|
||||||
|
|
||||||
|
Provides helpers to fetch, checkout, and diff PR branches in local repos
|
||||||
|
so Claude Code CLI can review code with full codebase context.
|
||||||
|
"""
|
||||||
|
|
||||||
|
import asyncio
|
||||||
|
import logging
|
||||||
|
import subprocess
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
_GIT_TIMEOUT_SECONDS = 60
|
||||||
|
|
||||||
|
|
||||||
|
async def _run_git(
|
||||||
|
args: list[str],
|
||||||
|
cwd: str,
|
||||||
|
timeout: int = _GIT_TIMEOUT_SECONDS,
|
||||||
|
) -> tuple[str, str, int]:
|
||||||
|
"""Run a git command in a thread pool and return (stdout, stderr, returncode)."""
|
||||||
|
cmd = ["git"] + args
|
||||||
|
|
||||||
|
def _run() -> tuple[str, str, int]:
|
||||||
|
result = subprocess.run(
|
||||||
|
cmd,
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
cwd=cwd,
|
||||||
|
timeout=timeout,
|
||||||
|
)
|
||||||
|
return (result.stdout, result.stderr, result.returncode)
|
||||||
|
|
||||||
|
loop = asyncio.get_event_loop()
|
||||||
|
return await loop.run_in_executor(None, _run)
|
||||||
|
|
||||||
|
|
||||||
|
async def prepare_pr_branch(
|
||||||
|
*,
|
||||||
|
repo_path: str | Path,
|
||||||
|
source_branch: str,
|
||||||
|
target_branch: str = "develop",
|
||||||
|
) -> tuple[str, str | None]:
|
||||||
|
"""Fetch and checkout the PR source branch, return git diff against target.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
repo_path: Absolute path to the local git repository.
|
||||||
|
source_branch: The PR source branch ref (e.g., "refs/heads/feat/my-feature").
|
||||||
|
target_branch: The PR target branch (e.g., "develop").
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
A tuple of (diff_text, error_message).
|
||||||
|
On success: (diff, None).
|
||||||
|
On failure: ("", error_description).
|
||||||
|
"""
|
||||||
|
repo = str(repo_path)
|
||||||
|
# Strip refs/heads/ prefix if present
|
||||||
|
branch = source_branch.removeprefix("refs/heads/")
|
||||||
|
target = target_branch.removeprefix("refs/heads/")
|
||||||
|
|
||||||
|
logger.info("Preparing PR branch '%s' in %s", branch, repo)
|
||||||
|
|
||||||
|
# 1. Fetch latest from remote
|
||||||
|
stdout, stderr, rc = await _run_git(["fetch", "origin"], cwd=repo)
|
||||||
|
if rc != 0:
|
||||||
|
return ("", f"git fetch failed: {stderr.strip()}")
|
||||||
|
|
||||||
|
# 2. Checkout the PR branch
|
||||||
|
# Try checkout existing local branch first, then try remote tracking
|
||||||
|
stdout, stderr, rc = await _run_git(["checkout", branch], cwd=repo)
|
||||||
|
if rc != 0:
|
||||||
|
# Branch might not exist locally yet, try creating from remote
|
||||||
|
stdout, stderr, rc = await _run_git(
|
||||||
|
["checkout", "-b", branch, f"origin/{branch}"],
|
||||||
|
cwd=repo,
|
||||||
|
)
|
||||||
|
if rc != 0:
|
||||||
|
return ("", f"git checkout failed: {stderr.strip()}")
|
||||||
|
|
||||||
|
# 3. Pull latest changes on the branch
|
||||||
|
stdout, stderr, rc = await _run_git(["pull", "origin", branch], cwd=repo)
|
||||||
|
if rc != 0:
|
||||||
|
logger.warning("git pull failed (non-fatal): %s", stderr.strip())
|
||||||
|
|
||||||
|
# 4. Generate diff against target branch
|
||||||
|
stdout, stderr, rc = await _run_git(
|
||||||
|
["diff", f"origin/{target}...HEAD", "--stat"],
|
||||||
|
cwd=repo,
|
||||||
|
)
|
||||||
|
stat_text = stdout.strip() if rc == 0 else ""
|
||||||
|
|
||||||
|
stdout, stderr, rc = await _run_git(
|
||||||
|
["diff", f"origin/{target}...HEAD"],
|
||||||
|
cwd=repo,
|
||||||
|
)
|
||||||
|
if rc != 0:
|
||||||
|
return ("", f"git diff failed: {stderr.strip()}")
|
||||||
|
|
||||||
|
diff_text = stdout.strip()
|
||||||
|
if stat_text:
|
||||||
|
diff_text = f"{stat_text}\n\n{diff_text}"
|
||||||
|
|
||||||
|
logger.info(
|
||||||
|
"PR branch '%s' ready, diff length: %d chars",
|
||||||
|
branch, len(diff_text),
|
||||||
|
)
|
||||||
|
return (diff_text, None)
|
||||||
|
|
||||||
|
|
||||||
|
async def restore_branch(
|
||||||
|
*,
|
||||||
|
repo_path: str | Path,
|
||||||
|
branch: str = "develop",
|
||||||
|
) -> None:
|
||||||
|
"""Checkout back to the specified branch (cleanup after review).
|
||||||
|
|
||||||
|
Best-effort: logs warnings on failure but does not raise.
|
||||||
|
"""
|
||||||
|
repo = str(repo_path)
|
||||||
|
target = branch.removeprefix("refs/heads/")
|
||||||
|
stdout, stderr, rc = await _run_git(["checkout", target], cwd=repo)
|
||||||
|
if rc != 0:
|
||||||
|
logger.warning("Failed to restore branch '%s': %s", target, stderr.strip())
|
||||||
Reference in New Issue
Block a user