fix(agents): enforcer l'invariant « 1 session vivante par agent » (singleton)
Un agent ne peut tourner que dans une seule cellule à la fois. La garde dans LaunchAgent refuse le spawn si l'agent est déjà vivant dans un autre node (AGENT_ALREADY_RUNNING) ; idempotent sur le même node ; le chemin resume (agent mort) reste inchangé. Le node_id est désormais plombé jusqu'au use case. Corrige le reset asymétrique d'une cellule au changement d'onglet : deux leaves partageant le même agent id rendaient session_for_agent/is_agent_live/stop_agent ambigus (cible arbitraire). Le churn reset/reattach déclenchait aussi les accents mélangés (FIFO intact, non touché). - snapshot agentWasRunning calculé par node (is_node_live) et non par agent - commande list_live_agents + live_agents()/node_for_agent()/is_node_live() - UI : dropdown grise les agents déjà placés ailleurs ; 2e cellule en doublon affiche « disponible » au lieu d'une relance fantôme Tests : cargo test (application + app-tauri) vert ; tsc + vitest vert. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@ -38,7 +38,7 @@ use domain::{PtySize, SessionId, SkillId, SkillRef};
|
||||
use uuid::Uuid;
|
||||
|
||||
use application::{
|
||||
CreateAgentFromScratch, CreateAgentInput, DeleteAgent, DeleteAgentInput, LaunchAgent,
|
||||
AppError, CreateAgentFromScratch, CreateAgentInput, DeleteAgent, DeleteAgentInput, LaunchAgent,
|
||||
LaunchAgentInput, ListAgents, ListAgentsInput, ReadAgentContext, ReadAgentContextInput,
|
||||
TerminalSessions, UpdateAgentContext, UpdateAgentContextInput,
|
||||
};
|
||||
@ -835,6 +835,137 @@ async fn launch_orders_prepare_then_injection_then_spawn() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Inserts a live agent session into the registry, simulating an already-running
|
||||
/// agent pinned on `node`. Returns the session id.
|
||||
fn seed_live_agent_session(
|
||||
sessions: &TerminalSessions,
|
||||
agent_id: AgentId,
|
||||
node: domain::NodeId,
|
||||
session_id: SessionId,
|
||||
) {
|
||||
let size = PtySize::new(24, 80).unwrap();
|
||||
let mut session = domain::TerminalSession::starting(
|
||||
session_id,
|
||||
node,
|
||||
ProjectPath::new("/home/me/proj/.ideai/run/x").unwrap(),
|
||||
domain::SessionKind::Agent { agent_id },
|
||||
size,
|
||||
);
|
||||
session.status = domain::SessionStatus::Running;
|
||||
sessions.insert(PtyHandle { session_id }, session);
|
||||
}
|
||||
|
||||
fn nid(n: u128) -> domain::NodeId {
|
||||
domain::NodeId::from_uuid(Uuid::from_u128(n))
|
||||
}
|
||||
|
||||
/// **Singleton invariant (T1)**: launching an agent that already owns a live
|
||||
/// session in *another* cell is refused with `AgentAlreadyRunning`, pointing at
|
||||
/// the existing host node. The registry is left untouched and the PTY is never
|
||||
/// spawned.
|
||||
#[tokio::test]
|
||||
async fn launch_refuses_agent_already_running_elsewhere() {
|
||||
let (launch, agent, _fs, pty, _bus, sessions, _tr, _session) = launch_fixture(
|
||||
ContextInjection::convention_file("CLAUDE.md").unwrap(),
|
||||
Some(ContextInjectionPlan::File {
|
||||
target: "CLAUDE.md".to_owned(),
|
||||
}),
|
||||
);
|
||||
|
||||
// The agent is already live in cell A.
|
||||
let host = nid(1);
|
||||
seed_live_agent_session(&sessions, agent.id, host, sid(42));
|
||||
let before = sessions.len();
|
||||
|
||||
// Attempt to launch the same agent into a *different* cell B.
|
||||
let mut input = launch_input(agent.id);
|
||||
input.node_id = Some(nid(2));
|
||||
let err = launch.execute(input).await.expect_err("must be refused");
|
||||
|
||||
match err {
|
||||
AppError::AgentAlreadyRunning { agent_id, node_id } => {
|
||||
assert_eq!(agent_id, agent.id);
|
||||
assert_eq!(node_id, host, "reports the existing host cell");
|
||||
}
|
||||
other => panic!("expected AgentAlreadyRunning, got {other:?}"),
|
||||
}
|
||||
assert_eq!(err.code(), "AGENT_ALREADY_RUNNING");
|
||||
// Invariant preserved: registry unchanged, no spawn happened.
|
||||
assert_eq!(sessions.len(), before, "registry must be unchanged");
|
||||
assert!(pty.spawns().is_empty(), "no PTY spawn on a refused launch");
|
||||
}
|
||||
|
||||
/// A launch with an unspecified node (`node_id: None`) for an already-live agent
|
||||
/// is also refused — it cannot be the same cell.
|
||||
#[tokio::test]
|
||||
async fn launch_refuses_already_running_agent_with_no_node() {
|
||||
let (launch, agent, _fs, pty, _bus, sessions, _tr, _session) = launch_fixture(
|
||||
ContextInjection::convention_file("CLAUDE.md").unwrap(),
|
||||
Some(ContextInjectionPlan::File {
|
||||
target: "CLAUDE.md".to_owned(),
|
||||
}),
|
||||
);
|
||||
seed_live_agent_session(&sessions, agent.id, nid(1), sid(42));
|
||||
|
||||
// node_id defaults to None in launch_input.
|
||||
let err = launch
|
||||
.execute(launch_input(agent.id))
|
||||
.await
|
||||
.expect_err("must be refused");
|
||||
assert!(matches!(err, AppError::AgentAlreadyRunning { .. }));
|
||||
assert!(pty.spawns().is_empty());
|
||||
}
|
||||
|
||||
/// Idempotence on the SAME node: a double-launch of the very same cell returns
|
||||
/// the already-registered session without respawning, and assigns no new
|
||||
/// conversation id.
|
||||
#[tokio::test]
|
||||
async fn launch_same_node_is_idempotent_no_respawn() {
|
||||
let (launch, agent, _fs, pty, _bus, sessions, _tr, _session) = launch_fixture(
|
||||
ContextInjection::convention_file("CLAUDE.md").unwrap(),
|
||||
Some(ContextInjectionPlan::File {
|
||||
target: "CLAUDE.md".to_owned(),
|
||||
}),
|
||||
);
|
||||
|
||||
let host = nid(7);
|
||||
seed_live_agent_session(&sessions, agent.id, host, sid(42));
|
||||
let before = sessions.len();
|
||||
|
||||
let mut input = launch_input(agent.id);
|
||||
input.node_id = Some(host);
|
||||
let out = launch.execute(input).await.expect("idempotent launch");
|
||||
|
||||
assert_eq!(out.session.id, sid(42), "returns the existing session");
|
||||
assert!(out.assigned_conversation_id.is_none(), "nothing new assigned");
|
||||
assert_eq!(sessions.len(), before, "no new session registered");
|
||||
assert!(pty.spawns().is_empty(), "no respawn on same-node relaunch");
|
||||
}
|
||||
|
||||
/// After the live session is removed (cell closed / agent exited), a fresh launch
|
||||
/// succeeds — the guard only blocks while the agent is actually live.
|
||||
#[tokio::test]
|
||||
async fn launch_succeeds_after_session_removed() {
|
||||
let (launch, agent, _fs, pty, _bus, sessions, _tr, _session) = launch_fixture(
|
||||
ContextInjection::convention_file("CLAUDE.md").unwrap(),
|
||||
Some(ContextInjectionPlan::File {
|
||||
target: "CLAUDE.md".to_owned(),
|
||||
}),
|
||||
);
|
||||
|
||||
// Live, then removed (close/exit).
|
||||
seed_live_agent_session(&sessions, agent.id, nid(1), sid(42));
|
||||
sessions.remove(&sid(42));
|
||||
assert!(sessions.session_for_agent(&agent.id).is_none());
|
||||
|
||||
let mut input = launch_input(agent.id);
|
||||
input.node_id = Some(nid(2));
|
||||
let out = launch.execute(input).await.expect("relaunch must succeed");
|
||||
|
||||
assert_eq!(out.session.id, sid(777), "spawned a fresh PTY session");
|
||||
assert_eq!(pty.spawns().len(), 1, "exactly one spawn after the agent died");
|
||||
}
|
||||
|
||||
/// **Anti-collision (ARCHITECTURE §14.1)**: two distinct agents of the *same*
|
||||
/// profile on the *same* project root must launch into two **distinct** cwd —
|
||||
/// each its own `.ideai/run/<agent-id>/` — and each writes its convention file
|
||||
|
||||
@ -121,23 +121,37 @@ impl ProjectStore for FakeStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// A controllable liveness registry: an agent is "live" iff it is in the set.
|
||||
/// A controllable liveness registry. Liveness is keyed on the hosting **node**
|
||||
/// (the leaf), matching the snapshot's per-cell query: a leaf is "live" iff its
|
||||
/// node id is in the set. An optional agent set backs the legacy
|
||||
/// `is_agent_live` query (unused by the snapshot now, kept for the trait).
|
||||
#[derive(Default, Clone)]
|
||||
struct FakeLive(Arc<Mutex<HashSet<AgentId>>>);
|
||||
struct FakeLive {
|
||||
nodes: Arc<Mutex<HashSet<NodeId>>>,
|
||||
agents: Arc<Mutex<HashSet<AgentId>>>,
|
||||
}
|
||||
|
||||
impl FakeLive {
|
||||
fn with(agents: &[AgentId]) -> Self {
|
||||
Self(Arc::new(Mutex::new(agents.iter().copied().collect())))
|
||||
/// Seeds the set of live **nodes** (the cells whose PTY is alive).
|
||||
fn with_nodes(nodes: &[NodeId]) -> Self {
|
||||
Self {
|
||||
nodes: Arc::new(Mutex::new(nodes.iter().copied().collect())),
|
||||
agents: Arc::new(Mutex::new(HashSet::new())),
|
||||
}
|
||||
}
|
||||
/// Simulates a PTY kill: forget every live agent.
|
||||
/// Simulates a PTY kill: forget every live node/agent.
|
||||
fn kill_all(&self) {
|
||||
self.0.lock().unwrap().clear();
|
||||
self.nodes.lock().unwrap().clear();
|
||||
self.agents.lock().unwrap().clear();
|
||||
}
|
||||
}
|
||||
|
||||
impl LiveAgentRegistry for FakeLive {
|
||||
fn is_agent_live(&self, agent_id: &AgentId) -> bool {
|
||||
self.0.lock().unwrap().contains(agent_id)
|
||||
self.agents.lock().unwrap().contains(agent_id)
|
||||
}
|
||||
fn is_node_live(&self, node_id: &NodeId) -> bool {
|
||||
self.nodes.lock().unwrap().contains(node_id)
|
||||
}
|
||||
}
|
||||
|
||||
@ -235,7 +249,7 @@ async fn live_agent_is_marked_running() {
|
||||
let agent = aid(100);
|
||||
seed_layouts(&fs, lid(1), &LayoutTree::single(agent_leaf(leaf, Some(agent))));
|
||||
|
||||
let live = FakeLive::with(&[agent]);
|
||||
let live = FakeLive::with_nodes(&[leaf]);
|
||||
let uc = make_use_case(&store, &fs, &live);
|
||||
|
||||
let out = uc
|
||||
@ -307,7 +321,7 @@ async fn mixed_tree_flags_each_agent_independently() {
|
||||
}));
|
||||
seed_layouts(&fs, lid(1), &tree);
|
||||
|
||||
let live = FakeLive::with(&[live_agent]);
|
||||
let live = FakeLive::with_nodes(&[live_leaf]);
|
||||
let uc = make_use_case(&store, &fs, &live);
|
||||
|
||||
let out = uc
|
||||
@ -335,7 +349,7 @@ async fn snapshot_reads_liveness_before_kill() {
|
||||
let agent = aid(100);
|
||||
seed_layouts(&fs, lid(1), &LayoutTree::single(agent_leaf(leaf, Some(agent))));
|
||||
|
||||
let live = FakeLive::with(&[agent]);
|
||||
let live = FakeLive::with_nodes(&[leaf]);
|
||||
let uc = make_use_case(&store, &fs, &live);
|
||||
|
||||
// Correct order (composition root contract): snapshot THEN kill.
|
||||
@ -387,3 +401,53 @@ async fn no_agent_leaf_is_noop() {
|
||||
assert_eq!(fs.writes(), writes_before);
|
||||
assert_eq!(was_running(&fs, leaf), Some(false));
|
||||
}
|
||||
|
||||
/// Duplicate leaves for the **same** agent (the singleton-invariant case): only
|
||||
/// the cell whose PTY is actually live is flagged running. The other leaf —
|
||||
/// pinning the same agent but never launched (or refused by the guard) — stays
|
||||
/// `false`. This is exactly why liveness is keyed on the node, not the agent:
|
||||
/// an agent-keyed query would wrongly mark BOTH leaves as running.
|
||||
#[tokio::test]
|
||||
async fn duplicate_leaves_same_agent_only_live_node_is_running() {
|
||||
let store = FakeStore::default();
|
||||
let fs = FakeFs::default();
|
||||
register_project(&store, pid(1)).await;
|
||||
|
||||
let leaf_a = nid(10);
|
||||
let leaf_b = nid(11);
|
||||
let agent = aid(100); // SAME agent on both leaves
|
||||
|
||||
let tree = LayoutTree::new(LayoutNode::Split(SplitContainer {
|
||||
id: nid(1),
|
||||
direction: Direction::Row,
|
||||
children: vec![
|
||||
WeightedChild {
|
||||
node: LayoutNode::Leaf(agent_leaf(leaf_a, Some(agent))),
|
||||
weight: 1.0,
|
||||
},
|
||||
WeightedChild {
|
||||
node: LayoutNode::Leaf(agent_leaf(leaf_b, Some(agent))),
|
||||
weight: 1.0,
|
||||
},
|
||||
],
|
||||
}));
|
||||
seed_layouts(&fs, lid(1), &tree);
|
||||
|
||||
// Only node A hosts a live session.
|
||||
let live = FakeLive::with_nodes(&[leaf_a]);
|
||||
let uc = make_use_case(&store, &fs, &live);
|
||||
|
||||
let out = uc
|
||||
.execute(SnapshotRunningAgentsInput { project_id: pid(1) })
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(out.running, 1, "only the live cell counts as running");
|
||||
assert_eq!(out.stopped, 1, "the duplicate (dead) cell counts as stopped");
|
||||
assert_eq!(was_running(&fs, leaf_a), Some(true));
|
||||
assert_eq!(
|
||||
was_running(&fs, leaf_b),
|
||||
Some(false),
|
||||
"the duplicate leaf for the same agent must NOT be marked running"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user