topology worker: fail runs when team container spawn fails, don't silently degrade
Coding loops with team_id set MUST run inside their team container so
agents have /workspace/repo bind-mounted. When try_team_gateway_url
silently returned None on error (mkdir perm denied, spawn_team failed,
etc.), the executor fell through to the shared runtime — which has no
repo mount — and the coding agents narrated for 6 minutes without
touching a file. Runs completed "green" with zero commits, hiding real
infra breakage.
Change try_team_gateway_url from Option<String> to Result<Option<String>, String>:
- Ok(Some) — team spawned/reattached; drive it.
- Ok(None) — no team binding on this run; existing per-topic/per-loop
fallback resolvers still apply.
- Err — team was expected but spawn failed; caller fails the run
with a descriptive error instead of silently degrading.
Each failure path in try_team_gateway_url now formats a specific
diagnostic string (docker connect, spawn_team, missing repo_workspace_path,
etc.) that surfaces into topology_runs.error.
The MCP-bearer mint failure is deliberately kept as a warning + best-
effort: some tools will 401 but the container still boots. Only failures
that would prevent code from being touched escalate to Err.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
5bbab870fc
commit
2a99bba6c1
@@ -185,7 +185,27 @@ async fn run_job(
|
||||
// Falls through to the legacy per-topic / per-loop URL resolvers
|
||||
// when there's no team binding — safe backward-compat for every
|
||||
// existing loop with team_id = NULL.
|
||||
let per_topic_url = try_team_gateway_url(pool, id, WorkspaceId::from(job.workspace_id)).await;
|
||||
// Team path is authoritative when the run's loop has team_id set:
|
||||
// if we can't spawn the team container, FAIL the run instead of
|
||||
// silently degrading to the shared runtime. The shared runtime
|
||||
// doesn't bind /workspace/repo, so coding agents would spend their
|
||||
// turns narrating without touching files — a much worse failure
|
||||
// mode than a red run with a clear error.
|
||||
let per_topic_url = match try_team_gateway_url(pool, id, WorkspaceId::from(job.workspace_id))
|
||||
.await
|
||||
{
|
||||
Ok(url) => url,
|
||||
Err(e) => {
|
||||
eprintln!("topology_worker: team gateway resolution failed for run {id}: {e}");
|
||||
let _ = cm_db::repo::topology_runs::fail(
|
||||
pool,
|
||||
id,
|
||||
&format!("team container unavailable — {e}"),
|
||||
)
|
||||
.await;
|
||||
return;
|
||||
}
|
||||
};
|
||||
let per_topic_url = if per_topic_url.is_some() {
|
||||
per_topic_url
|
||||
} else {
|
||||
@@ -624,42 +644,77 @@ async fn drive<E: TurnExecutor>(
|
||||
/// stderr and downgrades to `None` — a broken team resolution must
|
||||
/// never brick a run that could otherwise complete on the shared
|
||||
/// research container.
|
||||
/// Resolve the team-scoped ZeroClaw gateway URL for a run.
|
||||
///
|
||||
/// Returns:
|
||||
/// - `Ok(Some(url))` — this run's loop has a `team_id` and the team
|
||||
/// container is spawned (or reattached) and ready to drive.
|
||||
/// - `Ok(None)` — the run has no team binding at all; the caller should
|
||||
/// fall through to the legacy per-topic / per-loop / shared-runtime
|
||||
/// resolvers.
|
||||
/// - `Err(msg)` — the run's loop DOES have a `team_id` but the team
|
||||
/// container couldn't be spawned. The caller MUST fail the run;
|
||||
/// silently degrading to the shared runtime hides real infra breakage
|
||||
/// and leaves the agents narrating instead of touching the repo.
|
||||
async fn try_team_gateway_url(
|
||||
pool: &PgPool,
|
||||
run_id: Uuid,
|
||||
workspace_id: WorkspaceId,
|
||||
) -> Option<String> {
|
||||
let loop_id = cm_db::repo::topology_runs::loop_id_for_run(pool, run_id)
|
||||
) -> Result<Option<String>, String> {
|
||||
let Some(loop_id) = cm_db::repo::topology_runs::loop_id_for_run(pool, run_id)
|
||||
.await
|
||||
.ok()
|
||||
.flatten()?;
|
||||
let team_id = cm_db::repo::teams::team_for_loop(pool, loop_id)
|
||||
.flatten()
|
||||
else {
|
||||
return Ok(None);
|
||||
};
|
||||
let Some(team_id) = cm_db::repo::teams::team_for_loop(pool, loop_id)
|
||||
.await
|
||||
.ok()
|
||||
.flatten()?;
|
||||
.flatten()
|
||||
else {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
// Reattach fast path — team already has a persisted URL.
|
||||
if let Ok(Some((_container, Some(url)))) =
|
||||
cm_db::repo::teams::team_container_coords(pool, team_id, workspace_id).await
|
||||
{
|
||||
return Some(url);
|
||||
return Ok(Some(url));
|
||||
}
|
||||
|
||||
// Cold path — need to spawn. Repo path comes from the paired
|
||||
// research topic (loops.source_research_topic_id + research_topics.
|
||||
// repo_workspace_path). Without a repo we can't spawn a coding
|
||||
// team container (nothing meaningful to bind at /workspace/repo).
|
||||
let (source_topic_id, _consumed, _idx) =
|
||||
cm_db::repo::loops::source_research_context(pool, loop_id)
|
||||
.await
|
||||
.ok()
|
||||
.flatten()?;
|
||||
let source_topic_id = match cm_db::repo::loops::source_research_context(pool, loop_id).await {
|
||||
Ok(Some((tid, _consumed, _idx))) => tid,
|
||||
Ok(None) => {
|
||||
return Err(format!(
|
||||
"team {team_id} has no paired source_research_topic_id; \
|
||||
coding loops need a research topic to bind /workspace/repo"
|
||||
));
|
||||
}
|
||||
Err(e) => return Err(format!("source_research_context({loop_id}): {e}")),
|
||||
};
|
||||
let source_topic =
|
||||
cm_db::repo::research_topics::get(pool, source_topic_id, workspace_id.as_uuid())
|
||||
match cm_db::repo::research_topics::get(pool, source_topic_id, workspace_id.as_uuid())
|
||||
.await
|
||||
.ok()
|
||||
.flatten()?;
|
||||
let repo_path = source_topic.repo_workspace_path.clone()?;
|
||||
{
|
||||
Ok(Some(t)) => t,
|
||||
Ok(None) => {
|
||||
return Err(format!(
|
||||
"source research topic {source_topic_id} not found in workspace"
|
||||
));
|
||||
}
|
||||
Err(e) => return Err(format!("research_topics::get({source_topic_id}): {e}")),
|
||||
};
|
||||
let Some(repo_path) = source_topic.repo_workspace_path.clone() else {
|
||||
return Err(format!(
|
||||
"source research topic {source_topic_id} has no repo_workspace_path; \
|
||||
team can't bind /workspace/repo"
|
||||
));
|
||||
};
|
||||
|
||||
// Team's risk_profile → stamped into every [agents.*] binding on
|
||||
// the freshly-written config.toml.
|
||||
@@ -670,17 +725,15 @@ async fn try_team_gateway_url(
|
||||
.and_then(|c| c.risk_profile);
|
||||
|
||||
let docker = crate::research_container::connect()
|
||||
.map_err(|e| {
|
||||
eprintln!("try_team_gateway_url: docker connect failed for team {team_id}: {e}");
|
||||
e
|
||||
})
|
||||
.ok()?;
|
||||
.map_err(|e| format!("docker connect failed for team {team_id}: {e}"))?;
|
||||
let state_root = crate::research_container::team_state_root(team_id);
|
||||
|
||||
// Mint a workspace-owner service session so the team runtime's
|
||||
// clawmates_door MCP calls pass cm-auth (the static bearer baked
|
||||
// into the template config isn't a valid auth_sessions row and
|
||||
// gets 401'd, leaving every agent with 0 tools).
|
||||
// gets 401'd, leaving every agent with 0 tools). MCP is best-effort:
|
||||
// if the mint fails we still spawn the container with the stale
|
||||
// bearer — some tools will 401 but the run isn't wholly broken.
|
||||
let mcp_bearer = crate::runtime_provision::mint_workspace_service_token(pool, workspace_id)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
@@ -698,11 +751,7 @@ async fn try_team_gateway_url(
|
||||
mcp_bearer.as_deref(),
|
||||
)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
eprintln!("try_team_gateway_url: spawn_team failed for team {team_id}: {e}");
|
||||
e
|
||||
})
|
||||
.ok()?;
|
||||
.map_err(|e| format!("spawn_team({team_id}): {e}"))?;
|
||||
|
||||
// Persist coords so future iterations skip the spawn dance.
|
||||
if let Err(e) = cm_db::repo::teams::set_team_container_coords(
|
||||
@@ -717,5 +766,5 @@ async fn try_team_gateway_url(
|
||||
eprintln!("try_team_gateway_url: persist coords failed for team {team_id}: {e}");
|
||||
}
|
||||
|
||||
Some(spawned.gateway_url)
|
||||
Ok(Some(spawned.gateway_url))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user