refactor(#2756): replace the DAG terminal hook with real tail nodes
The queue carried a per-DAG `HookKind` that fired an inline side effect
from outside the graph when a container rolled up terminal. mara asked
three times why this could not be an ordinary node; the answer in the
code was a doc-comment claiming a node could not work, and it was wrong.
`DepWhen::AfterAny` already existed with two live users, and a weak edge
is satisfied by a `Cancelled` dep, so a tail node runs on success,
failure and cancel alike. What was genuinely missing was smaller than a
hook: a node had no way to learn how the work it followed ended.
So: `Claim` now carries `deps: Vec<DepOutcome>`, snapshotted at claim
time from the graph the scheduler already holds (no `hive-jobq` change).
`Claim::deps_state()` / `deps_error()` roll that up, and two new kinds
consume it — `ResolveApproval { approval_id }` and `EmitRebuilt { agent }`.
Templates append one as a group-root with `AfterAny` edges onto the DAG's
other group roots; a root's state is its subtree's roll-up, so that
covers every node without fanning out to each of them.
Deleted: `HookKind`, `DagSpec.hook`, `NodeKind::Dag.hook`, `DagMeta.hook`,
`TerminalDag`, `terminal_dag()`, `terminal_summary()`, `dag_agents()`,
`dag_rollup()`, `fire_terminal_hook()`, `run_terminal_hook()`,
`emit_rebuilt()`. `complete_node` returns `()`.
Load-bearing details:
- `JobQueue::cancel` spares tail nodes instead of cancelling the whole
subtree, and returns `bool`. Without this a cancelled approval DAG
would dangle its approval forever — the hazard `tests.rs` already
named. The spared tail's deps are `Cancelled`, which satisfies its weak
edge, so the scheduler claims it and it resolves the row as cancelled.
`hive-jobq` anticipated exactly this: `cancel_node`'s doc already says
to settle afterwards so "a weak-edge terminal node observing the
cancellation" can advance.
- The existing `complete(container)` call after cancelling is kept and is
deliberately a no-op when a tail was spared (a non-terminal child parks
the container back in `Finishing`), so power ops still settle
synchronously with no branch.
- `DeployTail` is NOT `is_tail()`: it does real compensating work, and a
cancelled DAG has nothing to compensate.
- `exec::failure_reason` falls back to `first_error(dag_id)` because a
group root that rolled up `Failed` from a child carries no error of its
own — without it every tail-reported failure would lose its reason.
- `EmitRebuilt` is per agent, so a multi-agent DAG reports each agent's
own outcome rather than painting all of them with the DAG roll-up.
- `ResolveApproval` is agentless: the approval row already names its
agent, and that is also what lets one tail close a multi-agent DAG.
Transients-derived-from-running-nodes and the frontend's node-kind
strings stay out of this change; they touch iris's slice and review
better next to their own diff.
This commit is contained in:
parent
896dfc6194
commit
e8e6998ac5
11 changed files with 521 additions and 324 deletions
|
|
@ -501,19 +501,26 @@ async fn run_approval_schedule_prompt(
|
|||
finish_approval(coord, &approval, result, None)
|
||||
}
|
||||
|
||||
/// Terminal hook for approval-carrying DAGs — the job queue's scheduler calls
|
||||
/// this exactly once when such a DAG settles terminal. Every approval-carrying
|
||||
/// template resolves here, deploys included: the deploy pipeline is ordinary
|
||||
/// queue nodes now, so the DAG's own terminal state is the authoritative
|
||||
/// outcome and there's no in-node resolution to skip around.
|
||||
/// Resolve an approval row from how its DAG's work ended — the body of the
|
||||
/// [`NodeKind::ResolveApproval`] tail node. Every approval-carrying template
|
||||
/// resolves here, deploys included: the deploy pipeline is ordinary queue nodes,
|
||||
/// so the work's terminal state is the authoritative outcome and there's no
|
||||
/// in-node resolution to skip around.
|
||||
///
|
||||
/// `state` / `error` come from the tail node's own dependency roll-up
|
||||
/// ([`Claim::deps_state`] / [`Claim::deps_error`]), so this runs on the success,
|
||||
/// failure **and cancel** paths alike.
|
||||
///
|
||||
/// [`NodeKind::ResolveApproval`]: crate::job_queue::NodeKind::ResolveApproval
|
||||
/// [`Claim::deps_state`]: crate::job_queue::Claim::deps_state
|
||||
/// [`Claim::deps_error`]: crate::job_queue::Claim::deps_error
|
||||
pub(crate) async fn resolve_approval_dag(
|
||||
coord: &Arc<Coordinator>,
|
||||
terminal: &crate::job_queue::TerminalDag,
|
||||
approval_id: i64,
|
||||
state: crate::job_queue::State,
|
||||
error: Option<&str>,
|
||||
) {
|
||||
use crate::job_queue::State;
|
||||
let Some(approval_id) = terminal.approval_id else {
|
||||
return;
|
||||
};
|
||||
let approval = match coord.approvals.get(approval_id) {
|
||||
Ok(Some(a)) => a,
|
||||
Ok(None) => {
|
||||
|
|
@ -525,16 +532,10 @@ pub(crate) async fn resolve_approval_dag(
|
|||
return;
|
||||
}
|
||||
};
|
||||
let result: Result<()> = match terminal.state {
|
||||
let result: Result<()> = match state {
|
||||
State::Done => Ok(()),
|
||||
State::Cancelled => Err(anyhow::anyhow!("cancelled before completion")),
|
||||
_ => Err(anyhow::anyhow!(
|
||||
"{}",
|
||||
terminal
|
||||
.error
|
||||
.clone()
|
||||
.unwrap_or_else(|| "job dag failed".to_owned())
|
||||
)),
|
||||
_ => Err(anyhow::anyhow!("{}", error.unwrap_or("job dag failed"))),
|
||||
};
|
||||
let mut terminal_tag = None;
|
||||
match approval.kind {
|
||||
|
|
@ -550,8 +551,7 @@ pub(crate) async fn resolve_approval_dag(
|
|||
}
|
||||
}
|
||||
ApprovalKind::MergeConfigPr => {
|
||||
terminal_tag =
|
||||
deploy_terminal_tag(approval.agent.as_str(), approval_id, terminal.state).await;
|
||||
terminal_tag = deploy_terminal_tag(approval.agent.as_str(), approval_id, state).await;
|
||||
// On a failed deploy, surface the failing build log back onto the
|
||||
// PR so the manager sees why it was rejected without leaving the
|
||||
// forge. Posted here rather than inside a node because this is the
|
||||
|
|
|
|||
Loading…
Reference in a new issue