diff --git a/crates/executor-core/src/module_resolver.rs b/crates/executor-core/src/module_resolver.rs index 53dcb8b..a481991 100644 --- a/crates/executor-core/src/module_resolver.rs +++ b/crates/executor-core/src/module_resolver.rs @@ -14,6 +14,14 @@ //! (a leaf can't shadow them — the trust boundary), while every SDK call //! *inside* a module still scopes to the inheriting app via `cx.app_id`. //! +//! **Extension points (§5.5).** A node may opt a module name OUT of sealing +//! by declaring it an extension point: an importer on that node's chain then +//! resolves the name against the *inheriting app's* effective view +//! (`App(cx.app_id)`), with the declared default body as fallback — so each +//! descendant app supplies its own impl. The "is this an extension point" +//! check keys on the importer's `origin` (trusted), so only the declaring +//! parent can open the inversion; a descendant can't hijack a sealed import. +//! //! Three runtime invariants are enforced: //! //! 1. **Lexical isolation** — `ModuleSource::resolve` is keyed by the @@ -377,8 +385,44 @@ impl ModuleResolver for PicloudModuleResolver { let origin = importer_source .and_then(decode_origin) .unwrap_or(self.default_origin); + + // Extension-point inversion (§5.5): if `path`'s nearest declaration on + // the importer's chain is an extension point (not a concrete module), + // resolve it against the INHERITING app's effective view instead of the + // sealed lexical chain — each descendant app supplies its own impl, + // falling back to the declared default body. The decision keys on + // `origin` (the trusted importer node), so only a declaration on the + // importer's own ancestry can open this; a descendant can never make a + // parent's sealed `import` dynamic, and a non-ext-point import never + // consults `App(cx.app_id)`. All branches funnel any backend error into + // the single redaction arm below. let lookup_result: Result, ModuleSourceError> = - tokio::task::block_in_place(|| handle.block_on(self.source.resolve(origin, path))); + (|| { + let ep = tokio::task::block_in_place(|| { + handle.block_on(self.source.resolve_extension_point(origin, path)) + })?; + let Some(ep) = ep else { + // Sealed lexical resolution (the default, unchanged). + return tokio::task::block_in_place(|| { + handle.block_on(self.source.resolve(origin, path)) + }); + }; + let app_origin = ScriptOwner::App(self.cx.app_id); + if let Some(m) = tokio::task::block_in_place(|| { + handle.block_on(self.source.resolve(app_origin, path)) + })? { + return Ok(Some(m)); + } + match ep.default_script_id { + // The default body is a module declared at the node that + // opened the ext point, i.e. on the importer's (`origin`) + // chain — resolve it there, not against the app. + Some(id) => tokio::task::block_in_place(|| { + handle.block_on(self.source.resolve_by_id(origin, id)) + }), + None => Ok(None), + } + })(); let module_row = match lookup_result { Ok(Some(m)) => m, diff --git a/crates/executor-core/tests/module_redaction_logging.rs b/crates/executor-core/tests/module_redaction_logging.rs index 4a34255..c89a067 100644 --- a/crates/executor-core/tests/module_redaction_logging.rs +++ b/crates/executor-core/tests/module_redaction_logging.rs @@ -34,6 +34,22 @@ impl ModuleSource for FailingSource { ) -> Result, ModuleSourceError> { Err(ModuleSourceError::Backend(SENTINEL.to_string())) } + + async fn resolve_extension_point( + &self, + _origin: ScriptOwner, + _name: &str, + ) -> Result, ModuleSourceError> { + Err(ModuleSourceError::Backend(SENTINEL.to_string())) + } + + async fn resolve_by_id( + &self, + _origin: ScriptOwner, + _script_id: picloud_shared::ScriptId, + ) -> Result, ModuleSourceError> { + Err(ModuleSourceError::Backend(SENTINEL.to_string())) + } } /// `MakeWriter` that appends to a shared buffer. diff --git a/crates/executor-core/tests/modules.rs b/crates/executor-core/tests/modules.rs index 8054dad..a0c1dc1 100644 --- a/crates/executor-core/tests/modules.rs +++ b/crates/executor-core/tests/modules.rs @@ -95,6 +95,30 @@ impl ModuleSource for CountingModuleSource { .get(&(app_id, name.to_string())) .cloned()) } + + async fn resolve_extension_point( + &self, + _origin: ScriptOwner, + _name: &str, + ) -> Result, ModuleSourceError> { + // This flat fake has no extension points; the inversion is covered by + // `LexicalModuleSource` below and the Postgres journey tests. + Ok(None) + } + + async fn resolve_by_id( + &self, + _origin: ScriptOwner, + script_id: ScriptId, + ) -> Result, ModuleSourceError> { + Ok(self + .table + .lock() + .await + .values() + .find(|m| m.script_id == script_id) + .cloned()) + } } fn services_with(modules: Arc) -> Services { @@ -623,6 +647,10 @@ fn validate_endpoint_skips_dynamic_imports_in_imports_list() { #[derive(Default)] struct LexicalModuleSource { table: Mutex>, + /// Extension-point declarations, keyed by exact declaring owner → + /// optional default module id. Flat (exact-owner) — the chain-walk + + /// shadowing tie-break is covered by the Postgres journey tests. + ext_points: Mutex>>, } impl LexicalModuleSource { @@ -630,15 +658,16 @@ impl LexicalModuleSource { Arc::new(Self::default()) } - async fn put(self: &Arc, owner: ScriptOwner, name: &str, source: &str) { + async fn put(self: &Arc, owner: ScriptOwner, name: &str, source: &str) -> ScriptId { let (app_id, group_id) = match owner { ScriptOwner::App(a) => (Some(a), None), ScriptOwner::Group(g) => (None, Some(g)), }; + let script_id = ScriptId::new(); self.table.lock().await.insert( (owner, name.to_string()), ModuleScript { - script_id: ScriptId::new(), + script_id, app_id, group_id, name: name.to_string(), @@ -646,6 +675,21 @@ impl LexicalModuleSource { updated_at: Utc::now(), }, ); + script_id + } + + /// Declare `name` an extension point at `owner`, with an optional default + /// module body id. + async fn put_ext_point( + self: &Arc, + owner: ScriptOwner, + name: &str, + default_script_id: Option, + ) { + self.ext_points + .lock() + .await + .insert((owner, name.to_string()), default_script_id); } } @@ -663,6 +707,35 @@ impl ModuleSource for LexicalModuleSource { .get(&(origin, name.to_string())) .cloned()) } + + async fn resolve_extension_point( + &self, + origin: ScriptOwner, + name: &str, + ) -> Result, ModuleSourceError> { + Ok(self + .ext_points + .lock() + .await + .get(&(origin, name.to_string())) + .map(|default_script_id| picloud_shared::ExtPointResolution { + default_script_id: *default_script_id, + })) + } + + async fn resolve_by_id( + &self, + _origin: ScriptOwner, + script_id: ScriptId, + ) -> Result, ModuleSourceError> { + Ok(self + .table + .lock() + .await + .values() + .find(|m| m.script_id == script_id) + .cloned()) + } } fn req_with_owner(app_id: AppId, owner: ScriptOwner) -> ExecRequest { @@ -736,3 +809,174 @@ async fn lexical_entry_origin_selects_app_module() { .expect("should execute"); assert_eq!(resp.body, serde_json::json!(999)); } + +// --------------------------------------------------------------------------- +// Extension points (§5.5) — the opt-in resolution inversion. A group declares +// a module name an extension point; an importer on the group's chain resolves +// it against the INHERITING APP, not the sealed lexical chain. +// --------------------------------------------------------------------------- + +/// A group script importing a declared extension point resolves it against the +/// executing app's own module — each tenant supplies its own body. +#[tokio::test(flavor = "multi_thread")] +async fn ext_point_resolves_to_app_module() { + let source = LexicalModuleSource::new(); + let app = AppId::new(); + let group = GroupId::new(); + // The group opens "theme" as an extension point (no default). + source + .put_ext_point(ScriptOwner::Group(group), "theme", None) + .await; + // The app provides its own "theme". + source + .put( + ScriptOwner::App(app), + "theme", + r#"fn name() { "app-theme" }"#, + ) + .await; + + let engine = engine_with(source.clone()); + // Entry runs as the inherited GROUP script (default_origin = group), but + // the executing app is `app` (cx.app_id). + let resp = engine + .execute( + r#"import "theme" as t; t::name()"#, + req_with_owner(app, ScriptOwner::Group(group)), + ) + .expect("should execute"); + assert_eq!( + resp.body, + serde_json::json!("app-theme"), + "extension point must resolve to the inheriting app's module" + ); +} + +/// When the app provides no module for the extension point, the declared +/// default body is used. +#[tokio::test(flavor = "multi_thread")] +async fn ext_point_falls_back_to_default() { + let source = LexicalModuleSource::new(); + let app = AppId::new(); + let group = GroupId::new(); + // The group's default "theme" body, registered as a group module. + let default_id = source + .put( + ScriptOwner::Group(group), + "default-theme", + r#"fn name() { "default-theme" }"#, + ) + .await; + source + .put_ext_point(ScriptOwner::Group(group), "theme", Some(default_id)) + .await; + // The app provides NO "theme". + + let engine = engine_with(source.clone()); + let resp = engine + .execute( + r#"import "theme" as t; t::name()"#, + req_with_owner(app, ScriptOwner::Group(group)), + ) + .expect("should execute"); + assert_eq!(resp.body, serde_json::json!("default-theme")); +} + +/// An extension point with neither an app provider nor a default is a clean +/// module-not-found at import time. +#[tokio::test(flavor = "multi_thread")] +async fn ext_point_no_provider_no_default_errors() { + let source = LexicalModuleSource::new(); + let app = AppId::new(); + let group = GroupId::new(); + source + .put_ext_point(ScriptOwner::Group(group), "theme", None) + .await; + + let engine = engine_with(source.clone()); + let err = engine + .execute( + r#"import "theme" as t; t::name()"#, + req_with_owner(app, ScriptOwner::Group(group)), + ) + .expect_err("ext point with no provider/default should fail"); + let msg = format!("{err:?}").to_lowercase(); + assert!( + msg.contains("module") || msg.contains("not found") || msg.contains("theme"), + "expected module-not-found-flavoured error, got {msg}" + ); +} + +/// A NORMAL (non-extension-point) group import is NOT hijacked by a leaf app's +/// module of the same name — the inversion only fires for declared extension +/// points, so the trust boundary holds. +#[tokio::test(flavor = "multi_thread")] +async fn sealed_import_not_hijacked_by_leaf_when_not_ext_point() { + let source = LexicalModuleSource::new(); + let app = AppId::new(); + let group = GroupId::new(); + // "auth" is a concrete group module — NOT an extension point. + source + .put( + ScriptOwner::Group(group), + "auth", + r#"fn who() { "group-auth" }"#, + ) + .await; + // The app has a trap "auth" that must NOT be selected. + source + .put(ScriptOwner::App(app), "auth", r#"fn who() { "leaf-trap" }"#) + .await; + + let engine = engine_with(source.clone()); + let resp = engine + .execute( + r#"import "auth" as a; a::who()"#, + req_with_owner(app, ScriptOwner::Group(group)), + ) + .expect("should execute"); + assert_eq!( + resp.body, + serde_json::json!("group-auth"), + "a non-ext-point import must seal to the group, never the leaf's trap" + ); +} + +/// Two apps inheriting the same extension point get their OWN bodies — the +/// id-keyed module cache does not bleed one tenant's body into another. +#[tokio::test(flavor = "multi_thread")] +async fn ext_point_two_apps_get_distinct_bodies() { + let source = LexicalModuleSource::new(); + let app1 = AppId::new(); + let app2 = AppId::new(); + let group = GroupId::new(); + source + .put_ext_point(ScriptOwner::Group(group), "theme", None) + .await; + source + .put(ScriptOwner::App(app1), "theme", r#"fn name() { "one" }"#) + .await; + source + .put(ScriptOwner::App(app2), "theme", r#"fn name() { "two" }"#) + .await; + + let engine = engine_with(source.clone()); + let r1 = engine + .execute( + r#"import "theme" as t; t::name()"#, + req_with_owner(app1, ScriptOwner::Group(group)), + ) + .expect("app1 executes"); + let r2 = engine + .execute( + r#"import "theme" as t; t::name()"#, + req_with_owner(app2, ScriptOwner::Group(group)), + ) + .expect("app2 executes"); + assert_eq!(r1.body, serde_json::json!("one")); + assert_eq!( + r2.body, + serde_json::json!("two"), + "no cross-tenant cache bleed" + ); +} diff --git a/crates/manager-core/migrations/0051_extension_points.sql b/crates/manager-core/migrations/0051_extension_points.sql new file mode 100644 index 0000000..685187b --- /dev/null +++ b/crates/manager-core/migrations/0051_extension_points.sql @@ -0,0 +1,49 @@ +-- Phase 4b+1 (v1.2 Hierarchies): extension points (§5.5). +-- +-- An extension point opts a MODULE NAME into dynamic, per-tenant resolution. +-- Normally a group/parent script's `import "x"` resolves sealed-lexically +-- against the importer's own defining node — a leaf app cannot shadow it +-- (the trust boundary). When a node declares `x` an extension point, an +-- importer whose defining node is on that node's chain instead resolves `x` +-- against the INHERITING app's effective view, so each descendant app may +-- supply its own `x`. Only the declaring (parent) node can open the hole; a +-- descendant cannot make one of its parent's sealed imports dynamic, because +-- the declaration must live on the importer's own ancestry. +-- +-- Owner-polymorphic exactly like vars (0048): exactly one of group_id/app_id. +-- Optional `default_script_id` is the fallback module body used when the +-- inheriting app provides no module of `name`. + +CREATE TABLE extension_points ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + -- Polymorphic owner: exactly one FK set (real FKs so ON DELETE CASCADE + -- works per owner kind and a dangling owner is impossible). + group_id UUID REFERENCES groups(id) ON DELETE CASCADE, + app_id UUID REFERENCES apps(id) ON DELETE CASCADE, + CONSTRAINT extension_points_owner_exactly_one + CHECK ((group_id IS NULL) <> (app_id IS NULL)), + name TEXT NOT NULL, + -- Optional fallback module (a module owned by the declaring node). SET + -- NULL on delete so deleting the module can never block — the fallback is + -- then absent and a non-providing app errors at import time (the next plan + -- shows the now-null default as an Update). A valid manifest apply can't + -- reach this: validation rejects a kept ext point whose default names a + -- module not in the bundle. + default_script_id UUID REFERENCES scripts(id) ON DELETE SET NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +-- One declaration per (owner, name), case-insensitive to match the module +-- name indexes (0050). Partial because the owner is split across two columns. +CREATE UNIQUE INDEX extension_points_group_uidx + ON extension_points (group_id, LOWER(name)) WHERE group_id IS NOT NULL; +CREATE UNIQUE INDEX extension_points_app_uidx + ON extension_points (app_id, LOWER(name)) WHERE app_id IS NOT NULL; + +CREATE INDEX extension_points_group_id_idx + ON extension_points (group_id) WHERE group_id IS NOT NULL; +CREATE INDEX extension_points_app_id_idx + ON extension_points (app_id) WHERE app_id IS NOT NULL; +CREATE INDEX extension_points_default_script_idx + ON extension_points (default_script_id) WHERE default_script_id IS NOT NULL; diff --git a/crates/manager-core/src/apply_api.rs b/crates/manager-core/src/apply_api.rs index 6033f43..041922d 100644 --- a/crates/manager-core/src/apply_api.rs +++ b/crates/manager-core/src/apply_api.rs @@ -8,7 +8,7 @@ use axum::{ extract::{Path, State}, http::StatusCode, response::{IntoResponse, Response}, - routing::post, + routing::{get, post}, Extension, Json, Router, }; use picloud_shared::{AppId, GroupId, Principal}; @@ -17,8 +17,8 @@ use serde_json::json; use crate::app_repo::AppRepository; use crate::apply_service::{ - ApplyError, ApplyOwner, ApplyReport, ApplyService, Bundle, BundleTrigger, NodeKind, PlanResult, - TreeBundle, TreePlanResult, + ApplyError, ApplyOwner, ApplyReport, ApplyService, Bundle, BundleTrigger, ExtPointView, + NodeKind, PlanResult, TreeBundle, TreePlanResult, }; use crate::authz::{require, AuthzDenied, Capability}; use crate::group_repo::GroupRepository; @@ -30,11 +30,53 @@ pub fn apply_router(service: ApplyService) -> Router { .route("/apps/{id}/apply", post(apply_handler)) .route("/groups/{id}/plan", post(group_plan_handler)) .route("/groups/{id}/apply", post(group_apply_handler)) + .route("/apps/{id}/extension-points", get(app_ext_points_handler)) + .route( + "/groups/{id}/extension-points", + get(group_ext_points_handler), + ) .route("/tree/plan", post(tree_plan_handler)) .route("/tree/apply", post(tree_apply_handler)) .with_state(service) } +/// `GET .../apps/{id}/extension-points` — list the app's OWN extension-point +/// declarations (for `pic pull`). Read-only; viewer-tier (`AppRead`). +async fn app_ext_points_handler( + State(svc): State, + Extension(principal): Extension, + Path(id_or_slug): Path, +) -> Result>, ApplyError> { + let app_id = resolve_app_id(svc.apps.as_ref(), &id_or_slug).await?; + require(svc.authz.as_ref(), &principal, Capability::AppRead(app_id)) + .await + .map_err(map_authz)?; + Ok(Json( + svc.list_extension_points(ApplyOwner::App(app_id)).await?, + )) +} + +/// `GET .../groups/{id}/extension-points` — list the group's OWN extension-point +/// declarations. Read-only; viewer-tier (`GroupScriptsRead`). +async fn group_ext_points_handler( + State(svc): State, + Extension(principal): Extension, + Path(id_or_slug): Path, +) -> Result>, ApplyError> { + let group_id = resolve_group_id(svc.groups.as_ref(), &id_or_slug).await?; + require( + svc.authz.as_ref(), + &principal, + Capability::GroupScriptsRead(group_id), + ) + .await + .map_err(map_authz)?; + Ok(Json( + svc.list_extension_points(ApplyOwner::Group(group_id)) + .await?, + )) +} + #[derive(Deserialize)] pub struct ApplyRequest { pub bundle: Bundle, @@ -232,7 +274,11 @@ async fn require_app_node_writes( require(svc.authz.as_ref(), principal, Capability::AppRead(app_id)) .await .map_err(map_authz)?; - if prune || !bundle.scripts.is_empty() { + // Extension points are module-resolution declarations, so they gate on the + // same script-write tier as modules — without this, an ext-point-only + // bundle (no `default`, so no script) would pass with viewer-tier `AppRead` + // and let a reader open the §5.5 import trust boundary. + if prune || !bundle.scripts.is_empty() || !bundle.extension_points.is_empty() { require( svc.authz.as_ref(), principal, @@ -292,7 +338,8 @@ async fn require_group_node_writes( bundle: &Bundle, prune: bool, ) -> Result<(), ApplyError> { - if prune || !bundle.scripts.is_empty() { + // Extension points gate on the script-write tier (see the app variant). + if prune || !bundle.scripts.is_empty() || !bundle.extension_points.is_empty() { require( svc.authz.as_ref(), principal, diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index b284dd2..e79d59c 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -83,6 +83,32 @@ pub struct Bundle { /// not expressible here; use `pic vars set --tombstone`. #[serde(default)] pub vars: std::collections::BTreeMap, + /// Declared extension points (§5.5): module names this node opens for + /// per-tenant resolution by inheriting apps. Reconciled at the node that + /// owns the importing script. + #[serde(default)] + pub extension_points: Vec, +} + +/// Desired extension point — a module name opened for dynamic, per-tenant +/// resolution, with an optional default body (a module declared at this node). +#[derive(Debug, Clone, Deserialize)] +pub struct ExtPointSpec { + pub name: String, + /// Name of a module declared at this node to use when an inheriting app + /// provides none. Omitted ⇒ no fallback (a non-providing app errors at + /// import time). + #[serde(default)] + pub default: Option, +} + +/// One of a node's OWN extension-point declarations, for the read/list +/// surface (`pic pull`). The default is its module NAME (not the raw id). +#[derive(Debug, Clone, Serialize)] +pub struct ExtPointView { + pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub default: Option, } #[derive(Debug, Clone, Deserialize)] @@ -315,6 +341,8 @@ pub struct Plan { pub secrets: Vec, #[serde(default)] pub vars: Vec, + #[serde(default)] + pub extension_points: Vec, } impl Plan { @@ -327,6 +355,7 @@ impl Plan { .chain(&self.triggers) .chain(&self.secrets) .chain(&self.vars) + .chain(&self.extension_points) .all(|c| c.op == Op::NoOp) } } @@ -403,6 +432,10 @@ pub struct CurrentState { /// the manifest reconciles. Inherited group vars and tombstones are /// excluded (the manifest manages only the app's own real values). pub vars: Vec<(String, serde_json::Value)>, + /// The owner's OWN extension-point declarations: `(name, default module + /// name)`. The default is resolved back to its module name (not the raw + /// id) so the diff is name-based and stable across re-applies. + pub extension_points: Vec<(String, Option)>, } // ---------------------------------------------------------------------------- @@ -773,6 +806,43 @@ impl ApplyService { } } + // 3c. Extension points (§5.5) — reconcile after scripts so a `default` + // module name resolves to its just-reconciled id. Upsert every + // Create/Update; the default (if named) must be a local module. + let bundle_ext: HashMap = bundle + .extension_points + .iter() + .map(|e| (e.name.to_lowercase(), e)) + .collect(); + for ch in &plan.extension_points { + match ch.op { + Op::Create | Op::Update => { + let spec = bundle_ext[&ch.key.to_lowercase()]; + let default_id = match &spec.default { + Some(name) => { + Some(*name_to_id.get(&name.to_lowercase()).ok_or_else(|| { + ApplyError::Invalid(format!( + "extension point `{}` default `{name}` is not a \ + module declared here", + spec.name + )) + })?) + } + None => None, + }; + owner + .set_ext_point_tx(&mut *tx, &spec.name, default_id) + .await?; + if ch.op == Op::Create { + report.extension_points_created += 1; + } else { + report.extension_points_updated += 1; + } + } + Op::NoOp | Op::Delete => {} + } + } + // 4. Prune (only with --prune): delete stale triggers, then scripts // (route removals already happened in the route delete-pass above). // Secret pruning is deliberately deferred (destructive + irreversible): @@ -848,6 +918,15 @@ impl ApplyService { report.vars_deleted += 1; } } + + // Extension points are prunable declarations (like vars): a live + // declaration dropped from the manifest is removed. + for ch in &plan.extension_points { + if ch.op == Op::Delete { + owner.delete_ext_point_tx(&mut *tx, &ch.key).await?; + report.extension_points_deleted += 1; + } + } } Ok(name_to_id) } @@ -1570,6 +1649,13 @@ impl ApplyService { .filter(|s| s.kind == ScriptKind::Module) .map(|s| s.name.to_lowercase()) .collect(); + // Extension points declared in THIS bundle also satisfy imports — the + // concrete body is supplied by an inheriting app at runtime (§5.5). + let local_ext: HashSet = bundle + .extension_points + .iter() + .map(|e| e.name.to_lowercase()) + .collect(); let origin = match owner { ApplyOwner::App(a) => picloud_shared::ScriptOwner::App(a), ApplyOwner::Group(g) => picloud_shared::ScriptOwner::Group(g), @@ -1579,7 +1665,7 @@ impl ApplyService { for s in &bundle.scripts { for imp in self.script_imports(s)? { let key = imp.to_lowercase(); - if local.contains(&key) || !checked.insert(key) { + if local.contains(&key) || local_ext.contains(&key) || !checked.insert(key) { continue; } let found = self @@ -1587,7 +1673,18 @@ impl ApplyService { .resolve(origin, &imp) .await .map_err(|e| ApplyError::Backend(e.to_string()))?; - if found.is_none() { + if found.is_some() { + continue; + } + // Not a concrete module — but an extension point declared on an + // ancestor also satisfies the import (resolved per-app at run + // time). Only a name that is neither is a dangling import. + let ep = self + .modules + .resolve_extension_point(origin, &imp) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + if ep.is_none() { return Err(ApplyError::Invalid(format!( "script `{}` imports unknown module `{imp}` \ (not declared here and not inherited from an ancestor)", @@ -1603,6 +1700,7 @@ impl ApplyService { /// scripts reachable from the app's chain that a route/trigger may bind to /// even though the manifest doesn't declare them (Phase 4). They count as /// known endpoints for the binding checks below. + #[allow(clippy::too_many_lines)] fn validate_bundle( &self, bundle: &Bundle, @@ -1722,6 +1820,33 @@ impl ApplyService { for key in bundle.vars.keys() { validate_var_key(key)?; } + + // Extension points (§5.5): unique kebab names; a `default` must name a + // module declared in this node (an endpoint is never importable). + let module_names: HashSet = bundle + .scripts + .iter() + .filter(|s| s.kind == ScriptKind::Module) + .map(|s| s.name.to_lowercase()) + .collect(); + let mut ext_names: HashSet = HashSet::new(); + for e in &bundle.extension_points { + validate_ext_point_name(&e.name)?; + if !ext_names.insert(e.name.to_lowercase()) { + return Err(ApplyError::Invalid(format!( + "duplicate extension point `{}`", + e.name + ))); + } + if let Some(default) = &e.default { + if !module_names.contains(&default.to_lowercase()) { + return Err(ApplyError::Invalid(format!( + "extension point `{}` default `{default}` is not a module declared here", + e.name + ))); + } + } + } Ok(()) } @@ -1769,15 +1894,58 @@ impl ApplyService { .filter(|r| r.environment_scope == "*" && !r.is_tombstone) .map(|r| (r.key, r.value)) .collect(); + let extension_points = self.load_extension_points(owner).await?; Ok(CurrentState { scripts, routes, triggers, secret_names, vars, + extension_points, }) } + /// The owner's OWN extension-point declarations, each with its default + /// module's NAME (resolved via the FK) so the diff is name-based. Read + /// directly off the pool (no repo trait — the pure `compute_diff` tests + /// bypass `load_current`, and this is integration-tested). + async fn load_extension_points( + &self, + owner: ApplyOwner, + ) -> Result)>, ApplyError> { + let (col, val) = match owner { + ApplyOwner::App(a) => ("app_id", a.into_inner()), + ApplyOwner::Group(g) => ("group_id", g.into_inner()), + }; + let rows: Vec<(String, Option)> = sqlx::query_as(&format!( + "SELECT e.name, ds.name AS default_name \ + FROM extension_points e \ + LEFT JOIN scripts ds ON ds.id = e.default_script_id \ + WHERE e.{col} = $1 ORDER BY LOWER(e.name)" + )) + .bind(val) + .fetch_all(&self.pool) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + Ok(rows) + } + + /// The owner's OWN extension-point declarations, for the `pic pull` export. + /// + /// # Errors + /// `Backend` on a repo failure. + pub async fn list_extension_points( + &self, + owner: ApplyOwner, + ) -> Result, ApplyError> { + Ok(self + .load_extension_points(owner) + .await? + .into_iter() + .map(|(name, default)| ExtPointView { name, default }) + .collect()) + } + async fn all_secret_names(&self, app_id: AppId) -> Result, ApplyError> { let mut names = Vec::new(); let mut cursor: Option = None; @@ -1838,6 +2006,66 @@ fn compute_diff_with_names( triggers: diff_triggers(current, bundle, &script_name_by_id), secrets: diff_secrets(current, bundle), vars: diff_vars(current, bundle), + extension_points: diff_extension_points(current, bundle), + } +} + +/// Diff extension points by name. The "value" is the default module name, so a +/// changed default is an `Update` (like vars, the declaration lives in the +/// manifest). Live declarations absent from the manifest are `Delete` (applied +/// only under `--prune`). Name comparison is case-insensitive to match the +/// per-owner `LOWER(name)` unique index. +fn diff_extension_points(current: &CurrentState, bundle: &Bundle) -> Vec { + let live: HashMap> = current + .extension_points + .iter() + .map(|(name, default)| (name.to_lowercase(), default.as_deref())) + .collect(); + let desired: HashSet = bundle + .extension_points + .iter() + .map(|e| e.name.to_lowercase()) + .collect(); + + let mut out = Vec::new(); + for e in &bundle.extension_points { + let want = e.default.as_deref(); + match live.get(&e.name.to_lowercase()) { + Some(cur) if eq_ci_opt(*cur, want) => out.push(ResourceChange { + op: Op::NoOp, + key: e.name.clone(), + detail: None, + }), + Some(_) => out.push(ResourceChange { + op: Op::Update, + key: e.name.clone(), + detail: Some("default changed".into()), + }), + None => out.push(ResourceChange { + op: Op::Create, + key: e.name.clone(), + detail: None, + }), + } + } + for (name, _) in ¤t.extension_points { + if !desired.contains(&name.to_lowercase()) { + out.push(ResourceChange { + op: Op::Delete, + key: name.clone(), + detail: None, + }); + } + } + out +} + +/// Case-insensitive equality for two optional module names. +fn eq_ci_opt(a: Option<&str>, b: Option<&str>) -> bool { + match (a, b) { + (Some(x), Some(y)) => x.eq_ignore_ascii_case(y), + (None, None) => true, + _ => false, } } @@ -1883,6 +2111,28 @@ fn diff_vars(current: &CurrentState, bundle: &Bundle) -> Vec { /// Kebab var key (`^[a-z0-9][a-z0-9-]{0,127}$`) — mirrors `vars_api`'s /// `validate_key` so the manifest and the interactive surface agree. +/// Extension-point name rule: the SAME identifier shape as a module name +/// (`^[a-zA-Z_][a-zA-Z0-9_]{0,63}$`, migration 0015). An extension point is +/// resolved by `import ""`, and the concrete module that satisfies it +/// must be a real module script — whose name is identifier-shaped — so a +/// hyphenated extension point could never be provided. Reject early. +fn validate_ext_point_name(name: &str) -> Result<(), ApplyError> { + let mut chars = name.chars(); + let ok = name.len() <= 64 + && chars + .next() + .is_some_and(|c| c.is_ascii_alphabetic() || c == '_') + && chars.all(|c| c.is_ascii_alphanumeric() || c == '_'); + if ok { + Ok(()) + } else { + Err(ApplyError::Invalid(format!( + "extension point name `{name}` must be 1–64 chars of letters, digits, and \ + underscores, starting with a letter or underscore (a module-name shape)" + ))) + } +} + fn validate_var_key(key: &str) -> Result<(), ApplyError> { let ok = !key.is_empty() && key.len() <= 128 @@ -2034,6 +2284,55 @@ impl ApplyOwner { Self::Group(g) => delete_group_var_tx(tx, g, key).await, } } + + async fn set_ext_point_tx( + self, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + name: &str, + default_script_id: Option, + ) -> Result<(), ApplyError> { + let default = default_script_id.map(ScriptId::into_inner); + let (col, val) = match self { + Self::App(a) => ("app_id", a.into_inner()), + Self::Group(g) => ("group_id", g.into_inner()), + }; + // Owner-kind-specific upsert; the conflict target is the matching + // partial-unique index, so its predicate is restated. Update both the + // default and updated_at so a re-applied default change takes effect. + sqlx::query(&format!( + "INSERT INTO extension_points ({col}, name, default_script_id) \ + VALUES ($1, $2, $3) \ + ON CONFLICT ({col}, LOWER(name)) WHERE {col} IS NOT NULL DO UPDATE \ + SET default_script_id = EXCLUDED.default_script_id, updated_at = NOW()" + )) + .bind(val) + .bind(name) + .bind(default) + .execute(&mut **tx) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + Ok(()) + } + + async fn delete_ext_point_tx( + self, + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + name: &str, + ) -> Result<(), ApplyError> { + let (col, val) = match self { + Self::App(a) => ("app_id", a.into_inner()), + Self::Group(g) => ("group_id", g.into_inner()), + }; + sqlx::query(&format!( + "DELETE FROM extension_points WHERE {col} = $1 AND LOWER(name) = LOWER($2)" + )) + .bind(val) + .bind(name) + .execute(&mut **tx) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + Ok(()) + } } fn diff_scripts(current: &CurrentState, bundle: &Bundle) -> Vec { @@ -2594,6 +2893,12 @@ pub struct ApplyReport { pub vars_updated: u32, #[serde(default)] pub vars_deleted: u32, + #[serde(default)] + pub extension_points_created: u32, + #[serde(default)] + pub extension_points_updated: u32, + #[serde(default)] + pub extension_points_deleted: u32, #[serde(skip_serializing_if = "Vec::is_empty")] pub warnings: Vec, } @@ -2818,6 +3123,15 @@ fn state_token_with_names( serde_json::to_string(value).unwrap_or_default() )); } + // Extension points: name + default module name. A changed default moves + // the token so the bound-plan check catches an edit between plan and apply. + for (name, default) in ¤t.extension_points { + parts.push(format!( + "e|{}|{}", + name.to_lowercase(), + default.as_deref().unwrap_or("") + )); + } // Order-independent: sort the per-resource tokens before hashing. parts.sort_unstable(); let mut h: u64 = 0xcbf2_9ce4_8422_2325; @@ -2898,6 +3212,7 @@ mod tests { triggers: vec![], secrets: vec!["S".into()], vars: std::collections::BTreeMap::new(), + extension_points: Vec::new(), }; let plan = compute_diff(¤t, &bundle); assert_eq!(plan.scripts.len(), 1); @@ -2920,6 +3235,7 @@ mod tests { triggers: vec![], secrets: vec!["S".into()], vars: std::collections::BTreeMap::new(), + extension_points: Vec::new(), }; let plan = compute_diff(¤t, &bundle); assert!(plan.is_noop(), "expected all no-op, got {plan:?}"); @@ -2937,6 +3253,7 @@ mod tests { triggers: vec![], secrets: vec![], vars: std::collections::BTreeMap::new(), + extension_points: Vec::new(), }; let plan = compute_diff(¤t, &bundle); assert_eq!(plan.scripts[0].op, Op::Update); @@ -2955,6 +3272,7 @@ mod tests { triggers: vec![], secrets: vec![], vars: std::collections::BTreeMap::new(), + extension_points: Vec::new(), }; let plan = compute_diff(¤t, &bundle); assert_eq!(plan.scripts[0].op, Op::Delete); @@ -3001,6 +3319,7 @@ mod tests { triggers: vec![], secrets: vec![], vars: std::collections::BTreeMap::new(), + extension_points: Vec::new(), }; let plan = compute_diff(¤t, &bundle); assert_eq!(plan.routes[0].op, Op::Update); @@ -3013,6 +3332,7 @@ mod tests { triggers: vec![], secrets: vec![], vars: std::collections::BTreeMap::new(), + extension_points: Vec::new(), } } @@ -3259,6 +3579,70 @@ mod tests { assert_eq!(state_token(&ab), state_token(&ba)); } + #[test] + fn diff_extension_points_create_update_noop_delete() { + let current = CurrentState { + extension_points: vec![ + ("theme".into(), Some("dtheme".into())), + ("keep".into(), None), + ("stale".into(), None), + ], + ..CurrentState::default() + }; + let bundle = Bundle { + extension_points: vec![ + ExtPointSpec { + name: "theme".into(), + default: Some("other".into()), + }, // default changed → Update + ExtPointSpec { + name: "Keep".into(), + default: None, + }, // identical (case-insensitive) → NoOp + ExtPointSpec { + name: "new".into(), + default: None, + }, // not live → Create + // "stale" is live but undeclared → Delete. + ], + ..empty_bundle() + }; + let plan = compute_diff(¤t, &bundle); + let op = |k: &str| { + plan.extension_points + .iter() + .find(|c| c.key.eq_ignore_ascii_case(k)) + .unwrap_or_else(|| panic!("no change for {k}: {plan:?}")) + .op + }; + assert_eq!(op("theme"), Op::Update); + assert_eq!(op("keep"), Op::NoOp); + assert_eq!(op("new"), Op::Create); + assert_eq!(op("stale"), Op::Delete); + assert!(!plan.is_noop()); + } + + #[test] + fn state_token_is_sensitive_to_extension_point_default() { + let base = CurrentState { + extension_points: vec![("theme".into(), Some("a".into()))], + ..CurrentState::default() + }; + // A default change must flip the token (bound-plan check for an + // out-of-band ext-point edit between plan and apply). + let changed = CurrentState { + extension_points: vec![("theme".into(), Some("b".into()))], + ..CurrentState::default() + }; + assert_ne!(state_token(&base), state_token(&changed)); + // And adding/removing a declaration moves it too. + let added = CurrentState { + extension_points: vec![("theme".into(), Some("a".into())), ("extra".into(), None)], + ..CurrentState::default() + }; + assert_ne!(state_token(&base), state_token(&added)); + } + #[test] fn state_token_ignores_dead_letter_triggers() { // The diff ignores dead-letter triggers (not manifest-representable), diff --git a/crates/manager-core/src/module_source.rs b/crates/manager-core/src/module_source.rs index b353efc..5f4cd4e 100644 --- a/crates/manager-core/src/module_source.rs +++ b/crates/manager-core/src/module_source.rs @@ -12,7 +12,9 @@ use async_trait::async_trait; use chrono::{DateTime, Utc}; -use picloud_shared::{ModuleScript, ModuleSource, ModuleSourceError, ScriptOwner}; +use picloud_shared::{ + ExtPointResolution, ModuleScript, ModuleSource, ModuleSourceError, ScriptId, ScriptOwner, +}; use sqlx::PgPool; use crate::config_resolver::{CHAIN_LEVELS_CTE, GROUP_CHAIN_LEVELS_CTE}; @@ -96,4 +98,92 @@ impl ModuleSource for PostgresModuleSource { .map_err(|e| ModuleSourceError::Backend(e.to_string()))?; Ok(row.map(Into::into)) } + + async fn resolve_extension_point( + &self, + origin: ScriptOwner, + name: &str, + ) -> Result, ModuleSourceError> { + // Find the nearest extension-point declaration of `name` on `origin`'s + // chain that is NOT shadowed by a concrete module of the same name at + // depth <= its own (tie → concrete wins). The NOT EXISTS encodes that + // shadowing rule; ORDER BY depth ASC LIMIT 1 picks the nearest + // surviving declaration. An App origin can see ext points declared on + // an ancestor group (or on the app itself); a Group origin only walks + // groups — same trust boundary as `resolve`. + let (cte, ep_join, mod_join) = match origin { + ScriptOwner::App(_) => ( + CHAIN_LEVELS_CTE, + "(e.group_id = c.group_owner OR e.app_id = c.app_owner)", + "(s.app_id = c2.app_owner OR s.group_id = c2.group_owner)", + ), + ScriptOwner::Group(_) => ( + GROUP_CHAIN_LEVELS_CTE, + "e.group_id = c.group_owner", + "s.group_id = c2.group_owner", + ), + }; + let query = format!( + "{cte} \ + SELECT e.default_script_id \ + FROM extension_points e \ + JOIN chain c ON {ep_join} \ + WHERE LOWER(e.name) = LOWER($2) \ + AND NOT EXISTS ( \ + SELECT 1 FROM scripts s \ + JOIN chain c2 ON {mod_join} \ + WHERE s.kind = 'module' AND LOWER(s.name) = LOWER($2) \ + AND c2.depth <= c.depth \ + ) \ + ORDER BY c.depth ASC LIMIT 1", + ); + let root = match origin { + ScriptOwner::App(a) => a.into_inner(), + ScriptOwner::Group(g) => g.into_inner(), + }; + let row: Option<(Option,)> = sqlx::query_as(&query) + .bind(root) + .bind(name) + .fetch_optional(&self.pool) + .await + .map_err(|e| ModuleSourceError::Backend(e.to_string()))?; + Ok(row.map(|(default_script_id,)| ExtPointResolution { + default_script_id: default_script_id.map(Into::into), + })) + } + + async fn resolve_by_id( + &self, + origin: ScriptOwner, + script_id: ScriptId, + ) -> Result, ModuleSourceError> { + // Load a module body by id (the ext-point fallback), but ONLY if it is + // owned by a node on `origin`'s chain — the same trust boundary as + // `resolve`. This keeps the resolver self-defending: a `default_script_id` + // that ever pointed off-chain resolves to None instead of executing + // cross-tenant code. Endpoints are never importable (kind filter). + let (cte, join) = match origin { + ScriptOwner::App(_) => ( + CHAIN_LEVELS_CTE, + "(s.app_id = c.app_owner OR s.group_id = c.group_owner)", + ), + ScriptOwner::Group(_) => (GROUP_CHAIN_LEVELS_CTE, "s.group_id = c.group_owner"), + }; + let root = match origin { + ScriptOwner::App(a) => a.into_inner(), + ScriptOwner::Group(g) => g.into_inner(), + }; + let row: Option = sqlx::query_as(&format!( + "{cte} \ + SELECT s.id, s.app_id, s.group_id, s.name, s.source, s.updated_at \ + FROM scripts s JOIN chain c ON {join} \ + WHERE s.id = $2 AND s.kind = 'module' LIMIT 1" + )) + .bind(root) + .bind(script_id.into_inner()) + .fetch_optional(&self.pool) + .await + .map_err(|e| ModuleSourceError::Backend(e.to_string()))?; + Ok(row.map(Into::into)) + } } diff --git a/crates/manager-core/tests/expected_schema.txt b/crates/manager-core/tests/expected_schema.txt index a1bdbba..6045afe 100644 --- a/crates/manager-core/tests/expected_schema.txt +++ b/crates/manager-core/tests/expected_schema.txt @@ -198,6 +198,15 @@ table: execution_logs app_id: uuid NOT NULL source: text NOT NULL default='http'::text +table: extension_points + id: uuid NOT NULL default=gen_random_uuid() + group_id: uuid NULL + app_id: uuid NULL + name: text NOT NULL + default_script_id: uuid NULL + created_at: timestamp with time zone NOT NULL default=now() + updated_at: timestamp with time zone NOT NULL default=now() + table: files app_id: uuid NOT NULL collection: text NOT NULL @@ -463,6 +472,14 @@ indexes on execution_logs: execution_logs_pkey: public.execution_logs USING btree (id) execution_logs_script_id_created_at_idx: public.execution_logs USING btree (script_id, created_at DESC) +indexes on extension_points: + extension_points_app_id_idx: public.extension_points USING btree (app_id) WHERE (app_id IS NOT NULL) + extension_points_app_uidx: public.extension_points USING btree (app_id, lower(name)) WHERE (app_id IS NOT NULL) + extension_points_default_script_idx: public.extension_points USING btree (default_script_id) WHERE (default_script_id IS NOT NULL) + extension_points_group_id_idx: public.extension_points USING btree (group_id) WHERE (group_id IS NOT NULL) + extension_points_group_uidx: public.extension_points USING btree (group_id, lower(name)) WHERE (group_id IS NOT NULL) + extension_points_pkey: public.extension_points USING btree (id) + indexes on files: files_pkey: public.files USING btree (app_id, collection, id) idx_files_app_collection: public.files USING btree (app_id, collection) @@ -656,6 +673,13 @@ constraints on execution_logs: [FOREIGN KEY] execution_logs_script_id_fkey: FOREIGN KEY (script_id) REFERENCES scripts(id) ON DELETE SET NULL [PRIMARY KEY] execution_logs_pkey: PRIMARY KEY (id) +constraints on extension_points: + [CHECK] extension_points_owner_exactly_one: CHECK (((group_id IS NULL) <> (app_id IS NULL))) + [FOREIGN KEY] extension_points_app_id_fkey: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE + [FOREIGN KEY] extension_points_default_script_id_fkey: FOREIGN KEY (default_script_id) REFERENCES scripts(id) ON DELETE SET NULL + [FOREIGN KEY] extension_points_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE + [PRIMARY KEY] extension_points_pkey: PRIMARY KEY (id) + constraints on files: [FOREIGN KEY] files_app_id_fkey: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE [PRIMARY KEY] files_pkey: PRIMARY KEY (app_id, collection, id) @@ -800,3 +824,4 @@ constraints on vars: 0048: vars 0049: group secrets 0050: group scripts + 0051: extension points diff --git a/crates/picloud-cli/src/client.rs b/crates/picloud-cli/src/client.rs index 8d8aee1..64c702b 100644 --- a/crates/picloud-cli/src/client.rs +++ b/crates/picloud-cli/src/client.rs @@ -123,6 +123,19 @@ impl Client { decode(resp).await } + /// `GET /api/v1/admin/apps/{id}/extension-points` — the app's OWN + /// extension-point declarations (for `pic pull`). + pub async fn extension_points_list(&self, ident: &str) -> Result> { + let resp = self + .request( + Method::GET, + &format!("/api/v1/admin/apps/{}/extension-points", seg(ident)), + ) + .send() + .await?; + decode(resp).await + } + /// `GET /api/v1/admin/scripts` — every script the caller can see /// (server filters by membership for `Member`). Lets `pic scripts ls` /// (no `--app`) collapse what used to be an N+1 per-app walk into a @@ -1320,6 +1333,8 @@ pub struct PlanDto { pub secrets: Vec, #[serde(default)] pub vars: Vec, + #[serde(default)] + pub extension_points: Vec, /// Fingerprint of the live state this plan was computed against; carried /// in `.picloud/` and replayed to `apply` for the bound-plan check. #[serde(default)] @@ -1334,6 +1349,14 @@ pub struct ChangeDto { pub detail: Option, } +/// One of an owner's OWN extension-point declarations (`pic pull`). +#[derive(Debug, Deserialize)] +pub struct ExtPointDto { + pub name: String, + #[serde(default)] + pub default: Option, +} + /// Response of `POST .../tree/plan` (Phase 5): a per-node diff + one combined /// bound-plan token for the whole subtree. #[derive(Debug, Deserialize)] @@ -1358,6 +1381,8 @@ pub struct NodePlanDto { pub secrets: Vec, #[serde(default)] pub vars: Vec, + #[serde(default)] + pub extension_points: Vec, } /// Response of `POST .../apply`: counts of what changed. @@ -1386,6 +1411,12 @@ pub struct ApplyReportDto { #[serde(default)] pub vars_deleted: u32, #[serde(default)] + pub extension_points_created: u32, + #[serde(default)] + pub extension_points_updated: u32, + #[serde(default)] + pub extension_points_deleted: u32, + #[serde(default)] pub warnings: Vec, } diff --git a/crates/picloud-cli/src/cmds/apply.rs b/crates/picloud-cli/src/cmds/apply.rs index b830170..f26f2d7 100644 --- a/crates/picloud-cli/src/cmds/apply.rs +++ b/crates/picloud-cli/src/cmds/apply.rs @@ -84,6 +84,15 @@ pub async fn run( "+{} ~{} -{}", report.vars_created, report.vars_updated, report.vars_deleted ), + ) + .field( + "extension-points", + format!( + "+{} ~{} -{}", + report.extension_points_created, + report.extension_points_updated, + report.extension_points_deleted + ), ); for w in &report.warnings { block.field("warning", w.clone()); @@ -151,6 +160,15 @@ pub async fn run_tree( "+{} ~{} -{}", report.vars_created, report.vars_updated, report.vars_deleted ), + ) + .field( + "extension-points", + format!( + "+{} ~{} -{}", + report.extension_points_created, + report.extension_points_updated, + report.extension_points_deleted + ), ); for w in &report.warnings { block.field("warning", w.clone()); diff --git a/crates/picloud-cli/src/cmds/init.rs b/crates/picloud-cli/src/cmds/init.rs index dec46ca..dad5a20 100644 --- a/crates/picloud-cli/src/cmds/init.rs +++ b/crates/picloud-cli/src/cmds/init.rs @@ -139,6 +139,7 @@ fn scaffold_manifest(slug: &str, name: &str) -> Manifest { triggers: crate::manifest::ManifestTriggers::default(), secrets: crate::manifest::ManifestSecrets::default(), vars: std::collections::BTreeMap::new(), + extension_points: Vec::new(), } } diff --git a/crates/picloud-cli/src/cmds/plan.rs b/crates/picloud-cli/src/cmds/plan.rs index 2c51e68..7fba587 100644 --- a/crates/picloud-cli/src/cmds/plan.rs +++ b/crates/picloud-cli/src/cmds/plan.rs @@ -64,12 +64,13 @@ fn render_tree(plan: &TreePlanDto, mode: OutputMode) { let mut table = Table::new(["node", "kind", "op", "resource", "detail"]); for n in &plan.nodes { let node = format!("{}:{}", n.kind, n.slug); - let groups: [(&str, &Vec); 5] = [ + let groups: [(&str, &Vec); 6] = [ ("script", &n.scripts), ("route", &n.routes), ("trigger", &n.triggers), ("secret", &n.secrets), ("var", &n.vars), + ("extension-point", &n.extension_points), ]; for (rk, changes) in groups { for c in changes { @@ -154,12 +155,28 @@ pub fn build_bundle(manifest: &Manifest, base_dir: &Path) -> Result { ); } + // Extension points: name + optional default module. The server's + // `ExtPointSpec` deserializes this shape directly. + let extension_points: Vec = manifest + .extension_points + .iter() + .map(|e| { + let mut o = Map::new(); + o.insert("name".into(), json!(e.name)); + if let Some(d) = &e.default { + o.insert("default".into(), json!(d)); + } + Value::Object(o) + }) + .collect(); + Ok(json!({ "scripts": scripts, "routes": routes, "triggers": triggers, "secrets": manifest.secrets.names, "vars": vars, + "extension_points": extension_points, })) } @@ -174,12 +191,13 @@ fn tagged(kind: &str, spec: impl Serialize) -> Result { fn render(plan: &PlanDto, mode: OutputMode) { let mut table = Table::new(["kind", "op", "resource", "detail"]); - let groups: [(&str, &Vec); 5] = [ + let groups: [(&str, &Vec); 6] = [ ("script", &plan.scripts), ("route", &plan.routes), ("trigger", &plan.triggers), ("secret", &plan.secrets), ("var", &plan.vars), + ("extension-point", &plan.extension_points), ]; for (kind, changes) in groups { for c in changes { diff --git a/crates/picloud-cli/src/cmds/pull.rs b/crates/picloud-cli/src/cmds/pull.rs index d8a7320..2ba4364 100644 --- a/crates/picloud-cli/src/cmds/pull.rs +++ b/crates/picloud-cli/src/cmds/pull.rs @@ -18,8 +18,8 @@ use crate::client::{Client, VarOwnerArg}; use crate::config; use crate::manifest::{ CronTriggerSpec, DocsTriggerSpec, FilesTriggerSpec, KvTriggerSpec, Manifest, ManifestApp, - ManifestRoute, ManifestScript, ManifestSecrets, ManifestTriggers, PubsubTriggerSpec, - QueueTriggerSpec, MANIFEST_FILE, + ManifestExtensionPoint, ManifestRoute, ManifestScript, ManifestSecrets, ManifestTriggers, + PubsubTriggerSpec, QueueTriggerSpec, MANIFEST_FILE, }; use crate::output::{KvBlock, OutputMode}; @@ -210,6 +210,18 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> eprintln!("warning: skipping {s} trigger — not yet representable in the manifest"); } + // Extension points (§5.5): the app's own declarations, exported so a + // re-applied pulled manifest doesn't prune them. + let extension_points: Vec = client + .extension_points_list(app_ident) + .await? + .into_iter() + .map(|e| ManifestExtensionPoint { + name: e.name, + default: e.default, + }) + .collect(); + let manifest = Manifest { app: Some(ManifestApp { slug: app.app.slug.clone(), @@ -224,6 +236,7 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> names: secrets.iter().map(|s| s.name.clone()).collect(), }, vars: manifest_vars, + extension_points, }; std::fs::write(&manifest_path, manifest.to_toml()?) diff --git a/crates/picloud-cli/src/manifest.rs b/crates/picloud-cli/src/manifest.rs index 84cd2a9..76caf9e 100644 --- a/crates/picloud-cli/src/manifest.rs +++ b/crates/picloud-cli/src/manifest.rs @@ -49,6 +49,10 @@ pub struct Manifest { /// The overlay merges per-env, last-write-wins by key. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub vars: BTreeMap, + /// `[[extension_points]]` — module names this node opens for per-tenant + /// resolution (§5.5). Allowed on both app and group manifests. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub extension_points: Vec, } impl Manifest { @@ -397,6 +401,17 @@ impl ManifestSecrets { } } +/// `[[extension_points]]` — a module name opened for per-tenant resolution +/// (§5.5). A parent/group script importing `name` resolves it against the +/// inheriting app's module instead of the sealed lexical chain; `default` +/// names a local module used when an app provides none. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ManifestExtensionPoint { + pub name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub default: Option, +} + // ---- serde skip/default helpers ---- fn is_endpoint(kind: &ScriptKind) -> bool { @@ -488,6 +503,10 @@ mod tests { ("region".to_string(), toml::Value::String("eu".into())), ("max-retries".to_string(), toml::Value::Integer(3)), ]), + extension_points: vec![ManifestExtensionPoint { + name: "theme".into(), + default: Some("default-theme".into()), + }], } } @@ -591,12 +610,14 @@ mod tests { triggers: ManifestTriggers::default(), secrets: ManifestSecrets::default(), vars: BTreeMap::new(), + extension_points: Vec::new(), }; let text = m.to_toml().unwrap(); assert!(!text.contains("[[scripts]]"), "got:\n{text}"); assert!(!text.contains("triggers"), "got:\n{text}"); assert!(!text.contains("secrets"), "got:\n{text}"); assert!(!text.contains("vars"), "got:\n{text}"); + assert!(!text.contains("extension_points"), "got:\n{text}"); // Still round-trips. assert_eq!(m, Manifest::parse(&text).unwrap()); } diff --git a/crates/picloud-cli/tests/cli.rs b/crates/picloud-cli/tests/cli.rs index 00d007c..9782905 100644 --- a/crates/picloud-cli/tests/cli.rs +++ b/crates/picloud-cli/tests/cli.rs @@ -23,6 +23,7 @@ mod dead_letters; mod email_queue; mod enabled; mod env_overlay; +mod extension_points; mod group_modules; mod group_scripts; mod group_secrets; diff --git a/crates/picloud-cli/tests/extension_points.rs b/crates/picloud-cli/tests/extension_points.rs new file mode 100644 index 0000000..d876ecc --- /dev/null +++ b/crates/picloud-cli/tests/extension_points.rs @@ -0,0 +1,243 @@ +//! Extension points (§5.5) end-to-end via `pic apply` + `invoke`: +//! * a GROUP declares a module name `theme` an extension point (with a +//! default body) and a group endpoint `render` that imports it, +//! * an app under the group provides its OWN `theme`; invoking the inherited +//! `render` in that app's context resolves the APP's `theme` (the +//! inversion) — NOT sealed to the group, +//! * an app that provides no `theme` falls back to the group's default body, +//! * `pull` round-trips the declaration (a re-applied pulled manifest is a +//! no-op, so `--prune` never silently drops it). + +use std::fs; + +use tempfile::TempDir; + +use crate::common; +use crate::common::cleanup::{AppGuard, GroupGuard, ScriptGuard}; + +fn manifest_dir() -> TempDir { + let dir = TempDir::new().expect("tempdir"); + fs::create_dir_all(dir.path().join("scripts")).expect("scripts dir"); + dir +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn extension_point_resolves_per_app_with_default_fallback() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let group = common::unique_slug("ep-grp"); + let app_a = common::unique_slug("ep-a"); + let app_b = common::unique_slug("ep-b"); + + // GroupGuard first so it drops LAST (after apps + group scripts). + let _g = GroupGuard::new(&env.url, &env.token, &group); + common::pic_as(&env) + .args(["groups", "create", &group]) + .assert() + .success(); + + let dir = manifest_dir(); + + // --- Group manifest: a default `theme` body, a `render` endpoint that + // imports `theme`, and the `[[extension_points]]` declaration. --- + fs::write( + dir.path().join("scripts/defaulttheme.rhai"), + r#"fn name() { "default" }"#, + ) + .unwrap(); + fs::write( + dir.path().join("scripts/render.rhai"), + r#"import "theme" as t; t::name()"#, + ) + .unwrap(); + let group_manifest = format!( + "[group]\nslug = \"{group}\"\nname = \"EP Group\"\n\n\ + [[scripts]]\nname = \"defaulttheme\"\nfile = \"scripts/defaulttheme.rhai\"\nkind = \"module\"\n\n\ + [[scripts]]\nname = \"render\"\nfile = \"scripts/render.rhai\"\n\n\ + [[extension_points]]\nname = \"theme\"\ndefault = \"defaulttheme\"\n" + ); + let group_path = dir.path().join("group.toml"); + fs::write(&group_path, &group_manifest).unwrap(); + let out = common::pic_as(&env) + .args(["apply", "--file"]) + .arg(&group_path) + .output() + .expect("group apply"); + assert!( + out.status.success(), + "group apply failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + // Group scripts block group deletion (RESTRICT) — guard them. + let _gr = ScriptGuard::new( + &env.url, + &env.token, + &group_script_id(&env, &group, "render"), + ); + let _gd = ScriptGuard::new( + &env.url, + &env.token, + &group_script_id(&env, &group, "defaulttheme"), + ); + + // --- App A under the group: provides its OWN `theme` + a caller that + // invokes the inherited `render`. --- + let _ga = AppGuard::new(&env.url, &env.token, &app_a); + common::pic_as(&env) + .args(["apps", "create", &app_a, "--group", &group]) + .assert() + .success(); + fs::write( + dir.path().join("scripts/theme-a.rhai"), + r#"fn name() { "app-a" }"#, + ) + .unwrap(); + fs::write( + dir.path().join("scripts/caller.rhai"), + r#"invoke("render", #{})"#, + ) + .unwrap(); + let app_a_manifest = format!( + "[app]\nslug = \"{app_a}\"\nname = \"EP A\"\n\n\ + [[scripts]]\nname = \"theme\"\nfile = \"scripts/theme-a.rhai\"\nkind = \"module\"\n\n\ + [[scripts]]\nname = \"caller\"\nfile = \"scripts/caller.rhai\"\n" + ); + let app_a_path = dir.path().join("a.toml"); + fs::write(&app_a_path, &app_a_manifest).unwrap(); + common::pic_as(&env) + .args(["apply", "--file"]) + .arg(&app_a_path) + .assert() + .success(); + + // The inherited `render` imports the ext point `theme`; in App A's context + // it resolves App A's OWN `theme`. + let caller_a = app_script_id(&env, &app_a, "caller"); + assert_eq!( + invoke_body(&env, &caller_a), + serde_json::json!("app-a"), + "extension point must resolve the inheriting app's module" + ); + + // --- App B under the group: provides NO `theme` → render falls back to the + // group's default body. --- + let _gb = AppGuard::new(&env.url, &env.token, &app_b); + common::pic_as(&env) + .args(["apps", "create", &app_b, "--group", &group]) + .assert() + .success(); + let app_b_manifest = format!( + "[app]\nslug = \"{app_b}\"\nname = \"EP B\"\n\n\ + [[scripts]]\nname = \"caller\"\nfile = \"scripts/caller.rhai\"\n" + ); + let app_b_path = dir.path().join("b.toml"); + fs::write(&app_b_path, &app_b_manifest).unwrap(); + common::pic_as(&env) + .args(["apply", "--file"]) + .arg(&app_b_path) + .assert() + .success(); + let caller_b = app_script_id(&env, &app_b, "caller"); + assert_eq!( + invoke_body(&env, &caller_b), + serde_json::json!("default"), + "an app providing no module must fall back to the ext point's default" + ); +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn extension_point_round_trips_through_pull() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let app = common::unique_slug("ep-pull"); + + let _a = AppGuard::new(&env.url, &env.token, &app); + common::pic_as(&env) + .args(["apps", "create", &app]) + .assert() + .success(); + + // Apply an app manifest that declares an extension point with a default. + let dir = manifest_dir(); + fs::write( + dir.path().join("scripts/theme.rhai"), + r#"fn name() { "x" }"#, + ) + .unwrap(); + let manifest = format!( + "[app]\nslug = \"{app}\"\nname = \"EP Pull\"\n\n\ + [[scripts]]\nname = \"theme\"\nfile = \"scripts/theme.rhai\"\nkind = \"module\"\n\n\ + [[extension_points]]\nname = \"theme_slot\"\ndefault = \"theme\"\n" + ); + let manifest_path = dir.path().join("picloud.toml"); + fs::write(&manifest_path, &manifest).unwrap(); + common::pic_as(&env) + .args(["apply", "--file"]) + .arg(&manifest_path) + .assert() + .success(); + + // Pull into a fresh dir, then re-plan: the extension point must round-trip + // (no Create/Delete), so `--prune` would never drop it. + let pulled = manifest_dir(); + common::pic_as(&env) + .args(["pull", &app, "--dir"]) + .arg(pulled.path()) + .assert() + .success(); + let toml = fs::read_to_string(pulled.path().join("picloud.toml")).unwrap(); + assert!( + toml.contains("theme_slot"), + "pulled manifest must export the extension point:\n{toml}" + ); + let plan = common::pic_as(&env) + .args(["plan", "--file"]) + .arg(pulled.path().join("picloud.toml")) + .output() + .expect("plan"); + let stdout = String::from_utf8_lossy(&plan.stdout); + assert!( + !stdout.to_lowercase().contains("create") && !stdout.to_lowercase().contains("delete"), + "re-planning a pulled manifest must be a no-op:\n{stdout}" + ); +} + +/// Id of the named script in a group (`pic scripts ls --group `). +fn group_script_id(env: &common::TestEnv, group: &str, name: &str) -> String { + named_id(env, &["scripts", "ls", "--group", group], name) +} + +/// Id of the named script in an app (`pic scripts ls --app `). +fn app_script_id(env: &common::TestEnv, app: &str, name: &str) -> String { + named_id(env, &["scripts", "ls", "--app", app], name) +} + +fn named_id(env: &common::TestEnv, args: &[&str], name: &str) -> String { + let ls = common::pic_as(env).args(args).output().expect("scripts ls"); + let table = String::from_utf8(ls.stdout).unwrap(); + table + .lines() + .map(common::cells) + .find(|c| c.get(2) == Some(&name)) + .and_then(|c| c.first().map(|s| (*s).to_string())) + .unwrap_or_else(|| panic!("script `{name}` not found:\n{table}")) +} + +fn invoke_body(env: &common::TestEnv, id: &str) -> serde_json::Value { + let out = common::pic_as(env) + .args(["scripts", "invoke", id]) + .output() + .expect("scripts invoke"); + assert!( + out.status.success(), + "invoke failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + serde_json::from_slice(&out.stdout).expect("invoke body is JSON") +} diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index d8ee317..7ba57d0 100644 --- a/crates/shared/src/lib.rs +++ b/crates/shared/src/lib.rs @@ -76,7 +76,9 @@ pub use inbox::{ pub use invoke::{InvokeError, InvokeService, InvokeTarget, NoopInvokeService, ResolvedScript}; pub use kv::{KvError, KvListPage, KvService, NoopKvService}; pub use log_sink::{ExecutionLogSink, LogSinkError}; -pub use modules::{ModuleScript, ModuleSource, ModuleSourceError, NoopModuleSource}; +pub use modules::{ + ExtPointResolution, ModuleScript, ModuleSource, ModuleSourceError, NoopModuleSource, +}; pub use outbox_writer::{HttpDispatchPayload, NewHttpOutbox, OutboxWriter, OutboxWriterError}; pub use pubsub::{ topic_matches, validate_topic_pattern, NoopPubsubService, PubsubError, PubsubService, diff --git a/crates/shared/src/modules.rs b/crates/shared/src/modules.rs index 223b8dd..c2e88c9 100644 --- a/crates/shared/src/modules.rs +++ b/crates/shared/src/modules.rs @@ -62,6 +62,17 @@ impl ModuleScript { } } +/// Outcome of an extension-point lookup (§5.5). Present when `name` is +/// declared an extension point on `origin`'s chain *and* is not shadowed by +/// a nearer concrete module of the same name — i.e. the nearest declaration +/// of `name` is the extension point, so resolution inverts to the inheriting +/// app. `default_script_id` is the optional fallback module body to use when +/// the app provides no module of `name`. +#[derive(Debug, Clone)] +pub struct ExtPointResolution { + pub default_script_id: Option, +} + /// Lookup contract used by `PicloudModuleResolver`. `resolve` walks the /// module set up the chain rooted at `origin` (the importing script's /// defining node), nearest-owner-wins. The `origin` is trusted @@ -80,6 +91,32 @@ pub trait ModuleSource: Send + Sync { origin: ScriptOwner, name: &str, ) -> Result, ModuleSourceError>; + + /// Extension-point check (§5.5). Returns `Some` iff the **nearest** + /// declaration of `name` on `origin`'s chain is an extension point (not + /// a concrete module). On a tie in depth a concrete module wins (so a + /// nearer/peer real module shadows the extension-point declaration). The + /// `origin` is the trusted importer node, so the inversion can only be + /// opened by a declaration on the importer's own ancestry — a descendant + /// can never make a parent's sealed import dynamic. + async fn resolve_extension_point( + &self, + origin: ScriptOwner, + name: &str, + ) -> Result, ModuleSourceError>; + + /// Load the extension-point fallback module `script_id`, but ONLY if it is + /// a `kind = 'module'` script reachable on `origin`'s chain. The `origin` + /// is the trusted importer node; constraining the lookup to its chain makes + /// the resolver self-defending — a `default_script_id` that (through a bug + /// or DB-direct write) pointed at another tenant's module resolves to + /// `None` rather than executing cross-tenant code. Returns `None` if the id + /// is absent, names an endpoint, or is off-chain. + async fn resolve_by_id( + &self, + origin: ScriptOwner, + script_id: ScriptId, + ) -> Result, ModuleSourceError>; } /// Failure modes surfaced from `ModuleSource::resolve`. "Not found" is @@ -109,4 +146,20 @@ impl ModuleSource for NoopModuleSource { ) -> Result, ModuleSourceError> { Ok(None) } + + async fn resolve_extension_point( + &self, + _origin: ScriptOwner, + _name: &str, + ) -> Result, ModuleSourceError> { + Ok(None) + } + + async fn resolve_by_id( + &self, + _origin: ScriptOwner, + _script_id: ScriptId, + ) -> Result, ModuleSourceError> { + Ok(None) + } }