From 547004e32d78cb3769a1f211b53c18863030cbb1 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Tue, 30 Jun 2026 20:38:28 +0200 Subject: [PATCH] =?UTF-8?q?feat(modules):=20author=20group=20trigger=20tem?= =?UTF-8?q?plates=20(event=20kinds)=20(=C2=A711=20tail=20T2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A [group] node may now declare EVENT trigger templates (kv/docs/files/ pubsub) that bind a group-owned handler. cron/queue/email stay app-only (rejected on a group at both the manifest and the reconcile layer). - Trigger is now owner-polymorphic (app_id: Option, group_id: Option), mirroring Script's Phase-4 reshape; TriggerRow carries group_id (#[sqlx(default)] so interactive RETURNING clauses still hydrate). - insert_trigger_tx takes a ScriptOwner (App XOR Group) and writes the group_id column; the queue advisory-lock branch is app-only. - TriggerRepo::list_for_group loads a group's templates; load_current's Group arm uses it so the diff is NoOp on re-apply and prune-able. - apply_service reconcile drops the "triggers are app-only" assumption; validate_bundle_for rejects non-event kinds on a group. The semantic- identity diff is already per-owner. - CLI manifest allows event-kind [[triggers]] on [group], rejects cron/queue/email there. Templates don't fire yet — the live dispatch union lands in T3. All trigger/apply/manifest unit tests green; clippy -D clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/apply_service.rs | 55 +++++++++++---- crates/manager-core/src/trigger_repo.rs | 90 +++++++++++++++++++----- crates/manager-core/src/triggers_api.rs | 32 ++++++--- crates/picloud-cli/src/manifest.rs | 12 ++-- 4 files changed, 145 insertions(+), 44 deletions(-) diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index d5f1a5c..9dea69a 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -588,10 +588,25 @@ impl ApplyService { "a group manifest cannot declare routes — routes bind to an app".into(), )); } - if !bundle.triggers.is_empty() { - return Err(ApplyError::Invalid( - "a group manifest cannot declare triggers — triggers belong to an app".into(), - )); + // §11 tail: a group MAY declare trigger TEMPLATES, but only for the + // stateless EVENT kinds (kv/docs/files/pubsub) — those resolve live + // via the chain union at dispatch. cron/queue/email carry per-app + // state (last_fired_at, the one-consumer lock, a sealed secret) and + // would need materialization; reject them on a group, deferred. + for t in &bundle.triggers { + if !matches!( + t, + BundleTrigger::Kv { .. } + | BundleTrigger::Docs { .. } + | BundleTrigger::Files { .. } + | BundleTrigger::Pubsub { .. } + ) { + return Err(ApplyError::Invalid( + "a group trigger template must be an event kind \ + (kv, docs, files, pubsub); cron/queue/email are app-only" + .into(), + )); + } } } // §11.6: shared collections are owned by GROUPS. Reject them on an app @@ -793,17 +808,19 @@ impl ApplyService { if ch.op != Op::Create { continue; } - // Triggers only exist on app nodes (a group bundle has none). - let app_id = owner.app_id().expect("triggers only exist on app nodes"); + // §11 tail: an app node owns triggers; a group node owns trigger + // TEMPLATES (event kinds only — validated in `validate_bundle_for`). let bt = bundle_triggers[&ch.key]; let sid = resolve_script(&name_to_id, bt.script())?; if let BundleTrigger::Email { inbound_secret_ref, .. } = bt { - // Resolve the referenced secret, decrypt it, and re-seal it - // into the email trigger's detail row — the value never - // travels in the manifest. + // Email is app-only (a group template rejects it in + // `validate_bundle_for`). Resolve the referenced secret, + // decrypt it, and re-seal it into the email trigger's detail + // row — the value never travels in the manifest. + let app_id = owner.app_id().expect("email triggers are app-only"); let (ct, nonce) = self.resolve_and_seal(app_id, inbound_secret_ref).await?; insert_email_trigger_tx(&mut *tx, app_id, sid, actor, &ct, &nonce) .await @@ -815,7 +832,15 @@ impl ApplyService { self.trigger_config.queue_default_visibility_timeout_secs, ); insert_trigger_tx( - &mut *tx, app_id, sid, actor, dispatch, retry_max, backoff, base, &details, + &mut *tx, + owner.as_script_owner(), + sid, + actor, + dispatch, + retry_max, + backoff, + base, + &details, ) .await .map_err(map_trig)?; @@ -2080,7 +2105,12 @@ impl ApplyService { .await .map_err(|e| ApplyError::Backend(e.to_string()))?, Vec::new(), - Vec::new(), + // §11 tail: a group owns trigger TEMPLATES; load them so the + // diff sees existing markers (NoOp on re-apply, prune-able). + self.triggers + .list_for_group(group_id) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?, Vec::new(), VarOwner::Group(group_id), ), @@ -3491,7 +3521,8 @@ mod tests { }; Trigger { id: TriggerId::from(uuid::Uuid::new_v4()), - app_id: AppId::from(uuid::Uuid::nil()), + app_id: Some(AppId::from(uuid::Uuid::nil())), + group_id: None, script_id, name: "t".into(), kind, diff --git a/crates/manager-core/src/trigger_repo.rs b/crates/manager-core/src/trigger_repo.rs index 584a828..adf232c 100644 --- a/crates/manager-core/src/trigger_repo.rs +++ b/crates/manager-core/src/trigger_repo.rs @@ -7,7 +7,8 @@ use async_trait::async_trait; use chrono::{DateTime, Utc}; use futures::stream::{self, TryStreamExt}; use picloud_shared::{ - AdminUserId, AppId, DocsEventOp, FilesEventOp, KvEventOp, ScriptId, TriggerId, + AdminUserId, AppId, DocsEventOp, FilesEventOp, GroupId, KvEventOp, ScriptId, ScriptOwner, + TriggerId, }; use serde::{Deserialize, Serialize}; use sqlx::PgPool; @@ -32,7 +33,10 @@ pub enum TriggerRepoError { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Trigger { pub id: TriggerId, - pub app_id: AppId, + /// Polymorphic owner (§11 tail): an app owns a trigger; a group owns a + /// trigger TEMPLATE. Exactly one is set (DB CHECK). Mirrors `Script`. + pub app_id: Option, + pub group_id: Option, pub script_id: ScriptId, /// §4.5 per-app trigger identifier; the manifest merge/upsert key. pub name: String, @@ -414,6 +418,13 @@ pub trait TriggerRepo: Send + Sync { async fn list_for_app(&self, app_id: AppId) -> Result, TriggerRepoError>; + /// Group-owned trigger TEMPLATES (§11 tail) declared directly at `group_id`. + /// Used by the declarative apply diff for a `[group]` node. Defaults to + /// empty so non-Postgres impls (tests) need not provide it. + async fn list_for_group(&self, _group_id: GroupId) -> Result, TriggerRepoError> { + Ok(Vec::new()) + } + async fn get(&self, id: TriggerId) -> Result, TriggerRepoError>; async fn delete(&self, id: TriggerId) -> Result; @@ -511,7 +522,7 @@ impl PostgresTriggerRepo { #[allow(clippy::too_many_arguments, clippy::too_many_lines)] pub(crate) async fn insert_trigger_tx( tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, - app_id: AppId, + owner: ScriptOwner, script_id: ScriptId, registered_by: AdminUserId, dispatch_mode: TriggerDispatchMode, @@ -533,12 +544,20 @@ pub(crate) async fn insert_trigger_tx( )); } }; + // §11 tail: a trigger is owned by an app XOR a group (the latter a + // template, unioned into descendant apps via the chain CTE at dispatch). + let (owner_app_id, owner_group_id) = match owner { + ScriptOwner::App(a) => (Some(a.into_inner()), None), + ScriptOwner::Group(g) => (None, Some(g.into_inner())), + }; // Queue: enforce the one-consumer-per-(app_id, queue_name) invariant — // the same advisory-lock + existence guard the interactive // `create_queue_trigger` uses. Without this, a concurrent apply + // interactive create on disjoint locks could double-register a queue // consumer (there is no DB unique constraint backing the invariant). - if let TriggerDetails::Queue { queue_name, .. } = details { + // Queue is app-only (rejected on a group), so this never runs for a + // group owner. + if let (TriggerDetails::Queue { queue_name, .. }, ScriptOwner::App(app_id)) = (details, owner) { sqlx::query("SELECT pg_advisory_xact_lock($1)") .bind(advisory_lock_key(app_id, queue_name)) .execute(&mut **tx) @@ -560,12 +579,13 @@ pub(crate) async fn insert_trigger_tx( } let row: (Uuid,) = sqlx::query_as( "INSERT INTO triggers ( \ - app_id, script_id, kind, enabled, dispatch_mode, \ + app_id, group_id, script_id, kind, enabled, dispatch_mode, \ retry_max_attempts, retry_backoff, retry_base_ms, \ registered_by_principal \ - ) VALUES ($1, $2, $3, TRUE, $4, $5, $6, $7, $8) RETURNING id", + ) VALUES ($1, $2, $3, $4, TRUE, $5, $6, $7, $8, $9) RETURNING id", ) - .bind(app_id.into_inner()) + .bind(owner_app_id) + .bind(owner_group_id) .bind(script_id.into_inner()) .bind(kind) .bind(dispatch_mode.as_str()) @@ -766,7 +786,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::Kv, @@ -832,7 +853,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::Docs, @@ -893,7 +915,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::DeadLetter, @@ -960,7 +983,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::Cron, @@ -1027,7 +1051,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::Files, @@ -1089,7 +1114,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::Pubsub, @@ -1149,7 +1175,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::Email, @@ -1220,6 +1247,28 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(out) } + async fn list_for_group(&self, group_id: GroupId) -> Result, TriggerRepoError> { + let parents: Vec = sqlx::query_as( + "SELECT id, app_id, group_id, script_id, name, kind, enabled, dispatch_mode, \ + retry_max_attempts, retry_backoff, retry_base_ms, \ + registered_by_principal, created_at, updated_at \ + FROM triggers WHERE group_id = $1 ORDER BY created_at DESC", + ) + .bind(group_id.into_inner()) + .fetch_all(&self.pool) + .await?; + let pool = self.pool.clone(); + let out = stream::iter(parents.into_iter().map(Ok)) + .map_ok(|p| { + let pool = pool.clone(); + async move { hydrate_one(&pool, p).await } + }) + .try_buffered(8) + .try_collect::>() + .await?; + Ok(out) + } + async fn get(&self, id: TriggerId) -> Result, TriggerRepoError> { let parent: Option = sqlx::query_as( "SELECT id, app_id, script_id, name, kind, enabled, dispatch_mode, \ @@ -1501,7 +1550,8 @@ impl TriggerRepo for PostgresTriggerRepo { Ok(Trigger { id: parent.id.into(), - app_id: parent.app_id.into(), + app_id: parent.app_id.map(Into::into), + group_id: parent.group_id.map(Into::into), script_id: parent.script_id.into(), name: parent.name.clone(), kind: TriggerKind::Queue, @@ -1709,7 +1759,8 @@ async fn hydrate_one(pool: &PgPool, parent: TriggerRow) -> Result bool { #[derive(sqlx::FromRow)] struct TriggerRow { id: Uuid, - app_id: Uuid, + app_id: Option, + // `default` so the interactive-create RETURNING clauses (which don't list + // group_id — they only ever build app-owned rows) still hydrate; the + // group/app list queries SELECT it explicitly. + #[sqlx(default)] + group_id: Option, script_id: Uuid, name: String, kind: String, diff --git a/crates/manager-core/src/triggers_api.rs b/crates/manager-core/src/triggers_api.rs index a93654d..add0100 100644 --- a/crates/manager-core/src/triggers_api.rs +++ b/crates/manager-core/src/triggers_api.rs @@ -698,7 +698,9 @@ async fn delete_trigger( .get(trigger_id) .await? .ok_or(TriggersApiError::NotFound(trigger_id))?; - if trigger.app_id != app_id { + // Interactive trigger management is app-scoped; a group TEMPLATE + // (app_id NULL) is never addressable here, so a mismatch (incl. None) 404s. + if trigger.app_id != Some(app_id) { return Err(TriggersApiError::NotFound(trigger_id)); } if !s.triggers.delete(trigger_id).await? { @@ -900,7 +902,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: crate::trigger_repo::TriggerKind::Kv, @@ -929,7 +932,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: crate::trigger_repo::TriggerKind::Docs, @@ -958,7 +962,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: crate::trigger_repo::TriggerKind::DeadLetter, @@ -988,7 +993,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: TriggerKind::Email, @@ -1015,7 +1021,7 @@ mod tests { Ok(g.get(&trigger_id) .filter(|t| t.kind == TriggerKind::Email) .map(|t| EmailInboundTarget { - app_id: t.app_id, + app_id: t.app_id.expect("email trigger is app-owned"), script_id: t.script_id, enabled: t.enabled, dispatch_mode: t.dispatch_mode, @@ -1033,7 +1039,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: crate::trigger_repo::TriggerKind::Cron, @@ -1063,7 +1070,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: crate::trigger_repo::TriggerKind::Files, @@ -1092,7 +1100,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: crate::trigger_repo::TriggerKind::Pubsub, @@ -1117,7 +1126,7 @@ mod tests { .lock() .await .values() - .filter(|t| t.app_id == app_id) + .filter(|t| t.app_id == Some(app_id)) .cloned() .collect()) } @@ -1169,7 +1178,8 @@ mod tests { let id = TriggerId::new(); let trigger = Trigger { id, - app_id, + app_id: Some(app_id), + group_id: None, script_id: req.script_id, name: "mock".into(), kind: TriggerKind::Queue, diff --git a/crates/picloud-cli/src/manifest.rs b/crates/picloud-cli/src/manifest.rs index 451e9c5..c9a9a92 100644 --- a/crates/picloud-cli/src/manifest.rs +++ b/crates/picloud-cli/src/manifest.rs @@ -67,17 +67,21 @@ impl Manifest { anyhow::bail!("manifest declares neither [app] nor [group]") } } - // A group node owns only scripts + vars (+ secret names) — routes and - // triggers are app concerns. Reject them early with a clear message. + // A group node owns scripts + vars (+ secret names) and, as of §11 tail, + // EVENT trigger TEMPLATES (kv/docs/files/pubsub) that fan out live to + // descendant apps. Routes stay app-only; so do the stateful trigger + // kinds (cron/queue/email need per-app state/secrets → materialization). if m.group.is_some() { if !m.routes.is_empty() { anyhow::bail!( "a [group] manifest cannot declare [[routes]] — routes bind to an app" ); } - if !m.triggers.is_empty() { + let t = &m.triggers; + if !t.cron.is_empty() || !t.queue.is_empty() || !t.email.is_empty() { anyhow::bail!( - "a [group] manifest cannot declare [[triggers]] — triggers belong to an app" + "a [group] trigger template must be an event kind \ + (kv, docs, files, pubsub); cron/queue/email are app-only" ); } }