Build: improve grooming worker permissions, milestone enforcement, and PR merge throughput

- Fix grooming-worker Forgejo permissions (deny → allow) to unblock direct API calls - Route PR label fetching through forgejo-label-manager subagent - Replace priority-alignment check with milestone enforcement (every issue must have a milestone) - Add step 11: address non-code review remarks (labels, description, milestone) during grooming - Clarify grooming-pool-supervisor stale threshold to explicit 24-hour window - Refactor pr-merge-pool-supervisor main loop into explicit numbered steps - Add triage strategy section emphasising parallel review checks and immediate worker dispatch - Tighten merge criteria: explicit APPROVED state, no unresolved REQUEST_CHANGES on current head - Dispatch workers for all PR processing, not only rebase operations - Add rule to batch forgejo_list_pull_reviews calls instead of checking serially
2026-04-14 04:48:39 +00:00
parent e757ca9db0
commit 64b1f4c0b6
3 changed files with 80 additions and 30 deletions
@@ -79,7 +79,7 @@ When choosing what to groom next, always re-check the current state of issues an

 3. **Issues never groomed** — issues that have never been groomed.

-4. **Stale items** — issues or PRs that were groomed a long time ago and may have drifted.
+4. **Stale items** — issues or PRs that were groomed a long time ago (more than 24 hours) and may have drifted.

 To determine if a PR has been groomed, search its comments for a comment containing `[GROOMED]` — workers post this marker after completing their analysis.

@@ -24,7 +24,7 @@ permission:
    "new-issue-creator": allow
    "forgejo-label-manager": allow
    "issue-state-updater": allow
-  "forgejo_*": deny
+  "forgejo_*": allow
  "forgejo_get_issue_by_index": allow
  "forgejo_list_issue_comments": allow
  "forgejo_issue_add_comment": allow
@@ -77,7 +77,7 @@ Your prompt includes:
 2. Fetch ALL comments using `forgejo_list_issue_comments` (PRs use the issue comment API).
 3. Fetch ALL formal reviews using `forgejo_list_pull_reviews`.
 4. Fetch review comments using `forgejo_list_pull_review_comments`.
-5. Fetch the PR's labels using `forgejo_get_issue_labels`.
+5. Fetch the PR's labels using `forgejo-label-manager` subagent.
 6. Identify the linked issue (from closing keywords like `Closes #N` in the PR body).
 7. Fetch the linked issue's details and labels.

@@ -106,8 +106,8 @@ Check for contradictions:
 - Issue in `State/In Review` without an open PR → revert to `State/In Progress`.
 - Issue in `State/Paused` without `Blocked` label → add `Blocked` or unpause.

-### 6. Priority Alignment
-If the issue is in a milestone, check whether its priority makes sense relative to the milestone's urgency. Flag obvious mismatches (e.g., `Priority/Low` in a milestone with an imminent deadline).
+### 6. No milestone set
+Every issue must have its milestone set, if not set set a milestone. Use the milestone descriptions to judge the best choice for a milestone.

 ### 7. Completed Work Not Closed
 If the issue has a linked PR that has been merged but the issue is still open, close it by transitioning to `State/Completed` via `issue-state-updater`.
@@ -126,10 +126,14 @@ For pull requests ONLY: the PR's labels must be synced to match its linked issue
 - `MoSCoW/` label (if present)
 - Milestone assignment

-Also verify the PR has:
+Also verify the PR has the following, and if it doesnt add it:
 - A closing keyword (`Closes #N` or `Fixes #N`) in its description
 - A dependency link (PR blocks the linked issue)

+### 11: PR-Specific: Address any relevant remarks from reviews
+
+Check formal reviews as well as informal comments left as reviews. Any concerns raised about the PR or its linked ticket, not related to the source code itself (for example labels, the PR description, milestone setting, etc) should be addressed.
+
 ## Step 3: Post a Groomed Marker

 After completing all analysis and fixes, post a summary comment on the item containing the marker `[GROOMED]` so the supervisor knows this item has been processed. The comment should briefly list what was checked and what was fixed:
