diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_inbox/message.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_inbox/message.rs index 3f79f5270c..67bd3540c3 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_inbox/message.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_inbox/message.rs @@ -1492,9 +1492,7 @@ mod tests { #[test] fn superseded_delivery_can_follow_a_real_replacement_chain_but_not_cycle() { - use crate::definitions::orgs::{ - HierarchyMode, OrgDefinition, OrgMember, PlanApprovalPolicy, - }; + use crate::definitions::orgs::{FlatOrgMember, OrgDefinition, PlanApprovalPolicy}; let _sandbox = sandbox_with_inbox_schema(); let run_id = "run-delivery-chain"; @@ -1505,39 +1503,43 @@ mod tests { role: "Coordinator".into(), agent_id: "coordinator-agent".into(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![ - OrgMember { - id: "member-a".into(), + members: vec![ + FlatOrgMember { + member_id: "member-a".into(), name: "Member A".into(), role: "worker".into(), agent_id: "agent-a".into(), runtime_config: None, - children: Vec::new(), }, - OrgMember { - id: "member-b".into(), + FlatOrgMember { + member_id: "member-b".into(), name: "Member B".into(), role: "worker".into(), agent_id: "agent-b".into(), runtime_config: None, - children: Vec::new(), }, - OrgMember { - id: "member-c".into(), + FlatOrgMember { + member_id: "member-c".into(), name: "Member C".into(), role: "worker".into(), agent_id: "agent-c".into(), runtime_config: None, - children: Vec::new(), }, ], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), }; let conn = get_connection().expect("open sandbox database"); conn.execute( "UPDATE agent_org_runs SET org_snapshot_json=?1 WHERE id=?2", - params![serde_json::to_string(&org).unwrap(), run_id], + params![ + serde_json::to_string(&crate::definitions::orgs::AgentOrgLaunchSnapshot::from( + &org + )) + .unwrap(), + run_id + ], ) .expect("seed roster snapshot"); let now = chrono::Utc::now().to_rfc3339(); diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/tests.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/tests.rs index ea6a2282ef..b10061fe0c 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/tests.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/tests.rs @@ -12,7 +12,7 @@ use crate::coordination::agent_org_runs::{ use crate::coordination::agent_org_tasks::{ AgentOrgTaskStore, CreateTaskParams, TaskStatus, TASK_METADATA_EXECUTION_MODE, }; -use crate::definitions::orgs::{HierarchyMode, OrgDefinition, OrgMember}; +use crate::definitions::orgs::{FlatOrgMember, OrgDefinition}; fn setup(policy: PlanApprovalPolicy) -> (test_helpers::test_env::SandboxGuard, AgentOrgRunContext) { let sandbox = test_helpers::test_env::sandbox(); @@ -48,32 +48,31 @@ fn setup(policy: PlanApprovalPolicy) -> (test_helpers::test_env::SandboxGuard, A role: "lead".into(), agent_id: "coord-agent".into(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: policy, - children: vec![ - OrgMember { - id: "planner".into(), + members: vec![ + FlatOrgMember { + member_id: "planner".into(), name: "Planner".into(), role: "plan".into(), agent_id: "planner-agent".into(), runtime_config: None, - children: Vec::new(), }, - OrgMember { - id: "builder".into(), + FlatOrgMember { + member_id: "builder".into(), name: "Builder".into(), role: "build".into(), agent_id: "builder-agent".into(), runtime_config: None, - children: Vec::new(), }, ], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), }; let run = AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id: org.id.clone(), coordinator_agent_id: org.agent_id.clone(), root_session_id: Some("root-plan-approval".into()), - org_snapshot: org, + org_snapshot: (&org).into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -95,18 +94,16 @@ fn setup(policy: PlanApprovalPolicy) -> (test_helpers::test_env::SandboxGuard, A name: "Planner".into(), role: "plan".into(), agent_id: "planner-agent".into(), - parent_member_id: None, }, AgentOrgContextMember { member_id: "builder".into(), name: "Builder".into(), role: "build".into(), agent_id: "builder-agent".into(), - parent_member_id: None, }, ], - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: policy, + capability_index: Default::default(), root_session_id: Some("root-plan-approval".into()), }; (sandbox, context) diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/transitions.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/transitions.rs index aa813799a9..6e7337a93a 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/transitions.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_plan_approvals/transitions.rs @@ -12,7 +12,7 @@ use crate::coordination::agent_org_runs::COORDINATOR_MEMBER_ID; use crate::coordination::agent_org_tasks::{ AgentOrgTaskStore, TaskExecutionMode, TaskOutput, TaskStatus, TASK_METADATA_EXECUTION_MODE, }; -use crate::definitions::orgs::{OrgDefinition, OrgMember}; +use crate::definitions::orgs::AgentOrgLaunchSnapshot; use super::artifact::validate_owned_plan_path_with_connection; use super::persistence::insert_record; @@ -299,24 +299,19 @@ fn participant_agent_ids_in_tx( .map_err(|err| format!("agent_org_run_not_mutable: {run_id}: {err}"))?; let mut participants = HashMap::new(); if let Some(snapshot_json) = snapshot_json { - let snapshot: OrgDefinition = serde_json::from_str(&snapshot_json).map_err(|err| { - format!("failed to parse Agent Org launch snapshot for run {run_id}: {err}") - })?; - collect_participant_agent_ids(&snapshot.children, &mut participants); + let snapshot: AgentOrgLaunchSnapshot = + serde_json::from_str(&snapshot_json).map_err(|err| { + format!("failed to parse Agent Org launch snapshot for run {run_id}: {err}") + })?; + crate::definitions::orgs::validate_launch_snapshot(&snapshot) + .map_err(|err| format!("invalid Agent Org launch snapshot for run {run_id}: {err}"))?; + for member in snapshot.members { + participants.insert(member.member_id, member.agent_id); + } } Ok((coordinator_agent_id, participants)) } -fn collect_participant_agent_ids( - members: &[OrgMember], - participants: &mut HashMap, -) { - for member in members { - participants.insert(member.id.clone(), member.agent_id.clone()); - collect_participant_agent_ids(&member.children, participants); - } -} - pub(super) fn plan_approval_request_message(approval: &AgentOrgPlanApproval) -> AgentMessage { let plan_char_count = approval.plan_content.chars().count(); let mut inline_plan_content = diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/helpers.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/helpers.rs index a3fbd9bb73..8b28063fe4 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/helpers.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/helpers.rs @@ -1,6 +1,8 @@ use rusqlite::{params, Connection, OptionalExtension, Result as SqliteResult}; -use crate::definitions::orgs::{AgentOrgsStore, OrgDefinition, OrgMember}; +use crate::definitions::orgs::{ + validate_launch_snapshot, AgentOrgCapabilityIndex, AgentOrgLaunchSnapshot, +}; use database::db::get_connection; use super::{ @@ -146,64 +148,58 @@ pub(super) fn row_to_run(row: &rusqlite::Row<'_>) -> SqliteResult Result { - if let Some(snapshot_json) = run.org_snapshot_json.as_deref() { - let snapshot: OrgDefinition = serde_json::from_str(snapshot_json).map_err(|err| { - format!( - "failed to parse Agent Org launch snapshot for run {}: {}", - run.id, err - ) - })?; - return Ok(context_from_run_and_org(run, &snapshot)); - } - - let org = org_store.get(&run.org_id)?; - Ok(context_from_run_and_org(run, &org)) + let snapshot_json = run + .org_snapshot_json + .as_deref() + .ok_or_else(|| format!("Agent Org run {} has no immutable launch snapshot", run.id))?; + let snapshot: AgentOrgLaunchSnapshot = serde_json::from_str(snapshot_json).map_err(|err| { + format!( + "failed to parse Agent Org launch snapshot for run {}: {}", + run.id, err + ) + })?; + validate_launch_snapshot(&snapshot).map_err(|err| { + format!( + "Agent Org run {} has invalid launch snapshot: {err}", + run.id + ) + })?; + Ok(context_from_run_and_snapshot(run, &snapshot)) } -pub(super) fn context_from_run_and_org( +pub(super) fn context_from_run_and_snapshot( run: &AgentOrgRunRecord, - org: &OrgDefinition, + snapshot: &AgentOrgLaunchSnapshot, ) -> AgentOrgRunContext { AgentOrgRunContext { run_id: run.id.clone(), - org_id: org.id.clone(), - org_name: org.name.clone(), - org_role: org.role.clone(), + org_id: snapshot.org_id.clone(), + org_name: snapshot.org_name.clone(), + org_role: snapshot.coordinator_role.clone(), coordinator_agent_id: run.coordinator_agent_id.clone(), coordinator_name: DEFAULT_COORDINATOR_DISPLAY_NAME.to_string(), - coordinator_role: org.role.clone(), - members: flatten_members(&org.children, None), - hierarchy_mode: org.hierarchy_mode, - plan_approval_policy: org.plan_approval_policy, + coordinator_role: snapshot.coordinator_role.clone(), + members: flatten_members(&snapshot.members), + plan_approval_policy: snapshot.plan_approval_policy, + capability_index: AgentOrgCapabilityIndex::from_snapshot(snapshot), root_session_id: run.root_session_id.clone(), } } -/// Flatten the `OrgMember` tree into a `Vec`, -/// preserving each member's parent id (the immediate parent in -/// `OrgDefinition.children`). A `None` parent means the member is a -/// direct report of the coordinator. -/// -/// In `HierarchyMode::Flat` the parent ids are still emitted but the -/// system prompt and routing layer ignore them. +/// Project the immutable flat snapshot roster into runtime context rows. pub(super) fn flatten_members( - members: &[OrgMember], - parent_id: Option<&str>, + members: &[crate::definitions::orgs::FlatOrgMember], ) -> Vec { - let mut flattened = Vec::new(); - for member in members { - flattened.push(AgentOrgContextMember { - member_id: member.id.clone(), + members + .iter() + .map(|member| AgentOrgContextMember { + member_id: member.member_id.clone(), name: member.name.clone(), role: member.role.clone(), agent_id: member.agent_id.clone(), - parent_member_id: parent_id.map(|id| id.to_string()), - }); - flattened.extend(flatten_members(&member.children, Some(&member.id))); - } - flattened + }) + .collect() } pub(super) fn insert_run(conn: &Connection, run: &AgentOrgRunRecord) -> SqliteResult<()> { diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/mod.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/mod.rs index 3e37e7d01d..a6437d2a85 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/mod.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/mod.rs @@ -41,9 +41,31 @@ pub use worker::{WorkerSessionInfo, WorkerSessionRuntime}; use rusqlite::{Connection, Result as SqliteResult}; use serde::Serialize; -use crate::definitions::orgs::{HierarchyMode, OrgDefinition, PlanApprovalPolicy}; +use crate::definitions::orgs::{ + AgentOrgCapabilityIndex, AgentOrgLaunchSnapshot, PlanApprovalPolicy, +}; -pub const COORDINATOR_MEMBER_ID: &str = "coordinator"; +type RunSchemaColumn = (i64, String, String, i64, Option, i64); + +const LEGACY_RUN_SCHEMA: [(&str, &str, i64, Option<&str>, i64); 15] = [ + ("id", "TEXT", 0, None, 1), + ("org_id", "TEXT", 1, None, 0), + ("coordinator_agent_id", "TEXT", 1, None, 0), + ("root_session_id", "TEXT", 0, None, 0), + ("org_snapshot_json", "TEXT", 0, None, 0), + ("entry_mode", "TEXT", 1, None, 0), + ("status", "TEXT", 1, None, 0), + ("work_item_id", "TEXT", 0, None, 0), + ("project_slug", "TEXT", 0, None, 0), + ("routine_fire_id", "TEXT", 0, None, 0), + ("summary", "TEXT", 0, None, 0), + ("last_error", "TEXT", 0, None, 0), + ("created_at", "TEXT", 1, None, 0), + ("updated_at", "TEXT", 1, None, 0), + ("completed_at", "TEXT", 0, None, 0), +]; + +pub use core_types::agent_org::COORDINATOR_MEMBER_ID; pub(crate) const DEFAULT_COORDINATOR_DISPLAY_NAME: &str = "Coordinator"; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] @@ -125,12 +147,6 @@ pub struct AgentOrgContextMember { pub name: String, pub role: String, pub agent_id: String, - /// `id` of the member this one reports to in `OrgDefinition.children`. - /// `None` means the member sits directly under the coordinator. - /// Used by the LLM system prompt to render reports-to relationships - /// (in `Soft`/`Strict` modes) and by the runtime to enforce routing - /// rules when `HierarchyMode::Strict` is in effect. - pub parent_member_id: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize)] @@ -138,7 +154,6 @@ pub struct AgentOrgContextMember { pub struct AgentOrgParticipant { pub member_id: String, pub agent_id: String, - pub parent_member_id: Option, pub is_coordinator: bool, } @@ -170,12 +185,13 @@ pub struct AgentOrgRunContext { /// logic explicitly considers `{coordinator} ∪ members` as the /// eligible recipient set. pub members: Vec, - /// How the coordinator → members → reports-to relationship should be - /// surfaced in the LLM system prompt and enforced by - /// `org_send_message`. Mirror of `OrgDefinition.hierarchy_mode`. - pub hierarchy_mode: HierarchyMode, /// Plan-approval policy captured in the launch snapshot. pub plan_approval_policy: PlanApprovalPolicy, + /// Compiled capability facts for future Writer/peer activation. PR2 + /// persists and freezes these facts but does not use them to authorize + /// Task mutations or member-to-member delivery yet. + #[serde(skip)] + pub capability_index: AgentOrgCapabilityIndex, /// Session ID of the coordinator (root) session for this run. Used by /// the frontend to navigate directly to the coordinator's chat history /// when the run is paused or the coordinator is not the active session. @@ -184,12 +200,6 @@ pub struct AgentOrgRunContext { pub root_session_id: Option, } -/// Outcome of [`AgentOrgRunContext::check_routing`]. -/// -/// `Allowed` means the send is legitimate under the current -/// `HierarchyMode`. `Blocked` carries an LLM-readable hint that names -/// the legitimate routing options (immediate manager + the coordinator -/// escape hatch) so the model can self-correct without retrying blind. #[derive(Debug, Clone, PartialEq, Eq)] pub enum RoutingDecision { Allowed, @@ -201,7 +211,6 @@ impl AgentOrgRunContext { AgentOrgParticipant { member_id: COORDINATOR_MEMBER_ID.to_string(), agent_id: self.coordinator_agent_id.clone(), - parent_member_id: None, is_coordinator: true, } } @@ -212,7 +221,6 @@ impl AgentOrgRunContext { participants.extend(self.members.iter().map(|member| AgentOrgParticipant { member_id: member.member_id.clone(), agent_id: member.agent_id.clone(), - parent_member_id: member.parent_member_id.clone(), is_coordinator: false, })); participants @@ -228,7 +236,6 @@ impl AgentOrgRunContext { .map(|member| AgentOrgParticipant { member_id: member.member_id.clone(), agent_id: member.agent_id.clone(), - parent_member_id: member.parent_member_id.clone(), is_coordinator: false, }) } @@ -271,82 +278,24 @@ impl AgentOrgRunContext { return Vec::new(); } - let mut allowed = match self.hierarchy_mode { - HierarchyMode::Flat | HierarchyMode::Soft => self - .participants() - .into_iter() - .map(|participant| participant.member_id) - .filter(|member_id| member_id != sender_member_id) - .collect::>(), - HierarchyMode::Strict => { - if sender_member_id == COORDINATOR_MEMBER_ID { - self.members - .iter() - .map(|member| member.member_id.clone()) - .collect::>() - } else { - let mut ids = Vec::new(); - ids.push(COORDINATOR_MEMBER_ID.to_string()); - if let Some(sender) = self - .members - .iter() - .find(|member| member.member_id == sender_member_id) - { - if let Some(parent_member_id) = sender.parent_member_id.as_ref() { - ids.push(parent_member_id.clone()); - } - ids.extend( - self.members - .iter() - .filter(|member| { - member - .parent_member_id - .as_deref() - .is_some_and(|parent| parent == sender.member_id) - }) - .map(|member| member.member_id.clone()), - ); - } - ids.into_iter() - .filter(|member_id| member_id != sender_member_id) - .collect::>() - } - } + let mut allowed = if sender_member_id == COORDINATOR_MEMBER_ID { + self.members + .iter() + .map(|member| member.member_id.clone()) + .collect::>() + } else { + vec![COORDINATOR_MEMBER_ID.to_string()] }; allowed.sort(); allowed.dedup(); allowed } - /// Member ids that `manager_member_id` may directly supervise on the - /// shared task board. Task authority deliberately differs from message - /// routing: unrestricted peer discussion in `Soft` mode does not make - /// every peer every other peer's manager. `Flat` drops the hierarchy, so - /// only the coordinator has cross-member task authority in that mode. - pub fn direct_report_member_ids_for(&self, manager_member_id: &str) -> Vec { - if self.hierarchy_mode == HierarchyMode::Flat - || manager_member_id == COORDINATOR_MEMBER_ID - || self.participant_by_member_id(manager_member_id).is_none() - { - return Vec::new(); - } - - let mut direct_reports = self - .members - .iter() - .filter(|member| member.parent_member_id.as_deref() == Some(manager_member_id)) - .map(|member| member.member_id.clone()) - .collect::>(); - direct_reports.sort(); - direct_reports.dedup(); - direct_reports - } - /// Task assignees that `caller_member_id` is authorized to manage. /// /// - coordinator: itself plus every roster member; /// - ordinary member: itself; - /// - manager member in Soft/Strict: itself plus direct reports. + /// - ordinary member: itself only until PR7 activates configured Writers. /// /// This is the task-governance source of truth. It must not be replaced by /// `allowed_recipient_member_ids_for`: permission to talk to a peer is not @@ -362,9 +311,7 @@ impl AgentOrgRunContext { .map(|participant| participant.member_id) .collect::>() } else { - let mut member_ids = vec![caller_member_id.to_string()]; - member_ids.extend(self.direct_report_member_ids_for(caller_member_id)); - member_ids + vec![caller_member_id.to_string()] }; allowed.sort(); allowed.dedup(); @@ -387,7 +334,7 @@ impl AgentOrgRunContext { } RoutingDecision::Blocked(format!( - "recipient_member_id '{to_member_id}' is not currently routable from sender_member_id '{from_member_id}'. Allowed recipient_member_id values: {}", + "recipient_member_id '{to_member_id}' is not currently routable from sender_member_id '{from_member_id}'; member peer delivery is not enabled until the peer-send phase. Allowed recipient_member_id values: {}", self.allowed_recipient_member_ids_for(from_member_id).join(", ") )) } @@ -422,7 +369,7 @@ pub struct CreateAgentOrgRunParams { pub org_id: String, pub coordinator_agent_id: String, pub root_session_id: Option, - pub org_snapshot: OrgDefinition, + pub org_snapshot: AgentOrgLaunchSnapshot, pub entry_mode: AgentOrgRunEntryMode, pub status: AgentOrgRunStatus, pub work_item_id: Option, @@ -435,7 +382,7 @@ pub struct CreateStartingAgentOrgRunParams { pub org_id: String, pub coordinator_agent_id: String, pub root_session_id: String, - pub org_snapshot: OrgDefinition, + pub org_snapshot: AgentOrgLaunchSnapshot, pub entry_mode: AgentOrgRunEntryMode, pub work_item_id: Option, pub project_slug: Option, @@ -462,6 +409,61 @@ impl AgentOrgStartingFailure { /// Initialize runtime Agent Org tables in `sessions.db`. pub fn init_schema(conn: &Connection) -> SqliteResult<()> { + let columns = conn + .prepare( + "SELECT cid, name, type, \"notnull\", dflt_value, pk + FROM pragma_table_info('agent_org_runs') ORDER BY cid", + )? + .query_map([], |row| { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + row.get(5)?, + )) + })? + .collect::>>()?; + + let is_legacy_run_schema = columns.len() == LEGACY_RUN_SCHEMA.len() + && columns + .iter() + .zip(LEGACY_RUN_SCHEMA.iter()) + .enumerate() + .all(|(cid, (actual, expected))| { + actual.0 == cid as i64 + && actual.1 == expected.0 + && actual.2 == expected.1 + && actual.3 == expected.2 + && actual.4.as_deref() == expected.3 + && actual.5 == expected.4 + }); + + if is_legacy_run_schema { + let legacy_run_count: i64 = + conn.query_row("SELECT COUNT(*) FROM agent_org_runs", [], |row| row.get(0))?; + let tx = database::db::begin_immediate(conn)?; + tx.execute_batch( + "DROP TABLE IF EXISTS agent_org_initial_inputs; + DROP TABLE IF EXISTS agent_org_member_materializations; + DROP TABLE IF EXISTS agent_org_run_progress; + DROP TABLE agent_org_runs;", + )?; + create_canonical_schema(&tx)?; + tx.commit()?; + tracing::info!( + event = "agent_org_legacy_run_schema_reset", + legacy_run_count, + "reset legacy Agent Org runtime envelope" + ); + return Ok(()); + } + + create_canonical_schema(conn) +} + +fn create_canonical_schema(conn: &Connection) -> SqliteResult<()> { conn.execute_batch( "CREATE TABLE IF NOT EXISTS agent_org_runs ( id TEXT PRIMARY KEY, diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/store.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/store.rs index aa822106bb..54f1f52fe8 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/store.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/store.rs @@ -5,7 +5,6 @@ use rusqlite::{params, Connection, OptionalExtension}; use crate::coordination::agent_member_interventions::AgentMemberInterventionStore; use crate::coordination::agent_org_plan_approvals::AgentOrgPlanApprovalStore; use crate::coordination::agent_org_tasks::{AgentOrgTaskStore, Task, TaskStatus}; -use crate::definitions::orgs::AgentOrgsStore; use crate::session::SessionStatus; use database::db::{get_connection, with_sessions_writer}; @@ -31,6 +30,8 @@ use super::{ CreateStartingAgentOrgRunParams, COORDINATOR_MEMBER_ID, }; +use crate::definitions::orgs::serialize_launch_snapshot; + /// Stable machine prefix for permanent Session-identity failures raised by /// the materialization / finish-Starting certificate checks in this store. /// Launch recovery classifies retryable-vs-permanent on these prefixes; never @@ -130,8 +131,7 @@ impl AgentOrgRunStore { pub fn create(params: CreateAgentOrgRunParams) -> Result { let entry_mode = validate_entry_mode(params.entry_mode.as_str())?; let status = validate_status(params.status.as_str())?; - let org_snapshot_json = serde_json::to_string(¶ms.org_snapshot) - .map_err(|err| format!("failed to serialize Agent Org launch snapshot: {err}"))?; + let org_snapshot_json = serialize_launch_snapshot(¶ms.org_snapshot)?; let now = chrono::Utc::now().to_rfc3339(); let run = AgentOrgRunRecord { id: format!("agent-org-run-{}", uuid::Uuid::new_v4()), @@ -174,8 +174,7 @@ impl AgentOrgRunStore { params: CreateStartingAgentOrgRunParams, ) -> Result { let entry_mode = validate_entry_mode(params.entry_mode.as_str())?; - let org_snapshot_json = serde_json::to_string(¶ms.org_snapshot) - .map_err(|error| format!("failed to serialize Agent Org launch snapshot: {error}"))?; + let org_snapshot_json = serialize_launch_snapshot(¶ms.org_snapshot)?; let now = chrono::Utc::now().to_rfc3339(); let run = AgentOrgRunRecord { id: format!("agent-org-run-{}", uuid::Uuid::new_v4()), @@ -201,7 +200,7 @@ impl AgentOrgRunStore { let mut member_ids = HashSet::new(); let mut session_ids = HashSet::new(); - let mut expected_roster = flatten_members(¶ms.org_snapshot.children, None) + let mut expected_roster = flatten_members(¶ms.org_snapshot.members) .into_iter() .map(|member| (member.member_id, member.agent_id)) .collect::>(); @@ -967,24 +966,20 @@ impl AgentOrgRunStore { /// /// Bounded to `MAX_PARENT_WALK_DEPTH` hops so a corrupt or cyclic /// parent chain can't cause an unbounded scan during session init. - pub fn context_for_run( - run_id: &str, - org_store: &AgentOrgsStore, - ) -> Result, String> { + pub fn context_for_run(run_id: &str) -> Result, String> { let Some(run) = load_by_id(run_id).map_err(|err| err.to_string())? else { return Ok(None); }; - Ok(Some(context_for_run_record(&run, org_store)?)) + Ok(Some(context_for_run_record(&run)?)) } pub fn context_for_session_with_parent_walk( session_id: &str, - org_store: &AgentOrgsStore, ) -> Result, String> { let Some(run) = Self::run_for_session_with_parent_walk(session_id)? else { return Ok(None); }; - Ok(Some(context_for_run_record(&run, org_store)?)) + Ok(Some(context_for_run_record(&run)?)) } pub fn root_session_id_for_session_with_parent_walk( @@ -1409,12 +1404,15 @@ impl AgentOrgRunStore { let Some(snapshot_json) = snapshot_json else { return Ok(None); }; - let snapshot: crate::definitions::orgs::OrgDefinition = + let snapshot: crate::definitions::orgs::AgentOrgLaunchSnapshot = serde_json::from_str(&snapshot_json).map_err(|err| { format!("failed to parse Agent Org launch snapshot for run {org_run_id}: {err}") })?; + crate::definitions::orgs::validate_launch_snapshot(&snapshot).map_err(|err| { + format!("invalid Agent Org launch snapshot for run {org_run_id}: {err}") + })?; Ok(Some( - flatten_members(&snapshot.children, None) + flatten_members(&snapshot.members) .into_iter() .map(|member| member.member_id) .collect(), diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/tests.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/tests.rs index 8aa482630a..db0b21efb9 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/tests.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_runs/tests.rs @@ -2,10 +2,68 @@ use super::helpers::load_by_id; use super::*; use crate::core::session::persistence::{upsert_session, UnifiedSessionRecord}; use crate::core::session::SessionStatus; -use crate::definitions::orgs::{ - AgentOrgsStore, HierarchyMode, OrgDefinition, OrgMember, PlanApprovalPolicy, -}; -use rusqlite::params; +use crate::definitions::orgs::{AgentOrgsStore, FlatOrgMember, OrgDefinition, PlanApprovalPolicy}; +use rusqlite::{params, Connection}; + +const LEGACY_AGENT_ORG_RUNS_DDL: &str = "CREATE TABLE agent_org_runs ( + id TEXT PRIMARY KEY, + org_id TEXT NOT NULL, + coordinator_agent_id TEXT NOT NULL, + root_session_id TEXT, + org_snapshot_json TEXT, + entry_mode TEXT NOT NULL, + status TEXT NOT NULL, + work_item_id TEXT, + project_slug TEXT, + routine_fire_id TEXT, + summary TEXT, + last_error TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + completed_at TEXT +);"; + +fn memory_connection() -> Connection { + let conn = Connection::open_in_memory().expect("in-memory sqlite"); + conn.execute_batch("PRAGMA foreign_keys = ON;") + .expect("enable foreign keys"); + conn +} + +fn insert_legacy_run(conn: &Connection) { + conn.execute( + "INSERT INTO agent_org_runs ( + id, org_id, coordinator_agent_id, root_session_id, + org_snapshot_json, entry_mode, status, summary, last_error, + created_at, updated_at + ) VALUES ( + 'legacy-run', 'legacy-org', 'legacy-coordinator', 'legacy-root', + '{}', 'standalone_session', 'running', 'legacy summary', 'legacy error', + '2026-01-01T00:00:00Z', '2026-01-02T00:00:00Z' + )", + [], + ) + .expect("insert legacy run sentinel"); +} + +fn row_count(conn: &Connection, table: &str) -> i64 { + conn.query_row(&format!("SELECT COUNT(*) FROM {table}"), [], |row| { + row.get(0) + }) + .unwrap_or_else(|error| panic!("count {table}: {error}")) +} + +fn index_names(conn: &Connection) -> Vec { + conn.prepare( + "SELECT name FROM sqlite_master + WHERE type='index' AND name LIKE 'idx_agent_org_%' ORDER BY name", + ) + .expect("prepare index query") + .query_map([], |row| row.get(0)) + .expect("query indexes") + .collect::>>() + .expect("collect indexes") +} #[test] fn enum_values_round_trip() { @@ -81,6 +139,201 @@ fn canonical_schema_snapshot_contains_only_the_long_lived_run_states() { assert!(initial_input_ddl.contains("UNIQUE(message_id)")); } +#[test] +fn exact_legacy_run_schema_resets_only_the_agent_org_runtime_envelope() { + let conn = memory_connection(); + conn.execute_batch(LEGACY_AGENT_ORG_RUNS_DDL) + .expect("legacy run schema"); + insert_legacy_run(&conn); + materialization::init_schema(&conn).expect("materialization schema from bad binary"); + progress::init_schema(&conn).expect("legacy progress schema"); + conn.execute_batch( + "UPDATE agent_org_run_progress SET work_revision=7 WHERE org_run_id='legacy-run'; + INSERT INTO agent_org_member_materializations ( + org_run_id, member_id, agent_id, generation, session_id, + authority_class, status, created_at, updated_at + ) VALUES ( + 'legacy-run', 'legacy-member', 'legacy-agent', 1, 'legacy-member-session', + 'starting', 'succeeded', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00Z' + ); + INSERT INTO agent_org_initial_inputs ( + org_run_id, turn_intent_id, message_id, content, payload_json, + status, created_at, updated_at + ) VALUES ( + 'legacy-run', 'legacy-turn', 'legacy-message', 'legacy input', '{}', + 'dispatched', '2026-01-01T00:00:00Z', '2026-01-01T00:00:00Z' + ); + CREATE TABLE agent_sessions ( + session_id TEXT PRIMARY KEY, title TEXT, status TEXT, updated_at TEXT + ); + CREATE TABLE code_sessions ( + session_id TEXT PRIMARY KEY, cli_agent_type TEXT, status TEXT, updated_at TEXT + ); + INSERT INTO agent_sessions VALUES ( + 'rust-sentinel', 'Rust sentinel', 'idle', '2025-12-01T00:00:00Z' + ); + INSERT INTO code_sessions VALUES ( + 'cli-sentinel', 'codex', 'completed', '2025-12-02T00:00:00Z' + );", + ) + .expect("legacy runtime and ordinary session sentinels"); + + init_schema(&conn).expect("reset exact legacy schema"); + + for table in [ + "agent_org_initial_inputs", + "agent_org_member_materializations", + "agent_org_run_progress", + "agent_org_runs", + ] { + assert_eq!(row_count(&conn, table), 0, "{table} must be reset"); + } + let new_column_count: i64 = conn + .query_row( + "SELECT COUNT(*) FROM pragma_table_info('agent_org_runs') + WHERE name IN ('activation_generation', 'has_initial_work', 'failure_json', + 'last_activity_outcome', 'idled_at')", + [], + |row| row.get(0), + ) + .expect("canonical run columns"); + assert_eq!(new_column_count, 5); + let run_ddl: String = conn + .query_row( + "SELECT sql FROM sqlite_master WHERE type='table' AND name='agent_org_runs'", + [], + |row| row.get(0), + ) + .expect("canonical run DDL"); + assert!(run_ddl.contains("'starting', 'running', 'paused', 'idle', 'failed', 'archived'")); + assert!(!run_ddl.contains("completed_at")); + let rust_unchanged: i64 = conn + .query_row( + "SELECT COUNT(*) FROM agent_sessions WHERE session_id='rust-sentinel' + AND title='Rust sentinel' AND status='idle' AND updated_at='2025-12-01T00:00:00Z'", + [], + |row| row.get(0), + ) + .expect("Rust session sentinel"); + let cli_unchanged: i64 = conn + .query_row( + "SELECT COUNT(*) FROM code_sessions WHERE session_id='cli-sentinel' + AND cli_agent_type='codex' AND status='completed' + AND updated_at='2025-12-02T00:00:00Z'", + [], + |row| row.get(0), + ) + .expect("CLI session sentinel"); + assert_eq!((rust_unchanged, cli_unchanged), (1, 1)); + + conn.execute( + "INSERT INTO agent_org_runs ( + id, org_id, coordinator_agent_id, entry_mode, status, created_at, updated_at + ) VALUES ( + 'new-run', 'new-org', 'new-coordinator', 'standalone_session', 'starting', + '2026-02-01T00:00:00Z', '2026-02-01T00:00:00Z' + )", + [], + ) + .expect("insert canonical starting run"); + let defaults: (String, i64, i64) = conn + .query_row( + "SELECT status, activation_generation, has_initial_work + FROM agent_org_runs WHERE id='new-run'", + [], + |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)), + ) + .expect("read canonical starting run"); + assert_eq!(defaults, ("starting".into(), 1, 0)); +} + +#[test] +fn canonical_schema_init_is_idempotent_and_preserves_runtime_data() { + let conn = memory_connection(); + init_schema(&conn).expect("create canonical schema"); + conn.execute_batch( + "INSERT INTO agent_org_runs ( + id, org_id, coordinator_agent_id, entry_mode, status, created_at, updated_at + ) VALUES ( + 'current-run', 'current-org', 'current-coordinator', + 'standalone_session', 'starting', '2026-03-01T00:00:00Z', '2026-03-01T00:00:00Z' + ); + INSERT INTO agent_org_run_progress ( + org_run_id, work_revision, completion_requested, completion_summary, updated_at + ) VALUES ('current-run', 9, 1, 'done', '2026-03-01T00:00:00Z'); + INSERT INTO agent_org_member_materializations ( + org_run_id, member_id, agent_id, generation, session_id, + authority_class, status, created_at, updated_at + ) VALUES ( + 'current-run', 'member-a', 'agent-a', 1, 'session-a', 'starting', 'succeeded', + '2026-03-01T00:00:00Z', '2026-03-01T00:00:00Z' + ); + INSERT INTO agent_org_initial_inputs ( + org_run_id, turn_intent_id, message_id, content, payload_json, + status, created_at, updated_at + ) VALUES ( + 'current-run', 'turn-a', 'message-a', 'hello', '{}', 'queued', + '2026-03-01T00:00:00Z', '2026-03-01T00:00:00Z' + );", + ) + .expect("canonical runtime fixtures"); + let indexes_before = index_names(&conn); + + init_schema(&conn).expect("repeat canonical init"); + + assert_eq!(index_names(&conn), indexes_before); + assert_eq!(row_count(&conn, "agent_org_runs"), 1); + assert_eq!(row_count(&conn, "agent_org_run_progress"), 1); + assert_eq!(row_count(&conn, "agent_org_member_materializations"), 1); + assert_eq!(row_count(&conn, "agent_org_initial_inputs"), 1); + let preserved: i64 = conn + .query_row( + "SELECT COUNT(*) + FROM agent_org_run_progress progress + JOIN agent_org_member_materializations materialization + ON materialization.org_run_id=progress.org_run_id + JOIN agent_org_initial_inputs input ON input.org_run_id=progress.org_run_id + WHERE progress.work_revision=9 AND progress.completion_summary='done' + AND materialization.session_id='session-a' AND input.content='hello'", + [], + |row| row.get(0), + ) + .expect("preserved canonical data"); + assert_eq!(preserved, 1); +} + +#[test] +fn unknown_run_schemas_never_trigger_destructive_reset() { + for ddl in [ + LEGACY_AGENT_ORG_RUNS_DDL.replace( + "completed_at TEXT\n);", + "completed_at TEXT, unknown_column TEXT\n);", + ), + LEGACY_AGENT_ORG_RUNS_DDL.replace("org_id TEXT NOT NULL", "org_id BLOB NOT NULL"), + LEGACY_AGENT_ORG_RUNS_DDL + .replace("root_session_id TEXT,", "root_session_id TEXT NOT NULL,"), + LEGACY_AGENT_ORG_RUNS_DDL.replace("summary TEXT,", "summary TEXT DEFAULT 'legacy',"), + LEGACY_AGENT_ORG_RUNS_DDL.replace("id TEXT PRIMARY KEY", "id TEXT"), + ] { + let conn = memory_connection(); + conn.execute_batch(&ddl) + .expect("create unknown schema fixture"); + insert_legacy_run(&conn); + + let _ = init_schema(&conn); + + assert_eq!(row_count(&conn, "agent_org_runs"), 1); + let sentinel: String = conn + .query_row( + "SELECT summary FROM agent_org_runs WHERE id='legacy-run'", + [], + |row| row.get(0), + ) + .expect("read unknown schema sentinel"); + assert_eq!(sentinel, "legacy summary"); + } +} + /// Build an `AgentOrgsStore` pre-loaded with a single org definition. /// Bypasses the disk loader so tests stay hermetic — the sandbox /// already isolates `~/.orgii`, but we don't need to touch disk at @@ -98,16 +351,16 @@ fn sample_org() -> OrgDefinition { role: "lead".to_string(), agent_id: "agent-coord".to_string(), description: None, - hierarchy_mode: Default::default(), plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![OrgMember { - id: "member-w1".to_string(), + members: vec![FlatOrgMember { + member_id: "member-w1".to_string(), name: "Worker One".to_string(), role: "ic".to_string(), agent_id: "agent-w1".to_string(), runtime_config: None, - children: Vec::new(), }], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), } } @@ -184,7 +437,7 @@ fn create_run_for_root(org: &OrgDefinition, root_session_id: &str) -> AgentOrgRu org_id: org.id.clone(), coordinator_agent_id: "agent-coord".to_string(), root_session_id: Some(root_session_id.to_string()), - org_snapshot: org.clone(), + org_snapshot: org.into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -234,7 +487,7 @@ fn create_starting_fixture(has_initial_work: bool) -> AgentOrgRunRecord { org_id: org.id.clone(), coordinator_agent_id: org.agent_id.clone(), root_session_id: "starting-root".to_string(), - org_snapshot: org, + org_snapshot: (&org).into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, work_item_id: None, project_slug: None, @@ -869,11 +1122,11 @@ fn upsert_cli_session_row_for_member( fn context_for_session_with_parent_walk_root_session_direct_hit() { let _sandbox = test_helpers::test_env::sandbox(); let org = sample_org(); - let store = store_with_org(org.clone()); + let _store = store_with_org(org.clone()); let _run = create_run_for_root(&org, "root-session-1"); upsert_session_row("root-session-1", None); - let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("root-session-1", &store) + let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("root-session-1") .expect("walk ok") .expect("context resolved"); assert_eq!(ctx.coordinator_agent_id, "agent-coord"); @@ -884,7 +1137,18 @@ fn context_for_session_with_parent_walk_root_session_direct_hit() { #[test] fn context_for_run_uses_launch_snapshot_after_live_org_changes() { let _sandbox = test_helpers::test_env::sandbox(); - let org = sample_org(); + let mut org = sample_org(); + org.members.push(FlatOrgMember { + member_id: "member-w2".to_string(), + name: "Worker Two".to_string(), + role: "reviewer".to_string(), + agent_id: "agent-w2".to_string(), + runtime_config: None, + }); + org.additional_task_graph_writer_member_ids = vec!["member-w1".to_string()]; + org.member_communication_links = vec![ + crate::definitions::orgs::MemberCommunicationLink::canonical("member-w1", "member-w2"), + ]; let store = store_with_org(org.clone()); let run = create_run_for_root(&org, "root-session-snapshot"); upsert_session_row("root-session-snapshot", None); @@ -893,58 +1157,35 @@ fn context_for_run_uses_launch_snapshot_after_live_org_changes() { let mut orgs = store.orgs.lock().expect("org store lock"); orgs[0].name = "Edited Live Org".to_string(); orgs[0].role = "edited lead".to_string(); - orgs[0].children[0].id = "member-edited".to_string(); - orgs[0].children[0].agent_id = "agent-edited".to_string(); + orgs[0].members[0].member_id = "member-edited".to_string(); + orgs[0].members[0].agent_id = "agent-edited".to_string(); + orgs[0].additional_task_graph_writer_member_ids.clear(); + orgs[0].member_communication_links.clear(); } - let ctx = AgentOrgRunStore::context_for_run(&run.id, &store) + let ctx = AgentOrgRunStore::context_for_run(&run.id) .expect("context lookup ok") .expect("context resolved"); assert_eq!(ctx.org_name, "WalkTest Org"); assert_eq!(ctx.coordinator_role, "lead"); - assert_eq!(ctx.members.len(), 1); + assert_eq!(ctx.members.len(), 2); assert_eq!(ctx.members[0].member_id, "member-w1"); assert_eq!(ctx.members[0].agent_id, "agent-w1"); -} - -#[test] -fn context_for_session_preserves_org_hierarchy_mode() { - for hierarchy_mode in [ - HierarchyMode::Flat, - HierarchyMode::Soft, - HierarchyMode::Strict, - ] { - let _sandbox = test_helpers::test_env::sandbox(); - let mode_label = match hierarchy_mode { - HierarchyMode::Flat => "flat", - HierarchyMode::Soft => "soft", - HierarchyMode::Strict => "strict", - }; - let mut org = sample_org(); - org.id = format!("org-mode-{mode_label}"); - org.hierarchy_mode = hierarchy_mode; - let store = store_with_org(org.clone()); - let root_session_id = format!("root-session-{mode_label}"); - let _run = create_run_for_root(&org, &root_session_id); - upsert_session_row(&root_session_id, None); - - let ctx = AgentOrgRunStore::context_for_session_with_parent_walk(&root_session_id, &store) - .expect("walk ok") - .expect("context resolved"); - assert_eq!(ctx.hierarchy_mode, hierarchy_mode); - } + assert!(ctx.capability_index.is_additional_writer("member-w1")); + assert!(ctx + .capability_index + .members_can_communicate("member-w1", "member-w2")); } #[test] fn context_for_session_with_parent_walk_one_hop_subagent() { let _sandbox = test_helpers::test_env::sandbox(); let org = sample_org(); - let store = store_with_org(org.clone()); let _run = create_run_for_root(&org, "root-session-2"); upsert_session_row("root-session-2", None); upsert_session_row("worker-session-2", Some("root-session-2")); - let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("worker-session-2", &store) + let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("worker-session-2") .expect("walk ok") .expect("context resolved via parent walk"); assert_eq!(ctx.run_id, _run.id); @@ -955,7 +1196,6 @@ fn context_for_session_with_parent_walk_one_hop_subagent() { fn context_for_session_with_parent_walk_cli_member_session() { let _sandbox = test_helpers::test_env::sandbox(); let org = sample_org(); - let store = store_with_org(org.clone()); let _run = create_run_for_root(&org, "root-session-cli-walk"); upsert_session_row("root-session-cli-walk", None); upsert_cli_session_row_for_member( @@ -966,10 +1206,9 @@ fn context_for_session_with_parent_walk_cli_member_session() { "running", ); - let ctx = - AgentOrgRunStore::context_for_session_with_parent_walk("cli-worker-session-walk", &store) - .expect("walk ok") - .expect("context resolved via CLI parent walk"); + let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("cli-worker-session-walk") + .expect("walk ok") + .expect("context resolved via CLI parent walk"); assert_eq!(ctx.run_id, _run.id); assert_eq!(ctx.coordinator_agent_id, "agent-coord"); } @@ -978,13 +1217,12 @@ fn context_for_session_with_parent_walk_cli_member_session() { fn context_for_session_with_parent_walk_two_hop_chain() { let _sandbox = test_helpers::test_env::sandbox(); let org = sample_org(); - let store = store_with_org(org.clone()); let _run = create_run_for_root(&org, "root-session-3"); upsert_session_row("root-session-3", None); upsert_session_row("mid-session-3", Some("root-session-3")); upsert_session_row("leaf-session-3", Some("mid-session-3")); - let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("leaf-session-3", &store) + let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("leaf-session-3") .expect("walk ok") .expect("context resolved via 2-hop walk"); assert_eq!(ctx.run_id, _run.id); @@ -993,12 +1231,10 @@ fn context_for_session_with_parent_walk_two_hop_chain() { #[test] fn context_for_session_with_parent_walk_unrelated_session_returns_none() { let _sandbox = test_helpers::test_env::sandbox(); - let org = sample_org(); - let store = store_with_org(org); upsert_session_row("orphan-session", None); - let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("orphan-session", &store) - .expect("walk ok"); + let ctx = + AgentOrgRunStore::context_for_session_with_parent_walk("orphan-session").expect("walk ok"); assert!( ctx.is_none(), "session with no matching org_run should resolve to None" @@ -1011,12 +1247,10 @@ fn context_for_session_with_parent_walk_unknown_session_returns_none() { // (e.g. wire from a stale event) should terminate the walk // cleanly, not panic and not error. let _sandbox = test_helpers::test_env::sandbox(); - let org = sample_org(); - let store = store_with_org(org); ensure_runtime_schemas(); - let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("ghost-session", &store) - .expect("walk ok"); + let ctx = + AgentOrgRunStore::context_for_session_with_parent_walk("ghost-session").expect("walk ok"); assert!(ctx.is_none()); } @@ -1025,12 +1259,10 @@ fn context_for_session_with_parent_walk_breaks_on_cycle() { // Synthetic cycle: A → B → A. Should bail out cleanly with None // (and a warn log; we don't assert on logs here). let _sandbox = test_helpers::test_env::sandbox(); - let org = sample_org(); - let store = store_with_org(org); upsert_session_row("cycle-a", Some("cycle-b")); upsert_session_row("cycle-b", Some("cycle-a")); - let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("cycle-a", &store) + let ctx = AgentOrgRunStore::context_for_session_with_parent_walk("cycle-a") .expect("walk ok despite cycle"); assert!( ctx.is_none(), @@ -1822,215 +2054,3 @@ fn idle_cas_and_task_create_have_one_serializable_outcome() { (status, result) => panic!("non-serializable quiescence result: {status:?}, {result:?}"), } } - -// ── HierarchyMode routing checks ──────────────────────────────── -// -// Pure-function coverage for `AgentOrgRunContext::check_routing`. -// The fixture mirrors a real two-branch org so cross-branch hops -// and the coordinator escape hatch can be exercised independently. -// -// coordinator -// ├── lead-a (member-a, agent-a) -// │ └── ic-a (member-a-ic, agent-a-ic) -// └── lead-b (member-b, agent-b) -// └── ic-b (member-b-ic, agent-b-ic) -fn routing_ctx(mode: HierarchyMode) -> AgentOrgRunContext { - AgentOrgRunContext { - run_id: "run-routing".into(), - org_id: "org-routing".into(), - org_name: "RoutingOrg".into(), - org_role: "lead".into(), - coordinator_agent_id: "agent-coord".into(), - coordinator_name: "RoutingOrg".into(), - coordinator_role: "lead".into(), - members: vec![ - AgentOrgContextMember { - member_id: "member-a".into(), - name: "lead-a".into(), - role: "lead".into(), - agent_id: "agent-a".into(), - parent_member_id: None, - }, - AgentOrgContextMember { - member_id: "member-a-ic".into(), - name: "ic-a".into(), - role: "ic".into(), - agent_id: "agent-a-ic".into(), - parent_member_id: Some("member-a".into()), - }, - AgentOrgContextMember { - member_id: "member-b".into(), - name: "lead-b".into(), - role: "lead".into(), - agent_id: "agent-b".into(), - parent_member_id: None, - }, - AgentOrgContextMember { - member_id: "member-b-ic".into(), - name: "ic-b".into(), - role: "ic".into(), - agent_id: "agent-b-ic".into(), - parent_member_id: Some("member-b".into()), - }, - ], - hierarchy_mode: mode, - plan_approval_policy: PlanApprovalPolicy::Coordinator, - root_session_id: None, - } -} - -#[test] -fn routing_flat_allows_anything() { - let ctx = routing_ctx(HierarchyMode::Flat); - assert_eq!( - ctx.check_routing("member-a-ic", "member-b-ic"), - RoutingDecision::Allowed, - ); - assert_eq!( - ctx.check_routing("member-b", "member-a"), - RoutingDecision::Allowed, - ); -} - -#[test] -fn routing_soft_allows_anything() { - // Soft mode renders reports-to in the prompt as a hint but - // never enforces — same outcome as Flat for the runtime layer. - let ctx = routing_ctx(HierarchyMode::Soft); - assert_eq!( - ctx.check_routing("member-a-ic", "member-b-ic"), - RoutingDecision::Allowed, - ); -} - -#[test] -fn task_authority_is_not_peer_message_reachability() { - let soft = routing_ctx(HierarchyMode::Soft); - assert_eq!( - soft.allowed_task_target_member_ids_for("member-a"), - vec!["member-a".to_string(), "member-a-ic".to_string()] - ); - assert!(soft.can_assign_task_to("member-a", "member-a-ic")); - assert!( - !soft.can_assign_task_to("member-a", "member-b"), - "Soft permits peer discussion, not peer task assignment" - ); - - let strict = routing_ctx(HierarchyMode::Strict); - assert!(strict.can_assign_task_to("member-a", "member-a-ic")); - assert!(!strict.can_assign_task_to("member-a", "member-b")); - - let flat = routing_ctx(HierarchyMode::Flat); - assert_eq!( - flat.allowed_task_target_member_ids_for("member-a"), - vec!["member-a".to_string()], - "Flat drops reports-to authority for non-coordinator members" - ); -} - -#[test] -fn task_authority_coordinator_can_manage_every_participant() { - let ctx = routing_ctx(HierarchyMode::Strict); - let allowed = ctx.allowed_task_target_member_ids_for(COORDINATOR_MEMBER_ID); - assert_eq!(allowed.len(), ctx.members.len() + 1); - assert!(allowed.contains(&COORDINATOR_MEMBER_ID.to_string())); - assert!(ctx - .members - .iter() - .all(|member| allowed.contains(&member.member_id))); -} - -#[test] -fn routing_strict_allows_send_to_coordinator() { - let ctx = routing_ctx(HierarchyMode::Strict); - assert_eq!( - ctx.check_routing("member-a-ic", COORDINATOR_MEMBER_ID), - RoutingDecision::Allowed, - "anyone may escalate to the coordinator", - ); -} - -#[test] -fn routing_strict_allows_coordinator_to_anyone() { - let ctx = routing_ctx(HierarchyMode::Strict); - assert_eq!( - ctx.check_routing(COORDINATOR_MEMBER_ID, "member-a-ic"), - RoutingDecision::Allowed, - "coordinator escape hatch — may reach any member", - ); -} - -#[test] -fn routing_strict_allows_send_to_direct_manager() { - let ctx = routing_ctx(HierarchyMode::Strict); - assert_eq!( - ctx.check_routing("member-a-ic", "member-a"), - RoutingDecision::Allowed, - ); -} - -#[test] -fn routing_strict_allows_send_to_direct_report() { - let ctx = routing_ctx(HierarchyMode::Strict); - assert_eq!( - ctx.check_routing("member-a", "member-a-ic"), - RoutingDecision::Allowed, - ); -} - -#[test] -fn routing_strict_blocks_cross_branch() { - let ctx = routing_ctx(HierarchyMode::Strict); - let RoutingDecision::Blocked(hint) = ctx.check_routing("member-a-ic", "member-b-ic") else { - panic!("expected cross-branch send to be blocked"); - }; - assert!( - hint.contains("sender_member_id 'member-a-ic'"), - "hint should name the sender member id (got: {hint})", - ); - assert!( - hint.contains("recipient_member_id 'member-b-ic'"), - "hint should name the recipient member id (got: {hint})", - ); - assert!( - hint.contains("Allowed recipient_member_id values: coordinator, member-a"), - "hint should expose the canonical member-id allow-list (got: {hint})", - ); -} - -#[test] -fn routing_strict_blocks_skip_level_up() { - // ic-a sending to its grand-manager (the coordinator's other - // direct report) is also a violation — only direct manager is - // allowed. - let ctx = routing_ctx(HierarchyMode::Strict); - assert!(matches!( - ctx.check_routing("member-a-ic", "member-b"), - RoutingDecision::Blocked(_) - )); -} - -#[test] -fn routing_strict_blocks_peer_to_peer_lead() { - let ctx = routing_ctx(HierarchyMode::Strict); - let RoutingDecision::Blocked(hint) = ctx.check_routing("member-a", "member-b") else { - panic!("peer leads must not contact each other directly"); - }; - assert!( - hint.contains("Allowed recipient_member_id values: coordinator"), - "top-level lead should only be allowed to route through coordinator (got: {hint})", - ); -} - -#[test] -fn routing_strict_blocks_unknown_sender_with_useful_hint() { - // A sender that isn't in the roster (shouldn't happen in - // practice, but the function must not panic): the message - // should still surface a Blocked decision rather than silently - // letting it through. - let ctx = routing_ctx(HierarchyMode::Strict); - assert!(matches!( - ctx.check_routing("member-stranger", "member-a-ic"), - RoutingDecision::Blocked(_) - )); -} diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/store/validation.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/store/validation.rs index 8f8ce06efa..2c46e1d8c1 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/store/validation.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/store/validation.rs @@ -110,12 +110,11 @@ pub(super) fn validate_task_text_fields( } fn collect_roster_member_ids( - members: &[crate::definitions::orgs::OrgMember], + members: &[crate::definitions::orgs::FlatOrgMember], out: &mut HashSet, ) { for member in members { - out.insert(member.id.clone()); - collect_roster_member_ids(&member.children, out); + out.insert(member.member_id.clone()); } } @@ -241,12 +240,14 @@ pub(super) fn validate_task_persistence_invariants( .map_err(|err| err.to_string())? .flatten(); if let Some(snapshot_json) = snapshot_json { - let snapshot: crate::definitions::orgs::OrgDefinition = + let snapshot: crate::definitions::orgs::AgentOrgLaunchSnapshot = serde_json::from_str(&snapshot_json).map_err(|err| { format!("invalid Agent Org launch snapshot for {org_run_id}: {err}") })?; + crate::definitions::orgs::validate_launch_snapshot(&snapshot) + .map_err(|err| format!("invalid Agent Org launch snapshot for {org_run_id}: {err}"))?; let mut roster = HashSet::new(); - collect_roster_member_ids(&snapshot.children, &mut roster); + collect_roster_member_ids(&snapshot.members, &mut roster); for member_id in &eligible_member_ids { if !roster.contains(member_id) { return Err(format!( diff --git a/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/tests.rs b/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/tests.rs index 1ec49c5754..ca80eae6fa 100644 --- a/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/tests.rs +++ b/src-tauri/crates/agent-core/src/core/coordination/agent_org_tasks/tests.rs @@ -734,30 +734,31 @@ fn store_rejects_owner_and_eligibility_outside_launch_roster() { use crate::coordination::agent_org_runs::{ AgentOrgRunEntryMode, AgentOrgRunStatus, AgentOrgRunStore, CreateAgentOrgRunParams, }; - use crate::definitions::orgs::{HierarchyMode, OrgDefinition, OrgMember, PlanApprovalPolicy}; + use crate::definitions::orgs::{FlatOrgMember, OrgDefinition, PlanApprovalPolicy}; let _sandbox = task_store_sandbox(); let run = AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id: "org-roster".to_string(), coordinator_agent_id: "coord".to_string(), root_session_id: None, - org_snapshot: OrgDefinition { + org_snapshot: (&OrgDefinition { id: "org-roster".to_string(), name: "Roster".to_string(), role: "coordinator".to_string(), agent_id: "coord".to_string(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![OrgMember { - id: "member-a".to_string(), + members: vec![FlatOrgMember { + member_id: "member-a".to_string(), name: "A".to_string(), role: "worker".to_string(), agent_id: "agent-a".to_string(), runtime_config: None, - children: Vec::new(), }], - }, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), + }) + .into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, diff --git a/src-tauri/crates/agent-core/src/core/definitions/commands.rs b/src-tauri/crates/agent-core/src/core/definitions/commands.rs index 37b3652d8d..1c5a5c49fc 100644 --- a/src-tauri/crates/agent-core/src/core/definitions/commands.rs +++ b/src-tauri/crates/agent-core/src/core/definitions/commands.rs @@ -74,31 +74,24 @@ pub async fn agent_definitions_remove( pub async fn agent_orgs_list( state: tauri::State<'_, std::sync::Arc>, ) -> Result, String> { - let orgs = state - .orgs - .lock() - .map_err(|err| format!("Lock error: {}", err))?; - Ok(orgs.clone()) + state.list() } -#[tauri::command] -pub async fn agent_orgs_add( - state: tauri::State<'_, std::sync::Arc>, - org_json: String, -) -> Result { - let org: OrgDefinition = - serde_json::from_str(&org_json).map_err(|err| format!("Invalid org JSON: {}", err))?; - state.insert(org) +pub(in crate::core::definitions) struct TrustedAgentOrgSettingsActor { + _private: (), } #[tauri::command] -pub async fn agent_orgs_update( +pub async fn agent_orgs_save_trusted_settings( state: tauri::State<'_, std::sync::Arc>, org_json: String, -) -> Result<(), String> { +) -> Result { let org: OrgDefinition = serde_json::from_str(&org_json).map_err(|err| format!("Invalid org JSON: {}", err))?; - state.replace(org) + state + .inner() + .save_trusted_settings_async(org, TrustedAgentOrgSettingsActor { _private: () }) + .await } #[tauri::command] @@ -106,7 +99,7 @@ pub async fn agent_orgs_remove( state: tauri::State<'_, std::sync::Arc>, org_id: String, ) -> Result { - state.remove(&org_id) + state.inner().remove_async(org_id).await } /// One row in the Inbox flat chat list — a persisted agent-org run that diff --git a/src-tauri/crates/agent-core/src/core/definitions/orgs.rs b/src-tauri/crates/agent-core/src/core/definitions/orgs.rs index aa63d1149c..eed40e4c01 100644 --- a/src-tauri/crates/agent-core/src/core/definitions/orgs.rs +++ b/src-tauri/crates/agent-core/src/core/definitions/orgs.rs @@ -1,26 +1,28 @@ -//! Agent org definitions — CRUD + JSON file persistence. -//! -//! Stores agent organizations (team hierarchies) in `~/.orgii/agent-orgs.json`. +//! Flat Agent Team definitions with validated, atomic JSON persistence. use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; +use std::fs::{File, OpenOptions}; +use std::io::Write; +use std::path::{Path, PathBuf}; #[cfg(not(test))] use std::sync::OnceLock; use std::sync::{Arc, Mutex}; -use tracing::{error, info}; - -use key_vault::ModelType; +use std::time::{SystemTime, UNIX_EPOCH}; +use tracing::{error, info, warn}; use app_paths::agent_orgs as storage_path; +use key_vault::ModelType; #[cfg(not(test))] static PROCESS_STORE: OnceLock> = OnceLock::new(); -/// Process-wide shared `AgentOrgsStore` — same singleton contract as -/// `definitions_store()`. Tauri manages this `Arc`; library callers use -/// this accessor instead of constructing ad-hoc stores that re-read the -/// JSON file per call. Test builds return a fresh store per call for -/// `ORGII_HOME` tempdir isolation. +pub const CLI_AGENT_ORG_REFERENCE_PREFIX: &str = "cli:"; +pub const MAX_AGENT_ORG_MEMBERS: usize = 50; +pub const MAX_AGENT_ORG_DEFINITION_BYTES: usize = 256 * 1024; +const AGENT_ORGS_FILE_SCHEMA_VERSION: u32 = 2; +use core_types::agent_org::COORDINATOR_MEMBER_ID; + pub fn orgs_store() -> Arc { #[cfg(test)] { @@ -34,62 +36,19 @@ pub fn orgs_store() -> Arc { } } -// ── Types ── - -pub const CLI_AGENT_ORG_REFERENCE_PREFIX: &str = "cli:"; - pub fn parse_cli_agent_org_reference(agent_id: &str) -> Option { let raw = agent_id .trim() .strip_prefix(CLI_AGENT_ORG_REFERENCE_PREFIX)? .trim(); let model_type = ModelType::from_str(raw)?; - if model_type.is_cli_agent() { - Some(model_type) - } else { - None - } + model_type.is_cli_agent().then_some(model_type) } pub fn is_cli_agent_org_reference(agent_id: &str) -> bool { parse_cli_agent_org_reference(agent_id).is_some() } -/// How the hierarchy implied by `OrgMember.children` is interpreted at -/// runtime. Wired through to the Agent Org runtime + LLM system prompt. -/// -/// - `Flat`: hierarchy is dropped entirely. Agents see no reports-to -/// structure in their system prompt; routing is unrestricted. -/// - `Soft` (default): hierarchy is shown to agents as an *organizational -/// hint* — they are encouraged to coordinate through their manager but -/// may message any peer directly when appropriate. Routing is unrestricted. -/// - `Strict`: hierarchy is enforced at the routing layer. A member may -/// only message its manager, its direct reports, or the coordinator -/// (always reachable as escape hatch). Sibling-to-sibling sends are -/// rejected with a structured error suggesting escalation. -/// -/// Task authority is intentionally stricter than message reachability: -/// the coordinator may manage all tasks; a member may manage itself and its -/// direct reports in `Soft`/`Strict`; a `Flat` member may manage only itself. -/// Therefore Soft peer discussion never implies Soft peer delegation. -/// -/// Default is `Soft` for both new orgs and orgs migrated from the -/// previous schema (no `hierarchy_mode` field on disk) — this is the -/// closest match to the prior runtime behaviour, where the LLM saw the -/// indented tree in its prompt but routing was already flat. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] -#[serde(rename_all = "lowercase")] -pub enum HierarchyMode { - Flat, - #[default] - Soft, - Strict, -} - -/// Who reviews an Agent Org member's submitted planning task. -/// -/// The value is snapshotted into each run through `OrgDefinition`, so editing -/// a team never changes the policy of work that is already in progress. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "snake_case")] pub enum PlanApprovalPolicy { @@ -109,7 +68,7 @@ impl PlanApprovalPolicy { } } -#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct OrgMemberRuntimeConfig { #[serde(skip_serializing_if = "Option::is_none")] @@ -143,57 +102,275 @@ pub struct OrgMemberLaunchOverride { pub runtime_config: Option, } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct OrgMember { - pub id: String, +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct FlatOrgMember { + pub member_id: String, pub name: String, pub role: String, pub agent_id: String, #[serde(skip_serializing_if = "Option::is_none")] pub runtime_config: Option, - #[serde(default)] - pub children: Vec, } -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct MemberCommunicationLink { + pub member_a_id: String, + pub member_b_id: String, +} + +impl MemberCommunicationLink { + pub fn canonical(member_a_id: impl Into, member_b_id: impl Into) -> Self { + let member_a_id = member_a_id.into(); + let member_b_id = member_b_id.into(); + if member_a_id <= member_b_id { + Self { + member_a_id, + member_b_id, + } + } else { + Self { + member_a_id: member_b_id, + member_b_id: member_a_id, + } + } + } + + pub fn key(&self) -> (&str, &str) { + (&self.member_a_id, &self.member_b_id) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct OrgDefinition { pub id: String, pub name: String, pub role: String, - #[serde(default)] pub agent_id: String, #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, - /// How `children` is interpreted at runtime. See `HierarchyMode` doc. - #[serde(default)] - pub hierarchy_mode: HierarchyMode, - /// Who approves plans submitted by members during an Agent Org run. - #[serde(default)] pub plan_approval_policy: PlanApprovalPolicy, - #[serde(default)] - pub children: Vec, + pub members: Vec, + pub additional_task_graph_writer_member_ids: Vec, + pub member_communication_links: Vec, } impl OrgDefinition { - /// Count total members (recursive, including root). - pub fn member_count(&self) -> usize { - 1 + Self::count_recursive(&self.children) + /// Total number of participants in the Team: the coordinator plus + /// every `members` entry. Coordinator-inclusive by design — use + /// `members.len()` for the member roster size. + pub fn participant_count(&self) -> usize { + 1 + self.members.len() + } + + pub fn with_all_member_links(mut self) -> Self { + self.member_communication_links = all_member_links(&self.members); + self + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct AgentOrgLaunchSnapshot { + pub schema_version: u32, + pub org_id: String, + pub org_name: String, + pub coordinator_role: String, + pub coordinator_agent_id: String, + pub plan_approval_policy: PlanApprovalPolicy, + pub members: Vec, + pub additional_task_graph_writer_member_ids: Vec, + pub member_communication_links: Vec, +} + +impl From<&OrgDefinition> for AgentOrgLaunchSnapshot { + fn from(org: &OrgDefinition) -> Self { + Self { + schema_version: 1, + org_id: org.id.clone(), + org_name: org.name.clone(), + coordinator_role: org.role.clone(), + coordinator_agent_id: org.agent_id.clone(), + plan_approval_policy: org.plan_approval_policy, + members: org.members.clone(), + additional_task_graph_writer_member_ids: org + .additional_task_graph_writer_member_ids + .clone(), + member_communication_links: org.member_communication_links.clone(), + } + } +} + +/// Validate the frozen, self-contained Team contract without consulting the +/// mutable definition store. Runtime recovery must never reinterpret a launch +/// snapshot using a template that may have changed after the run started. +pub fn validate_launch_snapshot(snapshot: &AgentOrgLaunchSnapshot) -> Result<(), String> { + if snapshot.schema_version != 1 { + return Err(format!( + "unsupported Agent Org launch snapshot version {}", + snapshot.schema_version + )); + } + if snapshot.org_id.trim().is_empty() + || snapshot.org_name.trim().is_empty() + || snapshot.coordinator_agent_id.trim().is_empty() + { + return Err("Agent Org launch snapshot has an empty Team or Coordinator identity".into()); + } + if snapshot.members.is_empty() || snapshot.members.len() > MAX_AGENT_ORG_MEMBERS { + return Err(format!( + "Agent Org launch snapshot must contain between 1 and {} members", + MAX_AGENT_ORG_MEMBERS + )); + } + + let mut member_ids = HashSet::with_capacity(snapshot.members.len()); + for member in &snapshot.members { + let member_id = member.member_id.trim(); + if member_id.is_empty() || member_id.eq_ignore_ascii_case(COORDINATOR_MEMBER_ID) { + return Err("Agent Org launch snapshot has an empty or reserved member id".into()); + } + if member.agent_id.trim().is_empty() { + return Err(format!( + "Agent Org launch snapshot member '{}' has no agent definition", + member.member_id + )); + } + if !member_ids.insert(member_id.to_string()) { + return Err(format!( + "Agent Org launch snapshot has duplicate member id '{}'", + member.member_id + )); + } + } + + let mut writer_ids = HashSet::new(); + for writer_id in &snapshot.additional_task_graph_writer_member_ids { + if !member_ids.contains(writer_id) { + return Err(format!( + "Agent Org launch snapshot references unknown Writer member '{}'", + writer_id + )); + } + if !writer_ids.insert(writer_id.as_str()) { + return Err(format!( + "Agent Org launch snapshot has duplicate Writer member '{}'", + writer_id + )); + } + } + + let mut link_keys = HashSet::new(); + for link in &snapshot.member_communication_links { + if link.member_a_id == link.member_b_id { + return Err(format!( + "Agent Org launch snapshot has self communication link '{}'", + link.member_a_id + )); + } + if !member_ids.contains(&link.member_a_id) || !member_ids.contains(&link.member_b_id) { + return Err("Agent Org launch snapshot has a link with an unknown member".into()); + } + let canonical = + MemberCommunicationLink::canonical(link.member_a_id.clone(), link.member_b_id.clone()); + if canonical != *link { + return Err(format!( + "Agent Org launch snapshot has non-canonical communication link '{} ↔ {}'", + link.member_a_id, link.member_b_id + )); + } + if !link_keys.insert((link.member_a_id.as_str(), link.member_b_id.as_str())) { + return Err(format!( + "Agent Org launch snapshot has duplicate communication link '{} ↔ {}'", + link.member_a_id, link.member_b_id + )); + } + } + + let encoded = serde_json::to_vec(snapshot) + .map_err(|err| format!("failed to serialize Agent Org launch snapshot: {err}"))?; + if encoded.len() > MAX_AGENT_ORG_DEFINITION_BYTES { + return Err(format!( + "Agent Org launch snapshot exceeds {} bytes", + MAX_AGENT_ORG_DEFINITION_BYTES + )); + } + Ok(()) +} + +/// Shared write-path serializer used by every run creation entry point. +pub fn serialize_launch_snapshot(snapshot: &AgentOrgLaunchSnapshot) -> Result { + validate_launch_snapshot(snapshot)?; + serde_json::to_string(snapshot) + .map_err(|err| format!("failed to serialize Agent Org launch snapshot: {err}")) +} + +#[derive(Debug, Clone, Default)] +pub struct AgentOrgCapabilityIndex { + writer_member_ids: HashSet, + communication_pairs: HashSet<(String, String)>, +} + +impl AgentOrgCapabilityIndex { + pub fn from_snapshot(snapshot: &AgentOrgLaunchSnapshot) -> Self { + Self { + writer_member_ids: snapshot + .additional_task_graph_writer_member_ids + .iter() + .cloned() + .collect(), + communication_pairs: snapshot + .member_communication_links + .iter() + .map(|link| (link.member_a_id.clone(), link.member_b_id.clone())) + .collect(), + } } - fn count_recursive(members: &[OrgMember]) -> usize { - members - .iter() - .map(|m| 1 + Self::count_recursive(&m.children)) - .sum() + pub fn is_additional_writer(&self, member_id: &str) -> bool { + self.writer_member_ids.contains(member_id) + } + + pub fn members_can_communicate(&self, member_a_id: &str, member_b_id: &str) -> bool { + let link = MemberCommunicationLink::canonical(member_a_id, member_b_id); + self.communication_pairs + .contains(&(link.member_a_id, link.member_b_id)) } } -// ── In-memory store ── +pub fn all_member_links(members: &[FlatOrgMember]) -> Vec { + let mut links = Vec::with_capacity(members.len().saturating_mul(members.len()) / 2); + for (index, member_a) in members.iter().enumerate() { + for member_b in members.iter().skip(index + 1) { + links.push(MemberCommunicationLink::canonical( + member_a.member_id.clone(), + member_b.member_id.clone(), + )); + } + } + links.sort_by(|left, right| left.key().cmp(&right.key())); + links +} + +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct AgentOrgDefinitionsFile { + schema_version: u32, + definitions: Vec, +} + +enum LoadOutcome { + Missing, + Loaded(Vec), + LegacyReset, + Blocked(String), +} pub struct AgentOrgsStore { pub(crate) orgs: Mutex>, + persistence_blocked: Option, } impl Default for AgentOrgsStore { @@ -205,18 +382,47 @@ impl Default for AgentOrgsStore { impl AgentOrgsStore { pub fn new() -> Self { let path = storage_path(); - let mut orgs = load_from_disk(&path); - if ensure_default_template_team(&mut orgs) { - if let Err(err) = save_to_disk(&path, &orgs) { - error!("[agent-orgs] Failed to persist default orgs: {}", err); + let (mut orgs, persistence_blocked, should_persist) = match load_from_disk(&path) { + LoadOutcome::Missing => (Vec::new(), None, true), + LoadOutcome::Loaded(orgs) => (orgs, None, false), + LoadOutcome::LegacyReset => (Vec::new(), None, true), + LoadOutcome::Blocked(message) => { + error!("[agent-orgs] {}", message); + (Vec::new(), Some(message), false) + } + }; + + let defaults_changed = + persistence_blocked.is_none() && ensure_default_template_team(&mut orgs); + if persistence_blocked.is_none() && (should_persist || defaults_changed) { + if let Err(err) = validate_definitions(&orgs).and_then(|_| save_to_disk(&path, &orgs)) { + error!( + "[agent-orgs] Failed to persist canonical definitions: {}", + err + ); + return Self { + orgs: Mutex::new(Vec::new()), + persistence_blocked: Some(err), + }; } } + Self { orgs: Mutex::new(orgs), + persistence_blocked, } } + pub fn list(&self) -> Result, String> { + self.ensure_writable()?; + self.orgs + .lock() + .map(|orgs| orgs.clone()) + .map_err(|err| format!("Lock error: {}", err)) + } + pub fn get(&self, org_id: &str) -> Result { + self.ensure_writable()?; let orgs = self .orgs .lock() @@ -229,117 +435,35 @@ impl AgentOrgsStore { pub fn coordinator_agent_id(&self, org_id: &str) -> Result { let org = self.get(org_id)?; - let coordinator_agent_id = org.agent_id.trim(); - if coordinator_agent_id.is_empty() { + if org.agent_id.trim().is_empty() { return Err(format!( "Agent Org '{}' has no coordinator agent configured", org.name )); } - Ok(coordinator_agent_id.to_string()) + Ok(org.agent_id) } - pub(crate) fn persist(&self, orgs: &[OrgDefinition]) { - let path = storage_path(); - if let Err(err) = save_to_disk(&path, orgs) { - error!("[agent-orgs] Failed to persist: {}", err); - } - } - - /// Names of orgs that reference `agent_id` (as coordinator or any - /// member, recursively). Used by `AgentDefinitionsStore::remove` to - /// refuse deleting agents with dangling org references. pub fn org_names_referencing_agent(&self, agent_id: &str) -> Vec { - fn members_reference(members: &[OrgMember], agent_id: &str) -> bool { - members - .iter() - .any(|m| m.agent_id == agent_id || members_reference(&m.children, agent_id)) - } let Ok(orgs) = self.orgs.lock() else { return Vec::new(); }; orgs.iter() - .filter(|org| org.agent_id == agent_id || members_reference(&org.children, agent_id)) + .filter(|org| { + org.agent_id == agent_id + || org.members.iter().any(|member| member.agent_id == agent_id) + }) .map(|org| org.name.clone()) .collect() } - /// Validate that every agent referenced by `org` (coordinator + all - /// members) resolves to a known agent definition or a valid `cli:*` - /// reference. Write-time enforcement so dangling references fail at - /// save instead of at launch. - fn validate_agent_references(org: &OrgDefinition) -> Result<(), String> { - fn check( - agent_id: &str, - where_: &str, - missing: &mut Vec, - unsupported_cli: &mut Vec, - ) { - let id = agent_id.trim(); - if id.is_empty() { - return; - } - if parse_cli_agent_org_reference(id).is_some() { - unsupported_cli.push(format!("{} ({})", id, where_)); - return; - } - if super::definitions_store().get(id).is_none() { - missing.push(format!("{} ({})", id, where_)); - } - } - fn walk( - members: &[OrgMember], - missing: &mut Vec, - unsupported_cli: &mut Vec, - ) { - for member in members { - let location = format!("member_id={} name={}", member.id, member.name); - check(&member.agent_id, &location, missing, unsupported_cli); - walk(&member.children, missing, unsupported_cli); - } - } - let mut missing = Vec::new(); - let mut unsupported_cli = Vec::new(); - check( - &org.agent_id, - "coordinator", - &mut missing, - &mut unsupported_cli, - ); - walk(&org.children, &mut missing, &mut unsupported_cli); - if !unsupported_cli.is_empty() { - return Err(format!( - "CLI Agent Org participants are not supported yet because they cannot drain the Agent Org inbox or use task tools: {}", - unsupported_cli.join(", ") - )); - } - if missing.is_empty() { - Ok(()) - } else { - Err(format!( - "Org '{}' references unknown agent definition(s): {}", - org.name, - missing.join(", ") - )) - } - } - - /// Insert a new org. The single creation chokepoint: validates agent - /// references, rejects duplicate ids AND duplicate names - /// (case-insensitive — the LLM tool path and RPC path previously used - /// different uniqueness rules). pub fn insert(&self, org: OrgDefinition) -> Result { - Self::validate_agent_references(&org)?; let id = org.id.clone(); - let snapshot = { - let mut guard = self - .orgs - .lock() - .map_err(|err| format!("Lock error: {}", err))?; - if guard.iter().any(|existing| existing.id == id) { + self.commit_candidate(|orgs| { + if orgs.iter().any(|existing| existing.id == id) { return Err(format!("Org with id '{}' already exists", id)); } - if guard + if orgs .iter() .any(|existing| existing.name.eq_ignore_ascii_case(&org.name)) { @@ -348,46 +472,64 @@ impl AgentOrgsStore { org.name )); } - guard.push(org); - guard.clone() - }; - self.persist(&snapshot); + orgs.push(org); + Ok(()) + })?; Ok(id) } - /// Replace an existing org by id. Validates agent references. pub fn replace(&self, org: OrgDefinition) -> Result<(), String> { - Self::validate_agent_references(&org)?; - let snapshot = { - let mut guard = self - .orgs - .lock() - .map_err(|err| format!("Lock error: {}", err))?; - let idx = guard + self.commit_candidate(|orgs| { + let index = orgs .iter() .position(|existing| existing.id == org.id) .ok_or_else(|| format!("Org '{}' not found", org.id))?; - guard[idx] = org; - guard.clone() - }; - self.persist(&snapshot); - Ok(()) + if orgs.iter().enumerate().any(|(candidate_index, existing)| { + candidate_index != index && existing.name.eq_ignore_ascii_case(&org.name) + }) { + return Err(format!("An org named '{}' already exists", org.name)); + } + orgs[index] = org; + Ok(()) + }) + } + + pub(in crate::core::definitions) fn save_trusted_settings( + &self, + org: OrgDefinition, + _actor: super::commands::TrustedAgentOrgSettingsActor, + ) -> Result { + let saved = org.clone(); + self.commit_candidate(|orgs| { + if let Some(index) = orgs.iter().position(|existing| existing.id == org.id) { + if orgs.iter().enumerate().any(|(candidate_index, existing)| { + candidate_index != index && existing.name.eq_ignore_ascii_case(&org.name) + }) { + return Err(format!("An org named '{}' already exists", org.name)); + } + orgs[index] = org; + } else { + if orgs + .iter() + .any(|existing| existing.name.eq_ignore_ascii_case(&org.name)) + { + return Err(format!("An org named '{}' already exists", org.name)); + } + orgs.push(org); + } + Ok(()) + })?; + self.get(&saved.id) } - /// Remove an org by id. Returns `true` when an org was removed. pub fn remove(&self, org_id: &str) -> Result { - let (removed, snapshot) = { - let mut guard = self - .orgs - .lock() - .map_err(|err| format!("Lock error: {}", err))?; - let len_before = guard.len(); - guard.retain(|org| org.id != org_id); - (guard.len() < len_before, guard.clone()) - }; - if removed { - self.persist(&snapshot); - } + let mut removed = false; + self.commit_candidate(|orgs| { + let before = orgs.len(); + orgs.retain(|org| org.id != org_id); + removed = orgs.len() != before; + Ok(()) + })?; Ok(removed) } @@ -399,107 +541,286 @@ impl AgentOrgsStore { if overrides.is_empty() { return Ok(()); } - - let mut orgs = self - .orgs - .lock() - .map_err(|err| format!("Lock error: {}", err))?; - let org = orgs - .iter_mut() - .find(|existing| existing.id == org_id) - .ok_or_else(|| format!("Agent Org '{}' not found", org_id))?; - - let context = format!("Agent Org '{}'", org.name); - apply_overrides_to_member_tree(&mut org.children, overrides, &context)?; - let snapshot = orgs.clone(); - drop(orgs); - self.persist(&snapshot); - Ok(()) + self.commit_candidate(|orgs| { + let org = orgs + .iter_mut() + .find(|existing| existing.id == org_id) + .ok_or_else(|| format!("Agent Org '{}' not found", org_id))?; + let context = format!("Agent Org '{}'", org.name); + apply_overrides_to_members(&mut org.members, overrides, &context) + }) } - /// Insert (or replace by id) a single `OrgDefinition` into the in-memory - /// store and flush to disk. Used exclusively by the debug-only - /// `/agent/test/agent-org/seed` endpoint to set up an Agent Org for E2E - /// tests without going through the rendered Agent Org wizard. - /// - /// Gated behind `cfg(debug_assertions)` so release builds do not - /// accidentally expose a write surface that bypasses validation done - /// elsewhere (e.g. agent-existence checks, role uniqueness). Tests are - /// responsible for handing in a self-consistent definition; this method - /// is a thin store mutation, not a validator. #[cfg(debug_assertions)] pub fn seed_for_test(&self, def: OrgDefinition) -> Result<(), String> { - let mut orgs = self + self.commit_candidate(|orgs| { + if let Some(slot) = orgs.iter_mut().find(|existing| existing.id == def.id) { + *slot = def; + } else { + orgs.push(def); + } + Ok(()) + }) + } + + fn ensure_writable(&self) -> Result<(), String> { + match &self.persistence_blocked { + Some(message) => Err(format!( + "Agent Org definitions are unavailable until the on-disk error is resolved: {}", + message + )), + None => Ok(()), + } + } + + fn commit_candidate(&self, mutate: F) -> Result<(), String> + where + F: FnOnce(&mut Vec) -> Result<(), String>, + { + self.ensure_writable()?; + let mut guard = self .orgs .lock() .map_err(|err| format!("Lock error: {}", err))?; - if let Some(slot) = orgs.iter_mut().find(|existing| existing.id == def.id) { - *slot = def; - } else { - orgs.push(def); - } - let snapshot = orgs.clone(); - drop(orgs); - self.persist(&snapshot); + let mut candidate = guard.clone(); + mutate(&mut candidate)?; + canonicalize_and_validate_definitions(&mut candidate)?; + save_to_disk(&storage_path(), &candidate)?; + *guard = candidate; Ok(()) } } -/// Apply member launch overrides to an org member tree, erroring on any -/// override that references an unknown member id. SHARED implementation — -/// both the persisted-org path (`apply_member_launch_overrides`) and the -/// run-snapshot path (`session::launch`) call this; they previously held -/// line-for-line copies that could drift. -pub fn apply_overrides_to_member_tree( - members: &mut [OrgMember], +/// Async wrappers for every mutating store entry point. +/// +/// Store mutations fsync under the store mutex; calling them directly from +/// an async context stalls the executor. These wrappers are the single +/// spawn_blocking owner — async callers (Tauri commands, model tools, +/// launch) go through here instead of hand-rolling their own offloading. +impl AgentOrgsStore { + async fn run_blocking(self: &Arc, task: F) -> Result + where + T: Send + 'static, + F: FnOnce(&AgentOrgsStore) -> Result + Send + 'static, + { + let store = Arc::clone(self); + tokio::task::spawn_blocking(move || task(&store)) + .await + .map_err(|err| format!("Agent Org store task failed: {err}"))? + } + + pub async fn insert_async(self: &Arc, org: OrgDefinition) -> Result { + self.run_blocking(move |store| store.insert(org)).await + } + + pub async fn replace_async(self: &Arc, org: OrgDefinition) -> Result<(), String> { + self.run_blocking(move |store| store.replace(org)).await + } + + pub async fn remove_async(self: &Arc, org_id: String) -> Result { + self.run_blocking(move |store| store.remove(&org_id)).await + } + + pub async fn apply_member_launch_overrides_async( + self: &Arc, + org_id: String, + overrides: HashMap, + ) -> Result<(), String> { + self.run_blocking(move |store| store.apply_member_launch_overrides(&org_id, &overrides)) + .await + } + + pub(in crate::core::definitions) async fn save_trusted_settings_async( + self: &Arc, + org: OrgDefinition, + actor: super::commands::TrustedAgentOrgSettingsActor, + ) -> Result { + self.run_blocking(move |store| store.save_trusted_settings(org, actor)) + .await + } +} + +pub fn apply_overrides_to_members( + members: &mut [FlatOrgMember], overrides: &HashMap, context_label: &str, ) -> Result<(), String> { - let mut applied_member_ids = HashSet::new(); - apply_overrides_to_members(members, overrides, &mut applied_member_ids)?; - let mut unknown_member_ids = overrides + let known_ids: HashSet<&str> = members + .iter() + .map(|member| member.member_id.as_str()) + .collect(); + let mut unknown_ids = overrides .keys() - .filter(|member_id| !applied_member_ids.contains(*member_id)) + .filter(|member_id| !known_ids.contains(member_id.as_str())) .cloned() .collect::>(); - unknown_member_ids.sort(); - if unknown_member_ids.is_empty() { - Ok(()) - } else { - Err(format!( + unknown_ids.sort(); + if !unknown_ids.is_empty() { + return Err(format!( "{} has no member id(s) for override: {}", context_label, - unknown_member_ids.join(", ") - )) + unknown_ids.join(", ") + )); + } + for member in members { + let Some(member_override) = overrides.get(&member.member_id) else { + continue; + }; + if let Some(agent_id) = member_override + .agent_id + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + { + member.agent_id = agent_id.to_string(); + } + if let Some(runtime_config) = member_override.runtime_config.clone() { + member.runtime_config = Some(runtime_config); + } } + Ok(()) } -fn apply_overrides_to_members( - members: &mut [OrgMember], - overrides: &HashMap, - applied_member_ids: &mut HashSet, -) -> Result<(), String> { - for member in members { - if let Some(member_override) = overrides.get(&member.id) { - applied_member_ids.insert(member.id.clone()); - if let Some(agent_id) = member_override - .agent_id - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - { - member.agent_id = agent_id.to_string(); - } - if let Some(runtime_config) = member_override.runtime_config.clone() { - member.runtime_config = Some(runtime_config); - } +pub fn canonicalize_and_validate_definition(org: &mut OrgDefinition) -> Result<(), String> { + if org.id.trim().is_empty() || org.name.trim().is_empty() { + return Err("Agent Org id and name must not be empty".to_string()); + } + if org.members.is_empty() || org.members.len() > MAX_AGENT_ORG_MEMBERS { + return Err(format!( + "Agent Org '{}' must contain between 1 and {} members", + org.name, MAX_AGENT_ORG_MEMBERS + )); + } + + let mut member_ids = HashSet::with_capacity(org.members.len()); + for member in &org.members { + let member_id = member.member_id.trim(); + if member_id.is_empty() || member_id.eq_ignore_ascii_case(COORDINATOR_MEMBER_ID) { + return Err(format!( + "Agent Org '{}' contains an empty or reserved member id", + org.name + )); + } + if !member_ids.insert(member_id.to_string()) { + return Err(format!( + "Agent Org '{}' contains duplicate member id '{}'", + org.name, member_id + )); + } + } + + let mut writer_ids = HashSet::new(); + for writer_id in &org.additional_task_graph_writer_member_ids { + if !member_ids.contains(writer_id) { + return Err(format!( + "Agent Org '{}' references unknown Writer member '{}'", + org.name, writer_id + )); + } + if !writer_ids.insert(writer_id.clone()) { + return Err(format!( + "Agent Org '{}' contains duplicate Writer member '{}'", + org.name, writer_id + )); + } + } + org.additional_task_graph_writer_member_ids.sort(); + + let mut link_keys = HashSet::new(); + let mut canonical_links = Vec::with_capacity(org.member_communication_links.len()); + for link in &org.member_communication_links { + if link.member_a_id == link.member_b_id { + return Err(format!( + "Agent Org '{}' contains self communication link '{}'", + org.name, link.member_a_id + )); + } + if !member_ids.contains(&link.member_a_id) || !member_ids.contains(&link.member_b_id) { + return Err(format!( + "Agent Org '{}' contains a communication link with an unknown member", + org.name + )); + } + let canonical = + MemberCommunicationLink::canonical(link.member_a_id.clone(), link.member_b_id.clone()); + if !link_keys.insert((canonical.member_a_id.clone(), canonical.member_b_id.clone())) { + return Err(format!( + "Agent Org '{}' contains a duplicate communication link '{} ↔ {}'", + org.name, canonical.member_a_id, canonical.member_b_id + )); + } + canonical_links.push(canonical); + } + canonical_links.sort_by(|left, right| left.key().cmp(&right.key())); + org.member_communication_links = canonical_links; + + validate_agent_references(org)?; + let encoded = serde_json::to_vec(org) + .map_err(|err| format!("Failed to encode Agent Org '{}': {}", org.name, err))?; + if encoded.len() > MAX_AGENT_ORG_DEFINITION_BYTES { + return Err(format!( + "Agent Org '{}' exceeds the {} byte definition limit", + org.name, MAX_AGENT_ORG_DEFINITION_BYTES + )); + } + Ok(()) +} + +fn canonicalize_and_validate_definitions(orgs: &mut [OrgDefinition]) -> Result<(), String> { + let mut ids = HashSet::new(); + let mut names = HashSet::new(); + for org in orgs { + canonicalize_and_validate_definition(org)?; + if !ids.insert(org.id.clone()) { + return Err(format!("Duplicate Agent Org id '{}'", org.id)); + } + if !names.insert(org.name.to_lowercase()) { + return Err(format!("Duplicate Agent Org name '{}'", org.name)); } - apply_overrides_to_members(&mut member.children, overrides, applied_member_ids)?; } Ok(()) } -// ── Built-in default templates ── +fn validate_definitions(orgs: &[OrgDefinition]) -> Result<(), String> { + let mut candidate = orgs.to_vec(); + canonicalize_and_validate_definitions(&mut candidate) +} + +fn validate_agent_references(org: &OrgDefinition) -> Result<(), String> { + let mut missing = Vec::new(); + let mut unsupported_cli = Vec::new(); + let mut check = |agent_id: &str, location: String| { + let id = agent_id.trim(); + if id.is_empty() { + missing.push(format!(" ({})", location)); + } else if parse_cli_agent_org_reference(id).is_some() { + unsupported_cli.push(format!("{} ({})", id, location)); + } else if super::definitions_store().get(id).is_none() { + missing.push(format!("{} ({})", id, location)); + } + }; + check(&org.agent_id, "coordinator".to_string()); + for member in &org.members { + check( + &member.agent_id, + format!("member_id={} name={}", member.member_id, member.name), + ); + } + if !unsupported_cli.is_empty() { + return Err(format!( + "CLI Agent Org participants are not supported: {}", + unsupported_cli.join(", ") + )); + } + if !missing.is_empty() { + return Err(format!( + "Org '{}' references unknown agent definition(s): {}", + org.name, + missing.join(", ") + )); + } + Ok(()) +} const DEFAULT_SDE_TEMPLATE_TEAM_ID: &str = "default:sde-feature-team"; const DEFAULT_DS_TEMPLATE_TEAM_ID: &str = "default:ds-analysis-team"; @@ -507,57 +828,70 @@ const BUILTIN_SDE_AGENT_ID: &str = "builtin:sde"; const BUILTIN_DS_AGENT_ID: &str = "builtin:ds"; fn ensure_default_template_team(orgs: &mut Vec) -> bool { - let mut changed = ensure_default_org( - orgs, - DEFAULT_SDE_TEMPLATE_TEAM_ID, - default_sde_template_team, - default_sde_template_team_is_current, - ); - changed |= ensure_default_org( - orgs, - DEFAULT_DS_TEMPLATE_TEAM_ID, - default_ds_template_team, - default_ds_template_team_is_current, - ); + let mut changed = reconcile_default_org(orgs, default_sde_template_team()); + changed |= reconcile_default_org(orgs, default_ds_template_team()); changed } -fn ensure_default_org( - orgs: &mut Vec, - org_id: &str, - build: fn() -> OrgDefinition, - is_current: fn(&OrgDefinition) -> bool, -) -> bool { - let default_template = build(); - if let Some(existing) = orgs.iter_mut().find(|org| org.id == org_id) { - if is_current(existing) { - return false; - } - *existing = default_template; +fn reconcile_default_org(orgs: &mut Vec, mut canonical: OrgDefinition) -> bool { + let Some(index) = orgs.iter().position(|org| org.id == canonical.id) else { + orgs.push(canonical); return true; - } + }; + let existing = &orgs[index]; + let canonical_ids: HashSet<&str> = canonical + .members + .iter() + .map(|member| member.member_id.as_str()) + .collect(); + canonical.additional_task_graph_writer_member_ids = existing + .additional_task_graph_writer_member_ids + .iter() + .filter(|member_id| canonical_ids.contains(member_id.as_str())) + .cloned() + .collect(); + let existing_ids: HashSet<&str> = existing + .members + .iter() + .map(|member| member.member_id.as_str()) + .collect(); + let existing_links: HashSet<(String, String)> = existing + .member_communication_links + .iter() + .map(|link| { + let canonical = MemberCommunicationLink::canonical( + link.member_a_id.clone(), + link.member_b_id.clone(), + ); + (canonical.member_a_id, canonical.member_b_id) + }) + .collect(); + canonical.member_communication_links = all_member_links(&canonical.members) + .into_iter() + .filter(|link| { + let both_survived = existing_ids.contains(link.member_a_id.as_str()) + && existing_ids.contains(link.member_b_id.as_str()); + !both_survived + || existing_links.contains(&(link.member_a_id.clone(), link.member_b_id.clone())) + }) + .collect(); - orgs.push(default_template); - true + if orgs[index] == canonical { + false + } else { + orgs[index] = canonical; + true + } } -fn default_sde_template_team_is_current(org: &OrgDefinition) -> bool { - const DEFAULT_MEMBER_IDS: [&str; 4] = [ - "sde-planner", - "sde-implementer", - "sde-reviewer", - "sde-tester", - ]; - - org.agent_id == BUILTIN_SDE_AGENT_ID - && org.children.len() == DEFAULT_MEMBER_IDS.len() - && DEFAULT_MEMBER_IDS.iter().all(|member_id| { - org.children.iter().any(|member| { - member.id == *member_id - && member.agent_id == BUILTIN_SDE_AGENT_ID - && member.children.is_empty() - }) - }) +fn flat_member(member_id: &str, name: &str, role: &str, agent_id: &str) -> FlatOrgMember { + FlatOrgMember { + member_id: member_id.to_string(), + name: name.to_string(), + role: role.to_string(), + agent_id: agent_id.to_string(), + runtime_config: None, + } } fn default_sde_template_team() -> OrgDefinition { @@ -570,57 +904,37 @@ fn default_sde_template_team() -> OrgDefinition { "Stable built-in Agent Org for cross-repo UI reproduction and teammate testing." .to_string(), ), - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![ - OrgMember { - id: "sde-planner".to_string(), - name: "Planner".to_string(), - role: "Breaks down the request and tracks execution state".to_string(), - agent_id: BUILTIN_SDE_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, - OrgMember { - id: "sde-implementer".to_string(), - name: "Implementer".to_string(), - role: "Makes the code changes".to_string(), - agent_id: BUILTIN_SDE_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, - OrgMember { - id: "sde-reviewer".to_string(), - name: "Reviewer".to_string(), - role: "Reviews correctness, naming, and maintainability".to_string(), - agent_id: BUILTIN_SDE_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, - OrgMember { - id: "sde-tester".to_string(), - name: "Tester".to_string(), - role: "Runs verification and reports failures".to_string(), - agent_id: BUILTIN_SDE_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, + members: vec![ + flat_member( + "sde-planner", + "Planner", + "Breaks down the request and tracks execution state", + BUILTIN_SDE_AGENT_ID, + ), + flat_member( + "sde-implementer", + "Implementer", + "Makes the code changes", + BUILTIN_SDE_AGENT_ID, + ), + flat_member( + "sde-reviewer", + "Reviewer", + "Reviews correctness, naming, and maintainability", + BUILTIN_SDE_AGENT_ID, + ), + flat_member( + "sde-tester", + "Tester", + "Runs verification and reports failures", + BUILTIN_SDE_AGENT_ID, + ), ], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), } -} - -fn default_ds_template_team_is_current(org: &OrgDefinition) -> bool { - const DEFAULT_MEMBER_IDS: [&str; 3] = ["ds-analyst", "ds-engineer", "ds-visualizer"]; - - org.agent_id == BUILTIN_DS_AGENT_ID - && org.children.len() == DEFAULT_MEMBER_IDS.len() - && DEFAULT_MEMBER_IDS.iter().all(|member_id| { - org.children.iter().any(|member| { - member.id == *member_id - && member.agent_id == BUILTIN_DS_AGENT_ID - && member.children.is_empty() - }) - }) + .with_all_member_links() } fn default_ds_template_team() -> OrgDefinition { @@ -633,77 +947,203 @@ fn default_ds_template_team() -> OrgDefinition { "Built-in Agent Org for SQL analysis, metrics review, data validation, and reporting." .to_string(), ), - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![ - OrgMember { - id: "ds-analyst".to_string(), - name: "Analyst".to_string(), - role: "Explores datasets, defines metrics, and answers analytical questions" - .to_string(), - agent_id: BUILTIN_DS_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, - OrgMember { - id: "ds-engineer".to_string(), - name: "Data Engineer".to_string(), - role: "Checks data quality, joins, schemas, and reproducibility".to_string(), - agent_id: BUILTIN_DS_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, - OrgMember { - id: "ds-visualizer".to_string(), - name: "Visualizer".to_string(), - role: "Turns findings into concise tables, charts, and decision-ready summaries" - .to_string(), - agent_id: BUILTIN_DS_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, + members: vec![ + flat_member( + "ds-analyst", + "Analyst", + "Explores datasets, defines metrics, and answers analytical questions", + BUILTIN_DS_AGENT_ID, + ), + flat_member( + "ds-engineer", + "Data Engineer", + "Checks data quality, joins, schemas, and reproducibility", + BUILTIN_DS_AGENT_ID, + ), + flat_member( + "ds-visualizer", + "Visualizer", + "Turns findings into concise tables, charts, and decision-ready summaries", + BUILTIN_DS_AGENT_ID, + ), ], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), } + .with_all_member_links() } -// ── File I/O ── - -fn load_from_disk(path: &std::path::Path) -> Vec { +fn load_from_disk(path: &Path) -> LoadOutcome { if !path.exists() { - return Vec::new(); - } - match std::fs::read_to_string(path) { - Ok(content) => match serde_json::from_str::>(&content) { - Ok(orgs) => { - info!( - "[agent-orgs] Loaded {} orgs from {}", - orgs.len(), - path.display() + return LoadOutcome::Missing; + } + let bytes = match std::fs::read(path) { + Ok(bytes) => bytes, + Err(err) => { + return LoadOutcome::Blocked(format!("Failed to read {}: {}", path.display(), err)); + } + }; + let value: serde_json::Value = match serde_json::from_slice(&bytes) { + Ok(value) => value, + Err(err) => { + return LoadOutcome::Blocked(format!("Failed to parse {}: {}", path.display(), err)); + } + }; + + if value.is_array() || contains_legacy_hierarchy(&value) { + match backup_legacy_file(path, &bytes) { + Ok(backup_path) => { + warn!( + "[agent-orgs] Backed up legacy recursive definitions to {} before reset", + backup_path.display() ); - orgs + return LoadOutcome::LegacyReset; } - Err(err) => { - error!("[agent-orgs] Failed to parse {}: {}", path.display(), err); - Vec::new() - } - }, + Err(err) => return LoadOutcome::Blocked(err), + } + } + + let mut file: AgentOrgDefinitionsFile = match serde_json::from_value(value) { + Ok(file) => file, Err(err) => { - error!("[agent-orgs] Failed to read {}: {}", path.display(), err); - Vec::new() + return LoadOutcome::Blocked(format!( + "Unrecognized Agent Org definitions file {}: {}", + path.display(), + err + )); } + }; + if file.schema_version != AGENT_ORGS_FILE_SCHEMA_VERSION { + return LoadOutcome::Blocked(format!( + "Unsupported Agent Org definitions schema version {} in {}", + file.schema_version, + path.display() + )); + } + if let Err(err) = canonicalize_and_validate_definitions(&mut file.definitions) { + return LoadOutcome::Blocked(format!( + "Invalid Agent Org definitions in {}: {}", + path.display(), + err + )); } + info!( + "[agent-orgs] Loaded {} definitions from {}", + file.definitions.len(), + path.display() + ); + LoadOutcome::Loaded(file.definitions) } -fn save_to_disk(path: &std::path::Path, orgs: &[OrgDefinition]) -> Result<(), String> { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent) - .map_err(|err| format!("Failed to create directory: {}", err))?; +fn contains_legacy_hierarchy(value: &serde_json::Value) -> bool { + match value { + serde_json::Value::Object(object) => { + object.contains_key("children") + || object.contains_key("hierarchyMode") + || object.values().any(contains_legacy_hierarchy) + } + serde_json::Value::Array(values) => values.iter().any(contains_legacy_hierarchy), + _ => false, } - let content = serde_json::to_string_pretty(orgs) - .map_err(|err| format!("Failed to serialize orgs: {}", err))?; - std::fs::write(path, content).map_err(|err| format!("Failed to write orgs: {}", err))?; +} + +fn backup_legacy_file(path: &Path, bytes: &[u8]) -> Result { + let timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_err(|err| { + format!( + "System clock error while backing up legacy definitions: {}", + err + ) + })? + .as_millis(); + let file_name = path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("agent-orgs.json"); + let parent = path.parent().unwrap_or_else(|| Path::new(".")); + for suffix in 0..1000u16 { + let suffix_text = if suffix == 0 { + String::new() + } else { + format!("-{}", suffix) + }; + let backup_path = parent.join(format!( + "{}.legacy-{}{}.bak", + file_name, timestamp, suffix_text + )); + match OpenOptions::new() + .write(true) + .create_new(true) + .open(&backup_path) + { + Ok(mut file) => { + file.write_all(bytes) + .and_then(|_| file.sync_all()) + .map_err(|err| { + format!( + "Failed to write legacy Agent Org backup {}: {}", + backup_path.display(), + err + ) + })?; + return Ok(backup_path); + } + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(err) => { + return Err(format!( + "Failed to create legacy Agent Org backup {}: {}", + backup_path.display(), + err + )); + } + } + } + Err("Could not allocate a unique legacy Agent Org backup path".to_string()) +} + +fn save_to_disk(path: &Path, orgs: &[OrgDefinition]) -> Result<(), String> { + let file = AgentOrgDefinitionsFile { + schema_version: AGENT_ORGS_FILE_SCHEMA_VERSION, + definitions: orgs.to_vec(), + }; + let content = serde_json::to_vec_pretty(&file) + .map_err(|err| format!("Failed to serialize Agent Org definitions: {}", err))?; + let parent = path.parent().unwrap_or_else(|| Path::new(".")); + std::fs::create_dir_all(parent) + .map_err(|err| format!("Failed to create Agent Org directory: {}", err))?; + let mut temp = tempfile::Builder::new() + .prefix(".agent-orgs-") + .suffix(".tmp") + .tempfile_in(parent) + .map_err(|err| { + format!( + "Failed to create temp file in {}: {}", + parent.display(), + err + ) + })?; + let temp_path = temp.path().to_path_buf(); + let write_result = (|| -> Result<(), String> { + temp.write_all(&content) + .and_then(|_| temp.as_file().sync_all()) + .map_err(|err| format!("Failed to sync {}: {}", temp_path.display(), err))?; + temp.persist(path).map_err(|err| { + format!( + "Failed to atomically replace {}: {}", + path.display(), + err.error + ) + })?; + if let Ok(directory) = File::open(parent) { + let _ = directory.sync_all(); + } + Ok(()) + })(); + write_result?; info!( - "[agent-orgs] Saved {} orgs to {}", + "[agent-orgs] Saved {} definitions to {}", orgs.len(), path.display() ); @@ -714,96 +1154,223 @@ fn save_to_disk(path: &std::path::Path, orgs: &[OrgDefinition]) -> Result<(), St mod tests { use super::*; - fn custom_org() -> OrgDefinition { + fn test_member(member_id: &str) -> FlatOrgMember { + flat_member(member_id, member_id, "member", BUILTIN_SDE_AGENT_ID) + } + + fn custom_org(member_ids: &[&str]) -> OrgDefinition { OrgDefinition { id: "custom-org".to_string(), name: "Custom Org".to_string(), role: "Coordinator".to_string(), - agent_id: "builtin:sde".to_string(), + agent_id: BUILTIN_SDE_AGENT_ID.to_string(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: Vec::new(), + members: member_ids.iter().map(|id| test_member(id)).collect(), + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), } + .with_all_member_links() } #[test] - fn new_persists_default_org_when_file_is_missing() { - let _sandbox = test_helpers::test_env::sandbox(); + fn all_member_links_materializes_every_canonical_pair() { + let members = (0..50) + .map(|index| test_member(&format!("member-{index:02}"))) + .collect::>(); + let links = all_member_links(&members); + assert_eq!(links.len(), 1_225); + assert!(links + .windows(2) + .all(|window| window[0].key() < window[1].key())); + } - let store = AgentOrgsStore::new(); - let sde_org = store - .get(DEFAULT_SDE_TEMPLATE_TEAM_ID) - .expect("default SDE Agent Org should be available"); - let ds_org = store - .get(DEFAULT_DS_TEMPLATE_TEAM_ID) - .expect("default DS Agent Org should be available"); - assert_eq!(sde_org.name, "Default Agent Org"); - assert_eq!(ds_org.name, "Data Science Agent Org"); - - let persisted = load_from_disk(&storage_path()); - assert!(persisted - .iter() - .any(|org| org.id == DEFAULT_SDE_TEMPLATE_TEAM_ID)); - assert!(persisted - .iter() - .any(|org| org.id == DEFAULT_DS_TEMPLATE_TEAM_ID)); + #[test] + fn validator_canonicalizes_links_and_keeps_writer_independent() { + let mut org = custom_org(&["alice", "bob", "carol"]); + org.additional_task_graph_writer_member_ids = vec!["bob".to_string()]; + org.member_communication_links = vec![MemberCommunicationLink { + member_a_id: "bob".to_string(), + member_b_id: "alice".to_string(), + }]; + canonicalize_and_validate_definition(&mut org).expect("valid definition"); + assert_eq!( + org.member_communication_links, + vec![MemberCommunicationLink::canonical("alice", "bob")] + ); + assert_eq!( + org.additional_task_graph_writer_member_ids, + vec!["bob".to_string()] + ); + } + + #[test] + fn validator_rejects_unknown_self_duplicate_and_too_many_members() { + let mut org = custom_org(&["alice", "bob"]); + org.member_communication_links = vec![MemberCommunicationLink::canonical("alice", "alice")]; + assert!(canonicalize_and_validate_definition(&mut org) + .unwrap_err() + .contains("self communication")); + + let mut org = custom_org(&["alice", "bob"]); + org.member_communication_links = vec![ + MemberCommunicationLink::canonical("alice", "bob"), + MemberCommunicationLink::canonical("bob", "alice"), + ]; + assert!(canonicalize_and_validate_definition(&mut org) + .unwrap_err() + .contains("duplicate communication")); + + let ids = (0..=MAX_AGENT_ORG_MEMBERS) + .map(|index| format!("member-{index}")) + .collect::>(); + let refs = ids.iter().map(String::as_str).collect::>(); + let mut org = custom_org(&refs); + assert!(canonicalize_and_validate_definition(&mut org) + .unwrap_err() + .contains("between 1 and 50")); + + let mut org = custom_org(&["alice", "bob"]); + org.additional_task_graph_writer_member_ids = vec!["unknown".to_string()]; + assert!(canonicalize_and_validate_definition(&mut org) + .unwrap_err() + .contains("unknown Writer")); + + let mut org = custom_org(&["alice", "bob"]); + org.member_communication_links = + vec![MemberCommunicationLink::canonical("alice", "unknown")]; + assert!(canonicalize_and_validate_definition(&mut org) + .unwrap_err() + .contains("unknown member")); + } + + #[test] + fn validator_enforces_definition_byte_limit() { + let mut org = custom_org(&["alice"]); + org.description = Some("x".repeat(MAX_AGENT_ORG_DEFINITION_BYTES)); + assert!(canonicalize_and_validate_definition(&mut org) + .unwrap_err() + .contains("definition limit")); + } + + #[test] + fn capability_index_uses_canonical_undirected_pairs() { + let org = custom_org(&["alice", "bob", "carol"]); + let snapshot = AgentOrgLaunchSnapshot::from(&org); + let index = AgentOrgCapabilityIndex::from_snapshot(&snapshot); + assert!(index.members_can_communicate("alice", "bob")); + assert!(index.members_can_communicate("bob", "alice")); + assert!(!index.is_additional_writer("alice")); } #[test] - fn new_backfills_default_org_when_user_file_already_exists() { + fn fifty_member_snapshot_validates_and_compiles_full_capability_index() { + let ids = (0..50) + .map(|index| format!("member-{index:02}")) + .collect::>(); + let refs = ids.iter().map(String::as_str).collect::>(); + let mut org = custom_org(&refs); + org.additional_task_graph_writer_member_ids = vec!["member-49".to_string()]; + let snapshot = AgentOrgLaunchSnapshot::from(&org); + validate_launch_snapshot(&snapshot).expect("valid frozen 50-Member snapshot"); + assert_eq!(snapshot.member_communication_links.len(), 1_225); + + let index = AgentOrgCapabilityIndex::from_snapshot(&snapshot); + assert!(index.members_can_communicate("member-00", "member-49")); + assert!(index.members_can_communicate("member-49", "member-00")); + assert!(index.is_additional_writer("member-49")); + assert!(!index.is_additional_writer("member-00")); + } + + #[test] + fn launch_snapshot_validator_rejects_noncanonical_or_unknown_capabilities() { + let org = custom_org(&["alice", "bob"]); + let mut snapshot = AgentOrgLaunchSnapshot::from(&org); + snapshot.member_communication_links = vec![MemberCommunicationLink { + member_a_id: "bob".to_string(), + member_b_id: "alice".to_string(), + }]; + assert!(validate_launch_snapshot(&snapshot) + .unwrap_err() + .contains("non-canonical")); + + let mut snapshot = AgentOrgLaunchSnapshot::from(&org); + snapshot.additional_task_graph_writer_member_ids = vec!["unknown".to_string()]; + assert!(validate_launch_snapshot(&snapshot) + .unwrap_err() + .contains("unknown Writer")); + + let mut snapshot = AgentOrgLaunchSnapshot::from(&org); + snapshot.member_communication_links.clear(); + snapshot.additional_task_graph_writer_member_ids = vec!["alice".to_string()]; + validate_launch_snapshot(&snapshot).expect("Writer does not imply a communication link"); + let index = AgentOrgCapabilityIndex::from_snapshot(&snapshot); + assert!(index.is_additional_writer("alice")); + assert!(!index.members_can_communicate("alice", "bob")); + } + + #[test] + fn legacy_array_is_backed_up_before_reset() { let _sandbox = test_helpers::test_env::sandbox(); let path = storage_path(); - save_to_disk(&path, &[custom_org()]).expect("seed custom org file"); - + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + let legacy = br#"[{"id":"old","children":[]}]"#; + std::fs::write(&path, legacy).unwrap(); let store = AgentOrgsStore::new(); - assert!(store.get("custom-org").is_ok()); assert!(store.get(DEFAULT_SDE_TEMPLATE_TEAM_ID).is_ok()); - assert!(store.get(DEFAULT_DS_TEMPLATE_TEAM_ID).is_ok()); - - let persisted = load_from_disk(&path); - assert!(persisted.iter().any(|org| org.id == "custom-org")); - assert!(persisted - .iter() - .any(|org| org.id == DEFAULT_SDE_TEMPLATE_TEAM_ID)); - assert!(persisted - .iter() - .any(|org| org.id == DEFAULT_DS_TEMPLATE_TEAM_ID)); + let backups = std::fs::read_dir(path.parent().unwrap()) + .unwrap() + .filter_map(Result::ok) + .filter(|entry| entry.file_name().to_string_lossy().contains("legacy-")) + .collect::>(); + assert_eq!(backups.len(), 1); + assert_eq!(std::fs::read(backups[0].path()).unwrap(), legacy); } #[test] - fn new_repairs_stale_default_org_template() { + fn invalid_canonical_file_fails_closed_without_overwrite() { let _sandbox = test_helpers::test_env::sandbox(); let path = storage_path(); - let mut stale_default = custom_org(); - stale_default.id = DEFAULT_SDE_TEMPLATE_TEAM_ID.to_string(); - stale_default.name = "Stale Empty Default Org".to_string(); - save_to_disk(&path, &[stale_default]).expect("seed stale default org"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + let invalid = br#"{"schemaVersion":2,"definitions":[{"id":"bad"}]}"#; + std::fs::write(&path, invalid).unwrap(); + let store = AgentOrgsStore::new(); + assert!(store.list().is_err()); + assert_eq!(std::fs::read(&path).unwrap(), invalid); + } + #[test] + fn canonical_file_round_trips_stable_ids_grants_and_links() { + let _sandbox = test_helpers::test_env::sandbox(); let store = AgentOrgsStore::new(); - let org = store - .get(DEFAULT_SDE_TEMPLATE_TEAM_ID) - .expect("default org should remain available"); - assert_eq!(org.name, "Default Agent Org"); - assert!(default_sde_template_team_is_current(&org)); + let mut org = custom_org(&["alice", "bob", "carol"]); + org.id = "round-trip-org".to_string(); + org.name = "Round Trip Org".to_string(); + org.additional_task_graph_writer_member_ids = vec!["bob".to_string()]; + org.member_communication_links = vec![MemberCommunicationLink::canonical("alice", "carol")]; + store.insert(org.clone()).expect("persist custom Team"); + + let restarted = AgentOrgsStore::new(); + assert_eq!(restarted.get(&org.id).expect("reloaded Team"), org); } #[test] - fn validation_rejects_cli_member_with_member_id_and_transport() { + fn failed_disk_commit_does_not_swap_candidate_into_memory() { let _sandbox = test_helpers::test_env::sandbox(); - let mut org = custom_org(); - org.children.push(OrgMember { - id: "cli-worker".to_string(), - name: "Legacy CLI Worker".to_string(), - role: "worker".to_string(), - agent_id: "cli:claude_code".to_string(), - runtime_config: None, - children: Vec::new(), - }); - - let error = AgentOrgsStore::validate_agent_references(&org) - .expect_err("CLI Agent Org members must be rejected at save time"); - assert!(error.contains("member_id=cli-worker"), "{error}"); - assert!(error.contains("cli:claude_code"), "{error}"); + let store = AgentOrgsStore::new(); + let before = store.list().expect("initial definitions"); + let path = storage_path(); + std::fs::remove_file(&path).expect("replace canonical file with failure fixture"); + std::fs::create_dir(&path).expect("directory blocks atomic file rename"); + + let mut org = custom_org(&["alice"]); + org.id = "must-not-commit".to_string(); + org.name = "Must Not Commit".to_string(); + assert!(store.insert(org).is_err()); + assert_eq!(store.list().expect("memory remains readable"), before); + assert!( + path.is_dir(), + "failed save must not replace the disk fixture" + ); } } diff --git a/src-tauri/crates/agent-core/src/core/session/launch/launch_helpers.rs b/src-tauri/crates/agent-core/src/core/session/launch/launch_helpers.rs index f2d718684a..783ceda9ad 100644 --- a/src-tauri/crates/agent-core/src/core/session/launch/launch_helpers.rs +++ b/src-tauri/crates/agent-core/src/core/session/launch/launch_helpers.rs @@ -10,7 +10,7 @@ use core_types::key_source::KeySource; use crate::coordination::agent_org_runs::{ AgentOrgRunStatus, AgentOrgRunStore, AgentOrgStartingFailure, }; -use crate::definitions::orgs::{OrgMember, OrgMemberRuntimeConfig}; +use crate::definitions::orgs::{FlatOrgMember, OrgMemberRuntimeConfig}; use crate::session::turn::streaming::{ broadcast_agent_error_structured, classify_streaming_error_message, StreamingError, }; @@ -95,10 +95,10 @@ pub(super) async fn handle_background_launch_failure( } pub(super) fn apply_member_launch_overrides_to_snapshot( - members: &mut [OrgMember], + members: &mut [FlatOrgMember], overrides: &HashMap, ) -> Result<(), String> { - crate::definitions::orgs::apply_overrides_to_member_tree( + crate::definitions::orgs::apply_overrides_to_members( members, overrides, "Agent Org launch override", @@ -109,9 +109,6 @@ pub(super) fn validate_launch_agent_definitions( agent_definition_id: Option<&str>, org_definition: Option<&crate::definitions::orgs::OrgDefinition>, ) -> Result<(), String> { - use std::collections::HashSet; - - use crate::coordination::agent_org_runs::COORDINATOR_MEMBER_ID; use crate::definitions::orgs::is_cli_agent_org_reference; let store = crate::definitions::definitions_store(); @@ -126,11 +123,11 @@ pub(super) fn validate_launch_agent_definitions( } if let Some(org) = org_definition { + crate::definitions::orgs::validate_launch_snapshot( + &crate::definitions::orgs::AgentOrgLaunchSnapshot::from(org), + )?; let mut missing: Vec = Vec::new(); let mut unsupported_cli: Vec = Vec::new(); - let mut member_ids = HashSet::new(); - let mut invalid_member_ids: Vec = Vec::new(); - let mut duplicate_member_ids: Vec = Vec::new(); if !org.agent_id.trim().is_empty() { if is_cli_agent_org_reference(&org.agent_id) { unsupported_cli.push(format!("coordinator '{}'", org.agent_id)); @@ -138,21 +135,12 @@ pub(super) fn validate_launch_agent_definitions( missing.push(format!("coordinator '{}'", org.agent_id)); } } - for member in flatten_org_members(&org.children) { - let member_id = member.id.trim(); - if member_id.is_empty() { - invalid_member_ids.push(format!("member '{}' has empty id", member.name)); - } else if member_id == COORDINATOR_MEMBER_ID { - invalid_member_ids.push(format!( - "member '{}' uses reserved id '{}'", - member.name, COORDINATOR_MEMBER_ID - )); - } else if !member_ids.insert(member_id.to_string()) { - duplicate_member_ids.push(member_id.to_string()); - } - + for member in &org.members { if is_cli_agent_org_reference(&member.agent_id) { - unsupported_cli.push(format!("member '{}' ({})", member.id, member.agent_id)); + unsupported_cli.push(format!( + "member '{}' ({})", + member.member_id, member.agent_id + )); } else if store.get(&member.agent_id).is_none() { missing.push(format!("member '{}' ({})", member.name, member.agent_id)); } @@ -163,23 +151,6 @@ pub(super) fn validate_launch_agent_definitions( unsupported_cli.join(", ") )); } - duplicate_member_ids.sort(); - duplicate_member_ids.dedup(); - if !invalid_member_ids.is_empty() || !duplicate_member_ids.is_empty() { - let mut reasons = Vec::new(); - reasons.extend(invalid_member_ids); - if !duplicate_member_ids.is_empty() { - reasons.push(format!( - "duplicate member_id value(s): {}", - duplicate_member_ids.join(", ") - )); - } - return Err(format!( - "Agent Org '{}' has invalid member_id configuration: {}", - org.name, - reasons.join(", ") - )); - } if !missing.is_empty() { return Err(format!( "Agent Org '{}' references missing Agent definition(s): {}", @@ -319,15 +290,6 @@ pub(super) fn derive_name(explicit: Option<&str>, content: &str) -> String { } } -pub(super) fn flatten_org_members(members: &[OrgMember]) -> Vec { - let mut flattened = Vec::new(); - for member in members { - flattened.push(member.clone()); - flattened.extend(flatten_org_members(&member.children)); - } - flattened -} - #[cfg(test)] mod provenance_tests { use super::provenance_fields; diff --git a/src-tauri/crates/agent-core/src/core/session/launch/launch_org.rs b/src-tauri/crates/agent-core/src/core/session/launch/launch_org.rs index d6ddf71b58..7fb24306ad 100644 --- a/src-tauri/crates/agent-core/src/core/session/launch/launch_org.rs +++ b/src-tauri/crates/agent-core/src/core/session/launch/launch_org.rs @@ -11,7 +11,9 @@ use core_types::key_source::KeySource; use crate::coordination::agent_org_runs::{ AgentOrgMaterializationStatus, AgentOrgRunStore, AgentOrgStartingFailure, COORDINATOR_MEMBER_ID, }; -use crate::definitions::orgs::{is_cli_agent_org_reference, OrgDefinition, OrgMember}; +use crate::definitions::orgs::{ + is_cli_agent_org_reference, validate_launch_snapshot, AgentOrgLaunchSnapshot, FlatOrgMember, +}; use crate::session::persistence::{ self as session_persistence, session_type, UnifiedSessionRecord, }; @@ -19,8 +21,8 @@ use crate::session::IdeContext; use crate::state::AgentAppState; use super::launch_helpers::{ - flatten_org_members, member_runtime_account_id, member_runtime_key_source, - member_runtime_model, member_runtime_native_harness_type, + member_runtime_account_id, member_runtime_key_source, member_runtime_model, + member_runtime_native_harness_type, }; #[derive(Debug)] @@ -65,7 +67,7 @@ impl std::fmt::Display for AgentOrgMaterializationError { #[allow(clippy::too_many_arguments)] pub(super) async fn materialize_org_member_sessions( org_run_id: &str, - org: &OrgDefinition, + org: &AgentOrgLaunchSnapshot, root_session_id: &str, _root_session_name: &str, workspace_path: &str, @@ -77,9 +79,12 @@ pub(super) async fn materialize_org_member_sessions( work_item_id: Option, project_slug: Option, ) -> Result, AgentOrgMaterializationError> { + validate_launch_snapshot(org).map_err(|error| { + AgentOrgMaterializationError::permanent("invalid_launch_snapshot", error) + })?; let workspace_path = workspace_path.to_string(); let root_session_id = root_session_id.to_string(); - let org_name = org.name.clone(); + let org_name = org.org_name.clone(); let model = model.filter(|value| !value.trim().is_empty()); let key_source = match key_source .as_deref() @@ -107,10 +112,12 @@ pub(super) async fn materialize_org_member_sessions( }) .transpose()?; let agent_exec_mode = agent_exec_mode.filter(|mode| !mode.trim().is_empty()); - let members = flatten_org_members(&org.children) - .into_iter() - .map(|member| (member.id.clone(), member)) - .collect::>(); + let members = org + .members + .iter() + .cloned() + .map(|member| (member.member_id.clone(), member)) + .collect::>(); let receipts = AgentOrgRunStore::materializations(org_run_id) .map_err(AgentOrgMaterializationError::retryable)?; let mut materialized_session_ids = Vec::new(); @@ -145,7 +152,7 @@ pub(super) async fn materialize_org_member_sessions( "unsupported_member_runtime", format!( "CLI Agent Org member {} cannot be materialized on the canonical lifecycle path", - member.id + member.member_id ), )); } @@ -179,7 +186,7 @@ pub(super) async fn materialize_org_member_sessions( { let identity_matches = existing.agent_definition_id.as_deref() == Some(member.agent_id.as_str()) - && existing.org_member_id.as_deref() == Some(member.id.as_str()) + && existing.org_member_id.as_deref() == Some(member.member_id.as_str()) && existing.parent_session_id.as_deref() == Some(root_session_id.as_str()); if !identity_matches { return Err(AgentOrgMaterializationError::permanent( @@ -208,7 +215,7 @@ pub(super) async fn materialize_org_member_sessions( agent_role: Some(member.role), project_slug, agent_definition_id: Some(member.agent_id), - org_member_id: Some(member.id), + org_member_id: Some(member.member_id), parent_session_id: Some(root_session_id), key_source: member_key_source, agent_exec_mode, diff --git a/src-tauri/crates/agent-core/src/core/session/launch/launch_tests.rs b/src-tauri/crates/agent-core/src/core/session/launch/launch_tests.rs index fa5dd34184..7f4b868efa 100644 --- a/src-tauri/crates/agent-core/src/core/session/launch/launch_tests.rs +++ b/src-tauri/crates/agent-core/src/core/session/launch/launch_tests.rs @@ -11,7 +11,7 @@ use crate::core::session::persistence::{self, UnifiedSessionRecord}; use crate::core::session::SessionStatus; use crate::definitions::builtin::SDE_AGENT_ID; use crate::definitions::orgs::{ - HierarchyMode, OrgDefinition, OrgMember, OrgMemberLaunchOverride, OrgMemberRuntimeConfig, + FlatOrgMember, OrgDefinition, OrgMemberLaunchOverride, OrgMemberRuntimeConfig, PlanApprovalPolicy, }; use core_types::key_source::KeySource; @@ -48,16 +48,17 @@ fn launch_validation_rejects_missing_agent_definition_before_session_create() { assert!(error.contains("does not exist"), "{error}"); } -fn valid_org_with_children(children: Vec) -> OrgDefinition { +fn valid_org_with_members(members: Vec) -> OrgDefinition { OrgDefinition { id: "test:member-id-org".to_string(), name: "Member Id Org".to_string(), role: "Coordinator".to_string(), agent_id: SDE_AGENT_ID.to_string(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children, + members, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), } } @@ -86,12 +87,18 @@ async fn late_launch_failure_does_not_fail_a_running_team_coordinator() { ..Default::default() }) .expect("persist coordinator Session"); - let org = valid_org_with_children(Vec::new()); + let org = valid_org_with_members(vec![FlatOrgMember { + member_id: "worker".to_string(), + name: "Worker".to_string(), + role: "Builder".to_string(), + agent_id: SDE_AGENT_ID.to_string(), + runtime_config: None, + }]); let run = AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id: org.id.clone(), coordinator_agent_id: org.agent_id.clone(), root_session_id: Some(session_id.to_string()), - org_snapshot: org, + org_snapshot: crate::definitions::orgs::AgentOrgLaunchSnapshot::from(&org), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -129,22 +136,23 @@ async fn late_launch_failure_does_not_fail_a_running_team_coordinator() { } #[test] -fn launch_overrides_apply_recursively_to_effective_org_snapshot() { - let mut org = valid_org_with_children(vec![OrgMember { - id: "lead".to_string(), - name: "Lead".to_string(), - role: "Lead".to_string(), - agent_id: SDE_AGENT_ID.to_string(), - runtime_config: None, - children: vec![OrgMember { - id: "child".to_string(), +fn launch_overrides_apply_to_flat_effective_org_snapshot() { + let mut org = valid_org_with_members(vec![ + FlatOrgMember { + member_id: "lead".to_string(), + name: "Lead".to_string(), + role: "Lead".to_string(), + agent_id: SDE_AGENT_ID.to_string(), + runtime_config: None, + }, + FlatOrgMember { + member_id: "child".to_string(), name: "Child".to_string(), role: "Worker".to_string(), agent_id: SDE_AGENT_ID.to_string(), runtime_config: None, - children: Vec::new(), - }], - }]); + }, + ]); let mut overrides = HashMap::new(); overrides.insert( "child".to_string(), @@ -159,10 +167,10 @@ fn launch_overrides_apply_recursively_to_effective_org_snapshot() { }, ); - apply_member_launch_overrides_to_snapshot(&mut org.children, &overrides) + apply_member_launch_overrides_to_snapshot(&mut org.members, &overrides) .expect("override should apply"); - let child = &org.children[0].children[0]; + let child = &org.members[1]; assert_eq!(child.agent_id, "cli:claude_code"); let runtime_config = child.runtime_config.as_ref().expect("runtime config"); assert_eq!(runtime_config.account_id.as_deref(), Some("account-child")); @@ -171,13 +179,12 @@ fn launch_overrides_apply_recursively_to_effective_org_snapshot() { #[test] fn launch_overrides_reject_unknown_member_ids() { - let mut org = valid_org_with_children(vec![OrgMember { - id: "lead".to_string(), + let mut org = valid_org_with_members(vec![FlatOrgMember { + member_id: "lead".to_string(), name: "Lead".to_string(), role: "Lead".to_string(), agent_id: SDE_AGENT_ID.to_string(), runtime_config: None, - children: Vec::new(), }]); let mut overrides = HashMap::new(); overrides.insert( @@ -188,7 +195,7 @@ fn launch_overrides_reject_unknown_member_ids() { }, ); - let error = apply_member_launch_overrides_to_snapshot(&mut org.children, &overrides) + let error = apply_member_launch_overrides_to_snapshot(&mut org.members, &overrides) .expect_err("unknown member override must fail"); assert!(error.contains("missing"), "{error}"); @@ -238,16 +245,16 @@ fn launch_validation_rejects_agent_org_with_missing_member_definition() { role: "Coordinator".to_string(), agent_id: SDE_AGENT_ID.to_string(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![OrgMember { - id: "worker".to_string(), + members: vec![FlatOrgMember { + member_id: "worker".to_string(), name: "Worker".to_string(), role: "Builder".to_string(), agent_id: "custom:deleted-worker".to_string(), runtime_config: None, - children: Vec::new(), }], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), }; let error = validate_launch_agent_definitions(Some(SDE_AGENT_ID), Some(&org)) @@ -260,13 +267,12 @@ fn launch_validation_rejects_agent_org_with_missing_member_definition() { #[test] fn launch_validation_rejects_cli_member_before_run_materialization() { let _sandbox = test_helpers::test_env::sandbox(); - let org = valid_org_with_children(vec![OrgMember { - id: "cli-worker".to_string(), + let org = valid_org_with_members(vec![FlatOrgMember { + member_id: "cli-worker".to_string(), name: "CLI Worker".to_string(), role: "Builder".to_string(), agent_id: "cli:claude_code".to_string(), runtime_config: None, - children: Vec::new(), }]); let error = validate_launch_agent_definitions(Some(SDE_AGENT_ID), Some(&org)) @@ -279,57 +285,46 @@ fn launch_validation_rejects_cli_member_before_run_materialization() { #[test] fn launch_validation_rejects_duplicate_member_ids() { let _sandbox = test_helpers::test_env::sandbox(); - let org = valid_org_with_children(vec![ - OrgMember { - id: "worker".to_string(), + let org = valid_org_with_members(vec![ + FlatOrgMember { + member_id: "worker".to_string(), name: "Worker A".to_string(), role: "Builder".to_string(), agent_id: SDE_AGENT_ID.to_string(), runtime_config: None, - children: Vec::new(), }, - OrgMember { - id: "worker".to_string(), + FlatOrgMember { + member_id: "worker".to_string(), name: "Worker B".to_string(), role: "Reviewer".to_string(), agent_id: SDE_AGENT_ID.to_string(), runtime_config: None, - children: Vec::new(), }, ]); let error = validate_launch_agent_definitions(Some(SDE_AGENT_ID), Some(&org)) .expect_err("duplicate member_id must fail before session creation"); - assert!(error.contains("duplicate member_id"), "{error}"); + assert!(error.contains("duplicate member id"), "{error}"); assert!(error.contains("worker"), "{error}"); } #[test] fn launch_validation_rejects_reserved_and_empty_member_ids() { let _sandbox = test_helpers::test_env::sandbox(); - let org = valid_org_with_children(vec![ - OrgMember { - id: COORDINATOR_MEMBER_ID.to_string(), - name: "Reserved".to_string(), + for (member_id, expected) in [ + (COORDINATOR_MEMBER_ID, "reserved member id"), + ("", "empty or reserved member id"), + ] { + let org = valid_org_with_members(vec![FlatOrgMember { + member_id: member_id.to_string(), + name: "Invalid".to_string(), role: "Builder".to_string(), agent_id: SDE_AGENT_ID.to_string(), runtime_config: None, - children: Vec::new(), - }, - OrgMember { - id: " ".to_string(), - name: "Blank".to_string(), - role: "Reviewer".to_string(), - agent_id: SDE_AGENT_ID.to_string(), - runtime_config: None, - children: Vec::new(), - }, - ]); - - let error = validate_launch_agent_definitions(Some(SDE_AGENT_ID), Some(&org)) - .expect_err("invalid member_id values must fail before session creation"); - - assert!(error.contains("reserved id"), "{error}"); - assert!(error.contains("empty id"), "{error}"); + }]); + let error = validate_launch_agent_definitions(Some(SDE_AGENT_ID), Some(&org)) + .expect_err("invalid member_id values must fail before session creation"); + assert!(error.contains(expected), "{error}"); + } } diff --git a/src-tauri/crates/agent-core/src/core/session/launch/mod.rs b/src-tauri/crates/agent-core/src/core/session/launch/mod.rs index f26003102f..04dcca6c43 100644 --- a/src-tauri/crates/agent-core/src/core/session/launch/mod.rs +++ b/src-tauri/crates/agent-core/src/core/session/launch/mod.rs @@ -28,9 +28,8 @@ use crate::state::AgentAppState; use project_management::projects::types as project_types; use launch_helpers::{ - apply_member_launch_overrides_to_snapshot, derive_name, flatten_org_members, - handle_background_launch_failure, provenance_fields, provenance_lock_reason, - validate_launch_agent_definitions, + apply_member_launch_overrides_to_snapshot, derive_name, handle_background_launch_failure, + provenance_fields, provenance_lock_reason, validate_launch_agent_definitions, }; use launch_org::{ cleanup_session_after_org_run_create_failure, materialize_org_member_sessions, @@ -447,7 +446,7 @@ pub async fn launch_agent_session( pub(crate) async fn launch_rust_agent_run( state: &AgentAppState, - org_store: Option<&AgentOrgsStore>, + org_store: Option<&std::sync::Arc>, request: AgentRunLaunchRequest, ) -> Result { if matches!(&request.target, AgentRunTarget::AgentOrg { .. }) { @@ -536,17 +535,27 @@ pub(crate) async fn launch_rust_agent_run( .as_ref() .map(|org| { let mut effective_org = org.clone(); - apply_member_launch_overrides_to_snapshot( - &mut effective_org.children, - &member_overrides, - ) - .map(|()| effective_org) + apply_member_launch_overrides_to_snapshot(&mut effective_org.members, &member_overrides) + .map(|()| effective_org) }) .transpose()?; validate_launch_agent_definitions( agent_definition_id.as_deref(), effective_org_definition.as_ref(), )?; + // A requested template mutation is durable before the immutable launch + // snapshot or any Team lifecycle row is created. The effective snapshot + // above contains the same overrides, while a disk failure leaves no run + // that could appear to have accepted an unpersisted future policy. + if apply_member_overrides_for_future { + if let (Some(store), Some(org_id)) = (org_store, agent_org_id.as_ref()) { + // Store commits fsync under the store mutex; route through the + // store's spawn_blocking wrapper instead of the async executor. + store + .apply_member_launch_overrides_async(org_id.clone(), member_overrides.clone()) + .await?; + } + } let (project_slug, work_item_id, agent_role, routine_fire_id) = provenance_fields(&request.provenance); @@ -624,14 +633,14 @@ pub(crate) async fn launch_rust_agent_run( session_id: root_session_id.clone(), succeeded: true, }]; - for member in flatten_org_members(&org_snapshot.children) { + for member in &org_snapshot.members { let prefix = crate::definitions::prefix_lookup::session_prefix_for_launch( Some(&member.agent_id), !workspace_path.is_empty(), ); materialization_intents.push(CreateAgentOrgMaterializationIntent { - member_id: member.id, - agent_id: member.agent_id, + member_id: member.member_id.clone(), + agent_id: member.agent_id.clone(), session_id: format!("{prefix}{}", uuid::Uuid::new_v4()), succeeded: false, }); @@ -640,7 +649,7 @@ pub(crate) async fn launch_rust_agent_run( org_id: org_id.clone(), coordinator_agent_id: coordinator_id.clone(), root_session_id: root_session_id.clone(), - org_snapshot: org_snapshot.clone(), + org_snapshot: crate::definitions::orgs::AgentOrgLaunchSnapshot::from(org_snapshot), entry_mode: AgentOrgRunEntryMode::StandaloneSession, work_item_id: work_item_id.clone(), project_slug: project_slug.clone(), @@ -739,12 +748,6 @@ pub(crate) async fn launch_rust_agent_run( } let agent_org_run_id = starting_run.as_ref().map(|run| run.id.clone()); - if starting_run.is_some() && apply_member_overrides_for_future { - if let (Some(store), Some(org_id)) = (org_store, agent_org_id.as_ref()) { - store.apply_member_launch_overrides(org_id, &member_overrides)?; - } - } - let created_at = chrono::Utc::now().to_rfc3339(); let native_harness_type_for_send = request .resources @@ -778,9 +781,11 @@ pub(crate) async fn launch_rust_agent_run( let app_handle_for_background = state.app_handle.clone(); let request_key_source_for_background = request.resources.key_source.clone(); let request_native_harness_for_background = request.resources.native_harness_type.clone(); - let org_for_background = effective_org_definition - .clone() - .expect("Agent Org launch has a validated snapshot"); + let org_for_background = crate::definitions::orgs::AgentOrgLaunchSnapshot::from( + effective_org_definition + .as_ref() + .expect("Agent Org launch has a validated snapshot"), + ); let starting_generation = starting_run .as_ref() .map(|run| run.activation_generation) @@ -1293,7 +1298,7 @@ async fn recover_agent_org_starting_runs(state: &AgentAppState) -> Result<(), St )?; continue; }; - let snapshot: crate::definitions::orgs::OrgDefinition = + let snapshot: crate::definitions::orgs::AgentOrgLaunchSnapshot = match serde_json::from_str(snapshot_raw) { Ok(snapshot) => snapshot, Err(error) => { diff --git a/src-tauri/crates/agent-core/src/core/session/persistence/sidebar.rs b/src-tauri/crates/agent-core/src/core/session/persistence/sidebar.rs index 20c5d3422c..996c629dcd 100644 --- a/src-tauri/crates/agent-core/src/core/session/persistence/sidebar.rs +++ b/src-tauri/crates/agent-core/src/core/session/persistence/sidebar.rs @@ -18,8 +18,9 @@ pub type NativeSessionPageCursor<'a> = (&'a str, &'a str); /// Return one bounded sidebar page of unpinned coding sessions that are not /// roots of any persisted Agent Org run. /// -/// Pin state and root membership are applied before LIMIT, so pinned rows, -/// Agent Org roots, and worker rows cannot consume standalone page capacity. +/// Pin state and Agent Org membership are applied before LIMIT, so pinned +/// rows, current roots, and orphaned Coordinator/member rows cannot consume +/// standalone page capacity. pub fn list_standalone_coding_sessions_page( limit: usize, cursor: Option>, @@ -62,7 +63,8 @@ fn list_agent_sessions_page( )" } Some(false) => { - "AND NOT EXISTS ( + "AND s.org_member_id IS NULL + AND NOT EXISTS ( SELECT 1 FROM agent_org_runs r WHERE r.root_session_id = s.session_id @@ -163,6 +165,26 @@ mod tests { type_name: &str, parent_session_id: Option<&str>, pinned: bool, + ) { + upsert_sidebar_session_with_membership( + session_id, + updated_at, + status, + type_name, + parent_session_id, + pinned, + None, + ); + } + + fn upsert_sidebar_session_with_membership( + session_id: &str, + updated_at: &str, + status: &str, + type_name: &str, + parent_session_id: Option<&str>, + pinned: bool, + org_member_id: Option<&str>, ) { ensure_runtime_schemas(); super::super::upsert_session(&UnifiedSessionRecord { @@ -171,6 +193,7 @@ mod tests { status: status.to_string(), session_type: type_name.to_string(), parent_session_id: parent_session_id.map(str::to_string), + org_member_id: org_member_id.map(str::to_string), created_at: updated_at.to_string(), updated_at: updated_at.to_string(), pinned, @@ -238,6 +261,15 @@ mod tests { session_type::CODING, None, ); + upsert_sidebar_session_with_membership( + "orphan-coordinator", + "2026-07-29T17:00:00Z", + "idle", + session_type::CODING, + None, + false, + Some("coordinator"), + ); upsert_sidebar_session( "legacy-coding-worker", "2026-07-29T16:00:00Z", @@ -275,6 +307,10 @@ mod tests { .collect::>(), vec!["standalone-a"] ); + assert!(first + .iter() + .chain(second.iter()) + .all(|session| session.session_id != "orphan-coordinator")); let roots = list_agent_org_root_sessions_page(10, None).expect("Agent Org root page"); assert_eq!( @@ -365,6 +401,7 @@ mod tests { AND s.session_type = 'sde' AND s.status != 'archived' AND s.parent_session_id IS NULL + AND s.org_member_id IS NULL AND NOT EXISTS ( SELECT 1 FROM agent_org_runs r diff --git a/src-tauri/crates/agent-core/src/core/session/prompt/section_builders.rs b/src-tauri/crates/agent-core/src/core/session/prompt/section_builders.rs index d01ac3ea4c..a7db48b14c 100644 --- a/src-tauri/crates/agent-core/src/core/session/prompt/section_builders.rs +++ b/src-tauri/crates/agent-core/src/core/session/prompt/section_builders.rs @@ -655,7 +655,7 @@ pub(crate) fn build_agent_org_context_section_with_task_snapshot( current_member_id: Option<&str>, task_snapshot: Result, String>, ) -> String { - use crate::definitions::orgs::{HierarchyMode, PlanApprovalPolicy}; + use crate::definitions::orgs::PlanApprovalPolicy; let identity_line = match current_member_id { Some(member_id) if context.participant_by_member_id(member_id).is_some() => format!( "- **Your identity in this org:** member_id `{member_id}`." @@ -670,17 +670,9 @@ pub(crate) fn build_agent_org_context_section_with_task_snapshot( "- **Your task authority:** coordinator — you may create, assign, reassign, edit, and repair tasks for every participant, and approve cross-workflow parallel overrides. You may NOT impersonate another member's work: only the current owner may set its task `in_progress`/`completed` or write its `output`. Assignment and dependency unblocking already wake the owner; do not start or complete the task on that member's behalf.".to_string() } Some(member_id) if context.participant_by_member_id(member_id).is_some() => { - let direct_reports = context.direct_report_member_ids_for(member_id); - if direct_reports.is_empty() { - format!( - "- **Your task authority:** worker — you may create and modify only tasks for `{member_id}`. You may talk to peers when routing allows, but you may not assign or rewrite their work. Only you may record `in_progress`, `completed`, and `output` for tasks you own." - ) - } else { - format!( - "- **Your task authority:** manager — you may administer your own tasks and direct-report tasks only: `{}`. Peer and cross-branch work must go through the coordinator. For every task, only its current owner may record `in_progress`, `completed`, or `output`; do not impersonate a direct report's work.", - direct_reports.join("`, `") - ) - } + format!( + "- **Your task authority:** worker — you may create and modify only tasks for `{member_id}`. Configured Writer grants are not active in this phase. You may not assign or rewrite peer work. Only you may record `in_progress`, `completed`, and `output` for tasks you own." + ) } _ => "- **Your task authority:** none — non-roster sessions cannot mutate the Agent Org task board.".to_string(), }; @@ -692,16 +684,7 @@ pub(crate) fn build_agent_org_context_section_with_task_snapshot( format!("- **Org:** {} (`{}`)", context.org_name, context.org_id), format!("- **Org role:** {}", context.org_role), "- **Coordinator member_id:** `coordinator`".to_string(), - format!( - "- **Hierarchy mode:** {}", - match context.hierarchy_mode { - HierarchyMode::Flat => "flat", - HierarchyMode::Soft => { - "soft (peer messaging is open; task authority follows the hierarchy)" - } - HierarchyMode::Strict => "strict (routing restricted — see rules below)", - } - ), + "- **Team structure:** flat Coordinator + peer Members".to_string(), ]; if context.members.is_empty() { @@ -709,21 +692,7 @@ pub(crate) fn build_agent_org_context_section_with_task_snapshot( } else { lines.push("- **Member IDs:**".to_string()); for member in &context.members { - match context.hierarchy_mode { - HierarchyMode::Flat => { - lines.push(format!(" - `{}`", member.member_id)); - } - HierarchyMode::Soft | HierarchyMode::Strict => { - let parent_member_id = member - .parent_member_id - .as_deref() - .unwrap_or(COORDINATOR_MEMBER_ID); - lines.push(format!( - " - `{}` / reports_to `{}`", - member.member_id, parent_member_id - )); - } - } + lines.push(format!(" - `{}`", member.member_id)); } } @@ -796,34 +765,11 @@ pub(crate) fn build_agent_org_context_section_with_task_snapshot( "Use the `org_send_message` tool to send a typed org message to exactly one coordinator/member participant in this org. The only routing field is `recipient_member_id`; never route by display name or agent id. Messages are persisted and surfaced to the recipient on its next turn — they do not interrupt the recipient's current turn. Every plain message to a non-coordinator worker must include `related_task_id` for unresolved, dependency-ready work already owned by that worker. Eligibility alone is not assignment; the coordinator must set `owner_member_id` before sending formal work instructions. Chat cannot create invisible work or bypass dependencies.".to_string(), ); - // Routing rules vary by hierarchy mode. The text below is what tells - // the LLM how to actually behave; the structural roster above is - // identical across modes (modulo the reports-to suffix). lines.push(String::new()); - match context.hierarchy_mode { - HierarchyMode::Flat => { - lines.push( - "**Routing (flat):** there is no reporting hierarchy. Any member may message any other member, the coordinator, or itself directly. Treat all members as peers and pick the most relevant recipient for each message." - .to_string(), - ); - } - HierarchyMode::Soft => { - lines.push( - "**Routing (soft hierarchy):** the reports-to relationships listed above are *organizational hints*, not enforced rules. Prefer to coordinate through your manager for cross-team or multi-step work, but you may message any peer directly for quick factual questions, peer-level technical debate, or when escalating through the chain would obviously waste time. The runtime does not block any send." - .to_string(), - ); - } - HierarchyMode::Strict => { - lines.push( - "**Routing (strict hierarchy):** the runtime enforces who you can message. From any non-coordinator member you may only `org_send_message` to:\n\ - 1. your manager (the member listed under \"reports to\" for you), or\n\ - 2. your direct reports (members whose \"reports to\" is you), or\n\ - 3. the coordinator (always reachable as escape hatch — use this when stuck or when the right recipient is a sibling).\n\ - Sibling-to-sibling sends are rejected with a structured error suggesting escalation. The coordinator may message any member directly. If you receive a sibling's request through the coordinator, treat it the same as a coordinator-issued request." - .to_string(), - ); - } - } + lines.push( + "**Routing:** the Coordinator and every Member are always mutually reachable. Member-to-Member communication links are frozen in the launch snapshot, but peer delivery remains disabled until the peer-send phase." + .to_string(), + ); lines.push(String::new()); lines.push( "**Messaging is not delegation.** Do not use a `plain` message to bypass task authority by telling a peer or another branch to start formal work. Use messages for questions, discussion, handoff context, and proposals. Formal work must already exist as an authority-checked task; if an unauthorized peer asks you to start new work, route the proposal to the coordinator instead of silently creating or executing a second task chain." diff --git a/src-tauri/crates/agent-core/src/core/session/prompt/section_tests.rs b/src-tauri/crates/agent-core/src/core/session/prompt/section_tests.rs index d9bda20145..394ca1f871 100644 --- a/src-tauri/crates/agent-core/src/core/session/prompt/section_tests.rs +++ b/src-tauri/crates/agent-core/src/core/session/prompt/section_tests.rs @@ -7,7 +7,7 @@ use crate::coordination::agent_org_runs::{ AgentOrgRunStore, CreateAgentOrgRunParams, COORDINATOR_MEMBER_ID, }; use crate::coordination::agent_org_tasks::{AgentOrgTaskStore, CreateTaskParams, TaskStatus}; -use crate::definitions::orgs::{HierarchyMode, OrgDefinition, OrgMember, PlanApprovalPolicy}; +use crate::definitions::orgs::{FlatOrgMember, OrgDefinition, PlanApprovalPolicy}; use serial_test::serial; use test_helpers::test_env; @@ -53,10 +53,9 @@ fn prompt_test_agent_org_context() -> AgentOrgRunContext { name: "Worker".to_string(), role: "implementer".to_string(), agent_id: "agent-worker".to_string(), - parent_member_id: None, }], - hierarchy_mode: HierarchyMode::Flat, plan_approval_policy: PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-prompt-test".to_string()), } } @@ -66,27 +65,28 @@ fn materialize_prompt_test_run(context: &AgentOrgRunContext) -> String { org_id: context.org_id.clone(), coordinator_agent_id: context.coordinator_agent_id.clone(), root_session_id: context.root_session_id.clone(), - org_snapshot: OrgDefinition { + org_snapshot: (&OrgDefinition { id: context.org_id.clone(), name: context.org_name.clone(), role: context.org_role.clone(), agent_id: context.coordinator_agent_id.clone(), description: None, - hierarchy_mode: context.hierarchy_mode, plan_approval_policy: context.plan_approval_policy, - children: context + members: context .members .iter() - .map(|member| OrgMember { - id: member.member_id.clone(), + .map(|member| FlatOrgMember { + member_id: member.member_id.clone(), name: member.name.clone(), role: member.role.clone(), agent_id: member.agent_id.clone(), runtime_config: None, - children: Vec::new(), }) .collect(), - }, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), + }) + .into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -203,19 +203,19 @@ fn agent_org_prompt_uses_task_board_for_roster_delegation() { } #[test] -fn agent_org_prompt_worker_cannot_confuse_soft_chat_with_peer_delegation() { - let mut context = prompt_test_agent_org_context(); - context.hierarchy_mode = HierarchyMode::Soft; +fn agent_org_prompt_worker_cannot_confuse_peer_chat_with_delegation() { + let context = prompt_test_agent_org_context(); let section = build_agent_org_context_section(&context, "agent-worker", Some("member-worker")); assert!( section.contains("Your task authority:** worker") - && section.contains("may not assign or rewrite their work") + && section.contains("may create and modify only tasks for `member-worker`") + && section.contains("Configured Writer grants are not active in this phase") && section.contains("Only you may record `in_progress`, `completed`, and `output`"), "worker prompt must explain self-only task authority: {section}" ); assert!( - section.contains("you may message any peer directly"), - "Soft routing should still permit peer discussion: {section}" + section.contains("peer delivery remains disabled until the peer-send phase"), + "prompt must not activate configured peer links before the peer-send phase: {section}" ); } diff --git a/src-tauri/crates/agent-core/src/core/session/turn/event_handler/mod.rs b/src-tauri/crates/agent-core/src/core/session/turn/event_handler/mod.rs index b7ea16a225..69bef586b5 100644 --- a/src-tauri/crates/agent-core/src/core/session/turn/event_handler/mod.rs +++ b/src-tauri/crates/agent-core/src/core/session/turn/event_handler/mod.rs @@ -1087,23 +1087,24 @@ mod tests { org_id: "org-stop-gate".to_string(), coordinator_agent_id: "coordinator".to_string(), root_session_id: None, - org_snapshot: crate::definitions::orgs::OrgDefinition { + org_snapshot: (&crate::definitions::orgs::OrgDefinition { id: "org-stop-gate".to_string(), name: "Stop Gate Test Org".to_string(), role: "coordinator".to_string(), agent_id: "coordinator".to_string(), description: None, - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, - children: vec![crate::definitions::orgs::OrgMember { - id: "member-worker".to_string(), + members: vec![crate::definitions::orgs::FlatOrgMember { + member_id: "member-worker".to_string(), name: "Worker".to_string(), role: "builder".to_string(), agent_id: "worker-agent".to_string(), runtime_config: None, - children: Vec::new(), }], - }, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), + }) + .into(), entry_mode: crate::coordination::agent_org_runs::AgentOrgRunEntryMode::StandaloneSession, status: crate::coordination::agent_org_runs::AgentOrgRunStatus::Running, diff --git a/src-tauri/crates/agent-core/src/core/session/turn/processor/inbox_drain/tests.rs b/src-tauri/crates/agent-core/src/core/session/turn/processor/inbox_drain/tests.rs index 1a9cef2b08..d9cba7b3c6 100644 --- a/src-tauri/crates/agent-core/src/core/session/turn/processor/inbox_drain/tests.rs +++ b/src-tauri/crates/agent-core/src/core/session/turn/processor/inbox_drain/tests.rs @@ -60,8 +60,8 @@ fn ctx_for(run_id: &str) -> AgentOrgRunContext { coordinator_name: "Org 1".into(), coordinator_role: "team".into(), members: vec![], - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-1".into()), } } @@ -78,7 +78,6 @@ fn ctx_for_with_member( agent_id: member_agent_id.into(), name: member_name.into(), role: "engineer".into(), - parent_member_id: None, }); ctx } @@ -90,7 +89,7 @@ fn running_ctx_for_members(members: &[(&str, &str, &str)]) -> AgentOrgRunContext use crate::coordination::agent_org_runs::{ AgentOrgRunEntryMode, AgentOrgRunStatus, AgentOrgRunStore, CreateAgentOrgRunParams, }; - use crate::core::definitions::orgs::{OrgDefinition, OrgMember}; + use crate::core::definitions::orgs::{FlatOrgMember, OrgDefinition}; ensure_inbox_schema(); let unique = uuid::Uuid::new_v4().to_string(); @@ -98,26 +97,27 @@ fn running_ctx_for_members(members: &[(&str, &str, &str)]) -> AgentOrgRunContext org_id: format!("org-inbox-drain-{unique}"), coordinator_agent_id: "coord".to_string(), root_session_id: Some(format!("root-{unique}")), - org_snapshot: OrgDefinition { + org_snapshot: (&OrgDefinition { id: format!("org-inbox-drain-{unique}"), name: "Inbox Drain Test Org".to_string(), role: "coordinator".to_string(), agent_id: "coord".to_string(), description: None, - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, - children: members + members: members .iter() - .map(|(member_id, agent_id, name)| OrgMember { - id: (*member_id).to_string(), + .map(|(member_id, agent_id, name)| FlatOrgMember { + member_id: (*member_id).to_string(), name: (*name).to_string(), role: "engineer".to_string(), agent_id: (*agent_id).to_string(), runtime_config: None, - children: Vec::new(), }) .collect(), - }, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), + }) + .into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -135,7 +135,6 @@ fn running_ctx_for_members(members: &[(&str, &str, &str)]) -> AgentOrgRunContext agent_id: (*agent_id).to_string(), name: (*name).to_string(), role: "engineer".to_string(), - parent_member_id: None, } }) .collect(); @@ -244,14 +243,12 @@ fn shared_agent_id_member_session_drains_only_its_member_inbox() { agent_id: shared_agent_id.into(), name: "Alice".into(), role: "planner".into(), - parent_member_id: None, }, crate::coordination::agent_org_runs::AgentOrgContextMember { member_id: "bob".into(), agent_id: shared_agent_id.into(), name: "Bob".into(), role: "implementer".into(), - parent_member_id: None, }, ]; upsert_org_member_session( @@ -1531,7 +1528,7 @@ fn drain_does_not_steal_task_from_stale_running_worker() { AgentOrgRunEntryMode, AgentOrgRunStatus, AgentOrgRunStore, CreateAgentOrgRunParams, }; use crate::coordination::agent_org_tasks::{AgentOrgTaskStore, CreateTaskParams, TaskStatus}; - use crate::core::definitions::orgs::{OrgDefinition, OrgMember}; + use crate::core::definitions::orgs::{FlatOrgMember, OrgDefinition}; use crate::session::persistence::{session_type, upsert_session, UnifiedSessionRecord}; let _sandbox = test_helpers::test_env::sandbox(); @@ -1555,33 +1552,33 @@ fn drain_does_not_steal_task_from_stale_running_worker() { org_id: "org-stale-drain".to_string(), coordinator_agent_id: "coord".to_string(), root_session_id: Some(root_session_id.clone()), - org_snapshot: OrgDefinition { + org_snapshot: (&OrgDefinition { id: "org-stale-drain".to_string(), name: "Stale Drain Org".to_string(), role: "coordinator".to_string(), agent_id: "coord".to_string(), description: None, - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, - children: vec![ - OrgMember { - id: "member-stale".to_string(), + members: vec![ + FlatOrgMember { + member_id: "member-stale".to_string(), name: "Stale Worker".to_string(), role: "worker".to_string(), agent_id: "stale-worker".to_string(), runtime_config: None, - children: Vec::new(), }, - OrgMember { - id: "member-fresh".to_string(), + FlatOrgMember { + member_id: "member-fresh".to_string(), name: "Fresh Worker".to_string(), role: "worker".to_string(), agent_id: "fresh-worker".to_string(), runtime_config: None, - children: Vec::new(), }, ], - }, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), + }) + .into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -1638,7 +1635,6 @@ fn drain_does_not_steal_task_from_stale_running_worker() { agent_id: "fresh-worker".to_string(), name: "Fresh Worker".to_string(), role: "worker".to_string(), - parent_member_id: None, }); ctx.root_session_id = Some(root_session_id.clone()); let mut messages = Vec::new(); @@ -1764,7 +1760,6 @@ fn drain_drops_exec_mode_request_from_non_coordinator_sender() { name: "Bob".into(), role: "engineer".into(), agent_id: "bob-agent".into(), - parent_member_id: None, }); AgentInboxStore::insert(InsertInboxParams { diff --git a/src-tauri/crates/agent-core/src/core/session/turn/processor/member_idle.rs b/src-tauri/crates/agent-core/src/core/session/turn/processor/member_idle.rs index cc598dbb23..4f65d9da2b 100644 --- a/src-tauri/crates/agent-core/src/core/session/turn/processor/member_idle.rs +++ b/src-tauri/crates/agent-core/src/core/session/turn/processor/member_idle.rs @@ -374,11 +374,10 @@ mod tests { name: name.into(), role: "engineer".into(), agent_id: agent_id.into(), - parent_member_id: None, }) .collect(), - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-1".into()), } } @@ -389,23 +388,24 @@ mod tests { org_id: "org-lifecycle-gate".to_string(), coordinator_agent_id: "coordinator".to_string(), root_session_id: None, - org_snapshot: crate::definitions::orgs::OrgDefinition { + org_snapshot: (&crate::definitions::orgs::OrgDefinition { id: "org-lifecycle-gate".to_string(), name: "Lifecycle Gate Test Org".to_string(), role: "coordinator".to_string(), agent_id: "coordinator".to_string(), description: None, - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, - children: vec![crate::definitions::orgs::OrgMember { - id: "member-worker".to_string(), + members: vec![crate::definitions::orgs::FlatOrgMember { + member_id: "member-worker".to_string(), name: "Worker".to_string(), role: "builder".to_string(), agent_id: "worker-agent".to_string(), runtime_config: None, - children: Vec::new(), }], - }, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), + }) + .into(), entry_mode: crate::coordination::agent_org_runs::AgentOrgRunEntryMode::StandaloneSession, status: crate::coordination::agent_org_runs::AgentOrgRunStatus::Running, diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/formatting.rs b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/formatting.rs index 5e816a6b7f..b82c3ebde4 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/formatting.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/formatting.rs @@ -1,6 +1,6 @@ //! Formatting helpers for agent and org display strings. -use crate::definitions::orgs::{OrgDefinition, OrgMember}; +use crate::definitions::orgs::OrgDefinition; use crate::definitions::AgentDefinition; pub fn format_agent_summary(agent: &AgentDefinition) -> String { @@ -57,10 +57,10 @@ pub fn format_agent_detail(agent: &AgentDefinition) -> String { pub fn format_org_summary(org: &OrgDefinition) -> String { let mut line = format!( - "- **{}** (id: `{}`, {} members)", + "- **{}** (id: `{}`, {} participants)", org.name, org.id, - org.member_count() + org.participant_count() ); if let Some(ref desc) = org.description { let preview: String = crate::utils::safe_truncate_chars_to_string(&desc, 80); @@ -80,23 +80,18 @@ pub fn format_org_detail(org: &OrgDefinition) -> String { if let Some(ref desc) = org.description { out.push_str(&format!("- **Description:** {}\n", desc)); } - out.push_str(&format!("- **Total members:** {}\n", org.member_count())); - if !org.children.is_empty() { + out.push_str(&format!( + "- **Total participants:** {}\n", + org.participant_count() + )); + if !org.members.is_empty() { out.push_str("\n## Team members\n\n"); - format_member_tree(&org.children, &mut out, 0); - } - out -} - -pub fn format_member_tree(members: &[OrgMember], out: &mut String, depth: usize) { - let indent = " ".repeat(depth); - for member in members { - out.push_str(&format!( - "{}- **{}** (role: {}, agent: `{}`)\n", - indent, member.name, member.role, member.agent_id - )); - if !member.children.is_empty() { - format_member_tree(&member.children, out, depth + 1); + for member in &org.members { + out.push_str(&format!( + "- **{}** (member_id: `{}`, role: {}, agent: `{}`)\n", + member.name, member.member_id, member.role, member.agent_id + )); } } + out } diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/mod.rs b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/mod.rs index 13581c4a78..fd92207c6d 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/mod.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/mod.rs @@ -44,10 +44,11 @@ impl AgentDefinitionTool { .inner() } - fn org_store(&self) -> &AgentOrgsStore { + fn org_store(&self) -> std::sync::Arc { self.app_handle .state::>() .inner() + .clone() } } @@ -83,11 +84,11 @@ impl Tool for AgentDefinitionTool { "update" => agent_actions::update_agent(self.store(), ¶ms), "remove" => agent_actions::remove_agent(self.store(), ¶ms), - "list_orgs" => org_actions::list_orgs(self.org_store()), - "get_org" => org_actions::get_org(self.org_store(), ¶ms), - "create_org" => org_actions::create_org(self.org_store(), ¶ms), - "update_org" => org_actions::update_org(self.org_store(), ¶ms), - "remove_org" => org_actions::remove_org(self.org_store(), ¶ms), + "list_orgs" => org_actions::list_orgs(&self.org_store()), + "get_org" => org_actions::get_org(&self.org_store(), ¶ms), + "create_org" => org_actions::create_org(&self.org_store(), ¶ms).await, + "update_org" => org_actions::update_org(&self.org_store(), ¶ms).await, + "remove_org" => org_actions::remove_org(&self.org_store(), ¶ms).await, _ => Err(ToolError::InvalidParams(format!( "Unknown action: '{}'. Valid: list, get, create, update, remove, \ diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/org_actions.rs b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/org_actions.rs index 455bb4b49a..ddfc3b1a24 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/org_actions.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/org_actions.rs @@ -1,19 +1,24 @@ //! CRUD handlers for `OrgDefinition` entries (agent organizations). +//! +//! Mutating actions are async and go through the store's `*_async` +//! wrappers (the single `spawn_blocking` owner) so fsync-under-mutex +//! store commits never run on the async executor. + +use std::sync::Arc; use serde_json::Value; use uuid::Uuid; -use crate::definitions::orgs::{AgentOrgsStore, OrgDefinition}; +use crate::definitions::orgs::{ + all_member_links, AgentOrgsStore, MemberCommunicationLink, OrgDefinition, +}; use crate::tools::traits::{optional_string, required_string, ToolError}; use super::formatting::{format_org_detail, format_org_summary}; use super::parsing::parse_org_members; pub(super) fn list_orgs(store: &AgentOrgsStore) -> Result { - let orgs = store - .orgs - .lock() - .map_err(|err| ToolError::ExecutionFailed(format!("Lock error: {}", err)))?; + let orgs = store.list().map_err(ToolError::ExecutionFailed)?; if orgs.is_empty() { return Ok("No agent organizations defined. Use 'create_org' to add one.".to_string()); @@ -29,30 +34,22 @@ pub(super) fn list_orgs(store: &AgentOrgsStore) -> Result { pub(super) fn get_org(store: &AgentOrgsStore, params: &Value) -> Result { let org_id = required_string(params, "org_id")?; - let orgs = store - .orgs - .lock() - .map_err(|err| ToolError::ExecutionFailed(format!("Lock error: {}", err)))?; - - let org = orgs - .iter() - .find(|o| o.id == org_id) - .ok_or_else(|| ToolError::ExecutionFailed(format!("Org '{}' not found", org_id)))?; + let org = store.get(&org_id).map_err(ToolError::ExecutionFailed)?; - Ok(format_org_detail(org)) + Ok(format_org_detail(&org)) } -pub(super) fn create_org(store: &AgentOrgsStore, params: &Value) -> Result { +pub(super) async fn create_org( + store: &Arc, + params: &Value, +) -> Result { let name = required_string(params, "name")?; let description = optional_string(params, "description"); let role = optional_string(params, "role").unwrap_or_else(|| "leader".to_string()); let leader_agent_id = optional_string(params, "agent_id").unwrap_or_default(); - let children = parse_org_members(params); - - let orgs = store - .orgs - .lock() - .map_err(|err| ToolError::ExecutionFailed(format!("Lock error: {}", err)))?; + // Model-authored create input never controls stable identities. + let members = parse_org_members(params, false).map_err(ToolError::InvalidParams)?; + let orgs = store.list().map_err(ToolError::ExecutionFailed)?; if orgs .iter() @@ -64,36 +61,35 @@ pub(super) fn create_org(store: &AgentOrgsStore, params: &Value) -> Result Result { +pub(super) async fn update_org( + store: &Arc, + params: &Value, +) -> Result { let org_id = required_string(params, "org_id")?; - let mut orgs = store - .orgs - .lock() - .map_err(|err| ToolError::ExecutionFailed(format!("Lock error: {}", err)))?; - - let org = orgs - .iter_mut() - .find(|o| o.id == org_id) - .ok_or_else(|| ToolError::ExecutionFailed(format!("Org '{}' not found", org_id)))?; + let mut org = store.get(&org_id).map_err(ToolError::ExecutionFailed)?; if let Some(name) = optional_string(params, "name") { org.name = name; @@ -108,28 +104,96 @@ pub(super) fn update_org(store: &AgentOrgsStore, params: &Value) -> Result>(); + let existing_ids = org + .members + .iter() + .map(|member| member.member_id.as_str()) + .collect::>(); + if let Some(unknown_id) = requested_member_ids + .iter() + .find(|member_id| !existing_ids.contains(**member_id)) + { + return Err(ToolError::ExecutionFailed(format!( + "Org '{}' has no existing member id '{}'", + org_id, unknown_id + ))); + } + // Model updates never carry runtime configuration; surviving members + // keep whatever runtime_config the user persisted for them. + for member in new_members.iter_mut() { + if let Some(existing) = org + .members + .iter() + .find(|existing| existing.member_id == member.member_id) + { + member.runtime_config = existing.runtime_config.clone(); + } + } + let retained_ids = new_members + .iter() + .filter(|member| existing_ids.contains(member.member_id.as_str())) + .map(|member| member.member_id.clone()) + .collect::>(); + let new_ids = new_members + .iter() + .filter(|member| !existing_ids.contains(member.member_id.as_str())) + .map(|member| member.member_id.clone()) + .collect::>(); + org.additional_task_graph_writer_member_ids + .retain(|member_id| retained_ids.contains(member_id)); + let existing_links = org + .member_communication_links + .iter() + .map(|link| { + let link = MemberCommunicationLink::canonical( + link.member_a_id.clone(), + link.member_b_id.clone(), + ); + (link.member_a_id, link.member_b_id) + }) + .collect::>(); + org.member_communication_links = all_member_links(&new_members) + .into_iter() + .filter(|link| { + new_ids.contains(&link.member_a_id) + || new_ids.contains(&link.member_b_id) + || existing_links + .contains(&(link.member_a_id.clone(), link.member_b_id.clone())) + }) + .collect(); + org.members = new_members; } let name = org.name.clone(); - let updated = org.clone(); - drop(orgs); - store.replace(updated).map_err(ToolError::ExecutionFailed)?; + store + .replace_async(org) + .await + .map_err(ToolError::ExecutionFailed)?; Ok(format!("Updated org '{}'.", name)) } -pub(super) fn remove_org(store: &AgentOrgsStore, params: &Value) -> Result { +pub(super) async fn remove_org( + store: &Arc, + params: &Value, +) -> Result { let org_id = required_string(params, "org_id")?; - let orgs = store - .orgs - .lock() - .map_err(|err| ToolError::ExecutionFailed(format!("Lock error: {}", err)))?; - - let removed_name = orgs.iter().find(|o| o.id == org_id).map(|o| o.name.clone()); - drop(orgs); - let removed = store.remove(&org_id).map_err(ToolError::ExecutionFailed)?; + let removed_name = store.get(&org_id).ok().map(|org| org.name); + let removed = store + .remove_async(org_id.clone()) + .await + .map_err(ToolError::ExecutionFailed)?; if removed { Ok(format!("Removed org '{}'.", removed_name.unwrap_or(org_id))) @@ -140,3 +204,218 @@ pub(super) fn remove_org(store: &AgentOrgsStore, params: &Value) -> Result Option> { @@ -22,38 +22,97 @@ pub fn parse_sub_agents(params: &Value) -> Option> { }) } -pub fn parse_org_members(params: &Value) -> Vec { - params - .get("members") - .and_then(|val| val.as_array()) - .map(|arr| arr.iter().filter_map(parse_single_member).collect()) - .unwrap_or_default() +/// Parse the `members` array of a create/update org call. +/// +/// Malformed entries are hard, structured errors — never silently dropped. +/// A silently dropped member used to cascade into Writer-grant and +/// communication-link removal on update, i.e. silent data destruction. +pub fn parse_org_members( + params: &Value, + accept_existing_ids: bool, +) -> Result, String> { + let Some(members_value) = params.get("members") else { + return Ok(Vec::new()); + }; + let Some(entries) = members_value.as_array() else { + return Err("'members' must be an array of member objects".to_string()); + }; + entries + .iter() + .enumerate() + .map(|(index, member)| parse_single_member(member, index, accept_existing_ids)) + .collect() } -fn parse_single_member(val: &Value) -> Option { - let name = val.get("name")?.as_str()?.to_string(); - let role = val - .get("role") - .and_then(|v| v.as_str()) - .unwrap_or("member") - .to_string(); - let agent_id = val - .get("agent_id") - .and_then(|v| v.as_str()) - .unwrap_or("") - .to_string(); - let children = val - .get("children") - .and_then(|v| v.as_array()) - .map(|arr| arr.iter().filter_map(parse_single_member).collect()) - .unwrap_or_default(); - Some(OrgMember { - id: Uuid::new_v4().to_string(), +fn parse_single_member( + val: &Value, + index: usize, + accept_existing_id: bool, +) -> Result { + let Some(entry) = val.as_object() else { + return Err(format!( + "members[{index}] must be an object with at least a 'name' field" + )); + }; + let name = match entry.get("name").map(|value| value.as_str()) { + Some(Some(name)) if !name.trim().is_empty() => name.to_string(), + Some(Some(_)) => { + return Err(format!("members[{index}] has an empty 'name'")); + } + Some(None) => { + return Err(format!("members[{index}] field 'name' must be a string")); + } + None => { + return Err(format!( + "members[{index}] is missing the required 'name' field" + )); + } + }; + let role = match entry.get("role") { + None => "member".to_string(), + Some(value) => value + .as_str() + .ok_or_else(|| format!("members[{index}] ('{name}') field 'role' must be a string"))? + .to_string(), + }; + let agent_id = match entry.get("agent_id") { + None => String::new(), + Some(value) => value + .as_str() + .ok_or_else(|| { + format!("members[{index}] ('{name}') field 'agent_id' must be a string") + })? + .to_string(), + }; + let member_id = if accept_existing_id { + match entry.get("member_id") { + None => Uuid::new_v4().to_string(), + Some(value) => { + let member_id = value.as_str().ok_or_else(|| { + format!("members[{index}] ('{name}') field 'member_id' must be a string") + })?; + if member_id.is_empty() { + return Err(format!( + "members[{index}] ('{name}') has an empty 'member_id'" + )); + } + if member_id.trim() != member_id { + return Err(format!( + "members[{index}] ('{name}') field 'member_id' must not have leading or trailing whitespace" + )); + } + member_id.to_string() + } + } + } else { + Uuid::new_v4().to_string() + }; + Ok(FlatOrgMember { + member_id, name, role, agent_id, runtime_config: None, - children, }) } diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/schema.rs b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/schema.rs index e9764d01ad..b09b1f6b38 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/schema.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/agent_def/schema.rs @@ -86,10 +86,15 @@ pub(super) fn parameters_schema() -> Value { "type": "array", "items": { "type": "object", - "properties": { "name": { "type": "string" }, "role": { "type": "string" }, "agent_id": { "type": "string" }, "children": { "type": "array", "items": { "type": "object" } } }, + "properties": { + "member_id": { "type": "string", "description": "Stable member id returned by get_org; include it on update to preserve that member" }, + "name": { "type": "string" }, + "role": { "type": "string" }, + "agent_id": { "type": "string" } + }, "required": ["name"] }, - "description": "Org team members" + "description": "Flat org team members. Writer grants and communication links are user-managed settings and are intentionally unavailable to this tool." } }); diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent/tests.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent/tests.rs index 8a1a6bf61d..8baed07f35 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent/tests.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent/tests.rs @@ -478,11 +478,10 @@ fn ctx_with_members(coordinator_id: &str, member_ids: &[&str]) -> AgentOrgRunCon name: (*id).to_string(), role: "worker".to_string(), agent_id: (*id).to_string(), - parent_member_id: None, }) .collect(), - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-test".to_string()), } } diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/inbox_repair.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/inbox_repair.rs index 70612e31db..ede831b8d2 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/inbox_repair.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/inbox_repair.rs @@ -196,7 +196,7 @@ mod tests { AgentOrgContextMember, AgentOrgRunContext, AgentOrgRunEntryMode, AgentOrgRunStatus, AgentOrgRunStore, CreateAgentOrgRunParams, }; - use crate::definitions::orgs::{HierarchyMode, OrgDefinition, OrgMember, PlanApprovalPolicy}; + use crate::definitions::orgs::{FlatOrgMember, OrgDefinition, PlanApprovalPolicy}; use crate::session::persistence::{upsert_session, UnifiedSessionRecord}; use crate::tools::impls::orchestration::org_send_message::NoopInboxWakeHook; use crate::tools::traits::Tool; @@ -224,22 +224,22 @@ mod tests { role: "Coordinator".into(), agent_id: "coordinator-agent".into(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![OrgMember { - id: "worker".into(), + members: vec![FlatOrgMember { + member_id: "worker".into(), name: "Worker".into(), role: "Implementer".into(), agent_id: "worker-agent".into(), runtime_config: None, - children: Vec::new(), }], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), }; let run = AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id: org.id.clone(), coordinator_agent_id: org.agent_id.clone(), root_session_id: Some("root-inbox-repair".into()), - org_snapshot: org, + org_snapshot: (&org).into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -297,10 +297,9 @@ mod tests { name: "Worker".into(), role: "Implementer".into(), agent_id: "worker-agent".into(), - parent_member_id: None, }], - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-inbox-repair".into()), }); let make_context = |member_id: &str, agent_id: &str| { diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message.rs index 43d1b49e1c..0cf550eb40 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message.rs @@ -113,25 +113,11 @@ impl OrgSendMessageTool { } } - fn hierarchy_mode_label(&self) -> &'static str { - match self.org_context.hierarchy_mode { - crate::definitions::orgs::HierarchyMode::Flat => "flat", - crate::definitions::orgs::HierarchyMode::Soft => "soft", - crate::definitions::orgs::HierarchyMode::Strict => "strict", - } - } - fn routing_description(&self) -> &'static str { - match self.org_context.hierarchy_mode { - crate::definitions::orgs::HierarchyMode::Flat => { - "flat: any participant may message any other participant except itself" - } - crate::definitions::orgs::HierarchyMode::Soft => { - "soft: same routable set as flat; reports_to is advisory only" - } - crate::definitions::orgs::HierarchyMode::Strict => { - "strict: coordinator may message members; members may message coordinator, manager, and direct reports only" - } + if self.sender.is_coordinator { + "coordinator may message any member" + } else { + "member may message the coordinator; peer delivery remains disabled until the peer-send phase" } } @@ -139,9 +125,8 @@ impl OrgSendMessageTool { let allowed = self.allowed_recipient_member_ids(); let kinds = self.allowed_message_kinds(); format!( - "{}\n\nCurrent Agent Org routing context:\n- hierarchy_mode: {}\n- sender_member_id: {}\n- routing_rule: {}\n- recipient_member_id enum: [{}]\n- kind enum for this sender: [{}]\n\nUse exactly one recipient_member_id from the enum. Do not route by display name or agent id.\n\nFormal-work rule:\n- A `plain` message to any non-coordinator worker MUST include `related_task_id`.\n- The task must be unresolved, dependency-ready, and already owned by that recipient. Eligibility alone is not an assignment.\n- Create and explicitly assign the durable task first; a chat message cannot replace a task, assign ownerless work, or bypass dependencies.\n- Worker → coordinator status/escalation messages do not need `related_task_id`.\n\nCoordinator planning protocol:\n- Create planning work with `task_create execution_mode=\"plan\"`; the assigned Planner starts in Plan mode automatically.\n- A member's `create_plan` call creates a durable approval bound to that planning task.\n- To answer a submitted member plan, send `kind = \"plan_approval_response\"`, echo the inbox `request_id`, and set `accepted = true` to complete the planning task and unlock its dependants, or `accepted = false` with non-empty `feedback` to wake the Planner once for revision.", + "{}\n\nCurrent Agent Org routing context:\n- sender_member_id: {}\n- routing_rule: {}\n- recipient_member_id enum: [{}]\n- kind enum for this sender: [{}]\n\nUse exactly one recipient_member_id from the enum. Do not route by display name or agent id.\n\nFormal-work rule:\n- A `plain` message to any non-coordinator worker MUST include `related_task_id`.\n- The task must be unresolved, dependency-ready, and already owned by that recipient. Eligibility alone is not an assignment.\n- Create and explicitly assign the durable task first; a chat message cannot replace a task, assign ownerless work, or bypass dependencies.\n- Worker → coordinator status/escalation messages do not need `related_task_id`.\n\nCoordinator planning protocol:\n- Create planning work with `task_create execution_mode=\"plan\"`; the assigned Planner starts in Plan mode automatically.\n- A member's `create_plan` call creates a durable approval bound to that planning task.\n- To answer a submitted member plan, send `kind = \"plan_approval_response\"`, echo the inbox `request_id`, and set `accepted = true` to complete the planning task and unlock its dependants, or `accepted = false` with non-empty `feedback` to wake the Planner once for revision.", ::description(self), - self.hierarchy_mode_label(), self.sender.member_id, self.routing_description(), allowed.join(", "), diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message/tests.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message/tests.rs index 2579d7723a..3317786382 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message/tests.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/send_message/tests.rs @@ -8,14 +8,9 @@ use crate::coordination::agent_org_runs::{AgentOrgContextMember, COORDINATOR_MEM use crate::coordination::agent_org_tasks::{ new_task_id, AgentOrgTaskStore, CreateTaskParams, TaskStatus, TASK_METADATA_ELIGIBLE_MEMBER_IDS, }; -use crate::definitions::orgs::HierarchyMode; use std::sync::Mutex; fn context() -> Arc { - context_with_mode(HierarchyMode::Strict) -} - -fn context_with_mode(hierarchy_mode: HierarchyMode) -> Arc { Arc::new(AgentOrgRunContext { run_id: "run-1".to_string(), org_id: "org-1".to_string(), @@ -30,18 +25,16 @@ fn context_with_mode(hierarchy_mode: HierarchyMode) -> Arc { name: "Planner".to_string(), role: "plan".to_string(), agent_id: "agent-shared".to_string(), - parent_member_id: None, }, AgentOrgContextMember { member_id: "builder".to_string(), name: "Builder".to_string(), role: "build".to_string(), agent_id: "agent-shared".to_string(), - parent_member_id: Some("planner".to_string()), }, ], - hierarchy_mode, plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-1".to_string()), }) } @@ -194,7 +187,7 @@ fn rejects_unroutable_member_id_with_allowed_ids() { assert!(error.contains("recipient_member_id 'ghost'"), "{error}"); assert!(error.contains("coordinator"), "{error}"); - assert!(error.contains("planner"), "{error}"); + assert!(!error.contains("planner"), "{error}"); } #[test] @@ -218,27 +211,17 @@ fn schema_keeps_openai_compatible_routing_fields() { } #[test] -fn llm_description_carries_flat_hierarchy_routing_hints() { - let tool = OrgSendMessageTool::new( - context_with_mode(HierarchyMode::Flat), - "builder".to_string(), - ); +fn llm_description_carries_current_routing_hints() { + let tool = OrgSendMessageTool::new(context(), "builder".to_string()); let description = tool.llm_description().expect("description"); - assert!(description.contains("hierarchy_mode: flat")); - assert!(description.contains("recipient_member_id enum: [coordinator, planner]")); + assert!(description.contains("recipient_member_id enum: [coordinator]")); } #[test] -fn llm_description_recipient_hints_follow_strict_hierarchy_mode() { - let coordinator_tool = OrgSendMessageTool::new( - context_with_mode(HierarchyMode::Strict), - COORDINATOR_MEMBER_ID.to_string(), - ); - let builder_tool = OrgSendMessageTool::new( - context_with_mode(HierarchyMode::Strict), - "builder".to_string(), - ); +fn llm_description_recipient_hints_keep_peer_send_disabled() { + let coordinator_tool = OrgSendMessageTool::new(context(), COORDINATOR_MEMBER_ID.to_string()); + let builder_tool = OrgSendMessageTool::new(context(), "builder".to_string()); assert!(coordinator_tool .llm_description() @@ -247,7 +230,7 @@ fn llm_description_recipient_hints_follow_strict_hierarchy_mode() { assert!(builder_tool .llm_description() .expect("description") - .contains("recipient_member_id enum: [coordinator, planner]")); + .contains("recipient_member_id enum: [coordinator]")); } #[test] @@ -297,9 +280,8 @@ fn llm_description_lists_only_member_ids() { let description = tool.llm_description().expect("description"); assert!(description.contains("Current Agent Org routing context")); - assert!(description.contains("hierarchy_mode: strict")); assert!(description.contains("sender_member_id: builder")); - assert!(description.contains("recipient_member_id enum: [coordinator, planner]")); + assert!(description.contains("recipient_member_id enum: [coordinator]")); assert!(!description.contains("recipient_agent_id")); assert!(!description.contains("recipient_name")); assert!(!description.contains("Builder")); diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_create.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_create.rs index 261cc99608..ed9326724b 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_create.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_create.rs @@ -199,10 +199,10 @@ impl Tool for TaskCreateTool { fn description(&self) -> &str { concat!( "Create a task on the org run's task board. The board is shared by every ", - "agent in this Agent Org run, but write authority follows the org structure: ", - "the coordinator may assign any participant; a member may assign itself and, ", - "in soft/strict hierarchy modes, its direct reports. Peer communication does ", - "not grant peer task-assignment authority. ", + "agent in this Agent Org run, but write authority remains deliberately narrow: ", + "the coordinator may assign any participant, while a member may assign only ", + "itself until additional Writer activation lands. Peer communication does not ", + "grant peer task-assignment authority. ", "Set `owner_member_id` to `coordinator` or an exact roster member_id for ", "direct assignment — a pending assignee will receive a `task_assigned` inbox ", "row on their next turn. If you leave `owner_member_id` unset, the task is ", @@ -303,7 +303,7 @@ impl Tool for TaskCreateTool { return self.ctx.authorization_denied_response( "task_create.assign_owner", denied, - "You may create work only for yourself or your direct reports. Ask the coordinator to create or assign work for a peer or another branch.", + "You may create work only for yourself. Ask the coordinator to create or assign work for another member.", ); } } diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_tests.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_tests.rs index 281ab235f2..5b6ab2beb4 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_tests.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_tests.rs @@ -11,7 +11,6 @@ use crate::coordination::agent_org_tasks::{ TaskExecutionMode, TaskStatus, TASK_DEPENDENCY_CYCLE_ERROR, }; use crate::core::session::persistence::{upsert_session, UnifiedSessionRecord}; -use crate::definitions::orgs::HierarchyMode; use crate::tools::impls::orchestration::org_send_message::{InboxWakeHook, NoopInboxWakeHook}; use crate::tools::traits::{Tool, ToolError}; use test_helpers::test_env; @@ -81,18 +80,16 @@ fn org_context() -> Arc { name: "Alice".into(), role: "engineer".into(), agent_id: "alice-1".into(), - parent_member_id: None, }, AgentOrgContextMember { member_id: "m-bob".into(), name: "Bob".into(), role: "engineer".into(), agent_id: "bob-1".into(), - parent_member_id: None, }, ], - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-tools-1".into()), }) } @@ -116,41 +113,38 @@ fn ctx_for_org( }) } -fn hierarchical_org_context(hierarchy_mode: HierarchyMode) -> Arc { +fn multi_member_org_context() -> Arc { Arc::new(AgentOrgRunContext { - run_id: "run-hierarchy-tools".into(), - org_id: "org-hierarchy-tools".into(), - org_name: "Hierarchy Tools Org".into(), + run_id: "run-flat-tools".into(), + org_id: "org-flat-tools".into(), + org_name: "Flat Tools Org".into(), org_role: "coordinator".into(), - coordinator_agent_id: "coord-hierarchy".into(), + coordinator_agent_id: "coord-flat".into(), coordinator_name: "Coordinator".into(), coordinator_role: "coordinator".into(), members: vec![ AgentOrgContextMember { - member_id: "manager".into(), - name: "Manager".into(), - role: "team lead".into(), - agent_id: "manager-agent".into(), - parent_member_id: None, + member_id: "builder".into(), + name: "Builder".into(), + role: "backend".into(), + agent_id: "builder-agent".into(), }, AgentOrgContextMember { - member_id: "report".into(), - name: "Direct Report".into(), + member_id: "reviewer".into(), + name: "Reviewer".into(), role: "implementer".into(), - agent_id: "report-agent".into(), - parent_member_id: Some("manager".into()), + agent_id: "reviewer-agent".into(), }, AgentOrgContextMember { member_id: "peer".into(), name: "Peer".into(), role: "reviewer".into(), agent_id: "peer-agent".into(), - parent_member_id: None, }, ], - hierarchy_mode, plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, - root_session_id: Some("root-hierarchy-tools".into()), + capability_index: Default::default(), + root_session_id: Some("root-flat-tools".into()), }) } @@ -204,10 +198,9 @@ fn shared_sde_ctx(caller_member_id: Option<&str>) -> Arc { name: "Planner".into(), role: "Plans".into(), agent_id: "builtin:sde".into(), - parent_member_id: None, }], - hierarchy_mode: Default::default(), plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-shared-sde".into()), }); Arc::new(TaskToolsContext { @@ -253,10 +246,10 @@ fn task_tools_sandbox() -> test_env::SandboxGuard { for (run_id, org_id, coordinator_agent_id, root_session_id) in [ ("run-tools-1", "org-tools-1", "coord-1", "root-tools-1"), ( - "run-hierarchy-tools", - "org-hierarchy-tools", - "coord-hierarchy", - "root-hierarchy-tools", + "run-flat-tools", + "org-flat-tools", + "coord-flat", + "root-flat-tools", ), ( "run-shared-sde", @@ -1057,50 +1050,34 @@ async fn task_create_member_can_start_self_work_in_progress() { } #[tokio::test] -async fn task_authority_manager_may_assign_direct_reports_only_when_hierarchy_exists() { +async fn task_authority_member_cannot_assign_another_member() { let _sandbox = task_tools_sandbox(); - for (mode, task_id, should_create) in [ - (HierarchyMode::Soft, "soft-report-task", true), - (HierarchyMode::Strict, "strict-report-task", true), - (HierarchyMode::Flat, "flat-report-task", false), - ] { - let tool = TaskCreateTool::new(ctx_for_org(hierarchical_org_context(mode), "manager")); - let response = tool - .execute_text( - json!({ - "id": task_id, - "subject": "Manager assigns a direct report", - "owner_member_id": "report" - }), - &test_ctx(), - ) - .await - .unwrap(); - let value: Value = serde_json::from_str(&response).unwrap(); - if should_create { - assert_eq!(value["task"]["owner"], "report", "mode={mode:?}"); - assert_eq!(value["already_exists"], false, "mode={mode:?}"); - AgentOrgTaskStore::delete("run-hierarchy-tools", task_id) - .expect("each hierarchy case must start with an empty scheduling board"); - } else { - assert_eq!(value["authorization_denied"], true, "mode={mode:?}"); - assert_eq!(value["denied_target_member_ids"], json!(["report"])); - } - } + let tool = TaskCreateTool::new(ctx_for_org(multi_member_org_context(), "builder")); + let response = tool + .execute_text( + json!({ + "id": "reviewer-task", + "subject": "Member attempts assignment", + "owner_member_id": "reviewer" + }), + &test_ctx(), + ) + .await + .unwrap(); + let value: Value = serde_json::from_str(&response).unwrap(); + assert_eq!(value["authorization_denied"], true); + assert_eq!(value["denied_target_member_ids"], json!(["reviewer"])); } #[tokio::test] -async fn task_authority_manager_cannot_assign_peer_even_when_soft_routing_allows_chat() { +async fn task_authority_member_cannot_assign_peer() { let _sandbox = task_tools_sandbox(); - let tool = TaskCreateTool::new(ctx_for_org( - hierarchical_org_context(HierarchyMode::Soft), - "manager", - )); + let tool = TaskCreateTool::new(ctx_for_org(multi_member_org_context(), "builder")); let response = tool .execute_text( json!({ - "id": "soft-peer-assignment", - "subject": "Manager attempted peer assignment", + "id": "peer-assignment", + "subject": "Member attempted peer assignment", "owner_member_id": "peer" }), &test_ctx(), @@ -1110,11 +1087,9 @@ async fn task_authority_manager_cannot_assign_peer_even_when_soft_routing_allows let value: Value = serde_json::from_str(&response).unwrap(); assert_eq!(value["authorization_denied"], true); assert_eq!(value["denied_target_member_ids"], json!(["peer"])); - assert!( - AgentOrgTaskStore::get("run-hierarchy-tools", "soft-peer-assignment") - .unwrap() - .is_none() - ); + assert!(AgentOrgTaskStore::get("run-flat-tools", "peer-assignment") + .unwrap() + .is_none()); } #[tokio::test] @@ -1642,14 +1617,11 @@ async fn task_authority_worker_cannot_unassign_into_preserved_cross_peer_pool() } #[tokio::test] -async fn task_authority_manager_can_edit_direct_report_but_not_peer_task() { +async fn task_authority_member_cannot_edit_other_members_tasks() { let _sandbox = task_tools_sandbox(); - let coordinator = ctx_for_org( - hierarchical_org_context(HierarchyMode::Soft), - COORDINATOR_MEMBER_ID, - ); + let coordinator = ctx_for_org(multi_member_org_context(), COORDINATOR_MEMBER_ID); let create = TaskCreateTool::new(coordinator); - for (id, owner) in [("report-work", "report"), ("peer-work", "peer")] { + for (id, owner) in [("reviewer-work", "reviewer"), ("peer-work", "peer")] { create .execute_text( json!({ "id": id, "subject": id, "owner_member_id": owner }), @@ -1659,27 +1631,25 @@ async fn task_authority_manager_can_edit_direct_report_but_not_peer_task() { .unwrap(); } - let manager = TaskUpdateTool::new(ctx_for_org( - hierarchical_org_context(HierarchyMode::Soft), - "manager", - )); - let report_response = manager + let member = TaskUpdateTool::new(ctx_for_org(multi_member_org_context(), "builder")); + let reviewer_response = member .execute_text( json!({ - "id": "report-work", - "description": "Authorized manager clarification" + "id": "reviewer-work", + "description": "Unauthorized cross-member clarification" }), &test_ctx(), ) .await .unwrap(); - let report_value: Value = serde_json::from_str(&report_response).unwrap(); + let reviewer_value: Value = serde_json::from_str(&reviewer_response).unwrap(); + assert_eq!(reviewer_value["authorization_denied"], true); assert_eq!( - report_value["task"]["description"], - "Authorized manager clarification" + reviewer_value["denied_target_member_ids"], + json!(["reviewer"]) ); - let peer_response = manager + let peer_response = member .execute_text( json!({ "id": "peer-work", "description": "Unauthorized peer edit" }), &test_ctx(), diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_update.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_update.rs index 208002b9bc..32f52a8a15 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_update.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/task_update.rs @@ -180,10 +180,10 @@ impl Tool for TaskUpdateTool { concat!( "Update a task on the org run's task board. Only the fields you set are ", "written; missing fields keep their current value. Task write authority ", - "has two separate parts. Administrative authority follows the org structure: ", + "has two separate parts. Administrative authority is capability-scoped: ", "the coordinator may create, assign, reassign, edit, and repair every task; a ", - "member may administer its own tasks and, in soft/strict hierarchy modes, tasks ", - "owned by its direct reports. Work-authorship authority is always owner-only: ", + "member may administer only its own tasks until additional Writer activation ", + "lands. Work-authorship authority is always owner-only: ", "only the current owner may set `status=\"in_progress\"`, set ", "`status=\"completed\"`, or write `output`. Assignment or dependency unblocking ", "already wakes the owner, so a coordinator/manager must not start or complete ", @@ -385,7 +385,7 @@ impl Tool for TaskUpdateTool { "task_update.modify" }, denied, - "You may modify only your own tasks or tasks owned by your direct reports. Ask the coordinator to modify, reassign, or delete peer and cross-branch work.", + "You may modify only your own tasks. Ask the coordinator to modify, reassign, or delete another member's work.", ); } @@ -427,7 +427,7 @@ impl Tool for TaskUpdateTool { return self.ctx.authorization_denied_response( "task_update.reassign_owner", denied, - "You may reassign work only to yourself or your direct reports. Ask the coordinator to reassign work to a peer or another branch.", + "You may reassign work only to yourself. Ask the coordinator to reassign work to another member.", ); } } diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/tasks.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/tasks.rs index 0b59505f98..7df294b6f2 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/tasks.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/agent_org/tasks.rs @@ -6,7 +6,7 @@ //! (i.e. it is the coordinator or one of the org members). //! - Coordinator and members both get the full set, but writes are //! authority-checked at the tool boundary: coordinator → anyone; -//! member → self + direct reports in Soft/Strict; Flat members → self. +//! members → self only until additional Writer activation lands in PR7. //! Tool availability is not task-administration authority. //! - Outside an org run the tools are not registered (so plain //! single-agent sessions can't accidentally create dangling task @@ -150,14 +150,8 @@ impl TaskToolsContext { pub(crate) fn task_authority_summary(&self) -> &'static str { if self.is_coordinator() { "coordinator: may create, assign, reassign, edit, and repair tasks for every participant, but may not impersonate another owner by setting that member's in_progress/completed lifecycle or writing that member's output" - } else if self - .org_context - .direct_report_member_ids_for(&self.caller_member_id) - .is_empty() - { - "worker: may manage only its own tasks and must update its own lifecycle/output" } else { - "manager: may administer its own tasks and direct-report tasks, but may update lifecycle/output only for work it personally owns" + "worker: may manage only its own tasks and must update its own lifecycle/output" } } diff --git a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/context_builders.rs b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/context_builders.rs index 48bd7bd9cc..0e8cbf1bf0 100644 --- a/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/context_builders.rs +++ b/src-tauri/crates/agent-core/src/core/tools/impls/orchestration/context_builders.rs @@ -162,10 +162,10 @@ pub fn build_agent_orgs_context() -> Option { let mut out = String::from("## Agent Organizations\n\n"); for org in &orgs { let mut line = format!( - "- **{}** (`{}`, {} members)", + "- **{}** (`{}`, {} participants)", org.name, org.id, - org.member_count() + org.participant_count() ); if let Some(ref desc) = org.description { let preview: String = crate::utils::safe_truncate_chars_to_string(&desc, 60); diff --git a/src-tauri/crates/agent-core/src/init/mod.rs b/src-tauri/crates/agent-core/src/init/mod.rs index d629e8e07f..fecffe5c7c 100644 --- a/src-tauri/crates/agent-core/src/init/mod.rs +++ b/src-tauri/crates/agent-core/src/init/mod.rs @@ -236,18 +236,15 @@ fn load_agent_org_context( state: &AgentAppState, session_id: &str, ) -> Option { - let Some(handle) = state.app_handle.as_ref() else { + let Some(_handle) = state.app_handle.as_ref() else { tracing::debug!( session_id = %session_id, "[init] agent_org_context lookup skipped (no app_handle — headless context)" ); return None; }; - use tauri::Manager; - let org_store = handle.state::>(); match crate::coordination::agent_org_runs::AgentOrgRunStore::context_for_session_with_parent_walk( session_id, - org_store.inner(), ) { Ok(Some(ctx)) => { // Surfacing this at info is intentional: the runtime visibility diff --git a/src-tauri/crates/agent-core/src/lifecycle.rs b/src-tauri/crates/agent-core/src/lifecycle.rs index 2285cb946b..6e13b40aad 100644 --- a/src-tauri/crates/agent-core/src/lifecycle.rs +++ b/src-tauri/crates/agent-core/src/lifecycle.rs @@ -332,9 +332,7 @@ fn requeue_agent_org_member_in_progress_work( let Some(member_id) = record.org_member_id else { return Ok(None); }; - let store = crate::definitions::orgs::orgs_store(); - let Some(context) = AgentOrgRunStore::context_for_session_with_parent_walk(session_id, &store)? - else { + let Some(context) = AgentOrgRunStore::context_for_session_with_parent_walk(session_id)? else { return Ok(None); }; let member_agent_id = context @@ -760,7 +758,7 @@ mod tests { AgentOrgTaskStore, CreateTaskParams, TaskStatus, TASK_METADATA_ELIGIBLE_MEMBER_IDS, TASK_METADATA_REQUIRED_ROLE, }; - use crate::definitions::orgs::{HierarchyMode, OrgDefinition, OrgMember}; + use crate::definitions::orgs::{FlatOrgMember, OrgDefinition}; use crate::session::persistence::{session_type, UnifiedSessionRecord}; use crate::session::turn::member_idle::{MemberIdleHook, MemberIdleHookGuard}; use std::sync::{Arc, Mutex}; @@ -914,26 +912,25 @@ mod tests { role: "coordinator".to_string(), agent_id: "builtin:coord".to_string(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, - children: vec![ - OrgMember { - id: "member-worker".to_string(), + members: vec![ + FlatOrgMember { + member_id: "member-worker".to_string(), name: "Worker".to_string(), role: "builder".to_string(), agent_id: member_agent_id.to_string(), runtime_config: None, - children: Vec::new(), }, - OrgMember { - id: "member-peer".to_string(), + FlatOrgMember { + member_id: "member-peer".to_string(), name: "Peer".to_string(), role: "builder".to_string(), agent_id: "builtin:sde".to_string(), runtime_config: None, - children: Vec::new(), }, ], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), } } @@ -969,7 +966,7 @@ mod tests { org_id: "org-lifecycle".to_string(), coordinator_agent_id: "builtin:coord".to_string(), root_session_id: Some("root-session".to_string()), - org_snapshot: org_definition(member_agent_id), + org_snapshot: (&org_definition(member_agent_id)).into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, diff --git a/src-tauri/crates/agent-core/src/state/commands/routines.rs b/src-tauri/crates/agent-core/src/state/commands/routines.rs index f7aaea7c3d..93cb4df076 100644 --- a/src-tauri/crates/agent-core/src/state/commands/routines.rs +++ b/src-tauri/crates/agent-core/src/state/commands/routines.rs @@ -39,7 +39,7 @@ pub async fn project_fire_routine( /// manual path passes `None`. pub async fn fire_routine_internal( state: &AgentAppState, - org_store: &AgentOrgsStore, + org_store: &std::sync::Arc, app: &tauri::AppHandle, routine: &types::RoutineDefinition, idempotency_key: Option, @@ -72,7 +72,7 @@ pub async fn fire_routine_internal( /// Also the entry point for dequeued (Queued → Pending) fires. pub async fn execute_pending_fire( state: &AgentAppState, - org_store: &AgentOrgsStore, + org_store: &std::sync::Arc, app: &tauri::AppHandle, routine: &types::RoutineDefinition, pending_fire: &types::RoutineFire, @@ -157,7 +157,7 @@ pub fn emit_routine_changed( async fn launch_routine_direct_session( state: &AgentAppState, - org_store: &AgentOrgsStore, + org_store: &std::sync::Arc, routine: &types::RoutineDefinition, pending_fire: &types::RoutineFire, ) -> Result { diff --git a/src-tauri/crates/agent-core/src/state/commands/session/debug/org_runtime.rs b/src-tauri/crates/agent-core/src/state/commands/session/debug/org_runtime.rs index 12d4271b40..0824926db4 100644 --- a/src-tauri/crates/agent-core/src/state/commands/session/debug/org_runtime.rs +++ b/src-tauri/crates/agent-core/src/state/commands/session/debug/org_runtime.rs @@ -18,7 +18,6 @@ use crate::coordination::agent_org_runs::{ AgentOrgRunContext, AgentOrgRunStore, COORDINATOR_MEMBER_ID, }; use crate::coordination::agent_org_tasks::{AgentOrgTaskStore, Task}; -use crate::definitions::orgs::AgentOrgsStore; use crate::session::persistence; use crate::state::AgentAppState; use crate::tools::impls::orchestration::agent_org::tasks::{ @@ -271,7 +270,6 @@ pub async fn debug_session_execute_org_tool( #[tauri::command] pub async fn debug_agent_org_execute_tool_as_agent( - org_store: tauri::State<'_, std::sync::Arc>, run_id: String, sender_member_id: String, tool_name: String, @@ -287,7 +285,7 @@ pub async fn debug_agent_org_execute_tool_as_agent( )); } - let org_context = AgentOrgRunStore::context_for_run(&run_id, &org_store)? + let org_context = AgentOrgRunStore::context_for_run(&run_id)? .ok_or_else(|| format!("Agent Org run not found: {run_id}"))?; let sender = org_context .participant_by_member_id(&sender_member_id) @@ -376,7 +374,6 @@ fn task_tools_context( #[tauri::command] pub async fn debug_agent_org_emit_member_idle( - org_store: tauri::State<'_, std::sync::Arc>, run_id: String, member_id: String, reason: String, @@ -408,7 +405,7 @@ pub async fn debug_agent_org_emit_member_idle( })?), None => None, }; - let context = AgentOrgRunStore::context_for_run(&run_id, &org_store)? + let context = AgentOrgRunStore::context_for_run(&run_id)? .ok_or_else(|| format!("Agent Org run not found: {run_id}"))?; let member = context .participant_by_member_id(&member_id) diff --git a/src-tauri/crates/agent-core/src/state/commands/session/launch.rs b/src-tauri/crates/agent-core/src/state/commands/session/launch.rs index b1793b4d86..2c9d864b51 100644 --- a/src-tauri/crates/agent-core/src/state/commands/session/launch.rs +++ b/src-tauri/crates/agent-core/src/state/commands/session/launch.rs @@ -139,7 +139,7 @@ pub struct SessionLaunchResult { pub async fn session_launch_impl( state: &AgentAppState, - org_store: Option<&AgentOrgsStore>, + org_store: Option<&std::sync::Arc>, mut params: SessionLaunchParams, ) -> Result { validate_workspace_launch_fields( @@ -261,7 +261,7 @@ fn validate_workspace_launch_fields( async fn launch_rust_agent( state: &AgentAppState, - org_store: Option<&AgentOrgsStore>, + org_store: Option<&std::sync::Arc>, params: SessionLaunchParams, name: String, ) -> Result { diff --git a/src-tauri/crates/agent-core/src/state/commands/session/message/tests.rs b/src-tauri/crates/agent-core/src/state/commands/session/message/tests.rs index 7e90f77657..3f6b46b1a4 100644 --- a/src-tauri/crates/agent-core/src/state/commands/session/message/tests.rs +++ b/src-tauri/crates/agent-core/src/state/commands/session/message/tests.rs @@ -31,7 +31,7 @@ use crate::coordination::agent_org_tasks::{ enqueue_task_assigned_to_with_tasks, AgentOrgTaskStore, CreateTaskParams, TaskStatus, TASK_METADATA_ELIGIBLE_MEMBER_IDS, TASK_METADATA_EXECUTION_MODE, }; -use crate::definitions::orgs::{HierarchyMode, OrgDefinition, OrgMember, PlanApprovalPolicy}; +use crate::definitions::orgs::{FlatOrgMember, OrgDefinition, PlanApprovalPolicy}; use crate::session::{AgentExecMode, SessionStatus}; use core_types::key_source::KeySource; @@ -58,22 +58,22 @@ fn setup_wake_mode_fixture(execution_mode: &str, task_status: TaskStatus) -> Wak role: "Coordinator".into(), agent_id: "coordinator-agent".into(), description: None, - hierarchy_mode: HierarchyMode::Soft, plan_approval_policy: PlanApprovalPolicy::Coordinator, - children: vec![OrgMember { - id: member_id.clone(), + members: vec![FlatOrgMember { + member_id: member_id.clone(), name: "Planner".into(), role: "Planner".into(), agent_id: "planner-agent".into(), runtime_config: None, - children: Vec::new(), }], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), }; let run = AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id: org.id.clone(), coordinator_agent_id: org.agent_id.clone(), root_session_id: Some("root-session".into()), - org_snapshot: org, + org_snapshot: (&org).into(), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, diff --git a/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/context.rs b/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/context.rs index c1ad72b30e..3692fd2b0c 100644 --- a/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/context.rs +++ b/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/context.rs @@ -66,10 +66,7 @@ pub(super) async fn session_org_read_context( let context = match runtime_context { Some(context) => Some(context), None => match org_store { - Some(store) => AgentOrgRunStore::context_for_session_with_parent_walk( - &session_id, - store.as_ref(), - )?, + Some(_) => AgentOrgRunStore::context_for_session_with_parent_walk(&session_id)?, None => None, }, }; diff --git a/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/run_view.rs b/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/run_view.rs index a409fcd3ad..03c18c82ed 100644 --- a/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/run_view.rs +++ b/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/run_view.rs @@ -57,7 +57,6 @@ pub struct AgentOrgRunMemberView { pub name: String, pub role: String, pub agent_id: String, - pub parent_member_id: Option, pub is_coordinator: bool, pub session_runtime: Option, pub unread_inbox_count: usize, @@ -604,7 +603,6 @@ fn coordinator_member_view( name: context.coordinator_name.clone(), role: context.coordinator_role.clone(), agent_id: context.coordinator_agent_id.clone(), - parent_member_id: None, is_coordinator: true, }, runtime, @@ -627,7 +625,6 @@ fn member_view( name: member.name.clone(), role: member.role.clone(), agent_id: member.agent_id.clone(), - parent_member_id: member.parent_member_id.clone(), is_coordinator: false, }, runtime, @@ -642,7 +639,6 @@ struct AgentOrgMemberViewIdentity { name: String, role: String, agent_id: String, - parent_member_id: Option, is_coordinator: bool, } @@ -658,7 +654,6 @@ fn member_view_from_parts( name, role, agent_id, - parent_member_id, is_coordinator, } = identity; let (inbox_activity_count, unread_inbox_count) = inbox_counts @@ -696,7 +691,6 @@ fn member_view_from_parts( name, role, agent_id, - parent_member_id, is_coordinator, session_runtime, unread_inbox_count, diff --git a/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/tests.rs b/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/tests.rs index 8e160cd8f0..d3ae7a4f8c 100644 --- a/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/tests.rs +++ b/src-tauri/crates/agent-core/src/state/commands/session/org_tasks/tests.rs @@ -22,7 +22,6 @@ use crate::coordination::agent_org_runs::{ AgentOrgContextMember, AgentOrgRunContext, AgentOrgRunStatus, COORDINATOR_MEMBER_ID, }; use crate::coordination::agent_org_tasks::{Task, TaskExecutionMode, TaskStatus, TaskSummary}; -use crate::definitions::orgs::HierarchyMode; fn context_with_shared_member_agent_id() -> AgentOrgRunContext { AgentOrgRunContext { @@ -39,18 +38,16 @@ fn context_with_shared_member_agent_id() -> AgentOrgRunContext { name: "Planner".to_string(), role: "Plan work".to_string(), agent_id: "builtin:sde".to_string(), - parent_member_id: None, }, AgentOrgContextMember { member_id: "member-builder".to_string(), name: "Builder".to_string(), role: "Build work".to_string(), agent_id: "builtin:sde".to_string(), - parent_member_id: Some("member-planner".to_string()), }, ], - hierarchy_mode: HierarchyMode::Strict, plan_approval_policy: crate::definitions::orgs::PlanApprovalPolicy::Coordinator, + capability_index: Default::default(), root_session_id: Some("root-shared-agent".to_string()), } } diff --git a/src-tauri/crates/types/src/agent_org.rs b/src-tauri/crates/types/src/agent_org.rs new file mode 100644 index 0000000000..f37922dd46 --- /dev/null +++ b/src-tauri/crates/types/src/agent_org.rs @@ -0,0 +1,12 @@ +//! Shared Agent Org (Team) identity constants. + +/// Reserved member id of the Team coordinator. +/// +/// The coordinator is not part of `members`; this id addresses it in task +/// ownership, inbox routing, and plan-approval flows. Definition and +/// snapshot validators reject `members` entries that claim this id. +/// +/// Single source of truth for both the definitions store +/// (`agent-core::core::definitions::orgs`) and the run coordination layer +/// (`agent-core::core::coordination::agent_org_runs`). +pub const COORDINATOR_MEMBER_ID: &str = "coordinator"; diff --git a/src-tauri/crates/types/src/lib.rs b/src-tauri/crates/types/src/lib.rs index 8a9ffc47fc..e5b948373a 100644 --- a/src-tauri/crates/types/src/lib.rs +++ b/src-tauri/crates/types/src/lib.rs @@ -23,6 +23,7 @@ //! prefer `serde`-derive-style annotations over impl blocks where possible. pub mod activity; +pub mod agent_org; pub mod cli_alias; pub mod extracted; pub mod jsonrpc; diff --git a/src-tauri/src/agent_sessions/session_directory/aggregation.rs b/src-tauri/src/agent_sessions/session_directory/aggregation.rs index 4aa38a18a8..4ee5386def 100644 --- a/src-tauri/src/agent_sessions/session_directory/aggregation.rs +++ b/src-tauri/src/agent_sessions/session_directory/aggregation.rs @@ -9,7 +9,7 @@ use std::collections::HashSet; use crate::agent_sessions::cli::persistence as cli_session_persistence; use agent_core::coordination::agent_org_runs::{AgentOrgRunRecord, AgentOrgRunStore}; -use agent_core::definitions::orgs::OrgDefinition; +use agent_core::definitions::orgs::AgentOrgLaunchSnapshot; use agent_core::session::persistence::{ self as session_persistence, list_agent_org_root_sessions_page, list_standalone_coding_sessions_page, list_unpinned_sessions_by_type_page, session_type, @@ -1123,8 +1123,8 @@ fn apply_pagination(sessions: &mut Vec, filter: &Session fn agent_org_display_name(run: &AgentOrgRunRecord) -> String { run.org_snapshot_json .as_deref() - .and_then(|json| serde_json::from_str::(json).ok()) - .map(|org| org.name) + .and_then(|json| serde_json::from_str::(json).ok()) + .map(|snapshot| snapshot.org_name) .unwrap_or_else(|| run.org_id.clone()) } @@ -1653,6 +1653,114 @@ mod tests { } } + #[test] + fn orphan_org_members_do_not_break_standalone_cursor_or_has_more() { + let _sandbox = crate::test_utils::test_env::sandbox(); + let conn = get_connection().expect("sandbox database"); + + for (session_id, updated_at, org_member_id) in [ + ( + "orphan-coordinator-new", + "2026-07-30T16:00:00Z", + Some("coordinator"), + ), + ("standalone-new", "2026-07-30T15:00:00Z", None), + ( + "orphan-coordinator-mid", + "2026-07-30T14:00:00Z", + Some("coordinator"), + ), + ("standalone-mid", "2026-07-30T13:00:00Z", None), + ("orphan-member", "2026-07-30T12:00:00Z", Some("alice")), + ("standalone-old", "2026-07-30T11:00:00Z", None), + ( + "valid-org-root", + "2026-07-30T10:00:00Z", + Some("coordinator"), + ), + ] { + session_persistence::upsert_session(&UnifiedSessionRecord { + session_id: session_id.to_string(), + name: session_id.to_string(), + status: "idle".to_string(), + session_type: session_type::CODING.to_string(), + org_member_id: org_member_id.map(str::to_string), + created_at: updated_at.to_string(), + updated_at: updated_at.to_string(), + ..Default::default() + }) + .expect("seed coding session"); + } + conn.execute( + "INSERT INTO agent_org_runs ( + id, org_id, coordinator_agent_id, root_session_id, + entry_mode, status, created_at, updated_at + ) VALUES ( + 'valid-org-run', 'valid-org', 'builtin:sde', 'valid-org-root', + 'standalone_session', 'idle', + '2026-07-30T10:00:00Z', '2026-07-30T10:00:00Z' + )", + [], + ) + .expect("seed valid Agent Org run"); + + let first = + list_native_sidebar_sessions(NativeSidebarSessionStream::StandaloneAgent, None, 2) + .expect("first standalone page"); + assert_eq!( + first + .sessions + .iter() + .map(|session| session.session_id.as_str()) + .collect::>(), + vec!["standalone-new", "standalone-mid"] + ); + assert!(first.has_more); + assert_eq!( + first.next_cursor.as_ref(), + Some(&NativeSidebarSessionCursor { + updated_at: "2026-07-30T13:00:00Z".to_string(), + session_id: "standalone-mid".to_string(), + }) + ); + + let second = list_native_sidebar_sessions( + NativeSidebarSessionStream::StandaloneAgent, + first.next_cursor.as_ref(), + 2, + ) + .expect("second standalone page"); + assert_eq!( + second + .sessions + .iter() + .map(|session| session.session_id.as_str()) + .collect::>(), + vec!["standalone-old"] + ); + assert!(!second.has_more); + assert_eq!( + second.next_cursor.as_ref(), + Some(&NativeSidebarSessionCursor { + updated_at: "2026-07-30T11:00:00Z".to_string(), + session_id: "standalone-old".to_string(), + }) + ); + + let roots = list_native_sidebar_sessions(NativeSidebarSessionStream::AgentOrgRoot, None, 2) + .expect("Agent Org root page"); + assert_eq!( + roots + .sessions + .iter() + .map(|session| session.session_id.as_str()) + .collect::>(), + vec!["valid-org-root"] + ); + assert_eq!(roots.sessions[0].agent_org_id.as_deref(), Some("valid-org")); + assert!(!roots.has_more); + } + #[test] fn pinned_native_page_merges_agent_and_cli_roots_in_stable_order() { let _sandbox = crate::test_utils::test_env::sandbox(); diff --git a/src-tauri/src/api/agent/test/agent_org.rs b/src-tauri/src/api/agent/test/agent_org.rs index bab0589a0e..4100c2b439 100644 --- a/src-tauri/src/api/agent/test/agent_org.rs +++ b/src-tauri/src/api/agent/test/agent_org.rs @@ -59,7 +59,10 @@ use agent_core::coordination::agent_org_runs::{ AgentOrgContextMember, AgentOrgRunContext, AgentOrgRunStore, }; use agent_core::coordination::agent_org_tasks::TASK_METADATA_ELIGIBLE_MEMBER_IDS; -use agent_core::definitions::orgs::{orgs_store, AgentOrgsStore, OrgDefinition, OrgMember}; +use agent_core::definitions::orgs::{ + all_member_links, AgentOrgCapabilityIndex, AgentOrgLaunchSnapshot, AgentOrgsStore, + FlatOrgMember, OrgDefinition, +}; use agent_core::state::commands::session::org_tasks::agent_org_session_run_view_impl; use agent_core::tools::error::ToolError; use agent_core::tools::impls::orchestration::agent_org::tasks::{ @@ -125,7 +128,7 @@ pub async fn test_agent_org_seed(Json(body): Json) -> Json = Vec::with_capacity(members_array.len()); + let mut members: Vec = Vec::with_capacity(members_array.len()); for (idx, item) in members_array.into_iter().enumerate() { let Some(obj) = item.as_object() else { return Json(serde_json::json!({ @@ -161,26 +164,27 @@ pub async fn test_agent_org_seed(Json(body): Json) -> Json, String>>()?; - let org_snapshot = OrgDefinition { + let mut org_definition = OrgDefinition { id: org_id.clone(), name: org_id.clone(), role: "coordinator".to_string(), agent_id: coordinator_agent_id.clone(), description: None, - hierarchy_mode: Default::default(), plan_approval_policy: Default::default(), - children: org_snapshot_children, + members: org_snapshot_members, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), }; + org_definition.member_communication_links = all_member_links(&org_definition.members); + let org_snapshot = AgentOrgLaunchSnapshot::from(&org_definition); let run = AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id, @@ -2170,7 +2176,6 @@ pub async fn test_agent_org_check_member_spawn_gate( .unwrap_or("worker") .to_string(), agent_id, - parent_member_id: None, }); } Some(AgentOrgRunContext { @@ -2182,8 +2187,8 @@ pub async fn test_agent_org_check_member_spawn_gate( coordinator_name: pluck_str("coordinator_name"), coordinator_role: pluck_str("coordinator_role"), members, - hierarchy_mode: Default::default(), plan_approval_policy: Default::default(), + capability_index: AgentOrgCapabilityIndex::default(), root_session_id: None, }) } @@ -2410,7 +2415,6 @@ pub async fn test_agent_org_post_member_idle( name, role, agent_id, - parent_member_id: None, }); } @@ -2423,8 +2427,8 @@ pub async fn test_agent_org_post_member_idle( coordinator_name, coordinator_role, members, - hierarchy_mode: Default::default(), plan_approval_policy: Default::default(), + capability_index: AgentOrgCapabilityIndex::default(), root_session_id: None, }; @@ -2519,7 +2523,7 @@ pub async fn test_agent_org_run_seed( let Some(members) = obj.get("members").and_then(|value| value.as_array()) else { return Json(serde_json::json!({ "ok": false, "error": "members must be an array" })); }; - let mut children = Vec::with_capacity(members.len()); + let mut flat_members = Vec::with_capacity(members.len()); for (index, member) in members.iter().enumerate() { let Some(member) = member.as_object() else { return Json(serde_json::json!({ @@ -2544,8 +2548,8 @@ pub async fn test_agent_org_run_seed( "error": format!("members[{index}].agent_id is required") })); }; - children.push(OrgMember { - id: member_id.clone(), + flat_members.push(FlatOrgMember { + member_id: member_id.clone(), name: member .get("name") .and_then(|value| value.as_str()) @@ -2560,25 +2564,27 @@ pub async fn test_agent_org_run_seed( .to_string(), agent_id: agent_id.to_string(), runtime_config: None, - children: Vec::new(), }); } let result = tokio::task::spawn_blocking(move || { + let mut definition = OrgDefinition { + id: org_id.clone(), + name: org_name, + role: org_role, + agent_id: coordinator_agent_id.clone(), + description: Some("Deterministic real-binary E2E fixture".to_string()), + plan_approval_policy: Default::default(), + members: flat_members, + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), + }; + definition.member_communication_links = all_member_links(&definition.members); AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id: org_id.clone(), coordinator_agent_id: coordinator_agent_id.clone(), root_session_id: None, - org_snapshot: OrgDefinition { - id: org_id, - name: org_name, - role: org_role, - agent_id: coordinator_agent_id, - description: Some("Deterministic real-binary E2E fixture".to_string()), - hierarchy_mode: Default::default(), - plan_approval_policy: Default::default(), - children, - }, + org_snapshot: AgentOrgLaunchSnapshot::from(&definition), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, @@ -2803,7 +2809,7 @@ pub async fn test_agent_org_tasks_seed( let eligible_member_ids = match requested_eligible_member_ids { Some(member_ids) => Some(member_ids), None if params.owner.is_none() && params.status == TaskStatus::Pending => Some( - AgentOrgRunStore::context_for_run(¶ms.org_run_id, &orgs_store())? + AgentOrgRunStore::context_for_run(¶ms.org_run_id)? .map(|context| { context .members @@ -3019,7 +3025,9 @@ pub async fn test_agent_org_seed_cli_member_run( AgentOrgRunEntryMode, AgentOrgRunStatus, AgentOrgRunStore, CreateAgentOrgRunParams, COORDINATOR_MEMBER_ID, }; - use agent_core::core::definitions::orgs::{OrgDefinition, OrgMember}; + use agent_core::core::definitions::orgs::{ + AgentOrgLaunchSnapshot, FlatOrgMember, OrgDefinition, + }; use agent_core::core::session::persistence::{ session_type, upsert_session, UnifiedSessionRecord, }; @@ -3083,29 +3091,29 @@ pub async fn test_agent_org_seed_cli_member_run( }) .map_err(|err| err.to_string())?; - let org_snapshot = OrgDefinition { + let org_definition = OrgDefinition { id: org_id.clone(), name: org_id.clone(), role: "coordinator".to_string(), agent_id: coordinator_agent_id.clone(), description: None, - hierarchy_mode: Default::default(), plan_approval_policy: Default::default(), - children: vec![OrgMember { - id: member_id.clone(), + members: vec![FlatOrgMember { + member_id: member_id.clone(), name: member_id.clone(), role: "worker".to_string(), agent_id: format!("cli:{cli_agent_type}"), runtime_config: None, - children: Vec::new(), }], + additional_task_graph_writer_member_ids: Vec::new(), + member_communication_links: Vec::new(), }; let run = AgentOrgRunStore::create(CreateAgentOrgRunParams { org_id, coordinator_agent_id, root_session_id: Some(root_session_id.clone()), - org_snapshot, + org_snapshot: AgentOrgLaunchSnapshot::from(&org_definition), entry_mode: AgentOrgRunEntryMode::StandaloneSession, status: AgentOrgRunStatus::Running, work_item_id: None, diff --git a/src-tauri/src/commands/handler_list.inc b/src-tauri/src/commands/handler_list.inc index 95f0ae41c7..1ae1291dc5 100644 --- a/src-tauri/src/commands/handler_list.inc +++ b/src-tauri/src/commands/handler_list.inc @@ -955,8 +955,7 @@ agent_core::definitions::commands::integrations_get, agent_core::definitions::commands::integrations_update_patch, // Agent Organizations agent_core::definitions::commands::agent_orgs_list, -agent_core::definitions::commands::agent_orgs_add, -agent_core::definitions::commands::agent_orgs_update, +agent_core::definitions::commands::agent_orgs_save_trusted_settings, agent_core::definitions::commands::agent_orgs_remove, agent_core::definitions::commands::agent_org_run_list, agent_core::state::commands::session::debug::org_runtime::debug_session_org_runtime_snapshot, diff --git a/src/api/tauri/agent/orgTasks.ts b/src/api/tauri/agent/orgTasks.ts index b0139999d1..8d2b5e3c91 100644 --- a/src/api/tauri/agent/orgTasks.ts +++ b/src/api/tauri/agent/orgTasks.ts @@ -40,7 +40,6 @@ export interface AgentOrgRunContextMember { name: string; role: string; agentId: string; - parentMemberId?: string | null; } export interface AgentOrgRunContext { @@ -52,7 +51,6 @@ export interface AgentOrgRunContext { coordinatorName: string; coordinatorRole: string; members: AgentOrgRunContextMember[]; - hierarchyMode: string; planApprovalPolicy: "coordinator" | "user" | "automatic"; /** Session ID of the coordinator (root) session. Used to navigate directly * to the coordinator's chat history when the run is paused or the user @@ -66,7 +64,6 @@ export interface AgentOrgRunMemberView { name: string; role: string; agentId: string; - parentMemberId?: string | null; isCoordinator: boolean; sessionRuntime?: AgentOrgOwnerRuntime | null; unreadInboxCount: number; diff --git a/src/api/tauri/rpc/procedures/agentOrgs.ts b/src/api/tauri/rpc/procedures/agentOrgs.ts index 29df587d4a..3454b13d16 100644 --- a/src/api/tauri/rpc/procedures/agentOrgs.ts +++ b/src/api/tauri/rpc/procedures/agentOrgs.ts @@ -176,13 +176,11 @@ const memory = { const orgs = { list: defineProcedure("agent_orgs_list") - .output(z.array(schemas.agentOrgs.OrgMemberSchema)) + .output(z.array(schemas.agentOrgs.OrgDefinitionSchema)) .build(), - add: defineProcedure("agent_orgs_add") - .input(schemas.agentOrgs.OrgJsonInput) - .build(), - update: defineProcedure("agent_orgs_update") + saveTrustedSettings: defineProcedure("agent_orgs_save_trusted_settings") .input(schemas.agentOrgs.OrgJsonInput) + .output(schemas.agentOrgs.OrgDefinitionSchema) .build(), remove: defineProcedure("agent_orgs_remove") .input(schemas.agentOrgs.OrgIdInput) diff --git a/src/api/tauri/rpc/schemas/agentOrgs.test.ts b/src/api/tauri/rpc/schemas/agentOrgs.test.ts new file mode 100644 index 0000000000..bc47cda2dd --- /dev/null +++ b/src/api/tauri/rpc/schemas/agentOrgs.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from "vitest"; + +import { OrgDefinitionSchema } from "./agentOrgs"; + +const validDefinition = { + id: "team-1", + name: "Delivery", + role: "Coordinator", + agentId: "builtin:sde", + planApprovalPolicy: "coordinator", + members: [ + { + memberId: "alice", + name: "Alice", + role: "Builder", + agentId: "builtin:sde", + }, + { + memberId: "bob", + name: "Bob", + role: "Reviewer", + agentId: "builtin:sde", + }, + ], + additionalTaskGraphWriterMemberIds: ["alice"], + memberCommunicationLinks: [{ memberAId: "alice", memberBId: "bob" }], +}; + +describe("Agent Org trusted settings wire schema", () => { + it("accepts the complete flat capability payload", () => { + expect(OrgDefinitionSchema.parse(validDefinition)).toEqual(validDefinition); + }); + + it("rejects recursive hierarchy payloads", () => { + expect(() => + OrgDefinitionSchema.parse({ + ...validDefinition, + hierarchyMode: "strict", + }) + ).toThrow(); + expect(() => + OrgDefinitionSchema.parse({ + ...validDefinition, + members: [ + { + ...validDefinition.members[0], + children: [validDefinition.members[1]], + }, + ], + }) + ).toThrow(); + }); + + it.each(["additionalTaskGraphWriterMemberIds", "memberCommunicationLinks"])( + "rejects a payload missing %s", + (field) => { + const payload = { ...validDefinition } as Record; + delete payload[field]; + expect(() => OrgDefinitionSchema.parse(payload)).toThrow(); + } + ); +}); diff --git a/src/api/tauri/rpc/schemas/agentOrgs.ts b/src/api/tauri/rpc/schemas/agentOrgs.ts index e15ca5717c..c088ffc874 100644 --- a/src/api/tauri/rpc/schemas/agentOrgs.ts +++ b/src/api/tauri/rpc/schemas/agentOrgs.ts @@ -125,7 +125,6 @@ export const CliConfigFileWriteInput = CliConfigFileInput.extend({ content: z.string(), }); -export const HierarchyModeSchema = z.enum(["flat", "soft", "strict"]); export const PlanApprovalPolicySchema = z.enum([ "coordinator", "user", @@ -148,31 +147,36 @@ export type OrgMemberRuntimeConfig = z.infer< typeof OrgMemberRuntimeConfigSchema >; -export type OrgMember = { - id: string; - name: string; - role: string; - agentId: string; - runtimeConfig?: OrgMemberRuntimeConfig; - description?: string; - hierarchyMode?: z.output; - planApprovalPolicy?: z.output; - children: OrgMember[]; -}; - -export const OrgMemberSchema: z.ZodType = z.lazy(() => - z.object({ - id: z.string(), +export const FlatOrgMemberSchema = z + .object({ + memberId: z.string(), name: z.string(), role: z.string(), agentId: z.string(), runtimeConfig: OrgMemberRuntimeConfigSchema.optional(), + }) + .strict(); + +export const MemberCommunicationLinkSchema = z + .object({ + memberAId: z.string(), + memberBId: z.string(), + }) + .strict(); + +export const OrgDefinitionSchema = z + .object({ + id: z.string(), + name: z.string(), + role: z.string(), + agentId: z.string(), description: z.string().optional(), - hierarchyMode: HierarchyModeSchema.optional(), - planApprovalPolicy: PlanApprovalPolicySchema.optional(), - children: z.array(OrgMemberSchema), + planApprovalPolicy: PlanApprovalPolicySchema, + members: z.array(FlatOrgMemberSchema), + additionalTaskGraphWriterMemberIds: z.array(z.string()), + memberCommunicationLinks: z.array(MemberCommunicationLinkSchema), }) -); + .strict(); export const OrgJsonInput = z.object({ orgJson: z.string(), diff --git a/src/components/TeamMemberTable/index.tsx b/src/components/TeamMemberTable/index.tsx index 9ead2b3fea..9241e50575 100644 --- a/src/components/TeamMemberTable/index.tsx +++ b/src/components/TeamMemberTable/index.tsx @@ -1,14 +1,9 @@ -/** - * TeamMemberTable — inline-editable table for agent team members. - * - * Built on the shared DragTable component for drag-to-reorder. - * Supports hierarchy via optional parentId field + "Reports to" column. - */ import { Plus, Trash2 } from "lucide-react"; import React, { useCallback, useMemo } from "react"; import { useTranslation } from "react-i18next"; import Button from "@src/components/Button"; +import Checkbox from "@src/components/Checkbox"; import DragTable, { type DragTableColumn } from "@src/components/DragTable"; import { DROPDOWN_CLASSES, @@ -16,61 +11,40 @@ import { DropdownFooter, } from "@src/components/Dropdown/exports"; import Input from "@src/components/Input"; -import Select from "@src/components/Select"; -import type { SelectOption } from "@src/components/Select"; +import Select, { type SelectOption } from "@src/components/Select"; import type { OrgMemberRuntimeConfig } from "@src/modules/MainApp/AgentOrgs/types"; -// ── Types ── - export interface TeamMember { id: string; name: string; role: string; agentId: string; runtimeConfig?: OrgMemberRuntimeConfig; - parentId?: string; } export interface TeamMemberTableProps { members: TeamMember[]; onChange: (members: TeamMember[]) => void; agentOptions: SelectOption[]; - /** Callback when user clicks "+ Add Agent" in the agent selector */ + writerMemberIds: ReadonlySet; + connectedCountByMemberId: ReadonlyMap; + onWriterChange: (memberId: string, checked: boolean) => void; + onManageCommunication: (memberId: string) => void; + onMemberAdded?: (memberId: string) => void; + onMemberRemoved?: (memberId: string) => void; onAddAgent?: () => void; - /** Header height: "compact" (32px) or "tall" (40px, default) — matches SettingsTable */ headerHeight?: "compact" | "tall"; - /** - * IDs of rows whose `name` should render with an error state (e.g. duplicate - * names within an Agent Team's routing namespace). The table itself is - * routing-agnostic; callers decide what counts as an error. - */ invalidNameRowIds?: ReadonlySet; - /** Tooltip surfaced on the name input for rows in `invalidNameRowIds`. */ invalidNameMessage?: string; - /** - * Hide the `Reports to` column entirely. Used by callers (Agent Team - * wizard) when the underlying hierarchy mode treats reports-to as - * meaningless — e.g. `HierarchyMode.flat`. The `parentId` field on - * each row is still preserved on the wire so toggling the column back - * on does not lose data. - */ - hideReportsTo?: boolean; - /** - * IDs of rows whose `parentId` should render with a warning state - * (e.g. members without a manager in `HierarchyMode.strict`, where - * they can only reach the coordinator). Independent from - * `invalidNameRowIds`; both can apply to the same row. - */ - warnReportsToRowIds?: ReadonlySet; - /** Tooltip surfaced on the reports-to cell for rows in `warnReportsToRowIds`. */ - warnReportsToMessage?: string; dataTestIdPrefix?: string; labels?: { name?: string; role?: string; agent?: string; - reportsTo?: string; - reportsToCoordinator?: string; + writer?: string; + connected?: string; + connectedCount?: (count: number) => string; + manageCommunication?: string; addMember?: string; namePlaceholder?: string; rolePlaceholder?: string; @@ -78,8 +52,6 @@ export interface TeamMemberTableProps { }; } -// ── Agent Select with footer action ── - interface AgentSelectProps { value: string; options: SelectOption[]; @@ -96,20 +68,11 @@ const AgentSelect: React.FC = ({ dataTestId, }) => { const { t } = useTranslation(); - - const handleChange = useCallback( - (val: string | number | (string | number)[]) => { - const strVal = String(val); - onChange(strVal); - }, - [onChange] - ); - const dropdownRender = useCallback( (menu: React.ReactNode) => (
{menu} - {onAddAgent && ( + {onAddAgent ? ( - )} + ) : null}
), [onAddAgent, t] ); - return ( updateMember(row.id, "name", val)} - placeholder={namePlaceholder} - size="default" - className="w-full" - data-testid={buildDataTestId(row, "name-input")} - error={hasNameError} - title={hasNameError ? invalidNameMessage : undefined} - /> - ); - }, + label: labels.name ?? "Name", + renderCell: (row) => ( + updateMember(row.id, "name", value)} + placeholder={labels.namePlaceholder} + size="default" + className="w-full" + data-testid={buildDataTestId(row, "name-input")} + error={invalidNameRowIds?.has(row.id) ?? false} + title={ + invalidNameRowIds?.has(row.id) ? invalidNameMessage : undefined + } + /> + ), }, { key: "role", - label: roleLabel, + label: labels.role ?? "Role", renderCell: (row) => ( updateMember(row.id, "role", val)} - placeholder={rolePlaceholder} + onChange={(value) => updateMember(row.id, "role", value)} + placeholder={labels.rolePlaceholder} size="default" className="w-full" data-testid={buildDataTestId(row, "role-input")} @@ -279,98 +196,104 @@ const TeamMemberTable: React.FC = ({ }, { key: "agent", - label: agentLabel, + label: labels.agent ?? "Agent", renderCell: (row) => ( updateMember(row.id, "agentId", val)} + onChange={(value) => updateMember(row.id, "agentId", value)} dataTestId={buildDataTestId(row, "agent-select")} /> ), }, - ]; - - if (!hideReportsTo) { - baseColumns.push({ - key: "reportsTo", - label: reportsToLabel, - renderCell: (row) => { - const warn = warnReportsToRowIds?.has(row.id) ?? false; - return ( -
- onChange(next as HierarchyMode)} - options={options} - style={SECTION_CONTROL_STYLE} - dataTestId="agent-orgs-hierarchy-mode-select" - /> -
- ); -}; - -export default HierarchyModeSelector; diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/MemberCommunicationPanel.test.ts b/src/scaffold/WizardSystem/variants/AgentOrg/MemberCommunicationPanel.test.ts new file mode 100644 index 0000000000..ff2aae00cd --- /dev/null +++ b/src/scaffold/WizardSystem/variants/AgentOrg/MemberCommunicationPanel.test.ts @@ -0,0 +1,200 @@ +// @vitest-environment jsdom +import { Provider, createStore } from "jotai"; +import React, { act } from "react"; +import { type Root, createRoot } from "react-dom/client"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; + +import type { TeamMember } from "@src/components/TeamMemberTable"; +import { activeOverlayCountAtom } from "@src/store/ui/overlayLayerAtom"; + +import MemberCommunicationPanel from "./MemberCommunicationPanel"; +import { allMemberPairKeys } from "./orgTree"; + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string, args?: { name?: string; defaultValue?: string }) => + args?.defaultValue ?? args?.name ?? key, + }), +})); + +const roster: TeamMember[] = [ + { id: "alice", name: "Alice", role: "Builder", agentId: "builtin:sde" }, + { id: "bob", name: "Bob", role: "Reviewer", agentId: "builtin:sde" }, + { id: "carol", name: "Carol", role: "Planner", agentId: "builtin:sde" }, +]; + +describe("MemberCommunicationPanel", () => { + let container: HTMLDivElement; + let root: Root; + let store: ReturnType; + const actEnvironment = globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + }; + + beforeAll(() => { + actEnvironment.IS_REACT_ACT_ENVIRONMENT = true; + }); + + beforeEach(() => { + store = createStore(); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + }); + + afterAll(() => { + Reflect.deleteProperty(actEnvironment, "IS_REACT_ACT_ENVIRONMENT"); + }); + + const renderPanel = async (pairKeys: ReadonlySet) => { + await act(async () => { + root.render( + React.createElement( + Provider, + { store }, + React.createElement(MemberCommunicationPanel, { + selectedMemberId: "alice", + members: roster, + pairKeys, + onPairChange: vi.fn(), + onClose: vi.fn(), + }) + ) + ); + await Promise.resolve(); + }); + }; + + it("renders only the selected Member's peers with all new-Team pairs checked", async () => { + await renderPanel(allMemberPairKeys(roster)); + + expect(document.body.textContent).toContain("Alice communication"); + expect( + document.querySelector('[data-testid="agent-orgs-peer-row-alice"]') + ).toBeNull(); + expect( + document.querySelectorAll('[data-testid^="agent-orgs-peer-row-"]') + ).toHaveLength(2); + expect( + [ + ...document.querySelectorAll("[data-checkbox-input]"), + ].every((input) => input.checked) + ).toBe(true); + expect(store.get(activeOverlayCountAtom)).toBe(1); + }); + + it("filters by role without changing hidden pair state", async () => { + const pairs = allMemberPairKeys(roster); + await renderPanel(pairs); + const search = document.querySelector( + '[data-testid="agent-orgs-communication-panel-search"]' + ); + expect(search).not.toBeNull(); + await act(async () => { + const setter = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + "value" + )?.set; + setter?.call(search, "planner"); + search?.dispatchEvent(new Event("input", { bubbles: true })); + }); + + expect( + document.querySelector('[data-testid="agent-orgs-peer-row-bob"]') + ).toBeNull(); + expect( + document.querySelector('[data-testid="agent-orgs-peer-row-carol"]') + ).not.toBeNull(); + expect(pairs.size).toBe(3); + }); + + it("renders exactly 49 peers for a 50-Member Team", async () => { + const largeRoster = Array.from({ length: 50 }, (_, index) => ({ + id: `member-${index}`, + name: `Member ${index}`, + role: "Builder", + agentId: "builtin:sde", + })); + await act(async () => { + root.render( + React.createElement( + Provider, + { store }, + React.createElement(MemberCommunicationPanel, { + selectedMemberId: "member-0", + members: largeRoster, + pairKeys: allMemberPairKeys(largeRoster), + onPairChange: vi.fn(), + onClose: vi.fn(), + }) + ) + ); + await Promise.resolve(); + }); + + expect( + document.querySelectorAll('[data-testid^="agent-orgs-peer-row-"]') + ).toHaveLength(49); + expect(document.querySelectorAll("[data-checkbox-input]")).toHaveLength(49); + }); + + it("closes on Escape, stops propagation, and returns focus to the opener", async () => { + const opener = document.createElement("button"); + const onClose = vi.fn(); + document.body.appendChild(opener); + opener.focus(); + const outerKeyHandler = vi.fn(); + document.body.addEventListener("keydown", outerKeyHandler); + + await act(async () => { + root.render( + React.createElement( + Provider, + { store }, + React.createElement(MemberCommunicationPanel, { + selectedMemberId: "alice", + members: roster, + pairKeys: allMemberPairKeys(roster), + onPairChange: vi.fn(), + onClose, + }) + ) + ); + await Promise.resolve(); + }); + const search = document.querySelector( + '[data-testid="agent-orgs-communication-panel-search"]' + ); + expect(search).not.toBeNull(); + act(() => search?.focus()); + expect(document.activeElement).toBe(search); + + await act(async () => { + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "Escape", bubbles: true }) + ); + root.render(React.createElement(Provider, { store }, null)); + await Promise.resolve(); + }); + + expect(onClose).toHaveBeenCalledOnce(); + expect(outerKeyHandler).not.toHaveBeenCalled(); + expect(document.activeElement).toBe(opener); + document.body.removeEventListener("keydown", outerKeyHandler); + opener.remove(); + }); +}); diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/MemberCommunicationPanel.tsx b/src/scaffold/WizardSystem/variants/AgentOrg/MemberCommunicationPanel.tsx new file mode 100644 index 0000000000..9f30e1dc0a --- /dev/null +++ b/src/scaffold/WizardSystem/variants/AgentOrg/MemberCommunicationPanel.tsx @@ -0,0 +1,193 @@ +import { X } from "lucide-react"; +import React, { + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from "react"; +import { createPortal } from "react-dom"; +import { useTranslation } from "react-i18next"; + +import Button from "@src/components/Button"; +import Checkbox from "@src/components/Checkbox"; +import Input from "@src/components/Input"; +import type { TeamMember } from "@src/components/TeamMemberTable"; +import { POPUP_Z_INDEX } from "@src/scaffold/shared/popupTokens"; +import { useOverlayLayer } from "@src/store/ui/overlayLayerAtom"; + +import { canonicalPairKey } from "./orgTree"; + +interface MemberCommunicationPanelProps { + selectedMemberId: string | null; + members: TeamMember[]; + pairKeys: ReadonlySet; + onPairChange: ( + memberAId: string, + memberBId: string, + checked: boolean + ) => void; + onClose: () => void; +} + +export default function MemberCommunicationPanel({ + selectedMemberId, + members, + pairKeys, + onPairChange, + onClose, +}: MemberCommunicationPanelProps) { + const { t } = useTranslation("integrations"); + const [query, setQuery] = useState(""); + const searchRef = useRef(null); + const panelRef = useRef(null); + const selected = members.find((member) => member.id === selectedMemberId); + useOverlayLayer(Boolean(selectedMemberId && selected)); + + useEffect(() => { + if (!selectedMemberId) return; + const previous = document.activeElement as HTMLElement | null; + const frame = requestAnimationFrame(() => searchRef.current?.focus()); + return () => { + cancelAnimationFrame(frame); + previous?.focus(); + }; + }, [selectedMemberId]); + + const closePanel = useCallback(() => { + setQuery(""); + onClose(); + }, [onClose]); + + useEffect(() => { + if (!selectedMemberId) return; + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === "Escape") { + event.preventDefault(); + event.stopPropagation(); + closePanel(); + return; + } + if (event.key !== "Tab") return; + const focusable = panelRef.current?.querySelectorAll( + 'button:not([disabled]), input:not([disabled]), [tabindex]:not([tabindex="-1"])' + ); + if (!focusable?.length) return; + const first = focusable[0]; + const last = focusable[focusable.length - 1]; + if (event.shiftKey && document.activeElement === first) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && document.activeElement === last) { + event.preventDefault(); + first.focus(); + } + }; + document.addEventListener("keydown", handleKeyDown, true); + return () => document.removeEventListener("keydown", handleKeyDown, true); + }, [closePanel, selectedMemberId]); + + const peers = useMemo(() => { + const normalized = query.trim().toLocaleLowerCase(); + return members.filter( + (member) => + member.id !== selectedMemberId && + (!normalized || + member.name.toLocaleLowerCase().includes(normalized) || + member.role.toLocaleLowerCase().includes(normalized)) + ); + }, [members, query, selectedMemberId]); + + if (!selectedMemberId || !selected) return null; + + return createPortal( +
+ +
, + document.body + ); +} diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/ReachabilityPreview.tsx b/src/scaffold/WizardSystem/variants/AgentOrg/ReachabilityPreview.tsx deleted file mode 100644 index 958280c5d5..0000000000 --- a/src/scaffold/WizardSystem/variants/AgentOrg/ReachabilityPreview.tsx +++ /dev/null @@ -1,158 +0,0 @@ -import { useMemo } from "react"; -import { useTranslation } from "react-i18next"; - -import type { OrgMember } from "@src/modules/MainApp/AgentOrgs/types"; -import { truncate } from "@src/util/string/truncate"; - -import { - type RoutingDecision, - buildPreviewGraph, - decideRouting, - findIsolatedMemberIds, -} from "./routingPreview"; - -interface ReachabilityPreviewProps { - /** - * Coordinator-rooted org tree. Must already include the wizard's - * current `hierarchyMode` on the root (or pass `hierarchyMode` - * explicitly to override). - */ - root: OrgMember; - /** - * Optional explicit override; if omitted, uses `root.hierarchyMode`. - * The component is only meant to render under Strict — the parent - * is responsible for hiding it otherwise. - */ - hierarchyMode?: OrgMember["hierarchyMode"]; -} - -/** - * Strict-mode reachability preview. Renders an N×N matrix of routing - * decisions (✓ allowed, ✗ blocked) so the user can sanity-check - * "which agent can directly contact which" before launching the org. - * - * Self routing (the diagonal) is shown as a muted dash — it isn't a - * real Strict denial, just self-routing being filtered upstream by - * `org_send_message`'s sender filter. - * - * Empty-state: an org with only the coordinator has nothing - * meaningful to display, so the component renders a single hint - * line instead of an empty matrix. - */ -export function ReachabilityPreview({ - root, - hierarchyMode, -}: ReachabilityPreviewProps) { - const { t } = useTranslation("integrations"); - - const graph = useMemo( - () => buildPreviewGraph(root, hierarchyMode), - [root, hierarchyMode] - ); - - const isolatedIds = useMemo(() => findIsolatedMemberIds(graph), [graph]); - - if (graph.nodes.length <= 1) { - return ( -
- {t("agentOrgs.orgWizard.reachability.empty")} -
- ); - } - - return ( -
- {isolatedIds.length > 0 ? ( -
- {t("agentOrgs.orgWizard.reachability.isolatedWarn", { - names: isolatedIds - .map( - (id) => graph.nodes.find((node) => node.id === id)?.name ?? id - ) - .join(", "), - })} -
- ) : null} - -
- - - - - {graph.nodes.map((node) => ( - - ))} - - - - {graph.nodes.map((from) => ( - - - {graph.nodes.map((to) => { - const decision: RoutingDecision = - from.id === to.id - ? "blocked" - : decideRouting(graph, from.id, to.id); - const isSelf = from.id === to.id; - return ( - - ); - })} - - ))} - -
- {t("agentOrgs.orgWizard.reachability.fromHeader")} - - {truncate(node.name, 14)} -
- {truncate(from.name, 14)} - - {cellGlyph(decision, isSelf)} -
-
-
- ); -} - -function cellClass(decision: RoutingDecision, isSelf: boolean): string { - if (isSelf) return "text-fg-muted px-2 py-1 text-center"; - return decision === "allowed" - ? "text-success-6 px-2 py-1 text-center" - : "text-warning-6 px-2 py-1 text-center"; -} - -function cellGlyph(decision: RoutingDecision, isSelf: boolean): string { - if (isSelf) return "—"; - return decision === "allowed" ? "✓" : "✗"; -} - -function cellTitle( - t: ReturnType["t"], - fromName: string, - toName: string, - decision: RoutingDecision, - isSelf: boolean -): string { - if (isSelf) return fromName; - return decision === "allowed" - ? t("agentOrgs.orgWizard.reachability.allowedTitle", { - from: fromName, - to: toName, - }) - : t("agentOrgs.orgWizard.reachability.blockedTitle", { - from: fromName, - to: toName, - }); -} diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/__tests__/routingPreview.test.ts b/src/scaffold/WizardSystem/variants/AgentOrg/__tests__/routingPreview.test.ts deleted file mode 100644 index 1b04e4a038..0000000000 --- a/src/scaffold/WizardSystem/variants/AgentOrg/__tests__/routingPreview.test.ts +++ /dev/null @@ -1,185 +0,0 @@ -import type { OrgMember } from "@src/modules/MainApp/AgentOrgs/types"; - -import { - buildPreviewGraph, - decideRouting, - findIsolatedMemberIds, -} from "../routingPreview"; - -/** - * Two-branch fixture mirroring `routing_ctx` in - * `agent_org_runs::tests`. Keep the topology in sync — the parity - * tests at the bottom of this file pin every Strict allow / block - * decision the Rust suite covers. - * - * coord - * ├── lead-a - * │ └── ic-a - * └── lead-b - * └── ic-b - */ -function buildOrg(): OrgMember { - return { - id: "coord", - name: "RoutingOrg", - role: "lead", - agentId: "agent-coord", - children: [ - { - id: "member-a", - name: "lead-a", - role: "lead", - agentId: "agent-a", - children: [ - { - id: "member-a-ic", - name: "ic-a", - role: "ic", - agentId: "agent-a-ic", - children: [], - }, - ], - }, - { - id: "member-b", - name: "lead-b", - role: "lead", - agentId: "agent-b", - children: [ - { - id: "member-b-ic", - name: "ic-b", - role: "ic", - agentId: "agent-b-ic", - children: [], - }, - ], - }, - ], - }; -} - -describe("buildPreviewGraph", () => { - it("flattens the tree depth-first with the coordinator as root", () => { - const graph = buildPreviewGraph(buildOrg(), "soft"); - expect(graph.coordinatorId).toBe("coord"); - expect(graph.nodes.map((node) => node.id)).toEqual([ - "coord", - "member-a", - "member-a-ic", - "member-b", - "member-b-ic", - ]); - }); - - it("populates parentId pointing at the immediate manager", () => { - const graph = buildPreviewGraph(buildOrg(), "soft"); - const ic = graph.nodes.find((node) => node.id === "member-a-ic"); - expect(ic?.parentId).toBe("member-a"); - const lead = graph.nodes.find((node) => node.id === "member-a"); - expect(lead?.parentId).toBe("coord"); - }); - - it("falls back to root.hierarchyMode then 'soft'", () => { - const root = { ...buildOrg(), hierarchyMode: "strict" as const }; - expect(buildPreviewGraph(root).hierarchyMode).toBe("strict"); - expect(buildPreviewGraph(buildOrg()).hierarchyMode).toBe("soft"); - }); -}); - -describe("decideRouting (parity with Rust check_routing)", () => { - it("Flat allows everything", () => { - const graph = buildPreviewGraph(buildOrg(), "flat"); - expect(decideRouting(graph, "member-a-ic", "member-b-ic")).toBe("allowed"); - expect(decideRouting(graph, "member-b", "member-a")).toBe("allowed"); - }); - - it("Soft allows everything (advisory only at runtime)", () => { - const graph = buildPreviewGraph(buildOrg(), "soft"); - expect(decideRouting(graph, "member-a-ic", "member-b-ic")).toBe("allowed"); - }); - - it("Strict allows anyone → coordinator", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "member-a-ic", "coord")).toBe("allowed"); - }); - - it("Strict allows coordinator → anyone (escape hatch)", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "coord", "member-b-ic")).toBe("allowed"); - }); - - it("Strict allows send to direct manager", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "member-a-ic", "member-a")).toBe("allowed"); - }); - - it("Strict allows send to direct report", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "member-a", "member-a-ic")).toBe("allowed"); - }); - - it("Strict blocks cross-branch", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "member-a-ic", "member-b-ic")).toBe("blocked"); - }); - - it("Strict blocks skip-level-up", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "member-a-ic", "member-b")).toBe("blocked"); - }); - - it("Strict blocks peer-to-peer leads", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "member-a", "member-b")).toBe("blocked"); - }); - - it("self routing is always blocked", () => { - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(decideRouting(graph, "member-a", "member-a")).toBe("blocked"); - }); -}); - -describe("findIsolatedMemberIds", () => { - it("returns [] under Flat", () => { - const graph = buildPreviewGraph(buildOrg(), "flat"); - expect(findIsolatedMemberIds(graph)).toEqual([]); - }); - - it("returns [] under Soft", () => { - const graph = buildPreviewGraph(buildOrg(), "soft"); - expect(findIsolatedMemberIds(graph)).toEqual([]); - }); - - it("flags an IC whose only non-coordinator peer is its lead — wait, that one is not isolated", () => { - // ic-a can reach lead-a, so it is NOT isolated. The fixture has - // no isolated members; this test pins that the fixture is - // healthy and the next test exercises the isolated case via - // a custom graph. - const graph = buildPreviewGraph(buildOrg(), "strict"); - expect(findIsolatedMemberIds(graph)).toEqual([]); - }); - - it("flags a top-level lead with no children and no peers", () => { - // Single lone lead under the coordinator: only "allowed" peer - // is the coordinator, so it counts as isolated for collaboration - // purposes. - const lonely: OrgMember = { - id: "coord", - name: "Lonely", - role: "lead", - agentId: "agent-coord", - children: [ - { - id: "loner", - name: "Solo", - role: "ic", - agentId: "agent-loner", - children: [], - }, - ], - }; - const graph = buildPreviewGraph(lonely, "strict"); - expect(findIsolatedMemberIds(graph)).toEqual(["loner"]); - }); -}); diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/index.ts b/src/scaffold/WizardSystem/variants/AgentOrg/index.ts index 1a0739949f..1acf6ca9ed 100644 --- a/src/scaffold/WizardSystem/variants/AgentOrg/index.ts +++ b/src/scaffold/WizardSystem/variants/AgentOrg/index.ts @@ -4,20 +4,13 @@ export { isOrgDraftValid, } from "./AgentTeamFormSections"; export type { AgentTeamFormSectionsProps } from "./AgentTeamFormSections"; -export { default as HierarchyModeSelector } from "./HierarchyModeSelector"; -export { ReachabilityPreview } from "./ReachabilityPreview"; export { - buildOrgTreeFromMembers, + allMemberPairKeys, + canonicalPairKey, + connectedCountByMemberId, findDuplicateMemberNameIds, - flattenOrgToMembers, + linksToPairSet, + sortedLinksFromPairSet, + toFlatOrgMembers, + toTeamMembers, } from "./orgTree"; -export { - buildPreviewGraph, - decideRouting, - findIsolatedMemberIds, -} from "./routingPreview"; -export type { - PreviewGraph, - PreviewNode, - RoutingDecision, -} from "./routingPreview"; diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/orgTree.test.ts b/src/scaffold/WizardSystem/variants/AgentOrg/orgTree.test.ts new file mode 100644 index 0000000000..a6389734d4 --- /dev/null +++ b/src/scaffold/WizardSystem/variants/AgentOrg/orgTree.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from "vitest"; + +import type { TeamMember } from "@src/components/TeamMemberTable"; + +import { + allMemberPairKeys, + canonicalPairKey, + connectedCountByMemberId, + linksToPairSet, + pairKeyIncludesMember, + pairKeysWithNewMember, + pairKeysWithoutMember, + sortedLinksFromPairSet, +} from "./orgTree"; + +function members(count: number): TeamMember[] { + return Array.from({ length: count }, (_, index) => ({ + id: `member-${index}`, + name: `Member ${index}`, + role: index % 2 ? "Reviewer" : "Builder", + agentId: "builtin:sde", + })); +} + +describe("flat Team communication draft", () => { + it("materializes every canonical pair for a new three-Member Team", () => { + const roster = members(3); + const pairs = allMemberPairKeys(roster); + + expect(pairs.size).toBe(3); + expect([...connectedCountByMemberId(roster, pairs).values()]).toEqual([ + 2, 2, 2, + ]); + expect(sortedLinksFromPairSet(pairs)).toEqual([ + { memberAId: "member-0", memberBId: "member-1" }, + { memberAId: "member-0", memberBId: "member-2" }, + { memberAId: "member-1", memberBId: "member-2" }, + ]); + }); + + it("materializes exactly 1,225 pairs for 50 Members", () => { + const roster = members(50); + const pairs = allMemberPairKeys(roster); + + expect(pairs.size).toBe(1_225); + expect(connectedCountByMemberId(roster, pairs).get("member-17")).toBe(49); + }); + + it("uses one undirected key from either panel endpoint", () => { + expect(canonicalPairKey("alice", "bob")).toBe( + canonicalPairKey("bob", "alice") + ); + }); + + it("removes one link and updates both endpoint counts only", () => { + const roster = members(3); + const pairs = allMemberPairKeys(roster); + pairs.delete(canonicalPairKey("member-0", "member-1")); + + expect([...connectedCountByMemberId(roster, pairs).values()]).toEqual([ + 1, 1, 2, + ]); + }); + + it("connects a newly added Member to every existing Member", () => { + const existing = members(3); + const pairs = allMemberPairKeys(existing); + const next = pairKeysWithNewMember(existing, pairs, "member-3"); + const roster = [...existing, members(4)[3]]; + + expect(next.size).toBe(6); + expect([...connectedCountByMemberId(roster, next).values()]).toEqual([ + 3, 3, 3, 3, + ]); + }); + + it("removes a deleted Member's links without changing surviving pair identity", () => { + const roster = members(3); + const original = allMemberPairKeys(roster); + const survivingKey = canonicalPairKey("member-0", "member-2"); + const next = pairKeysWithoutMember(original, "member-1"); + + expect(next).toEqual(new Set([survivingKey])); + expect(next.has(canonicalPairKey("member-2", "member-0"))).toBe(true); + }); + + it("keeps communication pair identity stable when a Member is renamed", () => { + const roster = members(2); + const pairs = allMemberPairKeys(roster); + const renamed = roster.map((member) => + member.id === "member-0" ? { ...member, name: "Renamed Alice" } : member + ); + + expect(allMemberPairKeys(renamed)).toEqual(pairs); + }); + + it("round-trips IDs containing separators without key collisions", () => { + const pairs = linksToPairSet([ + { memberAId: "a\u0000b", memberBId: "c" }, + { memberAId: "a", memberBId: "b\u0000c" }, + ]); + + expect(pairs.size).toBe(2); + expect(sortedLinksFromPairSet(pairs)).toHaveLength(2); + expect( + [...pairs].every((key) => pairKeyIncludesMember(key, "missing")) + ).toBe(false); + }); +}); diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/orgTree.ts b/src/scaffold/WizardSystem/variants/AgentOrg/orgTree.ts index 77c79d10e3..33c8d393f9 100644 --- a/src/scaffold/WizardSystem/variants/AgentOrg/orgTree.ts +++ b/src/scaffold/WizardSystem/variants/AgentOrg/orgTree.ts @@ -1,94 +1,113 @@ -/** - * Tree <-> flat list helpers shared between AgentTeamWizard (creation/editing - * inside the wizard) and OrgDetailView (always-editable detail panel). - * - * The on-disk shape is a tree (`OrgMember` with `children`); the table UI - * works on a flat `TeamMember[]` keyed by `parentId`. These helpers convert - * between the two representations. - */ import type { TeamMember } from "@src/components/TeamMemberTable"; -import type { OrgMember } from "@src/modules/MainApp/AgentOrgs/types"; +import type { + FlatOrgMember, + MemberCommunicationLink, +} from "@src/modules/MainApp/AgentOrgs/types"; -/** - * Compute the set of member IDs whose `name` collides (case-insensitively) - * with another row in the same list. Empty / whitespace-only names are - * skipped — that's a separate "name must not be empty" validation. - * - * Why uniqueness matters: names are still the human-facing labels in the - * editor. Runtime routing uses stable `member_id` values only, so duplicate - * labels are blocked here to keep the UI understandable without leaking - * display names into LLM tool routing. - */ export function findDuplicateMemberNameIds( members: { id: string; name: string }[] ): Set { const buckets = new Map(); for (const member of members) { const key = member.name.trim().toLowerCase(); - if (key.length === 0) continue; - const existing = buckets.get(key); - if (existing) { - existing.push(member.id); - } else { - buckets.set(key, [member.id]); - } + if (!key) continue; + buckets.set(key, [...(buckets.get(key) ?? []), member.id]); } - const duplicateIds = new Set(); - for (const ids of buckets.values()) { - if (ids.length > 1) { - for (const id of ids) duplicateIds.add(id); + return new Set( + [...buckets.values()].filter((ids) => ids.length > 1).flatMap((ids) => ids) + ); +} + +export function toTeamMembers(members: FlatOrgMember[]): TeamMember[] { + return members.map((member) => ({ + id: member.memberId, + name: member.name, + role: member.role, + agentId: member.agentId, + runtimeConfig: member.runtimeConfig, + })); +} + +export function toFlatOrgMembers(members: TeamMember[]): FlatOrgMember[] { + return members.map((member) => ({ + memberId: member.id, + name: member.name, + role: member.role, + agentId: member.agentId, + runtimeConfig: member.runtimeConfig, + })); +} + +export function canonicalPairKey(memberAId: string, memberBId: string): string { + return JSON.stringify( + memberAId < memberBId ? [memberAId, memberBId] : [memberBId, memberAId] + ); +} + +export function pairKeyToLink(key: string): MemberCommunicationLink { + const [memberAId, memberBId] = JSON.parse(key) as [string, string]; + return { memberAId, memberBId }; +} + +export function pairKeyIncludesMember(key: string, memberId: string): boolean { + const { memberAId, memberBId } = pairKeyToLink(key); + return memberAId === memberId || memberBId === memberId; +} + +export function linksToPairSet(links: MemberCommunicationLink[]): Set { + return new Set( + links.map((link) => canonicalPairKey(link.memberAId, link.memberBId)) + ); +} + +export function allMemberPairKeys(members: readonly TeamMember[]): Set { + const pairs = new Set(); + for (let left = 0; left < members.length; left += 1) { + for (let right = left + 1; right < members.length; right += 1) { + pairs.add(canonicalPairKey(members[left].id, members[right].id)); } } - return duplicateIds; + return pairs; } -/** Build an OrgMember[] tree from a flat TeamMember[] using parentId. */ -export function buildOrgTreeFromMembers(members: TeamMember[]): OrgMember[] { - const nodeMap = new Map(); +export function pairKeysWithNewMember( + members: readonly TeamMember[], + pairs: ReadonlySet, + newMemberId: string +): Set { + const next = new Set(pairs); for (const member of members) { - nodeMap.set(member.id, { - id: member.id, - name: member.name, - role: member.role, - agentId: member.agentId, - runtimeConfig: member.runtimeConfig, - children: [], - }); + next.add(canonicalPairKey(member.id, newMemberId)); } + return next; +} - const roots: OrgMember[] = []; - for (const member of members) { - const node = nodeMap.get(member.id)!; - const parent = member.parentId ? nodeMap.get(member.parentId) : undefined; - if (parent) { - parent.children.push(node); - } else { - roots.push(node); - } - } - return roots; +export function pairKeysWithoutMember( + pairs: ReadonlySet, + memberId: string +): Set { + return new Set( + [...pairs].filter((key) => !pairKeyIncludesMember(key, memberId)) + ); } -/** Flatten an OrgMember tree into a TeamMember[], preserving parentId. */ -export function flattenOrgToMembers( - nodes: OrgMember[], - parentId?: string -): TeamMember[] { - const result: TeamMember[] = []; - for (const node of nodes) { - if (node.role === "org") { - result.push(...flattenOrgToMembers(node.children, parentId)); - } else { - result.push({ - id: node.id, - name: node.name, - role: node.role, - agentId: node.agentId, - runtimeConfig: node.runtimeConfig, - parentId, - }); - result.push(...flattenOrgToMembers(node.children, node.id)); - } +export function connectedCountByMemberId( + members: readonly TeamMember[], + pairs: ReadonlySet +): Map { + const counts = new Map(members.map((member) => [member.id, 0])); + for (const key of pairs) { + const { memberAId, memberBId } = pairKeyToLink(key); + if (counts.has(memberAId)) + counts.set(memberAId, (counts.get(memberAId) ?? 0) + 1); + if (counts.has(memberBId)) + counts.set(memberBId, (counts.get(memberBId) ?? 0) + 1); } - return result; + return counts; +} + +export function sortedLinksFromPairSet( + pairs: ReadonlySet +): MemberCommunicationLink[] { + return [...pairs].sort().map(pairKeyToLink); } diff --git a/src/scaffold/WizardSystem/variants/AgentOrg/routingPreview.ts b/src/scaffold/WizardSystem/variants/AgentOrg/routingPreview.ts deleted file mode 100644 index daf1e0a03e..0000000000 --- a/src/scaffold/WizardSystem/variants/AgentOrg/routingPreview.ts +++ /dev/null @@ -1,148 +0,0 @@ -/** - * Pure routing-rule preview shared by `AgentTeamWizard` and `OrgDetailView`. - * - * This is a TS mirror of `AgentOrgRunContext::check_routing` in - * `src-tauri/src/agent_core/core/coordination/agent_org_runs.rs`. The - * Rust side is the runtime source of truth — `org_send_message` - * enforces these rules at send time. This module exists so the - * wizard can show users a Strict-mode reachability preview *before* - * anything launches, and surface "isolated member" warnings while - * the org is being edited. - * - * Parity invariant: every rule branch here has a corresponding - * `routing_strict_*` test in `agent_org_runs::tests`. If the Rust - * rules change, the matching parity test in this folder will fail - * and force the two implementations to be re-synced. - */ -import type { - HierarchyMode, - OrgMember, -} from "@src/modules/MainApp/AgentOrgs/types"; - -export type RoutingDecision = "allowed" | "blocked"; - -/** - * Flat view of a single org member used by the routing preview. The - * coordinator is represented as the entry whose `id` matches - * `coordinatorId` — exactly one such entry must exist. - */ -export interface PreviewNode { - id: string; - name: string; - parentId: string | null; -} - -export interface PreviewGraph { - /** `id` of the coordinator node (root of the OrgMember tree). */ - coordinatorId: string; - nodes: PreviewNode[]; - hierarchyMode: HierarchyMode; -} - -/** - * Build a `PreviewGraph` from a wizard-side root `OrgMember`. - * - * The wizard root is always the coordinator (we don't allow editing - * the root row directly; it inherits the org's identity). Children - * are walked depth-first; every entry's `parentId` points at its - * immediate manager, with the coordinator's children using the - * coordinator's id as their parent. - * - * `hierarchyMode` overrides whatever might be stamped on the root - * `OrgMember` so the wizard can preview a different mode without - * mutating state. When `undefined`, falls back to the root's mode - * or `"soft"` (matches `DEFAULT_HIERARCHY_MODE`). - */ -export function buildPreviewGraph( - root: OrgMember, - hierarchyMode?: HierarchyMode -): PreviewGraph { - const nodes: PreviewNode[] = [ - { id: root.id, name: root.name, parentId: null }, - ]; - - function walk(parent: OrgMember) { - for (const child of parent.children) { - if (child.role === "org") { - walk(child); - continue; - } - nodes.push({ - id: child.id, - name: child.name, - parentId: parent.id, - }); - walk(child); - } - } - walk(root); - - return { - coordinatorId: root.id, - nodes, - hierarchyMode: hierarchyMode ?? root.hierarchyMode ?? "soft", - }; -} - -/** - * Pure routing decision for a `from -> to` send under the graph's - * `hierarchyMode`. Mirrors `check_routing` in Rust: - * - * - `flat` / `soft` → always `"allowed"`. - * - `strict`: - * 1. anyone → coordinator → allowed (escalate) - * 2. coordinator → anyone → allowed (escape hatch) - * 3. from.parentId === to.id → allowed (report to manager) - * 4. to.parentId === from.id → allowed (manager to direct report) - * else → `"blocked"`. - * - * `from === to` is treated as `"blocked"` (self-routing) rather than - * exposing it as an entry in the matrix; callers should skip the - * diagonal when displaying. - */ -export function decideRouting( - graph: PreviewGraph, - fromId: string, - toId: string -): RoutingDecision { - if (fromId === toId) return "blocked"; - if (graph.hierarchyMode === "flat" || graph.hierarchyMode === "soft") { - return "allowed"; - } - const coord = graph.coordinatorId; - if (toId === coord || fromId === coord) return "allowed"; - - const from = graph.nodes.find((node) => node.id === fromId); - const to = graph.nodes.find((node) => node.id === toId); - if (!from || !to) return "blocked"; - - if (from.parentId === to.id) return "allowed"; - if (to.parentId === from.id) return "allowed"; - return "blocked"; -} - -/** - * Identify members that are "isolated" under Strict mode: they have - * exactly one reachable peer (the coordinator) and therefore cannot - * collaborate horizontally. The coordinator itself is excluded — - * isolation is only meaningful for non-root members. - * - * Returns an empty array under Flat / Soft. - */ -export function findIsolatedMemberIds(graph: PreviewGraph): string[] { - if (graph.hierarchyMode !== "strict") return []; - const isolated: string[] = []; - for (const node of graph.nodes) { - if (node.id === graph.coordinatorId) continue; - const reachableNonCoord = graph.nodes.filter( - (other) => - other.id !== node.id && - other.id !== graph.coordinatorId && - decideRouting(graph, node.id, other.id) === "allowed" - ); - if (reachableNonCoord.length === 0) { - isolated.push(node.id); - } - } - return isolated; -} diff --git a/src/store/workstation/tabs/__tests__/storage.test.ts b/src/store/workstation/tabs/__tests__/storage.test.ts index cab32d9225..3a1c835680 100644 --- a/src/store/workstation/tabs/__tests__/storage.test.ts +++ b/src/store/workstation/tabs/__tests__/storage.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { resolveAgentOrgDefinition } from "../agentConfigSnapshot"; import { LAYOUT_STORAGE_KEY, WORKSTATION_V3_GLOBAL_KEY, @@ -198,6 +199,104 @@ describe("v2 migration", () => { }); describe("v3 persistence keys", () => { + it("discards legacy Team snapshots without replacing them with empty members", () => { + const legacyOrgTab = tab( + "agent-config:org:default:sde-feature-team", + "agent-config", + { + data: { + variant: "org", + entityId: "default:sde-feature-team", + displayName: "Default Agent Org", + entitySnapshot: { + id: "default:sde-feature-team", + name: "Default Agent Org", + children: [{ id: "legacy-member" }], + }, + }, + } + ); + const currentOrgSnapshot = { + id: "current-team", + name: "Current Team", + role: "Coordinator", + agentId: "coordinator-agent", + planApprovalPolicy: "coordinator" as const, + members: [ + { + memberId: "alice", + name: "Alice", + role: "Engineer", + agentId: "alice-agent", + }, + ], + additionalTaskGraphWriterMemberIds: [], + memberCommunicationLinks: [], + }; + const currentOrgTab = tab("agent-config:org:current-team", "agent-config", { + data: { + variant: "org", + entityId: "current-team", + displayName: "Current Team", + entitySnapshot: currentOrgSnapshot, + }, + }); + + localStorage.setItem( + WORKSTATION_V3_MANIFEST_KEY, + JSON.stringify({ version: 3, sessionIds: [] }) + ); + localStorage.setItem( + WORKSTATION_V3_SHARED_KEY, + JSON.stringify({ tabs: [legacyOrgTab, currentOrgTab] }) + ); + + const loadedTabs = loadWorkstationTabsState().shared.tabs; + const loadedLegacyData = loadedTabs.find( + (item) => item.id === legacyOrgTab.id + )?.data; + const loadedCurrentData = loadedTabs.find( + (item) => item.id === currentOrgTab.id + )?.data; + + expect(loadedLegacyData).toEqual({ + variant: "org", + entityId: "default:sde-feature-team", + displayName: "Default Agent Org", + }); + expect(loadedLegacyData).not.toHaveProperty("entitySnapshot"); + expect(loadedCurrentData?.entitySnapshot).toEqual(currentOrgSnapshot); + + const legacySnapshot = legacyOrgTab.data.entitySnapshot; + expect( + resolveAgentOrgDefinition([], "default:sde-feature-team", legacySnapshot) + ).toBeUndefined(); + + const canonicalReloadedOrg = { + ...currentOrgSnapshot, + id: "default:sde-feature-team", + members: [ + ...currentOrgSnapshot.members, + { + memberId: "bob", + name: "Bob", + role: "Reviewer", + agentId: "bob-agent", + }, + ], + memberCommunicationLinks: [{ memberAId: "alice", memberBId: "bob" }], + }; + const resolvedOrg = resolveAgentOrgDefinition( + [canonicalReloadedOrg], + "default:sde-feature-team", + legacySnapshot + ); + expect(resolvedOrg?.members.map((member) => member.name)).toEqual([ + "Alice", + "Bob", + ]); + }); + it("writes shared/global/session scopes separately and encodes session IDs", () => { const state = emptyWorkstationTabsState(); state.shared.tabs = [tab("settings:main", "settings")]; diff --git a/src/store/workstation/tabs/agentConfigSnapshot.ts b/src/store/workstation/tabs/agentConfigSnapshot.ts new file mode 100644 index 0000000000..0a55176c42 --- /dev/null +++ b/src/store/workstation/tabs/agentConfigSnapshot.ts @@ -0,0 +1,27 @@ +import { OrgDefinitionSchema } from "@src/api/tauri/rpc/schemas/agentOrgs"; + +/** + * Cached tab snapshots are untrusted across app upgrades. Only reuse a Team + * snapshot when it still matches the current RPC shape; otherwise the tab + * renderer must reload the canonical definition by entityId. + */ +export function parseCurrentAgentOrgSnapshot(value: unknown) { + const result = OrgDefinitionSchema.safeParse(value); + return result.success ? result.data : undefined; +} + +type CurrentAgentOrgSnapshot = NonNullable< + ReturnType +>; + +export function resolveAgentOrgDefinition( + orgs: readonly CurrentAgentOrgSnapshot[], + entityId: string, + entitySnapshot: unknown +): CurrentAgentOrgSnapshot | undefined { + const canonicalOrg = orgs.find((org) => org.id === entityId); + if (canonicalOrg) return canonicalOrg; + + const cachedOrg = parseCurrentAgentOrgSnapshot(entitySnapshot); + return cachedOrg?.id === entityId ? cachedOrg : undefined; +} diff --git a/src/store/workstation/tabs/storage.ts b/src/store/workstation/tabs/storage.ts index 25138cfa76..f8c5e50cfa 100644 --- a/src/store/workstation/tabs/storage.ts +++ b/src/store/workstation/tabs/storage.ts @@ -1,5 +1,6 @@ import { createLogger } from "@src/hooks/logger"; +import { parseCurrentAgentOrgSnapshot } from "./agentConfigSnapshot"; import { type WorkStationTab, type WorkStationTabType, @@ -98,6 +99,21 @@ function isValidTab(value: unknown): value is WorkStationTab { ); } +function discardStaleAgentOrgSnapshot(tab: WorkStationTab): WorkStationTab { + if ( + tab.type !== "agent-config" || + tab.data.variant !== "org" || + !("entitySnapshot" in tab.data) || + parseCurrentAgentOrgSnapshot(tab.data.entitySnapshot) + ) { + return tab; + } + + const data = { ...tab.data }; + delete data.entitySnapshot; + return { ...tab, data }; +} + function sanitizeTabs(value: unknown): WorkStationTab[] { if (!Array.isArray(value)) return []; const seen = new Set(); @@ -106,7 +122,10 @@ function sanitizeTabs(value: unknown): WorkStationTab[] { if (!isValidTab(candidate) || seen.has(candidate.id)) continue; seen.add(candidate.id); // A dirty marker without a restored buffer is misleading after restart. - result.push({ ...candidate, hasUnsavedChanges: false }); + result.push({ + ...discardStaleAgentOrgSnapshot(candidate), + hasUnsavedChanges: false, + }); if (result.length >= MAX_TABS_PER_PARTITION) break; } return result; diff --git a/tests/e2e/specs/core/agent-org-settings-ui.spec.mjs b/tests/e2e/specs/core/agent-org-settings-ui.spec.mjs index 329f9b0114..90cb891064 100644 --- a/tests/e2e/specs/core/agent-org-settings-ui.spec.mjs +++ b/tests/e2e/specs/core/agent-org-settings-ui.spec.mjs @@ -73,10 +73,10 @@ describe("Agent Org settings and topology rendered UI", () => { await waitForApp(); }); - it("creates strict Agent Org structure in settings and launches that persisted runtime topology", async () => { + it("creates a flat Agent Org in settings and launches its immutable roster snapshot", async () => { const account = await getApiAccount(); const model = selectPreferredModel(account); - const orgName = `E2E Strict Org ${RUN_ID}`; + const orgName = `E2E Flat Org ${RUN_ID}`; const leadName = `E2E Lead ${RUN_ID}`; const childName = `E2E Child ${RUN_ID}`; await removeAgentOrgsByName(orgName); @@ -94,14 +94,14 @@ describe("Agent Org settings and topology rendered UI", () => { await configureCreatorForAgentOrg({ account, model, agentOrgId: org.id }); await selectRenderedAgentOrg(org.id); - const launchPrompt = `E2E true positive custom strict Agent Org topology ${RUN_ID}. Reply briefly.`; + const launchPrompt = `E2E true positive custom flat Agent Org topology ${RUN_ID}. Reply briefly.`; const sessionId = await sendFromRenderedCreator(launchPrompt); if (!sessionId) { throw new Error( - "Custom strict Agent Org launch did not create a session id" + "Custom flat Agent Org launch did not create a session id" ); } - await waitForRenderedAssistantReply("custom strict Agent Org launch"); + await waitForRenderedAssistantReply("custom flat Agent Org launch"); await waitForAgentOrgRunView( sessionId, @@ -116,13 +116,13 @@ describe("Agent Org settings and topology rendered UI", () => { child?.memberId && lead.agentId === BUILTIN_SDE_AGENT_ID && child.agentId === BUILTIN_SDE_AGENT_ID && - !lead.parentMemberId && - child.parentMemberId === lead.memberId && + !Object.hasOwn(lead, "parentMemberId") && + !Object.hasOwn(child, "parentMemberId") && lead.sessionRuntime?.sessionId && child.sessionRuntime?.sessionId ); }, - "custom strict Agent Org persisted members consumed by runtime" + "custom flat Agent Org persisted members consumed by runtime" ); }); @@ -140,11 +140,11 @@ describe("Agent Org settings and topology rendered UI", () => { leadName, childName, }); - const lead = (org.children ?? []).find( + const lead = (org.members ?? []).find( (member) => member.name === leadName ); - const child = lead?.children?.find((member) => member.name === childName); - if (!child?.id) { + const child = (org.members ?? []).find((member) => member.name === childName); + if (!child?.memberId) { throw new Error( `Override test could not resolve child member: ${JSON.stringify(org)}` ); @@ -154,7 +154,7 @@ describe("Agent Org settings and topology rendered UI", () => { await selectRenderedAgentOrg(org.id); const launchOnlyDraft = { agentOrgMemberOverrides: { - [child.id]: { + [child.memberId]: { runtimeConfig: { keySource: "own_key", accountId: account.id, @@ -198,7 +198,7 @@ describe("Agent Org settings and topology rendered UI", () => { org.id, (view) => { const overriddenMember = (view?.members ?? []).find( - (member) => member.memberId === child.id + (member) => member.memberId === child.memberId ); return Boolean(overriddenMember?.sessionRuntime?.sessionId); }, @@ -206,7 +206,7 @@ describe("Agent Org settings and topology rendered UI", () => { ); const launchOnlyChildRuntime = ( launchOnlyRunState?.view?.members ?? [] - ).find((member) => member.memberId === child.id)?.sessionRuntime; + ).find((member) => member.memberId === child.memberId)?.sessionRuntime; const launchOnlyChildSessionId = launchOnlyChildRuntime?.sessionId; if (!launchOnlyChildSessionId) { throw new Error( @@ -225,8 +225,8 @@ describe("Agent Org settings and topology rendered UI", () => { "listAgentOrgs after launch-only override" ).orgs.find((candidate) => candidate?.id === org.id); const afterLaunchOnlyChild = - afterLaunchOnlyOrg?.children?.[0]?.children?.find( - (member) => member.id === child.id + afterLaunchOnlyOrg?.members?.find( + (member) => member.memberId === child.memberId ); if (afterLaunchOnlyChild?.runtimeConfig) { throw new Error( @@ -272,14 +272,14 @@ describe("Agent Org settings and topology rendered UI", () => { org.id, (view) => { const overriddenMember = (view?.members ?? []).find( - (member) => member.memberId === child.id + (member) => member.memberId === child.memberId ); return Boolean(overriddenMember?.sessionRuntime?.sessionId); }, "persisted member override materialized" ); const persistedChildRuntime = (persistedRunState?.view?.members ?? []).find( - (member) => member.memberId === child.id + (member) => member.memberId === child.memberId )?.sessionRuntime; const persistedChildSessionId = persistedChildRuntime?.sessionId; if (!persistedChildSessionId) { @@ -298,8 +298,8 @@ describe("Agent Org settings and topology rendered UI", () => { await invokeE2E("listAgentOrgs"), "listAgentOrgs after persisted override" ).orgs.find((candidate) => candidate?.id === org.id); - const persistedChild = persistedOrg?.children?.[0]?.children?.find( - (member) => member.id === child.id + const persistedChild = persistedOrg?.members?.find( + (member) => member.memberId === child.memberId ); if (persistedChild?.runtimeConfig?.model !== overrideModel) { throw new Error( @@ -370,11 +370,11 @@ describe("Agent Org settings and topology rendered UI", () => { leadName, childName, }); - const lead = (org.children ?? []).find( + const lead = (org.members ?? []).find( (member) => member.name === leadName ); - const child = lead?.children?.find((member) => member.name === childName); - if (!child?.id) { + const child = (org.members ?? []).find((member) => member.name === childName); + if (!child?.memberId) { throw new Error( `Rendered member override could not resolve child: ${JSON.stringify(org)}` ); @@ -383,7 +383,7 @@ describe("Agent Org settings and topology rendered UI", () => { await configureCreatorForAgentOrg({ account, model, agentOrgId: org.id }); await selectRenderedAgentOrg(org.id); await selectRenderedOrgMemberAgentDefinition({ - memberId: child.id, + memberId: child.memberId, agentDefinitionId: overrideAgentId, expectedText: overrideAgentName, label: "rendered member AgentDefinition override", @@ -402,7 +402,7 @@ describe("Agent Org settings and topology rendered UI", () => { org.id, (view) => { const overriddenMember = (view?.members ?? []).find( - (member) => member.memberId === child.id + (member) => member.memberId === child.memberId ); return ( overriddenMember?.agentId === overrideAgentId && @@ -412,7 +412,7 @@ describe("Agent Org settings and topology rendered UI", () => { "rendered member AgentDefinition override materialized" ); const overriddenRuntime = (runState?.view?.members ?? []).find( - (member) => member.memberId === child.id + (member) => member.memberId === child.memberId )?.sessionRuntime; if (!overriddenRuntime?.sessionId) { throw new Error( diff --git a/tests/e2e/specs/core/agent-settings/org-detail-ui.spec.mjs b/tests/e2e/specs/core/agent-settings/org-detail-ui.spec.mjs index 99407b1ff8..3a9604d762 100644 --- a/tests/e2e/specs/core/agent-settings/org-detail-ui.spec.mjs +++ b/tests/e2e/specs/core/agent-settings/org-detail-ui.spec.mjs @@ -6,15 +6,28 @@ import { waitForScript, } from "../../../support/core/agent-settings/agentSettingsDriver.mjs"; import { - createRenderedStrictTwoMemberAgentOrg, invokeE2E, removeAgentOrgsByName, + seedFlatAgentOrg, unwrap, waitForAgentOrgByName, } from "../../../support/core/agentOrgUiDriver.mjs"; const RUN_MARKER = `E2E_ORG_DETAIL_${Date.now()}`; const ORGS_ROUTE = "/orgii/app/settings/agent-orgs/orgs"; + +async function openRenderedOrgTable(label) { + unwrap(await invokeE2E("navigateTo", ORGS_ROUTE), `navigate to orgs ${label}`); + await waitForScript( + `return !!document.querySelector('[data-testid="agent-orgs-add-org-button"]');`, + `Agent Teams table did not render ${label}` + ); + await browser.executeScript( + `window.dispatchEvent(new Event("orgii-agent-orgs-changed")); return true;`, + [] + ); +} + async function openOrgDetail(orgId, label, displayName) { unwrap(await invokeE2E("openOrgTab", orgId, displayName), `open org tab for ${label}`); await restoreWorkstationIfFocused(orgId, `${label} open org detail`); @@ -133,13 +146,18 @@ async function restoreWorkstationIfFocused(orgId, label) { document.querySelectorAll('[data-e2e-restore-workstation-target]').forEach((element) => { element.removeAttribute('data-e2e-restore-workstation-target'); }); + const preferred = document.querySelector('[data-testid="station-mode-my-station"]'); const candidates = Array.from(document.querySelectorAll('button[aria-label]')).filter((button) => { const label = button.getAttribute('aria-label') || ''; const buttonRect = button.getBoundingClientRect(); const style = window.getComputedStyle(button); - return /Workstation|工作站/.test(label) && buttonRect.width > 0 && buttonRect.height > 0 && style.display !== 'none' && style.visibility !== 'hidden'; + return /Workstation|工作站|My Station|我的工作站/.test(label) && buttonRect.width > 0 && buttonRect.height > 0 && style.display !== 'none' && style.visibility !== 'hidden'; }); - const button = candidates[0] ?? null; + const preferredRect = preferred?.getBoundingClientRect?.() ?? null; + const preferredStyle = preferred ? window.getComputedStyle(preferred) : null; + const preferredVisible = !!preferred && preferredRect.width > 0 && preferredRect.height > 0 && + preferredStyle.display !== 'none' && preferredStyle.visibility !== 'hidden'; + const button = (preferredVisible ? preferred : null) ?? candidates[0] ?? null; if (!button) { return { needed: true, @@ -198,7 +216,7 @@ describe("Agent Org detail Settings UI", () => { await removeAgentOrgsByName(originalName); await removeAgentOrgsByName(cancelledName); await removeAgentOrgsByName(savedName); - const org = await createRenderedStrictTwoMemberAgentOrg({ + const org = await seedFlatAgentOrg({ orgName: originalName, leadName, childName, @@ -206,6 +224,140 @@ describe("Agent Org detail Settings UI", () => { await openOrgDetail(org.id, "created org", originalName); const orgTabSelector = `[data-testid="agent-config-tab-org-${org.id}"]`; + const lead = (org.members ?? []).find((member) => member.name === leadName); + const child = (org.members ?? []).find((member) => member.name === childName); + if (!lead || !child) { + throw new Error(`Created Team did not expose its flat roster: ${JSON.stringify(org)}`); + } + + await pointerClick( + `${orgTabSelector} [data-testid="agent-orgs-member-${lead.memberId}-manage-communication"]`, + "lead Manage communication button", + { jsClick: true } + ); + await waitForScript( + `return !!document.querySelector('[data-testid="agent-orgs-communication-panel"]');`, + "lead communication panel did not open" + ); + const leadPanelContract = await browser.executeScript( + ` + const panel = document.querySelector('[data-testid="agent-orgs-communication-panel"]'); + const peer = document.querySelector('[data-testid="agent-orgs-peer-checkbox-' + arguments[0] + '"] [data-checkbox-input]'); + return { + panelText: panel?.textContent ?? null, + peerChecked: peer?.checked ?? null, + peerIds: [...document.querySelectorAll('[data-testid^="agent-orgs-peer-row-"]')] + .map((row) => row.getAttribute('data-testid')), + hasSelectedMemberPeer: !!document.querySelector('[data-testid="agent-orgs-peer-row-' + arguments[2] + '"]'), + }; + `, + [child.memberId, leadName, lead.memberId] + ); + if ( + !leadPanelContract.panelText?.includes(leadName) || + leadPanelContract.peerChecked !== true || + leadPanelContract.hasSelectedMemberPeer + ) { + throw new Error( + `Lead communication panel contract mismatch: ${JSON.stringify(leadPanelContract)}` + ); + } + await setTextInput( + '[data-testid="agent-orgs-communication-panel-search"]', + "child implementer", + "communication peer role search" + ); + await waitForScript( + `return !!document.querySelector('[data-testid="agent-orgs-peer-row-' + arguments[0] + '"]');`, + "communication search did not match peer role", + 30_000, + [child.memberId] + ); + await setTextInput( + '[data-testid="agent-orgs-communication-panel-search"]', + "", + "clear communication peer search" + ); + await pointerClick( + `[data-testid="agent-orgs-peer-checkbox-${child.memberId}"] [data-checkbox]`, + "disconnect child from lead", + { jsClick: true } + ); + await waitForScript( + ` + const root = document.querySelector(arguments[0]); + const leadCount = root?.querySelector('[data-testid="agent-orgs-member-' + arguments[1] + '-connected-count"]')?.textContent ?? ''; + const childCount = root?.querySelector('[data-testid="agent-orgs-member-' + arguments[2] + '-connected-count"]')?.textContent ?? ''; + const peer = document.querySelector('[data-testid="agent-orgs-peer-checkbox-' + arguments[2] + '"] [data-checkbox-input]'); + return leadCount.includes('0') && childCount.includes('0') && peer?.checked === false; + `, + "both Member rows did not reflect the disconnected draft", + 30_000, + [orgTabSelector, lead.memberId, child.memberId] + ); + const whilePanelOpen = unwrap( + await invokeE2E("listAgentOrgs"), + "list orgs while communication draft is open" + ).orgs.find((item) => item.id === org.id); + if (whilePanelOpen?.memberCommunicationLinks?.length !== 1) { + throw new Error( + `Communication panel saved independently of the main Team form: ${JSON.stringify(whilePanelOpen)}` + ); + } + await pointerClick( + '[data-testid="agent-orgs-communication-panel-close"]', + "close lead communication panel", + { jsClick: true } + ); + await pointerClick( + `${orgTabSelector} [data-testid="agent-orgs-member-${child.memberId}-manage-communication"]`, + "child Manage communication button", + { jsClick: true } + ); + await waitForScript( + `return document.querySelector('[data-testid="agent-orgs-peer-checkbox-' + arguments[0] + '"] [data-checkbox-input]')?.checked === false;`, + "reverse Member panel did not share the canonical disconnected pair", + 30_000, + [lead.memberId] + ); + await pointerClick( + `[data-testid="agent-orgs-peer-checkbox-${lead.memberId}"] [data-checkbox]`, + "reconnect lead from child panel", + { jsClick: true } + ); + await waitForScript( + ` + const root = document.querySelector(arguments[0]); + const leadCount = root?.querySelector('[data-testid="agent-orgs-member-' + arguments[1] + '-connected-count"]')?.textContent ?? ''; + const childCount = root?.querySelector('[data-testid="agent-orgs-member-' + arguments[2] + '-connected-count"]')?.textContent ?? ''; + return leadCount.includes('1') && childCount.includes('1'); + `, + "both Member rows did not reflect the reconnected canonical pair", + 30_000, + [orgTabSelector, lead.memberId, child.memberId] + ); + await pointerClick( + '[data-testid="agent-orgs-communication-panel-close"]', + "close child communication panel", + { jsClick: true } + ); + await pointerClick( + `${orgTabSelector} [data-testid="agent-orgs-member-${child.memberId}-writer-checkbox"] [data-checkbox]`, + "toggle child Writer grant", + { jsClick: true } + ); + await waitForScript( + ` + const root = document.querySelector(arguments[0]); + const writer = root?.querySelector('[data-testid="agent-orgs-member-' + arguments[1] + '-writer-checkbox"] [data-checkbox-input]'); + const leadCount = root?.querySelector('[data-testid="agent-orgs-member-' + arguments[2] + '-connected-count"]')?.textContent ?? ''; + const childCount = root?.querySelector('[data-testid="agent-orgs-member-' + arguments[1] + '-connected-count"]')?.textContent ?? ''; + return writer?.checked === true && leadCount.includes('1') && childCount.includes('1'); + `, + "Writer draft changed communication state", + 30_000, + [orgTabSelector, child.memberId, lead.memberId] + ); await setTextInput( `${orgTabSelector} [data-testid="agent-orgs-org-detail"] [data-testid="agent-orgs-org-name-input"]`, cancelledName, @@ -234,6 +386,15 @@ describe("Agent Org detail Settings UI", () => { `Cancelled org detail edit persisted: ${JSON.stringify(afterCancel)}` ); } + const cancelledOrg = (afterCancel ?? []).find((item) => item.id === org.id); + if ( + cancelledOrg?.additionalTaskGraphWriterMemberIds?.length !== 0 || + cancelledOrg?.memberCommunicationLinks?.length !== 1 + ) { + throw new Error( + `Main Team cancel did not discard Writer/link drafts: ${JSON.stringify(cancelledOrg)}` + ); + } await setTextInput( `${orgTabSelector} [data-testid="agent-orgs-org-detail"] [data-testid="agent-orgs-org-name-input"]`, @@ -299,7 +460,7 @@ describe("Agent Org detail Settings UI", () => { name: inputValue('agent-orgs-org-name-input'), description: inputValue('agent-orgs-org-description-input'), coordinator: root?.querySelector('[data-testid="agent-orgs-org-coordinator-select"]')?.textContent?.trim() ?? null, - hierarchy: root?.querySelector('[data-testid="agent-orgs-hierarchy-mode-select"]')?.textContent?.trim() ?? null, + communicationPanels: root?.querySelectorAll('[data-testid$="-manage-communication"]').length ?? 0, memberInputs, rootText: root?.textContent?.trim().replace(/\\s+/g, ' ').slice(0, 800) ?? null, }; @@ -323,7 +484,8 @@ describe("Agent Org detail Settings UI", () => { await restoreWorkstationIfFocused(org.id, "org detail save"); await pointerClick( `${orgTabSelector} [data-testid="agent-orgs-org-detail-save-button"]`, - "org detail save button" + "org detail save button", + { jsClick: true } ); const savedOrg = await waitForAgentOrgByName(savedName, "saved org detail edit"); if (savedOrg.description !== savedDescription) { @@ -331,9 +493,9 @@ describe("Agent Org detail Settings UI", () => { `Saved org detail description mismatch: ${JSON.stringify(savedOrg)}` ); } - const savedLead = (savedOrg.children ?? []).find((member) => member.name === leadName); - const savedChild = savedLead?.children?.find((member) => member.name === childName); - const savedAddedMember = (savedOrg.children ?? []).find( + const savedLead = (savedOrg.members ?? []).find((member) => member.name === leadName); + const savedChild = (savedOrg.members ?? []).find((member) => member.name === childName); + const savedAddedMember = (savedOrg.members ?? []).find( (member) => member.name === addedMemberName ); if ( @@ -341,22 +503,46 @@ describe("Agent Org detail Settings UI", () => { !savedChild || !savedAddedMember || savedAddedMember.role !== addedMemberRole || - savedOrg.hierarchyMode !== "strict" + savedOrg.memberCommunicationLinks?.length !== 3 ) { throw new Error( `Saved org detail clobbered topology: ${JSON.stringify(savedOrg)}` ); } - unwrap(await invokeE2E("navigateTo", ORGS_ROUTE), "navigate to orgs after org detail save"); - await waitForScript( - ` - const row = document.querySelector('[data-testid="agent-orgs-org-row-' + arguments[0] + '"]'); - return !!row && /\\b4\\b/.test(row.textContent || ''); - `, - "org table member count did not refresh after detail save", - 30_000, - [org.id] - ); + await openRenderedOrgTable("after org detail save"); + let savedRowContract = null; + try { + await browser.waitUntil( + async () => { + savedRowContract = await browser.executeScript( + ` + const row = document.querySelector('[data-testid="agent-orgs-org-row-' + arguments[0] + '"]'); + const memberCount = row?.querySelectorAll('td')[1]?.textContent?.trim() ?? null; + return { + location: window.location.pathname, + memberCount, + rowText: row?.textContent ?? null, + rowIds: [...document.querySelectorAll('[data-testid^="agent-orgs-org-row-"]')] + .map((item) => item.getAttribute('data-testid')), + bodyText: document.body.innerText.slice(0, 1000), + }; + `, + [org.id] + ); + return savedRowContract?.memberCount === "4"; + }, + { + timeout: 30_000, + interval: 250, + timeoutMsg: "org table member count did not refresh after detail save", + } + ); + } catch (error) { + throw new Error( + `org table member count did not refresh after detail save: ${JSON.stringify(savedRowContract)}`, + { cause: error } + ); + } } finally { await removeAgentOrgsByName(originalName); await removeAgentOrgsByName(cancelledName); @@ -371,13 +557,13 @@ describe("Agent Org detail Settings UI", () => { try { await removeAgentOrgsByName(orgName); - const org = await createRenderedStrictTwoMemberAgentOrg({ + const org = await seedFlatAgentOrg({ orgName, leadName, childName, }); - unwrap(await invokeE2E("navigateTo", ORGS_ROUTE), "navigate to orgs before table delete"); + await openRenderedOrgTable("before table delete"); await waitForScript( `return !!document.querySelector('[data-testid="agent-orgs-org-delete-row-button-${org.id}"]');`, "org table delete action did not render" @@ -389,7 +575,8 @@ describe("Agent Org detail Settings UI", () => { try { await pointerClick( `[data-testid="agent-orgs-org-delete-row-button-${org.id}"]`, - "org table delete action" + "org table delete action", + { jsClick: true } ); await waitForOrgDeleted(org.id, "table Delete action"); } finally { diff --git a/tests/e2e/support/core/agentOrgUiDriver.mjs b/tests/e2e/support/core/agentOrgUiDriver.mjs index 6fe4ebbb03..b0bf106c3f 100644 --- a/tests/e2e/support/core/agentOrgUiDriver.mjs +++ b/tests/e2e/support/core/agentOrgUiDriver.mjs @@ -581,6 +581,35 @@ export async function waitForAgentOrgByName(name, label) { return org; } +export async function seedFlatAgentOrg({ + orgName, + leadName, + childName, + memberAgentId = BUILTIN_SDE_AGENT_ID, +}) { + const id = `e2e-agent-org-fixture:${crypto.randomUUID()}`; + await postDebugJson("/agent/test/agent-org/seed", { + id, + name: orgName, + coordinator_agent_id: BUILTIN_SDE_AGENT_ID, + members: [ + { + id: `${id}:lead`, + name: leadName, + role: "Lead planner", + agent_id: memberAgentId, + }, + { + id: `${id}:child`, + name: childName, + role: "Child implementer", + agent_id: memberAgentId, + }, + ], + }); + return waitForAgentOrgByName(orgName, "flat Team seed"); +} + export async function createRenderedStrictTwoMemberAgentOrg({ orgName, leadName, @@ -630,11 +659,6 @@ export async function createRenderedStrictTwoMemberAgentOrg({ '[data-testid="agent-option-builtin:sde"]', "Agent Org coordinator" ); - await selectRenderedOption( - '[data-testid="agent-orgs-hierarchy-mode-select"]', - '[data-testid="agent-orgs-hierarchy-mode-strict"]', - "Agent Org strict hierarchy mode" - ); await selectRenderedOption( '[data-testid="agent-orgs-plan-approval-policy-select"]', `[data-testid="agent-orgs-plan-approval-policy-${planApprovalPolicy}"]`, @@ -679,48 +703,30 @@ export async function createRenderedStrictTwoMemberAgentOrg({ const leadId = await addMember(leadName, "Lead planner"); const childId = await addMember(childName, "Child implementer"); - const reportsToTriggerClick = await execJS( - js.click(`[data-testid="agent-orgs-member-${childId}-reports-to-select"]`) - ); - if (reportsToTriggerClick !== "clicked") { - throw new Error( - `child reports-to trigger did not click: ${reportsToTriggerClick}` - ); - } - let reportsToContract = null; + let communicationContract = null; await browser.waitUntil( async () => { - reportsToContract = await execJS(` + communicationContract = await execJS(` return { - hasCoordinator: !!document.querySelector('[data-testid="agent-orgs-member-reports-to-coordinator"]'), - hasLead: !!document.querySelector('[data-testid="agent-orgs-member-reports-to-${leadId}"]'), - hasUser: !!document.querySelector('[data-testid="agent-orgs-member-reports-to-user"]'), - options: Array.from(document.querySelectorAll('[data-testid^="agent-orgs-member-reports-to-"]')).map((option) => ({ - testId: option.getAttribute('data-testid'), - text: option.textContent || '', - })), + leadCount: document.querySelector('[data-testid="agent-orgs-member-${leadId}-connected-count"]')?.textContent ?? '', + childCount: document.querySelector('[data-testid="agent-orgs-member-${childId}-connected-count"]')?.textContent ?? '', + hasLeadManage: !!document.querySelector('[data-testid="agent-orgs-member-${leadId}-manage-communication"]'), + hasChildManage: !!document.querySelector('[data-testid="agent-orgs-member-${childId}-manage-communication"]'), }; `); return ( - reportsToContract.hasCoordinator && - reportsToContract.hasLead && - !reportsToContract.hasUser + communicationContract.hasLeadManage && + communicationContract.hasChildManage && + communicationContract.leadCount.includes("1") && + communicationContract.childCount.includes("1") ); }, { timeout: RENDER_TIMEOUT_MS, interval: 100, - timeoutMsg: `Reports-to dropdown contract mismatch: ${JSON.stringify(reportsToContract)}`, + timeoutMsg: `Flat communication summary contract mismatch: ${JSON.stringify(communicationContract)}`, } ); - const reportsToLeadClick = await execJS( - js.click(`[data-testid="agent-orgs-member-reports-to-${leadId}"]`) - ); - if (reportsToLeadClick !== "clicked") { - throw new Error( - `child reports-to lead option did not click: ${reportsToLeadClick}` - ); - } let saveState = null; try { @@ -768,21 +774,27 @@ export async function createRenderedStrictTwoMemberAgentOrg({ throw new Error(`Agent Org save did not click: ${saveResult}`); } const org = await waitForAgentOrgByName(orgName, "rendered org create"); - if (org?.hierarchyMode !== "strict") { - throw new Error( - `Created org did not persist strict hierarchy: ${JSON.stringify(org)}` - ); - } if (org?.planApprovalPolicy !== planApprovalPolicy) { throw new Error( `Created org did not persist plan approval policy ${planApprovalPolicy}: ${JSON.stringify(org)}` ); } - const lead = (org.children ?? []).find((member) => member.name === leadName); - const child = lead?.children?.find((member) => member.name === childName); + const lead = (org.members ?? []).find((member) => member.name === leadName); + const child = (org.members ?? []).find((member) => member.name === childName); if (!lead || !child) { throw new Error( - `Created org did not persist nested members: ${JSON.stringify(org)}` + `Created org did not persist flat members: ${JSON.stringify(org)}` + ); + } + if ( + org.memberCommunicationLinks?.length !== 1 || + org.memberCommunicationLinks[0]?.memberAId !== + [lead.memberId, child.memberId].sort()[0] || + org.memberCommunicationLinks[0]?.memberBId !== + [lead.memberId, child.memberId].sort()[1] + ) { + throw new Error( + `Created org did not persist one canonical Member pair: ${JSON.stringify(org)}` ); } return org;