diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 13745d90..1e4ba983 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -118,6 +118,7 @@ dependencies = [ "devkit-core", "devkit-harness", "libc", + "rusqlite", "serde", "serde_json", "tokio", diff --git a/rust/crates/devkit-core/src/discover.rs b/rust/crates/devkit-core/src/discover.rs index 5cdb502c..f61dcf03 100644 --- a/rust/crates/devkit-core/src/discover.rs +++ b/rust/crates/devkit-core/src/discover.rs @@ -292,6 +292,16 @@ fn local_utc_offset_secs(utc_secs: i64) -> i64 { } } +/// Process liveness: exists or permission-denied (EPERM = exists, not ours). +pub fn pid_alive(pid: i64) -> bool { + unsafe { + if libc::kill(pid as libc::pid_t, 0) == 0 { + return true; + } + } + std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) +} + #[cfg(test)] mod tests { use super::*; diff --git a/rust/crates/devkit-core/src/store/registry.rs b/rust/crates/devkit-core/src/store/registry.rs index 61d4e11a..929f9c17 100644 --- a/rust/crates/devkit-core/src/store/registry.rs +++ b/rust/crates/devkit-core/src/store/registry.rs @@ -268,10 +268,21 @@ impl Store { /// start-time name check. pub fn prune_dead(&self) -> Result { let conn = self.conn.lock().unwrap(); - Ok(conn.execute( - "DELETE FROM agents WHERE name IS NOT NULL AND gone_at IS NOT NULL", - [], - )?) + let pids: Vec = conn + .prepare("SELECT pid FROM agents WHERE name IS NOT NULL AND gone_at IS NOT NULL")? + .query_map([], |r| r.get(0))? + .filter_map(|r| r.ok()) + .collect(); + // gone_at is sweep bookkeeping — prune only tombstones whose pid is + // actually dead. A tombstoned-but-alive row keeps its registration; + // deleting it would free a name that is still in use. + let mut n = 0; + for pid in pids { + if !crate::discover::pid_alive(pid) { + n += conn.execute("DELETE FROM agents WHERE pid=?1", params![pid])?; + } + } + Ok(n) } /// Registry overlays for live rows: pid -> (name, pinned). Tombstones @@ -384,8 +395,10 @@ mod tests { #[test] fn tombstone_frees_name_via_prune_and_reuse() { let (s, _p) = tmp_store(); - s.apply_agent_snapshot(&[proc_started(10, 1000)]).unwrap(); - s.register_start(10, "my-agent", "codex", "/tmp", "tmux", &json!({})) + // i32::MAX is above every pid_max — guaranteed dead (pid 10 is a + // live kernel thread on Linux CI, which kept the tombstone alive). + s.apply_agent_snapshot(&[proc_started(i32::MAX as i64, 1000)]).unwrap(); + s.register_start(i32::MAX as i64, "my-agent", "codex", "/tmp", "tmux", &json!({})) .unwrap(); s.apply_agent_snapshot(&[]).unwrap(); // Dead name does not block a new start once pruned. diff --git a/rust/crates/devkitd/Cargo.toml b/rust/crates/devkitd/Cargo.toml index 4cffc6ee..10229a7d 100644 --- a/rust/crates/devkitd/Cargo.toml +++ b/rust/crates/devkitd/Cargo.toml @@ -14,3 +14,6 @@ libc.workspace = true tracing.workspace = true tracing-subscriber.workspace = true serde.workspace = true + +[dev-dependencies] +rusqlite.workspace = true diff --git a/rust/crates/devkitd/src/rpc/agent.rs b/rust/crates/devkitd/src/rpc/agent.rs index 8235e31e..46c29e36 100644 --- a/rust/crates/devkitd/src/rpc/agent.rs +++ b/rust/crates/devkitd/src/rpc/agent.rs @@ -52,12 +52,5 @@ fn base36(mut n: i64) -> String { out.iter().rev().map(|c| *c as char).collect() } -/// Process liveness: exists or permission-denied (EPERM = exists, not ours). -pub(super) fn pid_alive(pid: i64) -> bool { - unsafe { - if libc::kill(pid as libc::pid_t, 0) == 0 { - return true; - } - } - std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) -} +/// Process liveness lives in devkit-core (prune_dead needs it too). +pub(super) use devkit_core::discover::pid_alive; diff --git a/rust/crates/devkitd/src/rpc/agent/start.rs b/rust/crates/devkitd/src/rpc/agent/start.rs index 587cb206..3bb5e40a 100644 --- a/rust/crates/devkitd/src/rpc/agent/start.rs +++ b/rust/crates/devkitd/src/rpc/agent/start.rs @@ -72,12 +72,22 @@ pub fn run_with( let _start_guard = daemon.start_mutex.lock().unwrap(); daemon.store.prune_dead().map_err(RpcError::internal)?; + // Re-sync the legacy registry inside the mutex — agents started via the + // fallback path (or before daemon boot) live only in agents.db until + // this delta-merge; the name check must see them. + daemon + .store + .import_legacy_agents() + .map_err(RpcError::internal)?; if let Some(existing) = daemon .store .lookup_name(&name) .map_err(RpcError::internal)? { - if pid_alive(existing.pid) && existing.gone_at.is_none() { + // Live pid = in use, whatever gone_at says: a tombstone is sweep + // bookkeeping, not proof of death (imported legacy rows are + // tombstoned-by-default while their pids run fine). + if pid_alive(existing.pid) { return Err(RpcError::new( "name_in_use", format!( @@ -289,6 +299,56 @@ mod tests { assert!(valid_agent_name(&n), "generated name must validate"); } + #[test] + fn tombstoned_but_live_pid_still_blocks_name() { + // gone_at is sweep bookkeeping — a tombstone whose pid is alive + // (imported legacy row, or a process the sweep hasn't re-seen) must + // still block the name. + let f = FixtureDir::new("tomb-live"); + let d = daemon(&f); + let self_pid = std::process::id() as i64; + d.store + .register_start(self_pid, "my-agent", "claude", "/tmp", "tmux", &json!({})) + .unwrap(); + d.store.apply_agent_snapshot(&[]).unwrap(); // tombstones the row + let reg = d.store.lookup_name("my-agent").unwrap().unwrap(); + assert!(reg.gone_at.is_some()); + let e = run_with(&d, params(Some("my-agent"), "/tmp"), &runtimes(&f)).unwrap_err(); + assert_eq!(e.code, "name_in_use"); + } + + #[test] + fn legacy_row_written_after_daemon_boot_blocks_name() { + // An agent started via the fallback path registers only in + // agents.db — the start path must re-import before the name check. + let f = FixtureDir::new("legacy-post-boot"); + let d = daemon(&f); + let self_pid = std::process::id() as i64; + let legacy = f.0.join("agents.db"); + let conn = rusqlite::Connection::open(&legacy).unwrap(); + conn.execute_batch( + "CREATE TABLE agents ( + type TEXT NOT NULL, pid INTEGER NOT NULL, name TEXT NOT NULL UNIQUE, + tmux_session TEXT NOT NULL DEFAULT '', cwd TEXT NOT NULL DEFAULT '', + started_at TEXT NOT NULL, session_id TEXT NOT NULL DEFAULT '', + session_file_path TEXT NOT NULL DEFAULT '', updated_at TEXT NOT NULL, + pinned INTEGER NOT NULL DEFAULT 0, runtime TEXT NOT NULL DEFAULT 'tmux', + runtime_ref TEXT NOT NULL DEFAULT '', + PRIMARY KEY (type, pid));", + ) + .unwrap(); + conn.execute( + "INSERT INTO agents (type,pid,name,cwd,started_at,updated_at,runtime,runtime_ref) + VALUES ('claude',?1,'claude-1','/tmp','2026-10-16T00:00:00.000Z', + '2026-10-16T00:00:00.000Z','tmux','{\"session\":\"claude-1\"}')", + rusqlite::params![self_pid], + ) + .unwrap(); + d.store.set_legacy_db(legacy).unwrap(); + let e = run_with(&d, params(Some("claude-1"), "/tmp"), &runtimes(&f)).unwrap_err(); + assert_eq!(e.code, "name_in_use"); + } + #[test] fn started_event_payload_is_serializable_result() { let f = FixtureDir::new("payload");