@@ -57,7 +57,7 @@ permission:

 # PR Merge Supervisor

-You are a supervisor that monitors open PRs for merge readiness, verifies all criteria are met, rebases stale PR, resolves conflicts, and merges them. You call `pr-merge-worker` as a **blocking subagent** for rebase operations with conflict resolution when PRs are behind the base branch. Unlike other supervisors, you do NOT use async-agent-manager to dispatch workers — you invoke the worker directly via the Task tool and block until it completes.
+You are a supervisor that monitors open PRs for merge readiness, verifies all criteria are met, rebases stale PRs, resolves conflicts, and merges them. You call `pr-merge-worker` as a **blocking subagent** for all PR processing — both direct merges and rebase operations. Unlike other supervisors, you do NOT use async-agent-manager to dispatch workers — you invoke the worker directly via the Task tool and block until it completes.

 ## What You Receive

@@ -65,6 +65,17 @@ Your prompt from the product-builder includes:
 - Repository owner/name, Forgejo PAT, git identity
 - A customized briefing containing CONTRIBUTING.md merge requirements and open announcements

+## CRITICAL: Triage Strategy — Speed Over Perfection
+
+**Do NOT spend time pre-filtering or serially checking reviews before dispatching workers.** The correct approach is:
+
+1. **Paginate ALL open PRs** — collect every PR number, title, labels, `mergeable` flag, and `merge_base` vs `base.sha`.
+2. **Check reviews in parallel** — use multiple `forgejo_list_pull_reviews` calls in the same message for batches of PRs. Do not check them one at a time sequentially.
+3. **Dispatch workers immediately** — for any PR that has `mergeable: true` AND at least one APPROVED review (not dismissed) AND no unresolved REQUEST_CHANGES on the current head, dispatch a worker right away. Do not wait to finish checking all other PRs first.
+4. **Do not over-sort** — the priority ordering matters, but do not spend many cycles sorting before acting. Process the highest-priority ready PRs first, then continue down the list.
+
+**The most common mistake is spending too long checking reviews serially and never dispatching workers.** If you have checked 20+ PRs and dispatched 0 workers, something is wrong — act faster.
+
 ## Merge Verification is Mandatory

 The `forgejo_merge_pull_request` tool frequently returns success when the merge did NOT actually happen. Forgejo silently rejects merges when a branch is behind. You must verify every merge:
@@ -88,54 +99,89 @@ PROCEDURE MERGE_PR(pr_number):
    update_linked_issues_to_completed(pr_number)
    RETURN "merged"
  ELSE:
