From 86c57e2e43e965cf2539233c1d2d995be604e660 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 1 Jul 2026 20:04:32 +0200 Subject: [PATCH] feat(suppress): owner-polymorphic suppression repo + reconcile (M1.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit suppression_repo takes a ScriptOwner (app or group), binding the matching owner column. apply_service load_current / create / prune / suppression_report all thread owner.as_script_owner(); the validate_bundle_for group-suppress rejection and the manifest parse guard are dropped — a [group] may now declare [suppress] to decline a template it inherits from a higher ancestor. No runtime consumption effect yet (the chain filters land in M1.3/M1.4). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/apply_service.rs | 72 +++++++------- crates/manager-core/src/suppression_repo.rs | 103 +++++++++++++------- crates/picloud-cli/src/manifest.rs | 21 ++-- 3 files changed, 110 insertions(+), 86 deletions(-) diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index e589208..89fe9ca 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -767,20 +767,11 @@ impl ApplyService { )); } } - // §11 tail: only an APP opts out of inherited templates. A group that - // doesn't want a template simply doesn't declare it (the CLI already - // rejects `[suppress]` on a `[group]`; this guards a hand-rolled wire - // Bundle — the reconcile's `owner.app_id().expect(...)` would otherwise - // panic on a group). - if matches!(owner, ApplyOwner::Group(_)) - && (!bundle.suppress_triggers.is_empty() || !bundle.suppress_routes.is_empty()) - { - return Err(ApplyError::Invalid( - "a group cannot declare suppressions — only an app opts out of \ - inherited templates" - .into(), - )); - } + // §11 tail M1: both an app and a group may declare suppressions — an + // app declines an inherited template for itself, a group for its whole + // subtree. Both are inheritance-only (the dispatch filters gate to + // group-owned templates on the suppressing owner's chain), so no + // owner-kind guard here. self.validate_bundle(bundle, inherited_endpoints) } @@ -1065,17 +1056,22 @@ impl ApplyService { } // 3e. Per-app suppression markers (§11 tail) — insert each Create - // (idempotent). App-only; deletes happen in prune → the template - // re-inherits. Key is `"{target_kind}:{reference}"`; split on the FIRST - // `:` so a route reference containing `:` (a param path `/users/:id`) - // survives. + // (idempotent). Owner-polymorphic (§11 tail M1): an app declines for + // itself, a group for its whole subtree. Deletes happen in prune → the + // template re-inherits. Key is `"{target_kind}:{reference}"`; split on + // the FIRST `:` so a route reference containing `:` (a param path + // `/users/:id`) survives. for ch in &plan.suppressions { if ch.op == Op::Create { - let app_id = owner.app_id().expect("suppressions are app-only"); let (kind, reference) = ch.key.split_once(':').expect("suppression key has ':'"); - crate::suppression_repo::insert_suppression_tx(&mut *tx, app_id, kind, reference) - .await - .map_err(|e| ApplyError::Backend(e.to_string()))?; + crate::suppression_repo::insert_suppression_tx( + &mut *tx, + owner.as_script_owner(), + kind, + reference, + ) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; report.suppressions_created += 1; } } @@ -1188,15 +1184,17 @@ impl ApplyService { } } - // Per-app suppression markers are prunable config too (§11 tail). + // Suppression markers are prunable config too (§11 tail). // Removing a marker re-inherits the template. for ch in &plan.suppressions { if ch.op == Op::Delete { - let app_id = owner.app_id().expect("suppressions are app-only"); let (kind, reference) = ch.key.split_once(':').expect("suppression key has ':'"); crate::suppression_repo::delete_suppression_tx( - &mut *tx, app_id, kind, reference, + &mut *tx, + owner.as_script_owner(), + kind, + reference, ) .await .map_err(|e| ApplyError::Backend(e.to_string()))?; @@ -2221,17 +2219,14 @@ impl ApplyService { Ok(out) } - /// Read-only §11 tail report: an app's own suppression markers (the - /// inherited templates it declines). Group owner → empty (groups don't - /// suppress). Backs `pic suppress ls --app`. + /// Read-only §11 tail report: an owner's own suppression markers (the + /// inherited templates it declines). An app declines for itself, a group + /// for its subtree. Backs `pic suppress ls --app` / `--group`. pub async fn suppression_report( &self, owner: ApplyOwner, ) -> Result, ApplyError> { - let ApplyOwner::App(app_id) = owner else { - return Ok(Vec::new()); - }; - let rows = crate::suppression_repo::list_for_app(&self.pool, app_id) + let rows = crate::suppression_repo::list_for_owner(&self.pool, owner.as_script_owner()) .await .map_err(|e| ApplyError::Backend(e.to_string()))?; Ok(rows @@ -2529,14 +2524,13 @@ impl ApplyService { crate::group_collection_repo::list_all_for_owner(&self.pool, owner.as_script_owner()) .await .map_err(|e| ApplyError::Backend(e.to_string()))?; - // §11 tail per-app suppression markers — app-only (a group never - // suppresses; it just wouldn't declare the template). - let suppressions = match owner { - ApplyOwner::App(app_id) => crate::suppression_repo::list_for_app(&self.pool, app_id) + // §11 tail suppression markers declared directly at this node + // (§11 tail M1: owner-polymorphic — an app declines for itself, a group + // for its subtree). + let suppressions = + crate::suppression_repo::list_for_owner(&self.pool, owner.as_script_owner()) .await - .map_err(|e| ApplyError::Backend(e.to_string()))?, - ApplyOwner::Group(_) => Vec::new(), - }; + .map_err(|e| ApplyError::Backend(e.to_string()))?; Ok(CurrentState { scripts, routes, diff --git a/crates/manager-core/src/suppression_repo.rs b/crates/manager-core/src/suppression_repo.rs index a1b68af..631e108 100644 --- a/crates/manager-core/src/suppression_repo.rs +++ b/crates/manager-core/src/suppression_repo.rs @@ -1,52 +1,62 @@ //! Template-suppression markers (§11 tail) — the `template_suppressions` table -//! (0058). A marker `(app_id, target_kind, reference)` records that an app opts -//! OUT of an inherited group template: a handler script name it declines -//! (`target_kind='trigger'`) or a path it declines (`target_kind='route'`). +//! (0058, made owner-polymorphic in 0060). A marker `(owner, target_kind, +//! reference)` records that an OWNER opts OUT of an inherited group template: a +//! handler script name it declines (`target_kind='trigger'`) or a path it +//! declines (`target_kind='route'`). //! -//! App-only (a group would just not declare the template), so — unlike the -//! owner-polymorphic `extension_points` — these free functions take a plain -//! `AppId`. Same tx-function style: read over `&PgPool`, write over a -//! `&mut Transaction`. +//! Owner-polymorphic (like `extension_points`): an **app** declines for itself; +//! a **group** declines for its whole subtree. The functions take a +//! `ScriptOwner` and bind the matching owner column. Same tx-function style: +//! read over `&PgPool`, write over a `&mut Transaction`. -use picloud_shared::AppId; +use picloud_shared::ScriptOwner; use sqlx::{PgPool, Postgres, Transaction}; /// The two suppressible template kinds, as stored in `target_kind`. pub const TARGET_TRIGGER: &str = "trigger"; pub const TARGET_ROUTE: &str = "route"; -/// List the suppression markers declared at `app_id`, as `(target_kind, +/// List the suppression markers declared at `owner`, as `(target_kind, /// reference)` pairs, stably ordered. Backs `load_current` (the apply diff) and /// the read-only `suppress ls`. -pub async fn list_for_app( +pub async fn list_for_owner( pool: &PgPool, - app_id: AppId, + owner: ScriptOwner, ) -> Result, sqlx::Error> { - let rows: Vec<(String, String)> = sqlx::query_as( - "SELECT target_kind, reference FROM template_suppressions \ - WHERE app_id = $1 ORDER BY target_kind, LOWER(reference)", - ) - .bind(app_id.into_inner()) - .fetch_all(pool) - .await?; - Ok(rows) + let sql = match owner { + ScriptOwner::App(_) => { + "SELECT target_kind, reference FROM template_suppressions \ + WHERE app_id = $1 ORDER BY target_kind, LOWER(reference)" + } + ScriptOwner::Group(_) => { + "SELECT target_kind, reference FROM template_suppressions \ + WHERE group_id = $1 ORDER BY target_kind, LOWER(reference)" + } + }; + sqlx::query_as(sql) + .bind(owner_uuid(owner)) + .fetch_all(pool) + .await } /// Insert a suppression marker in the apply transaction. Idempotent: re-apply /// of an already-declared `(kind, reference)` is a no-op (`ON CONFLICT DO -/// NOTHING`), so the marker survives without a spurious version bump. +/// NOTHING` against the per-owner partial unique index), so the marker survives +/// without a spurious version bump. pub async fn insert_suppression_tx( tx: &mut Transaction<'_, Postgres>, - app_id: AppId, + owner: ScriptOwner, target_kind: &str, reference: &str, ) -> Result<(), sqlx::Error> { + let (app_id, group_id) = owner_columns(owner); sqlx::query( - "INSERT INTO template_suppressions (app_id, target_kind, reference) \ - VALUES ($1, $2, $3) \ - ON CONFLICT (app_id, target_kind, reference) DO NOTHING", + "INSERT INTO template_suppressions (app_id, group_id, target_kind, reference) \ + VALUES ($1, $2, $3, $4) \ + ON CONFLICT DO NOTHING", ) - .bind(app_id.into_inner()) + .bind(app_id) + .bind(group_id) .bind(target_kind) .bind(reference) .execute(&mut **tx) @@ -58,18 +68,41 @@ pub async fn insert_suppression_tx( /// the manifest stops declaring the reference → the template re-inherits. pub async fn delete_suppression_tx( tx: &mut Transaction<'_, Postgres>, - app_id: AppId, + owner: ScriptOwner, target_kind: &str, reference: &str, ) -> Result<(), sqlx::Error> { - sqlx::query( - "DELETE FROM template_suppressions \ - WHERE app_id = $1 AND target_kind = $2 AND reference = $3", - ) - .bind(app_id.into_inner()) - .bind(target_kind) - .bind(reference) - .execute(&mut **tx) - .await?; + let sql = match owner { + ScriptOwner::App(_) => { + "DELETE FROM template_suppressions \ + WHERE app_id = $1 AND target_kind = $2 AND reference = $3" + } + ScriptOwner::Group(_) => { + "DELETE FROM template_suppressions \ + WHERE group_id = $1 AND target_kind = $2 AND reference = $3" + } + }; + sqlx::query(sql) + .bind(owner_uuid(owner)) + .bind(target_kind) + .bind(reference) + .execute(&mut **tx) + .await?; Ok(()) } + +/// `(app_id, group_id)` bind pair for the polymorphic owner — exactly one set. +fn owner_columns(owner: ScriptOwner) -> (Option, Option) { + match owner { + ScriptOwner::App(a) => (Some(a.into_inner()), None), + ScriptOwner::Group(g) => (None, Some(g.into_inner())), + } +} + +/// The single owner UUID (app or group), for the WHERE-on-one-column queries. +fn owner_uuid(owner: ScriptOwner) -> uuid::Uuid { + match owner { + ScriptOwner::App(a) => a.into_inner(), + ScriptOwner::Group(g) => g.into_inner(), + } +} diff --git a/crates/picloud-cli/src/manifest.rs b/crates/picloud-cli/src/manifest.rs index 6aec5eb..52adda2 100644 --- a/crates/picloud-cli/src/manifest.rs +++ b/crates/picloud-cli/src/manifest.rs @@ -73,16 +73,12 @@ impl Manifest { } // A group node owns scripts + vars (+ secret names) and, as of §11 tail, // ROUTE TEMPLATES + EVENT trigger TEMPLATES (kv/docs/files/pubsub) that - // fan out live to descendant apps. The stateful trigger kinds + // fan out live to descendant apps. As of §11 tail M1 a group may also + // declare `[suppress]` to decline a template it inherits from a higher + // ancestor, for its whole subtree. The stateful trigger kinds // (cron/queue/email) stay app-only — they need per-app state/secrets → // materialization. if m.group.is_some() { - if !m.suppress.is_empty() { - anyhow::bail!( - "a [group] cannot declare [suppress] — only an app opts out of \ - inherited templates; a group simply doesn't declare the template" - ); - } let t = &m.triggers; if !t.cron.is_empty() || !t.queue.is_empty() || !t.email.is_empty() { anyhow::bail!( @@ -808,7 +804,7 @@ mod tests { } #[test] - fn app_suppress_parses_group_suppress_rejected() { + fn app_and_group_suppress_parse() { // §11 tail: an [app] declares [suppress] to opt out of inherited // templates — script names (triggers) + paths (routes). let m = Manifest::parse( @@ -819,13 +815,14 @@ mod tests { assert_eq!(m.suppress.triggers, ["audit"]); assert_eq!(m.suppress.routes, ["/hello"]); - // A [group] cannot suppress — it simply wouldn't declare the template. - let err = Manifest::parse( + // §11 tail M1: a [group] MAY suppress — it declines a template it + // inherits from a higher ancestor, for its whole subtree. + let g = Manifest::parse( "[group]\nslug = \"acme\"\nname = \"ACME\"\n\n\ [suppress]\ntriggers = [\"audit\"]\n", ) - .expect_err("group [suppress] is rejected"); - assert!(err.to_string().contains("suppress"), "got: {err}"); + .expect("group [suppress] must parse"); + assert_eq!(g.suppress.triggers, ["audit"]); // An unknown key inside [suppress] is a hard error (deny_unknown_fields). let typo = "[app]\nslug = \"x\"\nname = \"X\"\n\n[suppress]\ntrigger = [\"a\"]\n";