From 46cd2201d7c73fe37da42365ae2f66f5be5fedf8 Mon Sep 17 00:00:00 2001 From: AppzDev Date: Mon, 26 Oct 2026 13:43:24 +0530 Subject: [PATCH 1/3] fix(supervisor): post actionable PR comments when sweep will not merge When auto-merge is blocked, post GitHub markdown (why + checklist) instead of relying on labels or Telegram alone; improve repair-cap and stall wording. Co-authored-by: Cursor Agentflare-Agent: cursor-cli Agentflare-Branch: task/sweep-pr-merge-block-comments --- src/supervisor.rs | 2 + src/supervisor/merge.rs | 21 +++ src/supervisor/pr_merge_gate.rs | 261 ++++++++++++++++++++++++++++++++ src/supervisor/review_bots.rs | 36 +++-- 4 files changed, 311 insertions(+), 9 deletions(-) create mode 100644 src/supervisor/pr_merge_gate.rs diff --git a/src/supervisor.rs b/src/supervisor.rs index afbad435..285d4f15 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -2131,6 +2131,8 @@ fn job_in_flight(queue: &agentflare_jobs::Queue, item_id: &str) -> bool { /// keeps working unchanged. pub(crate) mod notify; pub(crate) use notify::*; +mod pr_merge_gate; +use pr_merge_gate::*; mod merge; pub(crate) use merge::*; mod auto_merge; diff --git a/src/supervisor/merge.rs b/src/supervisor/merge.rs index 13378444..b28a8b81 100644 --- a/src/supervisor/merge.rs +++ b/src/supervisor/merge.rs @@ -180,6 +180,7 @@ pub(super) fn handle_ci_green( number, PrApprovalBlocker::ApprovalLabel, ); + post_missing_approval_pr_comment(mcp, item, folder_path, number, PR_APPROVAL_LABEL); } } CiGreenMerge::BlockedOnReview { @@ -195,6 +196,15 @@ pub(super) fn handle_ci_green( label_missing, }, ); + post_github_review_block_pr_comment( + mcp, + item, + folder_path, + number, + changes_requested, + label_missing, + PR_APPROVAL_LABEL, + ); } } } @@ -227,6 +237,7 @@ pub(super) fn handle_ci_green( carries a dispatch gate label (needs-manual-dispatch / needs-human)." ), ); + post_item_dispatch_gate_pr_comment(mcp, item, folder_path, number); } result.skipped += 1; } else { @@ -556,6 +567,16 @@ pub(super) fn merge_or_repair_findings( // coming; threads merely waiting on the bot or a push are not stalls. let held = !review.unknown && review.escalated > 0; note_review_stall(mcp, queue, item, number, labels, folder_path, held, false); + post_review_threads_hold_pr_comment( + mcp, + item, + folder_path, + number, + review.blocking, + review.escalated, + review.waiting_push, + review.unknown, + ); return PassingPrOutcome::NotMerged; } note_review_stall(mcp, queue, item, number, labels, folder_path, false, false); diff --git a/src/supervisor/pr_merge_gate.rs b/src/supervisor/pr_merge_gate.rs new file mode 100644 index 00000000..0733ac9f --- /dev/null +++ b/src/supervisor/pr_merge_gate.rs @@ -0,0 +1,261 @@ +//! GitHub PR issue comments when the review sweep will not auto-merge. +//! Wording is GitHub-flavored markdown (not Telegram HTML). + +use crate::mcp_server::AgentflareMcp; +use agentflare_backend::item::Item; + +/// Marker prefix on a one-time “why merge is blocked” PR comment. +pub(super) const PR_MERGE_BLOCKED_MARKER: &str = "## supervisor — auto-merge blocked"; + +/// Item-metadata key: fingerprint of the last posted `PR_MERGE_BLOCKED_MARKER` body. +pub(super) const PR_MERGE_BLOCK_NOTIFIED_KEY: &str = "pr_merge_block_notified_for"; + +/// Builds a consistent PR comment: short title marker, why, checklist. +pub(super) fn format_merge_block_comment(why: &str, actions: &[&str]) -> String { + let mut body = format!("{PR_MERGE_BLOCKED_MARKER}\n\n"); + body.push_str("### Why\n\n"); + body.push_str(why.trim()); + body.push_str("\n\n### What you need to do\n\n"); + for action in actions { + body.push_str("- [ ] "); + body.push_str(action.trim()); + body.push('\n'); + } + body +} + +fn current_metadata( + mcp: &AgentflareMcp, + item: &Item, +) -> serde_json::Map { + let fresh = mcp + .with_backend_db(|conn| agentflare_backend::item::get(conn, &item.id)) + .ok() + .and_then(Result::ok) + .map(|i| i.metadata) + .unwrap_or_else(|| item.metadata.clone()); + crate::mcp_server::metadata_object(&fresh) +} + +fn update_metadata( + mcp: &AgentflareMcp, + item_id: &str, + merge: impl FnOnce(&mut serde_json::Map), +) -> bool { + mcp.with_backend_db(|conn| crate::mcp_server::merge_item_metadata(conn, item_id, merge)) + .ok() + .and_then(Result::ok) + .is_some() +} + +fn already_notified(mcp: &AgentflareMcp, item: &Item, fingerprint: &str) -> bool { + current_metadata(mcp, item) + .get(PR_MERGE_BLOCK_NOTIFIED_KEY) + .and_then(|v| v.as_str()) + == Some(fingerprint) +} + +/// Posts `body` on the PR once per `fingerprint` (stored on the item). Never +/// errors outward — same fail-open contract as `update_pr_stage`. +pub(super) fn post_pr_merge_block_once( + mcp: &AgentflareMcp, + item: &Item, + folder_path: &str, + pr_number: u64, + fingerprint: &str, + body: &str, +) { + if body.is_empty() || already_notified(mcp, item, fingerprint) { + return; + } + #[cfg(test)] + super::record_pr_stage_comment_for_test(body); + let Some(repo) = crate::github::RepoId::resolve_from_remote(std::path::Path::new(folder_path)) + else { + return; + }; + let Ok(client) = crate::github::Client::new() else { + return; + }; + if let Err(e) = crate::github::issues::comment(&client, &repo, pr_number, body) { + eprintln!( + "agentflare-supervisor: could not comment on PR #{pr_number}: {}", + e.log_safe() + ); + return; + } + update_metadata(mcp, &item.id, |m| { + m.insert(PR_MERGE_BLOCK_NOTIFIED_KEY.into(), fingerprint.into()); + }); +} + +/// PR comment when `status:pr:approved` is still missing. +pub(super) fn post_missing_approval_pr_comment( + mcp: &AgentflareMcp, + item: &Item, + folder_path: &str, + pr_number: u64, + approval_label: &str, +) { + let body = format_merge_block_comment( + &format!( + "This PR is CI-green and mergeable, but the review sweep will not merge it until a \ + human adds the `{approval_label}` label on GitHub." + ), + &[ + &format!("Add the `{approval_label}` label to this PR."), + "If `agentflare:needs-human` is also on the PR, read the supervisor comments above \ + and clear the underlying blocker before expecting auto-merge.", + ], + ); + post_pr_merge_block_once(mcp, item, folder_path, pr_number, "approval-label", &body); +} + +/// PR comment when the item carries `needs-human-gate` / `needs-manual-dispatch`. +pub(super) fn post_item_dispatch_gate_pr_comment( + mcp: &AgentflareMcp, + item: &Item, + folder_path: &str, + pr_number: u64, +) { + let body = format_merge_block_comment( + "This PR is CI-green and approved, but the sweep is skipping auto-merge because the \ + linked work item carries a dispatch gate label (`needs-human-gate` or \ + `needs-manual-dispatch`).", + &[ + "Open the work item and remove the gate label once you are ready for the sweep to \ + continue (or merge this PR manually on GitHub).", + "If the gate was set by a repair cap or review escalation, address those threads \ + first.", + ], + ); + post_pr_merge_block_once( + mcp, + item, + folder_path, + pr_number, + "item-dispatch-gate", + &body, + ); +} + +/// PR comment when GitHub branch protection blocks merge pending human review. +pub(super) fn post_github_review_block_pr_comment( + mcp: &AgentflareMcp, + item: &Item, + folder_path: &str, + pr_number: u64, + changes_requested: bool, + label_missing: bool, + approval_label: &str, +) { + let why = if changes_requested { + "GitHub is blocking the merge because a reviewer requested changes." + } else { + "GitHub is blocking the merge until an approving review lands (branch protection)." + }; + let mut actions: Vec = vec![ + "Read the open review feedback on the PR **Files changed** tab.".into(), + "Push fixes or reply on review threads until GitHub shows the PR as mergeable.".into(), + ]; + if label_missing { + actions.push(format!( + "Add `{approval_label}` once GitHub allows merge so the sweep can finish the merge." + )); + } + let action_refs: Vec<&str> = actions.iter().map(String::as_str).collect(); + let body = format_merge_block_comment(why, &action_refs); + post_pr_merge_block_once( + mcp, + item, + folder_path, + pr_number, + "github-review-block", + &body, + ); +} + +/// PR comment when unresolved review-bot threads hold merge (no repair dispatch this tick). +pub(super) fn post_review_threads_hold_pr_comment( + mcp: &AgentflareMcp, + item: &Item, + folder_path: &str, + pr_number: u64, + blocking: usize, + escalated: usize, + waiting_push: usize, + unknown: bool, +) { + let fingerprint = format!("review-hold:{blocking}:{escalated}:{waiting_push}:{unknown}"); + let why = if unknown { + "The sweep could not read review threads on this PR (GitHub API error or missing \ + credentials), so auto-merge is held." + .to_string() + } else if escalated > 0 { + format!( + "{escalated} review thread(s) exceeded the automatic repair round limit and need a \ + human decision; {blocking} non-optional thread(s) still block merge." + ) + } else if waiting_push > 0 { + format!( + "A fix was reported but its commit is not on the PR branch yet ({waiting_push} \ + thread(s) waiting on push)." + ) + } else { + format!( + "{blocking} unresolved review-bot thread(s) still block merge (waiting on bot \ + acceptance or human resolution)." + ) + }; + let actions = if unknown { + vec![ + "Retry after GitHub/API access is restored, or merge manually if you have verified the PR.", + ] + } else { + vec![ + "Resolve or reply on the open CodeRabbit (review-bot) threads on the PR.", + "Push any pending fixes, then record outcomes with `agentflare item review_result` if an agent worked the item.", + "Remove `agentflare:needs-human` after threads are resolved if the label was applied by the sweep.", + ] + }; + let body = format_merge_block_comment(&why, &actions); + post_pr_merge_block_once(mcp, item, folder_path, pr_number, &fingerprint, &body); +} + +/// Body for the review-repair cap (`CODERABBIT_REPAIR_CAP_MARKER`); also used on the PR via +/// `update_pr_stage` and idempotent `post_pr_merge_block_once`. +pub(super) fn format_repair_cap_pr_comment( + cap_marker: &str, + fingerprint: &str, + findings_count: usize, + cap: u32, +) -> String { + let why = format!( + "{findings_count} unresolved review-bot finding(s) remain after {cap} automatic repair \ + attempt(s) (self-repair cap)." + ); + let actions = [ + "Resolve or dismiss the open review threads on this PR, or merge manually if you accept the risk.", + "Remove `agentflare:needs-human` after you have handled the findings.", + "Clear `needs-human-gate` on the work item when you want agents to resume.", + ]; + let checklist = format_merge_block_comment(&why, &actions); + format!("{cap_marker}\n\nfindings fingerprint: {fingerprint}\n\n{checklist}") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn merge_block_comment_renders_markdown_sections() { + let body = format_merge_block_comment( + "CI is green but label missing.", + &["Add the label.", "Merge manually if needed."], + ); + assert!(body.starts_with(PR_MERGE_BLOCKED_MARKER)); + assert!(body.contains("### Why")); + assert!(body.contains("### What you need to do")); + assert!(body.contains("- [ ] Add the label.")); + } +} diff --git a/src/supervisor/review_bots.rs b/src/supervisor/review_bots.rs index 6d8fc158..21cc023d 100644 --- a/src/supervisor/review_bots.rs +++ b/src/supervisor/review_bots.rs @@ -936,10 +936,16 @@ pub(super) fn note_review_stall( if !notify { return; } - let message = format!( - "## supervisor — approved PR stalled on review threads - -This PR is approved and CI-green but unresolved review-bot threads have held the merge for {ticks} sweeps with no repair job running. Fix and push (then record the result with `review_result`), or resolve the threads, to unblock it." + let why = format!( + "This PR is approved and CI-green but unresolved review-bot threads have held the merge for {ticks} sweep(s) with no repair job running." + ); + let message = format_merge_block_comment( + &why, + &[ + "Resolve or reply on the open review-bot threads on this PR.", + "Push any fixes, then record the outcome with `agentflare item review_result` on the work item.", + "Remove `agentflare:needs-human` once threads are cleared so the sweep can merge.", + ], ); // Record the notification only once the label landed, so a failed label // add is retried next sweep instead of being skipped for good. @@ -1050,7 +1056,20 @@ pub(super) fn coderabbit_repair_or_gate( ensure_needs_human_item_gate(mcp, item, label_id_by_name); let pr_gated = labels.iter().any(|l| l == NEEDS_HUMAN_PR_LABEL); if pr_gated { - // Already announced, or gated for another reason: nothing to add. + let cap_message = format_repair_cap_pr_comment( + CODERABBIT_REPAIR_CAP_MARKER, + &fingerprint, + findings.len(), + crate::quota::decide::SELF_REPAIR_CAP, + ); + post_pr_merge_block_once( + mcp, + item, + folder_path, + pr_number, + &format!("repair-cap:{fingerprint}"), + &cap_message, + ); } else if coderabbit_cap_announced_for(mcp, item, &fingerprint) { // Announced, but the PR is not carrying its stage label (the // swap below failed, or it was reverted out-of-band): ensure it @@ -1063,10 +1082,9 @@ pub(super) fn coderabbit_repair_or_gate( "", ); } else { - let cap_message = format!( - "{CODERABBIT_REPAIR_CAP_MARKER}\n\nfindings fingerprint: {fingerprint}\n\n\ - {} unresolved finding(s) remain. {} automatic repair attempt(s) already made — \ - needs a human look.", + let cap_message = format_repair_cap_pr_comment( + CODERABBIT_REPAIR_CAP_MARKER, + &fingerprint, findings.len(), crate::quota::decide::SELF_REPAIR_CAP, ); From ff0d46c68cbb185343c2c6a8df44176a4b102ace Mon Sep 17 00:00:00 2001 From: AppzDev Date: Mon, 26 Oct 2026 14:11:20 +0530 Subject: [PATCH 2/3] fix(supervisor): clippy on merge gate + PR comments when CI repair skips Allow too_many_arguments on review-thread hold helper; post GitHub why/checklist when CI self-repair is skipped or deferred (gated, no agent, pressure, etc.). Co-authored-by: Cursor Agentflare-Agent: cursor-cli Agentflare-Branch: task/sweep-pr-merge-block-comments --- src/supervisor.rs | 76 +++++++++++++++++++++++++++++++++ src/supervisor/pr_merge_gate.rs | 33 ++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/src/supervisor.rs b/src/supervisor.rs index 285d4f15..71db0548 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -2283,7 +2283,35 @@ fn self_repair_or_gate( label_id_by_name: &std::collections::HashMap, folder_path: &str, ) -> SelfRepairOutcome { + let failing_checks = match trigger { + RepairTrigger::FailingChecks(checks) => Some(checks), + _ => None, + }; + let ci_repair_nudge = |reason_key: &str, why: &str, actions: &[&str], deferred: bool| { + if let Some(checks) = failing_checks { + post_ci_self_repair_blocked_pr_comment( + mcp, + item, + folder_path, + pr_number, + checks, + reason_key, + why, + actions, + deferred, + ); + } + }; if already_gated_or_in_flight(mcp, queue, item, label_id_by_name) { + ci_repair_nudge( + "gated", + "the work item is gated (`needs-human-gate` / `needs-manual-dispatch`) or a repair job is already in flight", + &[ + "Clear the dispatch gate label on the item if you want the sweep to retry.", + "Wait for the in-flight job to finish, or cancel it before expecting another dispatch.", + ], + false, + ); return SelfRepairOutcome::Skipped; } @@ -2359,6 +2387,12 @@ fn self_repair_or_gate( // Item #341: after the cap handling above, so an exhausted item still // reaches its human gate while the branch stays occupied. if branch_held_by_foreign_worktree(item, folder_path) { + ci_repair_nudge( + "foreign-worktree", + "another worktree holds this PR branch", + &["Finish work in the other worktree, or merge from the machine that owns the branch."], + true, + ); return SelfRepairOutcome::Deferred; } @@ -2386,20 +2420,47 @@ fn self_repair_or_gate( .and_then(Result::ok) .unwrap_or(false); if claim_still_live { + ci_repair_nudge( + "live-claim", + "another agent still holds the item claim", + &["Wait for the claim TTL to expire, then the sweep will retry."], + true, + ); return SelfRepairOutcome::Deferred; } // Resolved before the claim comment so a skipped repair never leaves a // claim marker behind to be re-posted on every TTL expiry. let Some(agent) = resolve_repair_agent(queue, item, pr_number) else { + ci_repair_nudge( + "no-agent", + "no autonomous repair agent could be resolved for this item/PR", + &[ + "Use branch `task/-…`, link the PR with `agentflare pr open --item `, and assign an autonomous agent on the item.", + "Fix failing checks locally and push, or merge manually once green.", + ], + false, + ); return SelfRepairOutcome::Skipped; }; if crate::auth_db::is_cooling_down(auth_conn, agent.as_str()) { + ci_repair_nudge( + "agent-cooldown", + "the repair agent is in cooldown after a recent failure", + &["Wait for cooldown to clear; the sweep will retry automatically."], + true, + ); return SelfRepairOutcome::Deferred; } // Independent of the per-agent cooldown above — the host's own // CPU-pressure tier. Both gates must pass. if host_policy.blocks_dispatch() { + ci_repair_nudge( + "cpu-pressure", + "host CPU pressure paused autonomous dispatches", + &["Wait for load to drop; the sweep will retry when dispatch is allowed again."], + true, + ); return SelfRepairOutcome::Deferred; } @@ -2421,11 +2482,26 @@ fn self_repair_or_gate( if let Some((repo, client)) = repo_and_client && !claim_self_repair(&client, &repo, pr_number, &item.id, crate::claims::now()) { + ci_repair_nudge( + "pr-claim", + "another workstation holds the self-repair claim on this PR", + &["Wait for the other instance to finish, or fix CI on the machine that owns the PR."], + true, + ); return SelfRepairOutcome::Deferred; } let reason = trigger.dispatch_reason(); let Some(info) = enqueue_work_job(queue, item, agent, Some(folder_path), Some(&reason)) else { + ci_repair_nudge( + "enqueue-failed", + "the repair job could not be enqueued", + &[ + "Check the agentflare jobs queue / logs on this machine.", + "Fix failing checks locally and push.", + ], + false, + ); return SelfRepairOutcome::Skipped; }; if announced_for.as_deref() == Some(fingerprint.as_str()) { diff --git a/src/supervisor/pr_merge_gate.rs b/src/supervisor/pr_merge_gate.rs index 0733ac9f..fc741034 100644 --- a/src/supervisor/pr_merge_gate.rs +++ b/src/supervisor/pr_merge_gate.rs @@ -176,6 +176,7 @@ pub(super) fn post_github_review_block_pr_comment( } /// PR comment when unresolved review-bot threads hold merge (no repair dispatch this tick). +#[allow(clippy::too_many_arguments)] pub(super) fn post_review_threads_hold_pr_comment( mcp: &AgentflareMcp, item: &Item, @@ -243,6 +244,38 @@ pub(super) fn format_repair_cap_pr_comment( format!("{cap_marker}\n\nfindings fingerprint: {fingerprint}\n\n{checklist}") } +/// PR comment when CI self-repair did not run (skipped or deferred). +pub(super) fn post_ci_self_repair_blocked_pr_comment( + mcp: &AgentflareMcp, + item: &Item, + folder_path: &str, + pr_number: u64, + failing_checks: &[String], + reason_key: &str, + why: &str, + actions: &[&str], + deferred: bool, +) { + let checks = if failing_checks.is_empty() { + "(unknown)".to_string() + } else { + failing_checks.join(", ") + }; + let lead = if deferred { + "CI self-repair is **deferred** this sweep" + } else { + "CI self-repair was **not dispatched**" + }; + let full_why = format!( + "{lead}: {why} + +Failing checks: `{checks}`." + ); + let fingerprint = format!("ci-repair-{reason_key}"); + let body = format_merge_block_comment(&full_why, actions); + post_pr_merge_block_once(mcp, item, folder_path, pr_number, &fingerprint, &body); +} + #[cfg(test)] mod tests { use super::*; From f3c5c80a038b25d4625bc5d43d6b9466da092cc0 Mon Sep 17 00:00:00 2001 From: AppzDev Date: Mon, 26 Oct 2026 14:45:40 +0530 Subject: [PATCH 3/3] fix(supervisor): CI repair comment clippy and stall test expectations Allow too_many_arguments on CI block helper; skip redundant review-hold PR comment when threads are escalated; align stall surfacing test with GFM body. Co-authored-by: Cursor Agentflare-Agent: cursor-cli Agentflare-Branch: task/sweep-pr-merge-block-comments --- src/supervisor/merge.rs | 22 ++++++++++++---------- src/supervisor/pr_merge_gate.rs | 1 + src/supervisor_coderabbit_tests.rs | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/supervisor/merge.rs b/src/supervisor/merge.rs index f75891fe..04ebf854 100644 --- a/src/supervisor/merge.rs +++ b/src/supervisor/merge.rs @@ -568,16 +568,18 @@ pub(super) fn merge_or_repair_findings( // coming; threads merely waiting on the bot or a push are not stalls. let held = !review.unknown && review.escalated > 0; note_review_stall(mcp, queue, item, number, labels, folder_path, held, false); - post_review_threads_hold_pr_comment( - mcp, - item, - folder_path, - number, - review.blocking, - review.escalated, - review.waiting_push, - review.unknown, - ); + if review.escalated == 0 { + post_review_threads_hold_pr_comment( + mcp, + item, + folder_path, + number, + review.blocking, + review.escalated, + review.waiting_push, + review.unknown, + ); + } return PassingPrOutcome::NotMerged; } note_review_stall(mcp, queue, item, number, labels, folder_path, false, false); diff --git a/src/supervisor/pr_merge_gate.rs b/src/supervisor/pr_merge_gate.rs index fc741034..0be12031 100644 --- a/src/supervisor/pr_merge_gate.rs +++ b/src/supervisor/pr_merge_gate.rs @@ -245,6 +245,7 @@ pub(super) fn format_repair_cap_pr_comment( } /// PR comment when CI self-repair did not run (skipped or deferred). +#[allow(clippy::too_many_arguments)] pub(super) fn post_ci_self_repair_blocked_pr_comment( mcp: &AgentflareMcp, item: &Item, diff --git a/src/supervisor_coderabbit_tests.rs b/src/supervisor_coderabbit_tests.rs index a6e4dc29..5b2a5d73 100644 --- a/src/supervisor_coderabbit_tests.rs +++ b/src/supervisor_coderabbit_tests.rs @@ -282,7 +282,7 @@ fn approved_green_pr_stalled_on_threads_is_surfaced_once() { assert!(get_meta().contains("review_stall_notified")); let cards = take_pr_stage_comments_for_test(); assert_eq!(cards.len(), 1, "surfaced exactly once: {cards:?}"); - assert!(cards[0].contains("stalled on review threads")); + assert!(cards[0].contains("held the merge for")); // Merely waiting on the bot is not a stall. let waiting = ReviewBotState { blocking: 1,