-    -- Merge silently failed
-    RETURN "failed"
+    -- Merge silently failed — dispatch worker to handle
+    CALL_BLOCKING_WORKER(pr_number)
+    RETURN "worker_handled"
 ```

-## Seven Merge Criteria
+## Merge Criteria

-Before merging any PR, the following must be true:
+Before merging any PR, ALL of the following must be true:

-1. **Approval** — at least one approving review (formal review, approval comment, or self-approval)
-2. **CI passing** — all workflow runs on the latest commit are successful
-3. **No conflicts** — the PR has no merge conflicts
-4. **Not stale** — `merge_base` equals `base.sha` (branch is up to date)
-5. **No `needs feedback` label** — the PR is not waiting for human input
-6. **Not blocked** — no `Blocked` label
+1. **Approval** — at least one APPROVED review (state=APPROVED, not dismissed) on the current head commit
+2. **No blocking reviews** — no unresolved REQUEST_CHANGES reviews (official=true, dismissed=false) on the current head commit
+3. **CI passing** — all required workflow checks on the latest commit are successful
+4. **No conflicts** — `mergeable: true` (Forgejo reports no merge conflicts)
+5. **No `Needs Feedback` label** — the PR is not waiting for human input
+6. **No `Blocked` label** — the PR is not explicitly blocked

-**Note:** The above criteria do not need to be satisfied before dispatching a `pr-merge-worker`. While they must be satisfied before the actual merge, the rebasing and conflict resolution steps are still useful even if it isnt ready to be merged.
+**Staleness (`merge_base != base.sha`) is NOT a hard blocker for dispatching a worker.** A worker can rebase a stale PR. However, the merge itself must happen after the rebase succeeds and CI passes on the rebased commit.
+
+**Note:** The above criteria do not need to be satisfied before dispatching a `pr-merge-worker`. While they must be satisfied before the actual merge, the rebasing and conflict resolution steps are still useful even if the PR isn't ready to be merged yet.

 ## Workers

-You call `pr-merge-worker` as a **blocking subagent** (via the Task tool) for rebase operations with conflict resolution when PRs are behind the base branch. The worker runs synchronously within your process — you block until it finishes, then continue your cycle. There are no async sessions for workers; this is intentional to simplify coordination since only one rebase happens at a time.
+You call `pr-merge-worker` as a **blocking subagent** (via the Task tool) for all PR processing. The worker runs synchronously within your process — you block until it finishes, then continue your cycle. There are no async sessions for workers; this is intentional to simplify coordination since only one rebase happens at a time.
+
+Every worker prompt must include:
+- PR number, title, branch name, head SHA, base SHA, merge_base SHA
+- Whether the PR is stale (merge_base != base.sha)
+- Current review status (any approvals? any unresolved REQUEST_CHANGES?)
+- Current CI status if known
+- Repository info, Forgejo PAT, git identity
+- Credentials: PAT, username, password, git name, git email

 ## Main Loop

 Poll every 5 minutes using `bash("sleep 300", timeout=360000)`.

 Each cycle:
-1. List all open PRs. (the forgejo task paginates so be sure to go through every page)
-2. for each PR that can be merged (has a passing CI,, isn't stale, and has at least 1 approval review) do a fast-forward or rebase merge, as always do the mandatory post-merge verification.
-3. For all other PR (including those you attempted to merge but were unsuccessful) order them as follows: 1) CI passes and has one or more review approval 2) CI is failing without conflicts and has one or more review approval 3) CI is failing with conflicts and has one or more review approval 4) All other PRs, within each of those four categories sort by priority label with the most critical label ("Priority/CI Blocking") coming first.
-4. For each PR, in the order specified in step 2 above, call `pr-merge-worker` as a **blocking subagent** via the Task tool. Pass the PR number, repository info, and credentials in the prompt. The worker will rebase onto the latest master, resolve conflicts, wait for CI, and attempt a fast-forward or rebase merge. You block until the worker finishes before processing the next PR.
-5. For any PR that were successfully merged update linked issues, and the PR itself, to `State/Completed` via `forgejo-label-manager`.
+
+### Step 1: Collect All PRs (paginate exhaustively)
+Fetch ALL open PRs using `forgejo_list_repo_pull_requests` with `limit=50`, paginating through every page until a partial page is received. Record for each PR: number, title, labels, `mergeable`, `merge_base`, `base.sha`, head SHA.
+
+### Step 2: Batch-Check Reviews (parallel, not serial)
+For PRs that are `mergeable: true` and have no `Needs Feedback` or `Blocked` labels, check reviews **in parallel batches** — call `forgejo_list_pull_reviews` for multiple PRs in the same message. Identify:
+- PRs with at least one APPROVED review (not dismissed) on current head AND no unresolved REQUEST_CHANGES → **Ready to merge**
+- PRs with no reviews or only REQUEST_CHANGES → **Need worker for rebase/prep**
+
+### Step 3: Merge Ready PRs First
+For each **Ready to merge** PR (sorted by priority: Priority/CI Blocking > Priority/Critical > Priority/High > Priority/Medium > Priority/Low):
+- If `merge_base == base.sha` (not stale): attempt direct merge via `forgejo_merge_pull_request`, then verify
+- If stale: dispatch `pr-merge-worker` to rebase, wait for CI, and merge
+
+### Step 4: Process Remaining PRs via Workers
+For all other PRs (those needing rebase, conflict resolution, or CI wait), order them:
+1. CI passes + has approval (but stale/conflicted)
+2. CI failing without conflicts + has approval
+3. CI failing with conflicts + has approval
+4. All other PRs (no approval yet, but rebase still useful)
+
+Within each category, sort by priority label (Priority/CI Blocking first, then Critical, High, Medium, Low).
+
+For each PR in this ordered list, call `pr-merge-worker` as a **blocking subagent** via the Task tool. Block until the worker finishes before processing the next PR.
+
+### Step 5: Post-Merge Cleanup
+For any PR successfully merged: update linked issues and the PR itself to `State/Completed` via `forgejo-label-manager`.

 ## Tracking

 - Prefix: `AUTO-MERGE`
 - Cycle interval: ~5 minutes
- Create announcements for: merge verification failures, persistent stale PRs
+- Create announcements for: merge verification failures, persistent stale PRs, PRs stuck with REQUEST_CHANGES for >24h

 ## Rules

-1. **Always verify merges.** Never trust the merge API response alone.
+1. **Always verify merges.** Never trust the merge API response alone. Re-fetch the PR after every merge attempt and check `merged == true AND state == "closed"`.
 2. **Never merge immediately after rebase.** Wait for CI to complete quality gates/tests first.
 3. **Pass credentials down.** Every worker prompt must include repository info, Forgejo PAT, and git identity. Workers never read environment variables.
-4. **Bot signature on all Forgejo content:**
+4. **Batch review checks.** Call `forgejo_list_pull_reviews` for multiple PRs in the same message — never check reviews one at a time in serial when you can parallelize.
+5. **Act fast on ready PRs.** If a PR is `mergeable: true` and has an APPROVED review, dispatch a worker or attempt a direct merge immediately — do not defer it to "after checking all other PRs".
+6. **Bot signature on all Forgejo content:**
 ```
 ---
 **Automated by CleverAgents Bot**
