From 5fbe4bd533dddd78a55996fa9e51db27115b4b1f Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 21:39:15 +0000 Subject: [PATCH] fix(agents): Add proper CI verification to ca-issue-worker before merging PRs - Implemented missing all_checks_passing() function to query Forgejo API - Added CI status verification via commit status endpoint - Added safety warnings against using force_merge flag - Fixed documentation to reflect correct merge responsibilities - Added error handling to default to 'checks not passing' on API failures This ensures PRs cannot be merged when CI checks are failing, respecting branch protection rules and quality gates defined in CONTRIBUTING.md. --- .opencode/agents/ca-issue-worker.md | 83 +++++++++++++++++++++++++++++ .opencode/agents/product-builder.md | 9 ++-- 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/.opencode/agents/ca-issue-worker.md b/.opencode/agents/ca-issue-worker.md index 61d803318..2947430b9 100644 --- a/.opencode/agents/ca-issue-worker.md +++ b/.opencode/agents/ca-issue-worker.md @@ -36,6 +36,13 @@ permission: # CleverAgents Issue Worker +**⚠️ CRITICAL MERGE SAFETY RULES ⚠️** +1. **NEVER use force_merge** - This flag is FORBIDDEN as it bypasses CI checks +2. **ALWAYS verify CI status** via Forgejo API before any merge attempt +3. **ALWAYS check required approvals** (1 for bot PRs, 2 for human PRs) +4. **If CI is failing, fix it** - Never assume checks are passing +5. **The all_checks_passing() function MUST query actual CI status** - Never skip this + You are a dual-mode worker that handles EITHER: 1. **Issue Implementation** (issue-impl mode): Implement a new issue from scratch through PR merge 2. **PR Fixing** (pr-fix mode): Fix an existing PR that needs work through merge @@ -199,6 +206,42 @@ elif work_type == "merge-conflicts": "---\n**Automated by CleverAgents Bot**\nSupervisor: Implementation | Agent: ca-issue-worker") elif work_type == "ready-to-merge": + # Helper function to check CI status + def all_checks_passing(): + """ + Check if all required CI checks are passing for the PR. + CRITICAL: This function MUST verify actual CI status via Forgejo API. + Returns True only if ALL required checks have passed. + """ + # Get the PR to find the head commit SHA + pr_data = forgejo_get_pull_request_by_index(owner, repo, pr_number) + head_sha = pr_data.head.sha + + # Query commit status via Forgejo API + # Note: This assumes Forgejo API has a commit status endpoint similar to GitHub + # The actual endpoint may need adjustment based on Forgejo's API + try: + # Get combined status for the commit + # This should be implemented using the actual Forgejo API endpoint + # Example: GET /repos/{owner}/{repo}/commits/{sha}/status + import requests + headers = {"Authorization": f"token {forgejo_pat}"} + status_url = f"https://git.cleverthis.com/api/v1/repos/{owner}/{repo}/commits/{head_sha}/status" + + response = requests.get(status_url, headers=headers) + if response.status_code == 200: + status_data = response.json() + # Check if overall state is success + # Forgejo/Gitea typically uses: success, error, failure, pending + return status_data.get("state", "").lower() == "success" + else: + # If we can't get status, assume checks are NOT passing + print(f"[WARNING] Could not fetch CI status: {response.status_code}") + return False + except Exception as e: + print(f"[ERROR] Failed to check CI status: {e}") + return False + # Helper function to check approvals def has_required_approvals(): """ @@ -225,10 +268,12 @@ elif work_type == "ready-to-merge": # Verify all checks pass if all_checks_passing() and has_required_approvals(): # Merge the PR + # CRITICAL: Never use force_merge=True - it bypasses CI checks! forgejo_merge_pull_request(owner, repo, pr_number, style="squash", title=pr.title, message=pr.body) + # Note: force_merge parameter is intentionally omitted (defaults to False) # Post on linked issue forgejo_create_issue_comment(owner, repo, issue_number, @@ -578,6 +623,13 @@ git stash pop # Re-apply uncommitted changes (if any) **This is where you earn your keep. You OWN this PR until it merges.** +**CRITICAL NOTE: The following section contains pseudo-code with undefined functions. +When implementing this agent, ensure ALL functions are properly defined, especially:** +- `all_checks_passing()` - MUST query Forgejo API for actual CI status +- `merge_pr()` - MUST use the implementation from the "Final Merge" section +- `fix_ci_failures()` - MUST invoke ca-pr-checker +- Other helper functions must be implemented or replaced with actual code + ```python pr_merged = False max_review_cycles = 10 @@ -710,6 +762,35 @@ def handle_merge_conflicts(): ```python def merge_pr(): """Merge PR if conditions are met""" + # Helper function to check CI status + def all_checks_passing(): + """ + Check if all required CI checks are passing for the PR. + CRITICAL: This function MUST verify actual CI status via Forgejo API. + Returns True only if ALL required checks have passed. + """ + # Get the PR to find the head commit SHA + pr_data = forgejo_get_pull_request_by_index(owner, repo, pr_number) + head_sha = pr_data.head.sha + + # Query commit status via Forgejo API + try: + import requests + headers = {"Authorization": f"token {forgejo_pat}"} + status_url = f"https://git.cleverthis.com/api/v1/repos/{owner}/{repo}/commits/{head_sha}/status" + + response = requests.get(status_url, headers=headers) + if response.status_code == 200: + status_data = response.json() + # Check if overall state is success + return status_data.get("state", "").lower() == "success" + else: + print(f"[WARNING] Could not fetch CI status: {response.status_code}") + return False + except Exception as e: + print(f"[ERROR] Failed to check CI status: {e}") + return False + # Check if PR has required approvals def has_required_approvals(): """ @@ -744,9 +825,11 @@ def merge_pr(): # All conditions met - merge it! print("[MERGING] All conditions met - merging PR") + # CRITICAL: Never use force_merge=True - it bypasses CI checks! result = forgejo_merge_pull_request(owner, repo, pr_number, style="squash", delete_branch_after_merge=True) + # Note: force_merge parameter is intentionally omitted (defaults to False) if result.success: # Post final comment on issue diff --git a/.opencode/agents/product-builder.md b/.opencode/agents/product-builder.md index 5eb8843ad..40057c59a 100644 --- a/.opencode/agents/product-builder.md +++ b/.opencode/agents/product-builder.md @@ -1378,9 +1378,12 @@ No exceptions — every comment, every issue body, every PR description. yourself. Continue working with the current spec on master while waiting for human approval. Monitor these PRs periodically (see Specification PR Monitoring section). -- **PRs are merged autonomously — but ONLY when CI passes.** The PR review - pool merges PRs only after ALL CI checks pass. The `force_merge` flag is - FORBIDDEN — it bypasses branch protection and violates CONTRIBUTING.md. +- **PRs are merged autonomously — but ONLY when CI passes.** The ca-issue-worker + merges PRs only after ALL CI checks pass (verified via Forgejo API) and required + approvals are met. The `force_merge` flag is FORBIDDEN — it bypasses branch + protection and violates CONTRIBUTING.md. Note: PR reviewers do NOT merge PRs, + they only provide reviews. The ca-issue-worker owns the PR lifecycle including + the final merge. PRs with the `needs feedback` label are the exception (human must merge). - **PRESERVE PR BODIES ON EVERY API UPDATE.** The Forgejo API (both REST and MCP) will wipe the PR description/body if it is not explicitly re-sent