fix(runtime): funnel every reap path through purge_agent; sweep node-placed orphans

Agent containers leaked two independent ways.

1. The four-step teardown (deprovision ZeroClaw -> reap_sandbox -> unlink
   .brain/.onion -> hard_purge) was inlined at three call sites and two had
   drifted. missions.rs::reap_mission_resources skipped reap_sandbox;
   topology_worker::maybe_teardown_ephemeral_team skipped it and the brain
   unlink; DELETE /api/claws/{id} (soft delete) released nothing at all, so an
   offline claw that can never run again kept its container and bind mount
   forever. All four now funnel through claws::purge_agent, with
   release_claw_resources for the soft-delete case (containers gone, rows kept).

2. Both orphan reapers listed only the local driver, so a container placed on a
   fleet node was invisible to the only backstop that could find it -- this is
   what accumulated 144 tc-agent-* orphans on one node. NodeDriverProvider gains
   node_ids() (backed by NodeHub::online_ids) and both reapers now sweep every
   connected node. The remote sweep is TTL-only on purpose: the boot pass runs
   with Duration::ZERO and would otherwise kill a container another instance is
   mid-provision on.

Why it was invisible: agent_containers.agent_id is ON DELETE CASCADE, so
hard_purge took the registry row with the agent and left the container
permanently unreferenceable.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
Omar Sobh
2026-07-30 10:41:22 -07:00
co-authored by Claude Opus 5
parent a78f308eea
commit c573480955
6 changed files with 213 additions and 37 deletions
+17
View File
@@ -120,6 +120,15 @@ impl NodeHub {
self.online.lock().map(|s| s.contains(&id)).unwrap_or(false) self.online.lock().map(|s| s.contains(&id)).unwrap_or(false)
} }
/// Every currently-connected node id. Sync (no await), like `is_connected`,
/// so the container reapers can enumerate nodes to sweep.
pub fn online_ids(&self) -> Vec<NodeId> {
self.online
.lock()
.map(|s| s.iter().copied().collect())
.unwrap_or_default()
}
/// Send a typed op with JSON args and await its result (20s default). /// Send a typed op with JSON args and await its result (20s default).
pub async fn call(&self, id: NodeId, op: &str, args: Value) -> Result<ExecOutput, String> { pub async fn call(&self, id: NodeId, op: &str, args: Value) -> Result<ExecOutput, String> {
self.call_timeout(id, op, args, 20).await self.call_timeout(id, op, args, 20).await
@@ -686,4 +695,12 @@ impl cm_runtime::NodeDriverProvider for HubDriverProvider {
None None
} }
} }
fn node_ids(&self) -> Vec<String> {
self.hub
.online_ids()
.into_iter()
.map(|id| id.to_string())
.collect()
}
} }
+67 -15
View File
@@ -170,6 +170,59 @@ pub(crate) fn brain_dir() -> std::path::PathBuf {
.unwrap_or_else(|_| std::env::temp_dir().join("clawmates-brains")) .unwrap_or_else(|_| std::env::temp_dir().join("clawmates-brains"))
} }
/// What [`purge_agent`] actually managed to tear down, so callers can report
/// per-stage progress without each re-implementing the sequence.
pub(crate) struct AgentPurgeReport {
pub had_container: bool,
pub brain_gone: bool,
pub counts: Result<cm_db::repo::agents::PurgeCounts, cm_db::DbError>,
}
/// Release the host-side resources a claw holds without touching its rows:
/// deprovision the ZeroClaw runtime agent, then reap its sandbox / browser /
/// terminal containers (which also clears the `agent_containers` rows).
///
/// Split out from [`purge_agent`] because the soft-delete path wants the
/// containers gone but the data kept. Best-effort; returns whether a container
/// was actually attached.
pub(crate) async fn release_claw_resources(
runtime: &cm_runtime::Runtime,
provisioner: Option<&crate::runtime_provision::RuntimeProvisioner>,
id: AgentId,
) -> bool {
if let Some(p) = provisioner {
let _ = p.deprovision_claw(id.as_uuid()).await;
}
runtime.reap_sandbox(id).await
}
/// The full per-claw teardown, in FK-safe order: deprovision the ZeroClaw
/// runtime agent → reap the sandbox/browser/terminal containers → unlink the
/// `.brain`/`.onion` files → transactionally purge every DB row.
///
/// Every reap path funnels through here. Three call sites used to inline their
/// own variant of this sequence and two of them had silently drifted — skipping
/// `reap_sandbox`, so deleting a mission or tearing down an ephemeral team left
/// live `tc-agent-*` containers and orphan `agent_containers` rows behind.
/// Steps 1–3 are best-effort; only the DB purge can fail the call.
pub(crate) async fn purge_agent(
pool: &sqlx::PgPool,
runtime: &cm_runtime::Runtime,
provisioner: Option<&crate::runtime_provision::RuntimeProvisioner>,
id: AgentId,
) -> AgentPurgeReport {
let had_container = release_claw_resources(runtime, provisioner, id).await;
let brain = brain_dir();
let brain_gone = std::fs::remove_file(brain.join(format!("claw_{id}.h5"))).is_ok();
let _ = std::fs::remove_file(brain.join(format!("claw_{id}.h5.onion")));
let counts = cm_db::repo::agents::hard_purge(pool, id).await;
AgentPurgeReport {
had_container,
brain_gone,
counts,
}
}
/// Open (or first-create) the claw's brain and read it into a response. Seeds /// Open (or first-create) the claw's brain and read it into a response. Seeds
/// the definition from Postgres on a fresh brain — mirrors the runtime's /// the definition from Postgres on a fresh brain — mirrors the runtime's
/// first-touch seeding so the cards always have real data. Pure/sync. /// first-touch seeding so the cards always have real data. Pure/sync.
@@ -1013,7 +1066,10 @@ pub async fn set_model(
} }
/// DELETE /api/claws/{id} — destructive (§7.7): workspace owners or the /// DELETE /api/claws/{id} — destructive (§7.7): workspace owners or the
/// claw's manager only. Soft delete keeps rows for audit. /// claw's manager only. Soft delete keeps rows for audit, but the claw's
/// host-side resources are released: a soft-deleted claw is `offline` and can
/// never run again, so leaving its container alive just burns the node's
/// memory and holds a workspace bind mount open indefinitely.
pub async fn delete( pub async fn delete(
State(state): State<AppState>, State(state): State<AppState>,
Authed(user): Authed, Authed(user): Authed,
@@ -1023,6 +1079,8 @@ pub async fn delete(
if !user.role.is_owner() && agent.managed_by != user.user_id { if !user.role.is_owner() && agent.managed_by != user.user_id {
return Err(ApiError::Forbidden); return Err(ApiError::Forbidden);
} }
let provisioner = crate::runtime_provision::RuntimeProvisioner::from_env();
let had_container = release_claw_resources(&state.runtime, provisioner.as_ref(), id).await;
cm_db::repo::agents::soft_delete(&state.pool, id).await?; cm_db::repo::agents::soft_delete(&state.pool, id).await?;
cm_db::repo::audit::append( cm_db::repo::audit::append(
&state.pool, &state.pool,
@@ -1031,7 +1089,7 @@ pub async fn delete(
"agent.deleted", "agent.deleted",
"agent", "agent",
&id.to_string(), &id.to_string(),
json!({"name": agent.name}), json!({"name": agent.name, "container_reaped": had_container}),
) )
.await?; .await?;
Ok(StatusCode::NO_CONTENT) Ok(StatusCode::NO_CONTENT)
@@ -1116,21 +1174,15 @@ pub async fn batch_delete(
let name = agent.name.clone(); let name = agent.name.clone();
yield sse(json!({"stage":"start","pct":base,"label":format!("Removing {name}…")})); yield sse(json!({"stage":"start","pct":base,"label":format!("Removing {name}…")}));
// 1. Deprovision the ZeroClaw runtime agent (best-effort). // Runtime → container → brain → DB, via the shared reaper. The
// whole sequence is sub-second, so the stage events are emitted
// from the report rather than interleaved.
yield sse(json!({"stage":"deprovision","pct":base,"label":format!("{name}: deprovisioning runtime…")})); yield sse(json!({"stage":"deprovision","pct":base,"label":format!("{name}: deprovisioning runtime…")}));
if let Some(p) = &provisioner { let report = purge_agent(&state.pool, &state.runtime, provisioner.as_ref(), id).await;
let _ = p.deprovision_claw(id.as_uuid()).await; yield sse(json!({"stage":"container","pct":base,"label":format!("{name}: {}", if report.had_container { "reaped sandbox container" } else { "no container attached" })}));
} yield sse(json!({"stage":"brain","pct":base,"label":format!("{name}: {}", if report.brain_gone { "deleted .brain file" } else { "no .brain file" })}));
// 2. Reap the sandbox/browser container if one is attached.
let had_container = state.runtime.reap_sandbox(id).await;
yield sse(json!({"stage":"container","pct":base,"label":format!("{name}: {}", if had_container { "reaped sandbox container" } else { "no container attached" })}));
// 3. Unlink the brain files.
let brain_gone = std::fs::remove_file(brain_dir().join(format!("claw_{id}.h5"))).is_ok();
let _ = std::fs::remove_file(brain_dir().join(format!("claw_{id}.h5.onion")));
yield sse(json!({"stage":"brain","pct":base,"label":format!("{name}: {}", if brain_gone { "deleted .brain file" } else { "no .brain file" })}));
// 4. Transactionally purge all DB rows + the agent itself.
yield sse(json!({"stage":"purge","pct":base,"label":format!("{name}: purging data…")})); yield sse(json!({"stage":"purge","pct":base,"label":format!("{name}: purging data…")}));
match cm_db::repo::agents::hard_purge(&state.pool, id).await { match report.counts {
Ok(c) => { Ok(c) => {
let _ = cm_db::repo::audit::append( let _ = cm_db::repo::audit::append(
&state.pool, user.workspace_id, Actor::User(user.user_id), &state.pool, user.workspace_id, Actor::User(user.user_id),
+11 -10
View File
@@ -437,18 +437,19 @@ async fn reap_mission_resources(state: &AppState, mission_id: Uuid) {
.unwrap_or_default() .unwrap_or_default()
}; };
// 2. Reap each claw: ZeroClaw config → .brain files → all DB rows. // 2. Reap each claw: ZeroClaw config → sandbox container → .brain files →
// all DB rows. Shared with the batch-delete reaper so this path cannot
// drift back into skipping the container teardown.
let provisioner = crate::runtime_provision::RuntimeProvisioner::from_env(); let provisioner = crate::runtime_provision::RuntimeProvisioner::from_env();
for cid in &claw_ids { for cid in &claw_ids {
if let Some(p) = &provisioner { let report = crate::routes::claws::purge_agent(
let _ = p.deprovision_claw(*cid).await; &state.pool,
} &state.runtime,
let brain = crate::routes::claws::brain_dir(); provisioner.as_ref(),
let _ = std::fs::remove_file(brain.join(format!("claw_{cid}.h5"))); cm_domain::AgentId::from(*cid),
let _ = std::fs::remove_file(brain.join(format!("claw_{cid}.h5.onion"))); )
if let Err(e) = .await;
cm_db::repo::agents::hard_purge(&state.pool, cm_domain::AgentId::from(*cid)).await if let Err(e) = report.counts {
{
eprintln!("missions::delete: hard_purge claw {cid} failed (continuing): {e}"); eprintln!("missions::delete: hard_purge claw {cid} failed (continuing): {e}");
} }
} }
+16 -12
View File
@@ -129,7 +129,7 @@ async fn run_job(
let _ = cm_db::repo::topology_runs::fail(pool, id, &e).await; let _ = cm_db::repo::topology_runs::fail(pool, id, &e).await;
} }
} }
maybe_teardown_ephemeral_team(pool, id).await; maybe_teardown_ephemeral_team(pool, runtime, id).await;
return; return;
} }
@@ -219,14 +219,14 @@ async fn run_job(
} }
} }
} }
maybe_teardown_ephemeral_team(pool, id).await; maybe_teardown_ephemeral_team(pool, runtime, id).await;
} }
/// Post-terminal hook: if this run's team is `ephemeral` and no siblings are /// Post-terminal hook: if this run's team is `ephemeral` and no siblings are
/// still in flight, deprovision every bound claw on the ZeroClaw daemon, /// still in flight, deprovision every bound claw on the ZeroClaw daemon,
/// delete the claw rows, and delete the team row. Best-effort — a failure to /// delete the claw rows, and delete the team row. Best-effort — a failure to
/// tear down leaves the team intact and logs; a future sweep can retry. /// tear down leaves the team intact and logs; a future sweep can retry.
async fn maybe_teardown_ephemeral_team(pool: &PgPool, id: Uuid) { async fn maybe_teardown_ephemeral_team(pool: &PgPool, runtime: &cm_runtime::Runtime, id: Uuid) {
let teardown = match cm_db::repo::topology_runs::check_ephemeral_teardown(pool, id).await { let teardown = match cm_db::repo::topology_runs::check_ephemeral_teardown(pool, id).await {
Ok(Some(t)) => t, Ok(Some(t)) => t,
Ok(None) => return, Ok(None) => return,
@@ -239,16 +239,20 @@ async fn maybe_teardown_ephemeral_team(pool: &PgPool, id: Uuid) {
// side fails we still delete our rows (the daemon can be swept for orphans // side fails we still delete our rows (the daemon can be swept for orphans
// by the fleet-reconcile timer). This is the trade cm-api owns everywhere: // by the fleet-reconcile timer). This is the trade cm-api owns everywhere:
// Postgres is authoritative, the daemon config is a cache. // Postgres is authoritative, the daemon config is a cache.
if let Some(prov) = crate::runtime_provision::RuntimeProvisioner::from_env() { //
for cid in &teardown.claw_ids { // Goes through the shared reaper so an ephemeral team's claws also get
if let Err(e) = prov.deprovision_claw(*cid).await { // their sandbox containers and `.brain` files removed — this path used to
eprintln!("topology_worker: deprovision_claw({cid}) failed: {e}"); // do the daemon + DB halves only, leaking a container per ephemeral run.
} let provisioner = crate::runtime_provision::RuntimeProvisioner::from_env();
}
}
for cid in &teardown.claw_ids { for cid in &teardown.claw_ids {
if let Err(e) = cm_db::repo::agents::hard_purge(pool, cm_domain::AgentId::from(*cid)).await let report = crate::routes::claws::purge_agent(
{ pool,
runtime,
provisioner.as_ref(),
cm_domain::AgentId::from(*cid),
)
.await;
if let Err(e) = report.counts {
eprintln!("topology_worker: agents::hard_purge({cid}) failed: {e}"); eprintln!("topology_worker: agents::hard_purge({cid}) failed: {e}");
} }
} }
+57
View File
@@ -25,6 +25,15 @@ impl std::fmt::Debug for SandboxManager {
/// is not connected (the manager falls back to local). /// is not connected (the manager falls back to local).
pub trait NodeDriverProvider: Send + Sync { pub trait NodeDriverProvider: Send + Sync {
fn driver(&self, node_id: &str) -> Option<Arc<dyn SandboxDriver>>; fn driver(&self, node_id: &str) -> Option<Arc<dyn SandboxDriver>>;
/// Every currently-connected node id, so the orphan reapers can sweep
/// node-placed containers too. Without this the reapers only ever list the
/// LOCAL engine, and a container placed on a fleet node whose registry row
/// is gone (`agent_containers` FK-cascades away with its agent) becomes
/// unreachable forever — the leak that accumulated 144 orphans on one node.
fn node_ids(&self) -> Vec<String> {
Vec::new()
}
} }
pub struct SandboxManager { pub struct SandboxManager {
@@ -344,6 +353,54 @@ impl SandboxManager {
Err(e) => eprintln!("sandbox reaper: failed to remove {}: {e}", m.id), Err(e) => eprintln!("sandbox reaper: failed to remove {}: {e}", m.id),
} }
} }
// Then every connected fleet node. Node-placed sandboxes were invisible
// to this sweep before, so they leaked one container per reap that
// skipped `release_agent`.
//
// Deliberately TTL-only: the boot sweep passes ZERO, which would remove
// EVERY untracked container of our kind on a shared node — including one
// another server instance is mid-provision on. The periodic reaper
// (5 min / 10 min TTL) collects them safely instead.
if min_age > 0 {
for node_id in self
.node_provider
.as_ref()
.map(|p| p.node_ids())
.unwrap_or_default()
{
if node_id == self.node_id {
continue;
}
let Some(driver) = self.node_provider.as_ref().and_then(|p| p.driver(&node_id))
else {
continue;
};
let remote = match driver.list_managed(kind).await {
Ok(m) => m,
Err(e) => {
eprintln!("sandbox reaper: list({kind}) on node {node_id} failed: {e}");
continue;
}
};
for m in remote {
if live.contains(&m.id) || now - m.created_unix < min_age {
continue;
}
let handle = SandboxHandle {
id: m.id.clone(),
name: m.id.clone(),
};
match driver.destroy(&handle).await {
Ok(()) => reaped += 1,
Err(e) => eprintln!(
"sandbox reaper: failed to remove {} on node {node_id}: {e}",
m.id
),
}
}
}
}
reaped reaped
} }
+45
View File
@@ -404,6 +404,51 @@ impl TerminalManager {
Err(e) => eprintln!("terminal reaper: failed to remove {}: {e}", m.id), Err(e) => eprintln!("terminal reaper: failed to remove {}: {e}", m.id),
} }
} }
// An agent placed on a fleet node runs its terminal there too, so sweep
// each connected node as well — otherwise a node-placed terminal whose
// registry row is gone can never be found again. TTL-only for the same
// reason as the sandbox reaper: the boot pass uses ZERO and must not
// touch containers on a shared node.
if min > 0 {
for node_id in self
.node_provider
.as_ref()
.map(|p| p.node_ids())
.unwrap_or_default()
{
if node_id == self.node_id {
continue;
}
let Some(driver) = self.node_provider.as_ref().and_then(|p| p.driver(&node_id))
else {
continue;
};
let remote = match driver.list_managed(SandboxKind::Terminal.label()).await {
Ok(m) => m,
Err(e) => {
eprintln!("terminal reaper: list on node {node_id} failed: {e}");
continue;
}
};
for m in remote {
if tracked.contains(&m.id) || now_unix - m.created_unix < min {
continue;
}
let handle = SandboxHandle {
id: m.id.clone(),
name: m.id.clone(),
};
match driver.destroy(&handle).await {
Ok(()) => reaped += 1,
Err(e) => eprintln!(
"terminal reaper: failed to remove {} on node {node_id}: {e}",
m.id
),
}
}
}
}
reaped reaped
} }