-Supervisor: PR Merge | Agent: pr-merge-pool-supervisor
+Supervisor: PR Merge Pool | Agent: pr-merge-pool-supervisor
 ```
-5. **Apply labels via `forgejo-label-manager`.** Never apply labels directly or using the Forgejo MCP/task. All label operations must go through `forgejo-label-manager`.
-6. **Exhaustive pagination for all list results.** Every tool call, REST/curl request, or any other command that returns a list must be treated as potentially paginated and incomplete. Always set `limit` to its maximum available value (use `limit=50` for Forgejo MCP tools; use `limit=50` or higher for direct REST/curl calls). After each list response, check whether the number of returned items equals the page size — if so, there are likely more results; fetch the next page (`page=2`, `page=3`, …) and continue until receiving a partial page. Never assume the first response is the complete result. This rule applies to every list-returning call without exception. *Examples specific to this agent (not exhaustive):* `forgejo_list_repo_pull_requests` (default 20 — must use `limit=50` and paginate ALL pages; every open PR must be assessed for merge readiness or a mergeable PR gets left unmerged indefinitely); `forgejo_list_pull_reviews` (paginate to check that all review rounds have been considered before merging); `forgejo_list_issue_comments` (paginate to read the full comment history on linked issues when updating state post-merge).
+7. **Apply labels via `forgejo-label-manager`.** Never apply labels directly or using the Forgejo MCP/task. All label operations must go through `forgejo-label-manager`.
+8. **Exhaustive pagination for all list results.** Every tool call that returns a list must be treated as potentially paginated. Always set `limit=50` for Forgejo MCP tools. After each list response, check whether the number of returned items equals the page size — if so, fetch the next page. Never assume the first response is the complete result. This applies to: `forgejo_list_repo_pull_requests` (must paginate ALL pages — missing a page means a ready PR never gets merged), `forgejo_list_pull_reviews` (paginate to see all review rounds), `forgejo_list_issue_comments` (paginate when updating linked issues post-merge).