fix(missions): bind graph nodes to claws via attrs, not a dropped top-level key
`inject_node_agents` wrote the claw alias as a top-level `"agent"` key on
each graph node, but `cm_topology::Node` only deserializes `{id, role,
level, attrs}` — serde silently dropped it. `TurnRequest::agent` came back
`None` and every mission turn fell back to `ZEROCLAW_DEFAULT_AGENT`
(`scout`), running with scout's workspace and tools instead of the
mission's claws. The runtime trace confirms it: every turn logged
`"agent_alias":"scout"`.
That is why mission agents reported an "empty greenfield" workspace and
emitted artifacts inline instead of writing them: scout is jailed to
`/zeroclaw-data/.zeroclaw/agents/scout/workspace` and cannot see
`/mission/repo`. The per-mission provisioning and `workspace.path` pinning
shipped earlier were correct — they were just applied to agents that
nothing ever drove.
- bind into `node.attrs["agent"]` (top-level key kept for display/debug)
- extract the DB-free `apply_node_agents` and add a regression test that
round-trips through the real `TopologyGraph` deserializer, which is the
guard that was missing
- log loudly in `topology_exec::run_turn` when a node falls back to the
default agent, instead of silently swapping in a different agent
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
0a647c0bfa
commit
9bc5f6a142
@@ -381,10 +381,21 @@ async fn close_finished_missions(pool: &PgPool) -> Result<(), String> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Walk the team's graph nodes and set `node.agent = "claw_<hex>"` for
|
/// Walk the team's graph nodes and bind each to its claw as
|
||||||
/// each based on the `team_members(team_id, node_id, claw_id)` map.
|
/// `node.attrs["agent"] = "claw_<hex>"`, based on the
|
||||||
/// Nodes without a matching member row are left alone (the executor
|
/// `team_members(team_id, node_id, claw_id)` map. Nodes without a
|
||||||
/// will fall through to the env alias map / default).
|
/// matching member row are left alone (the executor will fall through to
|
||||||
|
/// the env alias map / default).
|
||||||
|
///
|
||||||
|
/// IMPORTANT — the alias MUST live under `attrs`, not at the node's top
|
||||||
|
/// level. `cm_topology::graph::Node` only deserializes `{id, role, level,
|
||||||
|
/// attrs}`, so a top-level `"agent"` key is silently dropped by serde,
|
||||||
|
/// `TurnRequest::agent` comes back `None`, and every turn falls back to
|
||||||
|
/// `ZEROCLAW_DEFAULT_AGENT` (`scout`) — which is jailed to scout's own
|
||||||
|
/// workspace and cannot see `/mission/repo`. That produced whole missions
|
||||||
|
/// of agents burning tokens while reporting an "empty greenfield"
|
||||||
|
/// workspace. The top-level key is still written for display/debug, but
|
||||||
|
/// `attrs` is what actually binds. See `topology_exec::run_turn`.
|
||||||
///
|
///
|
||||||
/// The graph shape we care about: `{ "nodes": [ { "id": "n0", ... } ] }`.
|
/// The graph shape we care about: `{ "nodes": [ { "id": "n0", ... } ] }`.
|
||||||
/// Non-object graphs (or graphs without a nodes array) are returned
|
/// Non-object graphs (or graphs without a nodes array) are returned
|
||||||
@@ -414,6 +425,16 @@ async fn inject_node_agents(
|
|||||||
if by_node.is_empty() {
|
if by_node.is_empty() {
|
||||||
return graph;
|
return graph;
|
||||||
}
|
}
|
||||||
|
apply_node_agents(graph, &by_node)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Pure core of [`inject_node_agents`] — the DB-free half, so the binding
|
||||||
|
/// contract can be regression-tested against the real `TopologyGraph`
|
||||||
|
/// deserializer.
|
||||||
|
fn apply_node_agents(
|
||||||
|
graph: serde_json::Value,
|
||||||
|
by_node: &std::collections::HashMap<String, Uuid>,
|
||||||
|
) -> serde_json::Value {
|
||||||
let mut graph = graph;
|
let mut graph = graph;
|
||||||
if let Some(nodes) = graph.get_mut("nodes").and_then(|v| v.as_array_mut()) {
|
if let Some(nodes) = graph.get_mut("nodes").and_then(|v| v.as_array_mut()) {
|
||||||
for node in nodes {
|
for node in nodes {
|
||||||
@@ -423,12 +444,84 @@ async fn inject_node_agents(
|
|||||||
let id = obj.get("id").and_then(|v| v.as_str()).map(str::to_string);
|
let id = obj.get("id").and_then(|v| v.as_str()).map(str::to_string);
|
||||||
let Some(id) = id else { continue };
|
let Some(id) = id else { continue };
|
||||||
if let Some(claw_id) = by_node.get(&id) {
|
if let Some(claw_id) = by_node.get(&id) {
|
||||||
obj.insert(
|
let alias = crate::runtime_provision::claw_alias(*claw_id);
|
||||||
"agent".to_string(),
|
// The binding that actually takes effect (see doc comment).
|
||||||
serde_json::Value::String(crate::runtime_provision::claw_alias(*claw_id)),
|
match obj.get_mut("attrs").and_then(|v| v.as_object_mut()) {
|
||||||
);
|
Some(attrs) => {
|
||||||
|
attrs.insert("agent".to_string(), serde_json::Value::String(alias.clone()));
|
||||||
|
}
|
||||||
|
None => {
|
||||||
|
let mut attrs = serde_json::Map::new();
|
||||||
|
attrs.insert("agent".to_string(), serde_json::Value::String(alias.clone()));
|
||||||
|
obj.insert("attrs".to_string(), serde_json::Value::Object(attrs));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Kept for display/debug only — serde drops it on load.
|
||||||
|
obj.insert("agent".to_string(), serde_json::Value::String(alias));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
graph
|
graph
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
fn by_node(pairs: &[(&str, Uuid)]) -> std::collections::HashMap<String, Uuid> {
|
||||||
|
pairs.iter().map(|(k, v)| (k.to_string(), *v)).collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The regression guard: the alias must survive a round-trip through the
|
||||||
|
/// real `TopologyGraph` deserializer and land in `attrs`. A top-level
|
||||||
|
/// `"agent"` key alone is dropped by serde, which silently routed every
|
||||||
|
/// mission turn to the default `scout` agent.
|
||||||
|
#[test]
|
||||||
|
fn bound_alias_survives_topology_graph_deserialization() {
|
||||||
|
let claw = Uuid::nil();
|
||||||
|
let graph = serde_json::json!({
|
||||||
|
"kind": "pipeline",
|
||||||
|
"nodes": [{"id": "n0", "role": "coder"}],
|
||||||
|
"edges": [],
|
||||||
|
});
|
||||||
|
|
||||||
|
let out = apply_node_agents(graph, &by_node(&[("n0", claw)]));
|
||||||
|
let parsed: cm_topology::TopologyGraph =
|
||||||
|
serde_json::from_value(out).expect("graph deserializes");
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
parsed.nodes[0].attrs.get("agent").map(String::as_str),
|
||||||
|
Some(crate::runtime_provision::claw_alias(claw).as_str()),
|
||||||
|
"alias must be readable from attrs after a real deserialize"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn binding_preserves_existing_attrs() {
|
||||||
|
let claw = Uuid::nil();
|
||||||
|
let graph = serde_json::json!({
|
||||||
|
"kind": "pipeline",
|
||||||
|
"nodes": [{"id": "n0", "role": "coder", "attrs": {"budget": "5"}}],
|
||||||
|
"edges": [],
|
||||||
|
});
|
||||||
|
|
||||||
|
let out = apply_node_agents(graph, &by_node(&[("n0", claw)]));
|
||||||
|
let attrs = &out["nodes"][0]["attrs"];
|
||||||
|
assert_eq!(attrs["budget"], "5");
|
||||||
|
assert_eq!(attrs["agent"], crate::runtime_provision::claw_alias(claw));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn unmapped_nodes_are_left_unbound() {
|
||||||
|
let graph = serde_json::json!({
|
||||||
|
"kind": "pipeline",
|
||||||
|
"nodes": [{"id": "n0", "role": "coder"}, {"id": "n1", "role": "tester"}],
|
||||||
|
"edges": [],
|
||||||
|
});
|
||||||
|
|
||||||
|
let out = apply_node_agents(graph, &by_node(&[("n0", Uuid::nil())]));
|
||||||
|
assert!(out["nodes"][0]["attrs"]["agent"].is_string());
|
||||||
|
// n1 has no member row — the executor falls back to the alias map.
|
||||||
|
assert!(out["nodes"][1].get("attrs").is_none());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -354,7 +354,19 @@ impl TurnExecutor for ZeroClawDriveExecutor {
|
|||||||
.map(str::trim)
|
.map(str::trim)
|
||||||
.filter(|a| !a.is_empty())
|
.filter(|a| !a.is_empty())
|
||||||
.map(str::to_string)
|
.map(str::to_string)
|
||||||
.unwrap_or_else(|| self.alias_for(&req.role));
|
.unwrap_or_else(|| {
|
||||||
|
// Falling back here means the graph node was never bound to a
|
||||||
|
// claw, so the turn runs as the default agent with the DEFAULT
|
||||||
|
// agent's workspace and tools — not the mission's. That silently
|
||||||
|
// produced whole missions of unusable output, so say so loudly.
|
||||||
|
let fallback = self.alias_for(&req.role);
|
||||||
|
eprintln!(
|
||||||
|
"topology_exec: node={} role={} has no bound agent — falling back to `{fallback}` \
|
||||||
|
(its workspace/tools, NOT the mission's)",
|
||||||
|
req.node_id, req.role,
|
||||||
|
);
|
||||||
|
fallback
|
||||||
|
});
|
||||||
let prompt = Self::build_prompt(&req);
|
let prompt = Self::build_prompt(&req);
|
||||||
self.drive(&alias, &prompt).await
|
self.drive(&alias, &prompt).await
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user