fix(#2417): cancel stale config-pr approvals when the pr is no longer open
This commit is contained in:
parent
2bf6290aa0
commit
f025f32ccb
1 changed files with 78 additions and 1 deletions
|
|
@ -35,6 +35,13 @@ pub async fn poll_open_config_prs(core_token: &str, coord: &Arc<Coordinator>) ->
|
||||||
.map_err(anyhow::Error::from)
|
.map_err(anyhow::Error::from)
|
||||||
.and_then(|r| r.map_err(anyhow::Error::from))?;
|
.and_then(|r| r.map_err(anyhow::Error::from))?;
|
||||||
|
|
||||||
|
// (agent, pr_number) of every OPEN config PR seen this sweep, plus the set
|
||||||
|
// of agents whose PR list was fetched successfully. The reconcile pass
|
||||||
|
// below uses these to cancel pending approvals whose PR is no longer open,
|
||||||
|
// without wrongly cancelling one when a repo's list failed transiently.
|
||||||
|
let mut open_prs: std::collections::HashSet<(String, u64)> = std::collections::HashSet::new();
|
||||||
|
let mut scanned_agents: std::collections::HashSet<String> = std::collections::HashSet::new();
|
||||||
|
|
||||||
for repo in repos {
|
for repo in repos {
|
||||||
let Some(repo_name) = repo.name.as_deref() else {
|
let Some(repo_name) = repo.name.as_deref() else {
|
||||||
continue;
|
continue;
|
||||||
|
|
@ -60,7 +67,10 @@ pub async fn poll_open_config_prs(core_token: &str, coord: &Arc<Coordinator>) ->
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
Ok(Ok(prs)) => prs,
|
Ok(Ok(prs)) => {
|
||||||
|
scanned_agents.insert(agent.to_owned());
|
||||||
|
prs
|
||||||
|
}
|
||||||
Ok(Err(e)) => {
|
Ok(Err(e)) => {
|
||||||
tracing::debug!(
|
tracing::debug!(
|
||||||
%agent, error = %e,
|
%agent, error = %e,
|
||||||
|
|
@ -81,6 +91,7 @@ pub async fn poll_open_config_prs(core_token: &str, coord: &Arc<Coordinator>) ->
|
||||||
let Some(pr_number) = pr.number.and_then(|n| u64::try_from(n).ok()) else {
|
let Some(pr_number) = pr.number.and_then(|n| u64::try_from(n).ok()) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
|
open_prs.insert((agent.to_owned(), pr_number));
|
||||||
|
|
||||||
// Skip if a pending approval already exists for this PR.
|
// Skip if a pending approval already exists for this PR.
|
||||||
match coord
|
match coord
|
||||||
|
|
@ -126,5 +137,71 @@ pub async fn poll_open_config_prs(core_token: &str, coord: &Arc<Coordinator>) ->
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reconcile: cancel pending approvals whose PR is no longer open.
|
||||||
|
reconcile_stale_config_pr_approvals(coord, &open_prs, &scanned_agents);
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Cancel pending `MergeConfigPr` approvals whose PR is no longer open — merged
|
||||||
|
/// (incl. outside the approval flow) or closed. Without this the card lingers on
|
||||||
|
/// the dashboard forever, since the webhook only signals opened PRs and the
|
||||||
|
/// add-loop only ever queues.
|
||||||
|
///
|
||||||
|
/// `open_prs` is the set of `(agent, pr_number)` seen open this sweep and
|
||||||
|
/// `scanned_agents` the agents whose PR list was fetched successfully — the
|
||||||
|
/// reconcile is guarded to those so a transient list failure can't cancel a
|
||||||
|
/// still-valid approval.
|
||||||
|
fn reconcile_stale_config_pr_approvals(
|
||||||
|
coord: &Arc<Coordinator>,
|
||||||
|
open_prs: &std::collections::HashSet<(String, u64)>,
|
||||||
|
scanned_agents: &std::collections::HashSet<String>,
|
||||||
|
) {
|
||||||
|
let pending = match coord.approvals.pending() {
|
||||||
|
Ok(p) => p,
|
||||||
|
Err(e) => {
|
||||||
|
tracing::warn!(error = ?e, "config-pr poll: pending() failed, skipping reconcile");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
for a in pending {
|
||||||
|
if a.kind != hive_sh4re::ApprovalKind::MergeConfigPr || !scanned_agents.contains(&a.agent) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let Ok(pr_number) = a.commit_ref.parse::<u64>() else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
if open_prs.contains(&(a.agent.clone(), pr_number)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
match coord
|
||||||
|
.approvals
|
||||||
|
.mark_cancelled(a.id, "config-pr poll (PR no longer open)")
|
||||||
|
{
|
||||||
|
Ok(_) => {
|
||||||
|
tracing::info!(
|
||||||
|
agent = %a.agent, pr_number, id = a.id,
|
||||||
|
"config-pr poll: cancelled stale MergeConfigPr approval (PR merged/closed)"
|
||||||
|
);
|
||||||
|
coord.emit_approval_resolved(crate::coordinator::ApprovalResolved {
|
||||||
|
id: a.id,
|
||||||
|
agent: &a.agent,
|
||||||
|
approval_kind: "merge_config_pr",
|
||||||
|
sha_short: a
|
||||||
|
.fetched_sha
|
||||||
|
.as_deref()
|
||||||
|
.map(|s| s[..s.len().min(12)].to_owned()),
|
||||||
|
status: "cancelled",
|
||||||
|
note: Some("PR merged/closed outside the approval".to_owned()),
|
||||||
|
description: a.description.clone(),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
Err(e) => {
|
||||||
|
tracing::warn!(
|
||||||
|
agent = %a.agent, id = a.id, error = ?e,
|
||||||
|
"config-pr poll: failed to cancel stale MergeConfigPr approval"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue