diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 892082d96c6..c19c94756d1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -531,11 +531,12 @@ Note: Both `TriggerDef` and `ActionDef` use serde internally-tagged enums. Trigg **4 trigger types:** `message_posted`, `reaction_added`, `schedule`, `webhook` -**7 action types:** +**8 action types:** | Action | Description | |--------|-------------| | `send_message` | Post to the workflow's channel (or override channel) | +| `assign_agent` | Dispatch a task to exactly one agent by hex pubkey (or a single template resolving to one); fails closed if the assignee is not a channel member | | `send_dm` | Direct message to a user (pubkey hex or `{{trigger.author}}`) | | `set_channel_topic` | Update channel topic | | `add_reaction` | React to the trigger message | diff --git a/crates/buzz-relay/src/workflow_sink.rs b/crates/buzz-relay/src/workflow_sink.rs index 97c31c25611..cb4f29a84f4 100644 --- a/crates/buzz-relay/src/workflow_sink.rs +++ b/crates/buzz-relay/src/workflow_sink.rs @@ -362,6 +362,194 @@ impl ActionSink for RelayActionSink { Ok(event_id_hex) }) } + + fn assign_agent( + &self, + community_id: CommunityId, + channel_id: &str, + text: &str, + author_pubkey: &str, + agent_pubkey: &str, + task_id: Option<&str>, + ) -> Pin> + Send + '_>> { + let channel_id = channel_id.to_owned(); + let text = text.to_owned(); + let author_pubkey = author_pubkey.to_owned(); + let agent_pubkey = agent_pubkey.to_owned(); + let task_id = task_id.map(str::to_owned); + + Box::pin(async move { + // 0. Upgrade weak reference — fails only during shutdown. + let state = self + .state + .upgrade() + .ok_or_else(|| ActionSinkError::Database("relay is shutting down".into()))?; + + // 1. Resolve tenant for the run's community (see send_message for + // rationale). Fail closed if the community is no longer mapped. + let host = state + .db + .lookup_community_host(community_id) + .await + .map_err(|e| ActionSinkError::Database(e.to_string()))? + .ok_or_else(|| { + ActionSinkError::Database(format!( + "workflow run community {community_id} is not mapped to a host" + )) + })?; + let tenant = buzz_core::tenant::TenantContext::resolved(community_id, host); + + // 2. Validate text. + if text.trim().is_empty() { + return Err(ActionSinkError::EmptyContent); + } + + // 3. Parse/canonicalize channel UUID and look up channel. + let channel_uuid = Uuid::parse_str(&channel_id) + .map_err(|e| ActionSinkError::InvalidInput(format!("invalid UUID: {e}")))?; + let channel_id_canonical = channel_uuid.to_string(); + + let channel = state + .db + .get_channel(tenant.community(), channel_uuid) + .await + .map_err(|e| match &e { + buzz_db::DbError::ChannelNotFound(_) | buzz_db::DbError::NotFound(_) => { + ActionSinkError::ChannelNotFound(channel_id_canonical.clone()) + } + _ => ActionSinkError::Database(e.to_string()), + })?; + + if channel.archived_at.is_some() { + return Err(ActionSinkError::ChannelArchived( + channel_id_canonical.clone(), + )); + } + + // 4. Parse author (workflow owner) and verify their access. + let author_pubkey = nostr::PublicKey::from_hex(&author_pubkey).map_err(|e| { + ActionSinkError::InvalidInput(format!("invalid author pubkey: {e}")) + })?; + let author_pubkey_bytes = author_pubkey.to_bytes().to_vec(); + let author_pubkey_hex = author_pubkey.to_hex(); + let owner_is_member = state + .is_member_cached(tenant.community(), channel_uuid, &author_pubkey_bytes) + .await + .map_err(|e| ActionSinkError::Database(e.to_string()))?; + if !owner_is_member && channel.visibility != "open" { + return Err(ActionSinkError::InvalidInput( + "workflow owner does not have access to destination channel".into(), + )); + } + + // 5. Parse assignee and enforce membership (fail-closed). + // The assignee must already be a channel member; silently + // adding them would let a workflow escalate authority beyond + // what the owner granted at save time. + let agent_pk = nostr::PublicKey::from_hex(&agent_pubkey) + .map_err(|e| ActionSinkError::InvalidInput(format!("invalid agent pubkey: {e}")))?; + let agent_pk_bytes = agent_pk.to_bytes().to_vec(); + let agent_pk_hex = agent_pk.to_hex(); + let agent_is_member = state + .is_member_cached(tenant.community(), channel_uuid, &agent_pk_bytes) + .await + .map_err(|e| ActionSinkError::Database(e.to_string()))?; + if !agent_is_member { + return Err(ActionSinkError::AssigneeNotMember(agent_pk_hex)); + } + + // 6. Build the kind:9 event. + // - Owner attribution `p` tag (same as send_message) + // - Assignee wake `p` tag (dedup against owner) + // - No reverse-parse of `@Name` mentions in `text` — that is + // the failure mode assign_agent exists to avoid. + // - Optional `task` tag with the caller's correlation id. + let mut tags = vec![ + Tag::parse(["p", &author_pubkey_hex]) + .map_err(|e| ActionSinkError::EventBuild(format!("p tag: {e}")))?, + Tag::parse(["h", &channel_id_canonical]) + .map_err(|e| ActionSinkError::EventBuild(format!("h tag: {e}")))?, + Tag::parse(["buzz:workflow", "true"]) + .map_err(|e| ActionSinkError::EventBuild(format!("workflow tag: {e}")))?, + ]; + if agent_pk_hex != author_pubkey_hex { + tags.push( + Tag::parse(["p", &agent_pk_hex]) + .map_err(|e| ActionSinkError::EventBuild(format!("assignee p tag: {e}")))?, + ); + } + if let Some(tid) = task_id.as_deref() { + if !tid.trim().is_empty() { + tags.push( + Tag::parse(["task", tid]) + .map_err(|e| ActionSinkError::EventBuild(format!("task tag: {e}")))?, + ); + } + } + + let kind = Kind::from(KIND_STREAM_MESSAGE as u16); + let event = EventBuilder::new(kind, &text) + .tags(tags) + .sign_with_keys(&state.relay_keypair) + .map_err(|e| ActionSinkError::EventBuild(format!("signing: {e}")))?; + + let event_id_hex = event.id.to_hex(); + let event_id_bytes = event.id.as_bytes().to_vec(); + let kind_u32 = KIND_STREAM_MESSAGE; + + let event_created_at = { + let ts = event.created_at.as_secs() as i64; + chrono::DateTime::from_timestamp(ts, 0).unwrap_or_else(Utc::now) + }; + + info!( + event_id = %event_id_hex, + channel_id = %channel_id_canonical, + author = %author_pubkey, + agent = %agent_pk_hex, + "Workflow AssignAgent: posting kind {kind_u32} event" + ); + + // 7. Persist with thread metadata (top-level, same as send_message). + let thread_meta = Some(buzz_db::event::ThreadMetadataParams { + event_id: &event_id_bytes, + event_created_at, + channel_id: channel_uuid, + parent_event_id: None, + parent_event_created_at: None, + root_event_id: None, + root_event_created_at: None, + depth: 0, + broadcast: false, + }); + + let (stored_event, was_inserted) = state + .db + .insert_event_with_thread_metadata( + tenant.community(), + &event, + Some(channel_uuid), + thread_meta, + ) + .await + .map_err(|e| ActionSinkError::Database(e.to_string()))?; + + // 8. Post-persist fan-out (only on real insert). + if was_inserted { + let _ = dispatch_persistent_event( + &tenant, + &state, + &stored_event, + kind_u32, + &author_pubkey_hex, + None, + ) + .await; + } + + Ok(event_id_hex) + }) + } } #[cfg(test)] @@ -708,4 +896,202 @@ mod integration_tests { "mentioned member {agent_hex} must be p-tagged so it wakes; got {p_tag_targets:?}" ); } + + /// The identity-safety contract for `assign_agent`, exercised end to end + /// against real Postgres. The three assertions map directly to Airy's + /// review corrections (singular assignee, no prose parsing, membership + /// fail-closed): + /// + /// 1. Two channel members share the display name "Winnie" (the exact + /// duplicate-name repro from #4108b496). Dispatching by pubkey wakes + /// exactly the selected pubkey — never the other, never both. + /// 2. The message text contains `@Winnie`, which under `send_message` + /// would be dropped as ambiguous. `assign_agent` must NOT reverse-parse + /// that name; the `p`-tag set must be exactly `{owner, selected agent}`. + /// 3. A non-member assignee is rejected with `AssigneeNotMember` — never + /// silently added, never posted-but-unwaked. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn workflow_assign_agent_wakes_exact_pubkey_and_ignores_prose() { + let state = test_state().await; + + let author = nostr::Keys::generate(); + let author_hex = author.public_key().to_hex(); + + // Two agents sharing the display name "Winnie" (the duplicate-name + // hazard driving Slice 1). One is chosen; the other must not wake. + let winnie_a = nostr::Keys::generate(); + let winnie_a_bytes = winnie_a.public_key().to_bytes().to_vec(); + let winnie_a_hex = winnie_a.public_key().to_hex(); + let winnie_b = nostr::Keys::generate(); + let winnie_b_bytes = winnie_b.public_key().to_bytes().to_vec(); + let winnie_b_hex = winnie_b.public_key().to_hex(); + + let host = format!("wf-assign-{}.example", uuid::Uuid::new_v4().simple()); + let community = match state + .db + .create_community_with_owner(&host, &author_hex) + .await + .expect("create community") + { + CreateCommunityWithOwnerResult::Created(rec) => rec.id, + other => panic!("expected fresh community, got {other:?}"), + }; + + let channel = state + .db + .create_channel( + community, + "wf-assign", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &author.public_key().to_bytes(), + None, + ) + .await + .expect("create channel"); + + for bytes in [&winnie_a_bytes, &winnie_b_bytes] { + state + .db + .ensure_user(community, bytes) + .await + .expect("ensure user row"); + state + .db + .update_user_profile(community, bytes, Some("Winnie"), None, None, None) + .await + .expect("set display name"); + state + .db + .add_member( + community, + channel.id, + bytes, + MemberRole::Bot, + Some(&author.public_key().to_bytes()), + ) + .await + .expect("add member"); + } + + let sink = RelayActionSink::new(&state); + let event_id_hex = sink + .assign_agent( + community, + &channel.id.to_string(), + // Prose contains `@Winnie` — under send_message this is + // ambiguous and drops. assign_agent must NOT reverse-parse it. + "@Winnie please pick this up", + &author_hex, + &winnie_a_hex, + Some("11111111-2222-3333-4444-555555555555"), + ) + .await + .expect("assign_agent"); + + let id_bytes = nostr::EventId::from_hex(&event_id_hex) + .expect("event id") + .as_bytes() + .to_vec(); + let stored = state + .db + .get_event_by_id(community, &id_bytes) + .await + .expect("query event") + .expect("event persisted"); + + let p_tag_targets: Vec<&str> = stored + .event + .tags + .iter() + .filter(|t| t.as_slice().first().map(|s| s.as_str()) == Some("p")) + .filter_map(|t| t.as_slice().get(1).map(|s| s.as_str())) + .collect(); + + // Exactly two `p` tags — owner + selected agent — in that order. + assert_eq!( + p_tag_targets, + vec![author_hex.as_str(), winnie_a_hex.as_str()], + "p-tag set must be exactly {{owner, selected}}; got {p_tag_targets:?}" + ); + // The other same-name member must NOT wake. + assert!( + !p_tag_targets.contains(&winnie_b_hex.as_str()), + "second same-name member {winnie_b_hex} must NOT be p-tagged" + ); + + // The `task` correlation id is present. + let task_tag = stored + .event + .tags + .iter() + .find(|t| t.as_slice().first().map(|s| s.as_str()) == Some("task")) + .expect("task tag present"); + assert_eq!( + task_tag.as_slice().get(1).map(|s| s.as_str()), + Some("11111111-2222-3333-4444-555555555555") + ); + } + + /// A non-member assignee must be rejected fail-closed — the workflow + /// owner's authority cannot be silently extended by mid-run membership + /// changes. + #[tokio::test] + #[ignore = "requires Postgres"] + async fn workflow_assign_agent_rejects_non_member() { + let state = test_state().await; + + let author = nostr::Keys::generate(); + let author_hex = author.public_key().to_hex(); + let stranger = nostr::Keys::generate(); + let stranger_hex = stranger.public_key().to_hex(); + + let host = format!("wf-nonmember-{}.example", uuid::Uuid::new_v4().simple()); + let community = match state + .db + .create_community_with_owner(&host, &author_hex) + .await + .expect("create community") + { + CreateCommunityWithOwnerResult::Created(rec) => rec.id, + other => panic!("expected fresh community, got {other:?}"), + }; + + let channel = state + .db + .create_channel( + community, + "wf-nonmember", + ChannelType::Stream, + ChannelVisibility::Open, + None, + &author.public_key().to_bytes(), + None, + ) + .await + .expect("create channel"); + + // stranger is deliberately NOT added as a channel member. + let sink = RelayActionSink::new(&state); + let err = sink + .assign_agent( + community, + &channel.id.to_string(), + "please pick this up", + &author_hex, + &stranger_hex, + None, + ) + .await + .expect_err("assign_agent must fail when assignee is not a member"); + + match err { + ActionSinkError::AssigneeNotMember(pk) => { + assert_eq!(pk, stranger_hex); + } + other => panic!("expected AssigneeNotMember, got: {other}"), + } + } } diff --git a/crates/buzz-workflow/src/action_sink.rs b/crates/buzz-workflow/src/action_sink.rs index 0c6002e74eb..253ae7ff5b5 100644 --- a/crates/buzz-workflow/src/action_sink.rs +++ b/crates/buzz-workflow/src/action_sink.rs @@ -29,6 +29,13 @@ pub enum ActionSinkError { /// Message content is empty or whitespace-only. #[error("empty message content")] EmptyContent, + /// The target agent is not a member of the destination channel. + /// + /// `assign_agent` is fail-closed: the agent must already be a channel + /// member. Silently adding them would let a workflow escalate authority + /// beyond what the owner granted at save time. + #[error("assignee is not a channel member: {0}")] + AssigneeNotMember(String), } impl From for crate::WorkflowError { @@ -66,4 +73,36 @@ pub trait ActionSink: Send + Sync { text: &str, author_pubkey: &str, ) -> Pin> + Send + '_>>; + + /// Dispatch a task to exactly one agent by immutable pubkey. + /// + /// The relay-side implementation emits exactly two `p` tags on the + /// resulting `kind:9` message: `author_pubkey` (owner attribution) and + /// `agent_pubkey` (wake). The `text` is **not** scanned for `@Name` + /// mentions — that reverse-parse is the failure mode `assign_agent` + /// exists to avoid. + /// + /// Fails with [`ActionSinkError::AssigneeNotMember`] if `agent_pubkey` + /// is not a current member of `channel_id`. Adding them silently would + /// let a workflow escalate beyond the owner's saved authority. + /// + /// - `agent_pubkey`: hex-encoded pubkey of the sole assignee. + /// - `task_id`: optional caller-supplied correlation UUID emitted as a + /// `task` tag. Preserved verbatim; not validated as a UUID here — the + /// executor performs shape validation before calling. + /// + /// Returns the event ID hex string on success. + /// + /// No default implementation is provided intentionally: there is only + /// one production sink, and a runtime "unimplemented" would defeat the + /// identity-safety guarantees this method is being added to enforce. + fn assign_agent( + &self, + community_id: CommunityId, + channel_id: &str, + text: &str, + author_pubkey: &str, + agent_pubkey: &str, + task_id: Option<&str>, + ) -> Pin> + Send + '_>>; } diff --git a/crates/buzz-workflow/src/executor.rs b/crates/buzz-workflow/src/executor.rs index dffa4927168..21fc8af23b9 100644 --- a/crates/buzz-workflow/src/executor.rs +++ b/crates/buzz-workflow/src/executor.rs @@ -448,6 +448,17 @@ pub fn resolve_step_templates( Delay { duration } => Ok(Delay { duration: duration.clone(), }), + AssignAgent { + agent_pubkey, + text, + channel, + task_id, + } => Ok(AssignAgent { + agent_pubkey: t(agent_pubkey)?, + text: t(text)?, + channel: t_opt(channel)?, + task_id: t_opt(task_id)?, + }), } } @@ -596,6 +607,82 @@ pub async fn dispatch_action( }))) } + AssignAgent { + agent_pubkey, + text, + channel, + task_id, + } => { + // Re-validate the *resolved* agent_pubkey. Schema + // validation accepts either static 64-hex or a single + // `{{...}}` template; only after template resolution do we + // know what pubkey the run will actually wake. A resolved + // non-hex string is a definition/data error surfaced as a + // run failure — never a silent misroute or wrong-agent + // wake (an unknown template also passes through unchanged, + // so this catches both). + if !crate::schema::is_lowercase_hex_pubkey(agent_pubkey) { + return Err(WorkflowError::InvalidDefinition(format!( + "AssignAgent: resolved agent_pubkey '{agent_pubkey}' is not a \ + 64-char lowercase hex pubkey" + ))); + } + + let wf_run = engine + .db + .get_workflow_run(community_id, run_id) + .await + .map_err(|e| { + WorkflowError::WebhookError(format!( + "AssignAgent: failed to load workflow run {run_id}: {e}" + )) + })?; + let workflow = engine + .db + .get_workflow(community_id, wf_run.workflow_id) + .await + .map_err(|e| { + WorkflowError::WebhookError(format!( + "AssignAgent: failed to load workflow {}: {e}", + wf_run.workflow_id + )) + })?; + let channel_id = resolve_send_message_channel( + channel.as_deref(), + &trigger_ctx.channel_id, + workflow.channel_id, + )?; + let owner_pubkey_hex = hex::encode(&workflow.owner_pubkey); + + info!( + run_id = %run_id, + step = step_id, + channel = %channel_id, + agent = %agent_pubkey, + "AssignAgent → {channel_id}: {text}" + ); + + let event_id = engine + .action_sink()? + .assign_agent( + community_id, + &channel_id, + text, + &owner_pubkey_hex, + agent_pubkey, + task_id.as_deref(), + ) + .await + .map_err(WorkflowError::from)?; + + Ok(StepResult::Completed(serde_json::json!({ + "assigned": true, + "agent_pubkey": agent_pubkey, + "event_id": event_id, + "task_id": task_id, + }))) + } + SendDm { to, text: _ } => { warn!(run_id = %run_id, step = step_id, "SendDm not yet implemented (to={to})"); // TODO (WF-07): emit DM event. @@ -1871,4 +1958,89 @@ mod tests { .expect("override should be accepted"); assert_eq!(resolved, override_channel_id.to_string()); } + + // --- assign_agent template resolution ---------------------------------- + + /// A 64-char lowercase hex pubkey fixture used across the assign_agent + /// executor tests. + const AGENT_HEX_FIXTURE: &str = + "dcd584bd8bbd49fd62caf3a8a0a43afd38b9a91dcbd3a1c9dba7d082ca024e66"; + + fn assign_step(agent_pubkey: &str, text: &str, task_id: Option<&str>) -> Step { + Step { + id: "assign".to_owned(), + name: None, + if_expr: None, + timeout_secs: None, + action: ActionDef::AssignAgent { + agent_pubkey: agent_pubkey.to_owned(), + text: text.to_owned(), + channel: None, + task_id: task_id.map(str::to_owned), + }, + } + } + + #[test] + fn assign_agent_resolves_text_and_task_id_templates() { + let mut ctx = make_trigger(); + ctx.text = "P1 in prod".to_owned(); + let outputs = HashMap::new(); + let step = assign_step( + AGENT_HEX_FIXTURE, + "New incident: {{trigger.text}}", + Some("{{trigger.message_id}}"), + ); + let resolved = resolve_step_templates(&step, &ctx, &outputs).unwrap(); + match resolved { + ActionDef::AssignAgent { + agent_pubkey, + text, + task_id, + .. + } => { + // Static agent_pubkey is preserved verbatim. + assert_eq!(agent_pubkey, AGENT_HEX_FIXTURE); + assert_eq!(text, "New incident: P1 in prod"); + assert_eq!(task_id.as_deref(), Some("event-id-hex")); + } + other => panic!("unexpected resolved action: {other:?}"), + } + } + + #[test] + fn assign_agent_resolves_agent_pubkey_template_through_trigger_author() { + // `{{trigger.author}}` is the "reply to the sender" pattern — the + // executor must template-resolve agent_pubkey so this works. + let mut ctx = make_trigger(); + ctx.author = AGENT_HEX_FIXTURE.to_owned(); + let outputs = HashMap::new(); + let step = assign_step("{{trigger.author}}", "please respond", None); + let resolved = resolve_step_templates(&step, &ctx, &outputs).unwrap(); + match resolved { + ActionDef::AssignAgent { agent_pubkey, .. } => { + assert_eq!(agent_pubkey, AGENT_HEX_FIXTURE); + } + other => panic!("unexpected resolved action: {other:?}"), + } + } + + #[test] + fn is_lowercase_hex_pubkey_accepts_canonical_and_rejects_case_length_or_symbols() { + use crate::schema::is_lowercase_hex_pubkey; + + assert!(is_lowercase_hex_pubkey(AGENT_HEX_FIXTURE)); + + // Uppercase, short, long, and non-hex all fail. + assert!(!is_lowercase_hex_pubkey(&AGENT_HEX_FIXTURE.to_uppercase())); + assert!(!is_lowercase_hex_pubkey(&AGENT_HEX_FIXTURE[..63])); + assert!(!is_lowercase_hex_pubkey(&format!("{AGENT_HEX_FIXTURE}0"))); + let non_hex = format!("{}z", &AGENT_HEX_FIXTURE[..63]); + assert!(!is_lowercase_hex_pubkey(&non_hex)); + + // An unresolved template that passed through resolution unchanged must + // NOT pass as a hex pubkey — this is what protects against silent + // misroutes when a template variable name is misspelled. + assert!(!is_lowercase_hex_pubkey("{{trigger.author}}")); + } } diff --git a/crates/buzz-workflow/src/schema.rs b/crates/buzz-workflow/src/schema.rs index 9bc79aa48b3..64051045556 100644 --- a/crates/buzz-workflow/src/schema.rs +++ b/crates/buzz-workflow/src/schema.rs @@ -144,6 +144,37 @@ pub enum ActionDef { /// Duration string (e.g. `"5m"`, `"1h"`). duration: String, }, + /// Dispatch a task to exactly one agent identified by immutable pubkey. + /// + /// Unlike [`ActionDef::SendMessage`], which reverse-parses `@Name` from + /// prose against channel membership (ambiguous names silently wake no one; + /// renames silently rewrite the target), `assign_agent` binds dispatch to + /// the target's hex pubkey. The relay sink emits exactly two `p` tags on + /// the resulting message: the workflow owner (attribution) and + /// `agent_pubkey` (wake). The message body is never scanned for narrative + /// names. + /// + /// `agent_pubkey` accepts either a static 64-char lowercase hex pubkey or a + /// single `{{...}}` template placeholder (e.g. `{{trigger.author}}`). + /// Mixed literal+template strings are rejected so a stray `@Name` cannot + /// smuggle an identity into the field. The resolved value must still be + /// 64-char lowercase hex at dispatch time and must belong to a current + /// member of the destination channel — a non-member fails the run. + AssignAgent { + /// Hex pubkey (64 lowercase hex chars) OR a single template + /// placeholder that resolves to one. See variant docs. + agent_pubkey: String, + /// Task text posted to the channel (supports template variables). + text: String, + /// Optional channel UUID override. Must equal the workflow's channel + /// when the workflow is bound to one (matches `send_message`). + #[serde(default)] + channel: Option, + /// Optional caller-supplied correlation id for downstream tracking. + /// Must be a valid UUID when set. + #[serde(default)] + task_id: Option, + }, } impl WorkflowDef { @@ -203,6 +234,7 @@ impl WorkflowDef { step.id ))); } + validate_action(&step.id, &step.action)?; } if let TriggerDef::Schedule { cron, interval } = &self.trigger { @@ -242,6 +274,82 @@ impl WorkflowDef { } } +/// Per-action definition-time validation. Called by [`WorkflowDef::validate`] +/// for every step. Rejects malformed inputs that would otherwise only surface +/// as a runtime failure — for `assign_agent` this is the identity-safety line: +/// a workflow that mistypes an agent pubkey should never save. +fn validate_action(step_id: &str, action: &ActionDef) -> Result<(), WorkflowError> { + if let ActionDef::AssignAgent { + agent_pubkey, + text, + channel, + task_id, + } = action + { + if text.trim().is_empty() { + return Err(WorkflowError::InvalidDefinition(format!( + "assign_agent step '{step_id}': text must not be empty" + ))); + } + if !is_agent_pubkey_shape(agent_pubkey) { + return Err(WorkflowError::InvalidDefinition(format!( + "assign_agent step '{step_id}': agent_pubkey must be a 64-char lowercase hex pubkey \ + or a single {{{{...}}}} template placeholder (got '{agent_pubkey}')" + ))); + } + if let Some(ch) = channel { + let trimmed = ch.trim(); + if !trimmed.is_empty() && trimmed.parse::().is_err() { + return Err(WorkflowError::InvalidDefinition(format!( + "assign_agent step '{step_id}': channel override '{ch}' is not a valid UUID" + ))); + } + } + if let Some(tid) = task_id { + let trimmed = tid.trim(); + if trimmed.is_empty() || trimmed.parse::().is_err() { + return Err(WorkflowError::InvalidDefinition(format!( + "assign_agent step '{step_id}': task_id '{tid}' must be a valid UUID when set" + ))); + } + } + } + Ok(()) +} + +/// True when `s` is exactly 64 lowercase hex characters — the canonical +/// on-the-wire pubkey shape. Membership lookups are performed against these +/// bytes, so a mixed-case or short/long input can never match a real member +/// and must not slip through. +pub(crate) fn is_lowercase_hex_pubkey(s: &str) -> bool { + let trimmed = s.trim(); + trimmed.len() == 64 + && trimmed + .chars() + .all(|c| c.is_ascii_digit() || matches!(c, 'a'..='f')) +} + +/// True when `s` is a single, unbroken `{{...}}` template placeholder with +/// non-empty inner content — e.g. `{{trigger.author}}`. Rejects mixed +/// literal+template forms (`prefix-{{x}}`) so a caller cannot smuggle a +/// name-like segment into an identity field. +pub(crate) fn is_single_template(s: &str) -> bool { + let trimmed = s.trim(); + trimmed.starts_with("{{") + && trimmed.ends_with("}}") + && trimmed.len() >= 4 + && !trimmed[2..trimmed.len() - 2].contains("{{") + && !trimmed[2..trimmed.len() - 2].contains("}}") + && !trimmed[2..trimmed.len() - 2].trim().is_empty() +} + +/// Definition-time shape check for `agent_pubkey`: accepts either a static +/// pubkey or a single template placeholder. The resolved value is +/// re-validated with [`is_lowercase_hex_pubkey`] at dispatch time. +pub(crate) fn is_agent_pubkey_shape(s: &str) -> bool { + is_lowercase_hex_pubkey(s) || is_single_template(s) +} + /// Validate a cron expression using the `cron` crate. /// /// The `cron` crate requires 7 fields: `sec min hour dom month dow year`. @@ -359,9 +467,10 @@ mod tests { " - id: hook\n action: call_webhook\n url: https://hooks.example.com/notify\n method: POST\n", " - id: approve\n action: request_approval\n from: '@manager'\n message: Approve?\n timeout: 4h\n", " - id: wait\n action: delay\n duration: 5m\n", + " - id: assign\n action: assign_agent\n agent_pubkey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n text: Please take this\n", ); let (def, _) = parse_yaml(yaml).expect("parse failed"); - assert_eq!(def.steps.len(), 7); + assert_eq!(def.steps.len(), 8); assert!(matches!( &def.steps[0].action, @@ -385,6 +494,10 @@ mod tests { ActionDef::RequestApproval { .. } )); assert!(matches!(&def.steps[6].action, ActionDef::Delay { .. })); + assert!(matches!( + &def.steps[7].action, + ActionDef::AssignAgent { .. } + )); } #[test] @@ -888,4 +1001,183 @@ mod tests { TriggerDef::DiffPosted { filter: Some(_) } )); } + + // --- assign_agent ------------------------------------------------------- + + /// 64-char lowercase hex fixture — a valid `agent_pubkey` shape. + const AGENT_HEX: &str = "dcd584bd8bbd49fd62caf3a8a0a43afd38b9a91dcbd3a1c9dba7d082ca024e66"; + + #[test] + fn assign_agent_parses_minimal_fields() { + let yaml = format!( + "name: Dispatch\ntrigger:\n on: webhook\nsteps:\n - id: assign\n action: assign_agent\n agent_pubkey: {AGENT_HEX}\n text: 'Please pick up ticket 42'\n", + ); + let (def, _) = parse_yaml(&yaml).expect("minimal assign_agent should parse"); + match &def.steps[0].action { + ActionDef::AssignAgent { + agent_pubkey, + text, + channel, + task_id, + } => { + assert_eq!(agent_pubkey, AGENT_HEX); + assert_eq!(text, "Please pick up ticket 42"); + assert!(channel.is_none()); + assert!(task_id.is_none()); + } + other => panic!("unexpected action: {other:?}"), + } + } + + #[test] + fn assign_agent_round_trips_through_canonical_json() { + let channel_uuid = "4108b496-0efb-4fc6-85e3-6c88defb467c"; + let task_uuid = "11111111-2222-3333-4444-555555555555"; + let yaml = format!( + "name: Dispatch\ntrigger:\n on: webhook\nsteps:\n - id: assign\n action: assign_agent\n agent_pubkey: {AGENT_HEX}\n text: 'Do the thing'\n channel: {channel_uuid}\n task_id: {task_uuid}\n", + ); + let (def, json) = parse_yaml(&yaml).expect("assign_agent with all fields should parse"); + let reparsed: WorkflowDef = serde_json::from_str(&json).expect("json round-trip"); + match &reparsed.steps[0].action { + ActionDef::AssignAgent { + agent_pubkey, + text, + channel, + task_id, + } => { + assert_eq!(agent_pubkey, AGENT_HEX); + assert_eq!(text, "Do the thing"); + assert_eq!(channel.as_deref(), Some(channel_uuid)); + assert_eq!(task_id.as_deref(), Some(task_uuid)); + } + other => panic!("unexpected action: {other:?}"), + } + // Round-trip preserved the AssignAgent variant end-to-end. + assert_eq!(def.steps.len(), reparsed.steps.len()); + } + + #[test] + fn assign_agent_accepts_template_pubkey() { + // A single, unbroken template placeholder is allowed so a workflow can + // route back to the triggering author. The resolved value is + // re-validated at dispatch time. + let yaml = "name: Reply\ntrigger:\n on: message_posted\nsteps:\n - id: assign\n action: assign_agent\n agent_pubkey: '{{trigger.author}}'\n text: 'Follow up please'\n"; + let (def, _) = parse_yaml(yaml).expect("template agent_pubkey should parse"); + match &def.steps[0].action { + ActionDef::AssignAgent { agent_pubkey, .. } => { + assert_eq!(agent_pubkey, "{{trigger.author}}"); + } + other => panic!("unexpected action: {other:?}"), + } + } + + #[test] + fn assign_agent_rejects_uppercase_hex_pubkey() { + // Case matters for identity: `dcd…` and `DCD…` render the same but + // channel-member pubkeys are stored/compared as lowercase hex. Reject + // the mixed-case shape at definition time so a workflow does not save + // in a state that would then miss the membership check. + let upper = AGENT_HEX.to_uppercase(); + let yaml = format!( + "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: {upper}\n text: hi\n", + ); + let err = parse_yaml(&yaml).unwrap_err(); + match &err { + WorkflowError::InvalidDefinition(msg) => { + assert!( + msg.contains("agent_pubkey"), + "error should mention agent_pubkey, got: {msg}" + ); + } + other => panic!("expected InvalidDefinition, got: {other}"), + } + } + + #[test] + fn assign_agent_rejects_short_hex_pubkey() { + // 63 chars — one short of the pubkey length. + let short = &AGENT_HEX[..63]; + let yaml = format!( + "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: {short}\n text: hi\n", + ); + let err = parse_yaml(&yaml).unwrap_err(); + assert!(matches!(err, WorkflowError::InvalidDefinition(_))); + } + + #[test] + fn assign_agent_rejects_non_hex_pubkey() { + // 64 chars but includes a non-hex letter. + let non_hex = format!("{}z", &AGENT_HEX[..63]); + let yaml = format!( + "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: {non_hex}\n text: hi\n", + ); + let err = parse_yaml(&yaml).unwrap_err(); + assert!(matches!(err, WorkflowError::InvalidDefinition(_))); + } + + #[test] + fn assign_agent_rejects_mixed_literal_and_template_pubkey() { + // Mixed strings are the identity-smuggle vector — literal characters + // mean the resolved value can never be exactly one pubkey. Reject. + let yaml = "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: 'prefix-{{trigger.author}}'\n text: hi\n"; + let err = parse_yaml(yaml).unwrap_err(); + assert!(matches!(err, WorkflowError::InvalidDefinition(_))); + } + + #[test] + fn assign_agent_rejects_empty_template_pubkey() { + // `{{ }}` alone is not a well-formed template — reject rather than + // let it slip through as "template-shaped". + let yaml = "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: '{{ }}'\n text: hi\n"; + let err = parse_yaml(yaml).unwrap_err(); + assert!(matches!(err, WorkflowError::InvalidDefinition(_))); + } + + #[test] + fn assign_agent_rejects_empty_text() { + let yaml = format!( + "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: {AGENT_HEX}\n text: ' '\n", + ); + let err = parse_yaml(&yaml).unwrap_err(); + match &err { + WorkflowError::InvalidDefinition(msg) => { + assert!(msg.contains("text"), "error should mention text: {msg}"); + } + other => panic!("expected InvalidDefinition, got: {other}"), + } + } + + #[test] + fn assign_agent_rejects_invalid_task_id() { + let yaml = format!( + "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: {AGENT_HEX}\n text: hi\n task_id: not-a-uuid\n", + ); + let err = parse_yaml(&yaml).unwrap_err(); + match &err { + WorkflowError::InvalidDefinition(msg) => { + assert!( + msg.contains("task_id"), + "error should mention task_id: {msg}" + ); + } + other => panic!("expected InvalidDefinition, got: {other}"), + } + } + + #[test] + fn assign_agent_rejects_invalid_channel_uuid() { + let yaml = format!( + "name: Bad\ntrigger:\n on: webhook\nsteps:\n - id: s\n action: assign_agent\n agent_pubkey: {AGENT_HEX}\n text: hi\n channel: not-a-uuid\n", + ); + let err = parse_yaml(&yaml).unwrap_err(); + match &err { + WorkflowError::InvalidDefinition(msg) => { + assert!( + msg.contains("channel"), + "error should mention channel: {msg}" + ); + } + other => panic!("expected InvalidDefinition, got: {other}"), + } + } }