fix(#1940): carry pr_number on the live approval_added event

This commit is contained in:
damocles 2026-06-23 15:11:28 +02:00 committed by mara
commit 4c53898382
4 changed files with 74 additions and 31 deletions

View file

@ -389,6 +389,20 @@ pub struct ApprovalResolved<'a> {
pub description: Option<String>, pub description: Option<String>,
} }
/// Field-named payload for [`Coordinator::emit_approval_added`].
/// Mirrors the `ApprovalAdded` dashboard-event fields. `agent`
/// borrows from the caller; `approval_kind` is a compile-time
/// constant. `pr_number` is set for `merge_config_pr` only.
pub struct ApprovalAdded<'a> {
pub id: i64,
pub agent: &'a str,
pub approval_kind: &'static str,
pub sha_short: Option<String>,
pub diff: Option<String>,
pub description: Option<String>,
pub pr_number: Option<u64>,
}
/// Field-named payload for [`Coordinator::emit_question_added`]. /// Field-named payload for [`Coordinator::emit_question_added`].
/// Mirrors the `QuestionAdded` dashboard-event fields; all references /// Mirrors the `QuestionAdded` dashboard-event fields; all references
/// share the caller's lifetime. /// share the caller's lifetime.
@ -740,15 +754,16 @@ impl Coordinator {
/// Emit `ApprovalAdded` immediately after the row is inserted in /// Emit `ApprovalAdded` immediately after the row is inserted in
/// sqlite. Caller passes the diff text it already computed (or /// sqlite. Caller passes the diff text it already computed (or
/// `None` for spawn approvals which carry no diff). /// `None` for spawn approvals which carry no diff).
pub fn emit_approval_added( pub fn emit_approval_added(&self, ev: ApprovalAdded<'_>) {
&self, let ApprovalAdded {
id: i64, id,
agent: &str, agent,
approval_kind: &'static str, approval_kind,
sha_short: Option<String>, sha_short,
diff: Option<String>, diff,
description: Option<String>, description,
) { pr_number,
} = ev;
self.emit_dashboard_event(DashboardEvent::ApprovalAdded { self.emit_dashboard_event(DashboardEvent::ApprovalAdded {
seq: self.next_seq(), seq: self.next_seq(),
id, id,
@ -757,6 +772,7 @@ impl Coordinator {
sha_short, sha_short,
diff, diff,
description, description,
pr_number,
}); });
} }

View file

@ -1663,7 +1663,15 @@ async fn post_request_spawn(
// refetch. Spawn approvals carry no diff/sha. // refetch. Spawn approvals carry no diff/sha.
state state
.coord .coord
.emit_approval_added(id, &name, "spawn", None, None, None); .emit_approval_added(crate::coordinator::ApprovalAdded {
id,
agent: &name,
approval_kind: "spawn",
sha_short: None,
diff: None,
description: None,
pr_number: None,
});
(StatusCode::OK, "ok").into_response() (StatusCode::OK, "ok").into_response()
} }
Err(e) => error_response(&format!("request-spawn {name} failed: {e:#}")), Err(e) => error_response(&format!("request-spawn {name} failed: {e:#}")),

View file

@ -76,6 +76,13 @@ pub enum DashboardEvent {
sha_short: Option<String>, sha_short: Option<String>,
diff: Option<String>, diff: Option<String>,
description: Option<String>, description: Option<String>,
/// Forge PR number, for `merge_config_pr` approvals only — lets
/// the live `applyApprovalAdded` path build the "review PR on
/// forge" link without waiting for a cold `/api/state` refresh
/// (mirrors `ApprovalView::pr_number`). `None` for every other
/// kind.
#[serde(skip_serializing_if = "Option::is_none")]
pr_number: Option<u64>,
}, },
/// A pending approval transitioned to a terminal state /// A pending approval transitioned to a terminal state
/// (approved / denied / failed). Clients move the row out of the /// (approved / denied / failed). Clients move the row out of the
@ -346,6 +353,7 @@ mod tests {
sha_short: None, sha_short: None,
diff: None, diff: None,
description: None, description: None,
pr_number: None,
}, },
DashboardEvent::ApprovalResolved { DashboardEvent::ApprovalResolved {
seq: 1, seq: 1,

View file

@ -1438,14 +1438,15 @@ fn handle_request_update_meta_inputs(
} }
}; };
tracing::info!(%id, %label, "update_meta_inputs approval queued"); tracing::info!(%id, %label, "update_meta_inputs approval queued");
coord.emit_approval_added( coord.emit_approval_added(crate::coordinator::ApprovalAdded {
id, id,
requester, agent: requester,
"update_meta_inputs", approval_kind: "update_meta_inputs",
None, sha_short: None,
None, diff: None,
description.map(str::to_owned), description: description.map(str::to_owned),
); pr_number: None,
});
AgentResponse::Ok AgentResponse::Ok
} }
@ -1542,14 +1543,15 @@ fn handle_request_schedule_prompt(
interval = ?payload.interval_seconds, interval = ?payload.interval_seconds,
"schedule_prompt approval queued" "schedule_prompt approval queued"
); );
coord.emit_approval_added( coord.emit_approval_added(crate::coordinator::ApprovalAdded {
id, id,
requester, agent: requester,
"schedule_prompt", approval_kind: "schedule_prompt",
None, sha_short: None,
None, diff: None,
payload.description.clone(), description: payload.description.clone(),
); pr_number: None,
});
AgentResponse::Ok AgentResponse::Ok
} }
@ -1802,7 +1804,15 @@ pub(crate) fn submit_init_config(
) )
.map_err(|e| anyhow::anyhow!("queue approval row: {e:#}"))?; .map_err(|e| anyhow::anyhow!("queue approval row: {e:#}"))?;
tracing::info!(%id, %name, "init_config approval queued"); tracing::info!(%id, %name, "init_config approval queued");
coord.emit_approval_added(id, name, "init_config", None, None, description); coord.emit_approval_added(crate::coordinator::ApprovalAdded {
id,
agent: name,
approval_kind: "init_config",
sha_short: None,
diff: None,
description,
pr_number: None,
});
Ok(id) Ok(id)
} }
@ -1928,14 +1938,15 @@ pub(crate) async fn submit_apply_commit(
// get a fully-formed row without a snapshot refetch. `sha_short` // get a fully-formed row without a snapshot refetch. `sha_short`
// is reused from the dedup gate above. // is reused from the dedup gate above.
let diff = crate::dashboard::approval_diff(agent, id).await; let diff = crate::dashboard::approval_diff(agent, id).await;
coord.emit_approval_added( coord.emit_approval_added(crate::coordinator::ApprovalAdded {
id, id,
agent, agent,
"apply_commit", approval_kind: "apply_commit",
Some(sha_short), sha_short: Some(sha_short),
Some(diff), diff: Some(diff),
description.map(str::to_owned), description: description.map(str::to_owned),
); pr_number: None,
});
Ok((id, sha)) Ok((id, sha))
} }