From 8f966783fe9906855200453fc3eafba4b20a3348 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Mon, 29 Jun 2026 20:07:59 +0200 Subject: [PATCH] =?UTF-8?q?feat(modules):=20extension-point-aware=20resolv?= =?UTF-8?q?er=20policy=20(=C2=A75.5=20C2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the runtime heart of extension points: `ModuleSource::resolve_policy` decides lexical vs dynamic resolution by the NEAREST declaration of a name walking up the importing origin's chain — a concrete module → lexical (seal to origin, Phase 4b); an extension-point marker → dynamic, resolved against the inheriting app (`cx.app_id`) so the app can override, falling back to the default body up-chain; the EP wins a depth tie. - shared: `ModuleResolution { Module | NoProvider | NotFound }` + a `resolve_policy(origin, inheriting_app, name)` trait method with a lexical-only default impl (existing fakes inherit it unchanged). - PostgresModuleSource: one-round-trip nearest-declaration query (min EP depth vs min module depth over the chain CTEs), then the lexical/dynamic branch. - module_resolver: calls `resolve_policy(origin, cx.app_id, path)`; maps NoProvider → a clear "extension point has no provider" error, NotFound → module-not-found. Cache + AST-source sealing unchanged. - executor-core tests: a policy fake + 3 cases (app override binds dynamically, no-provider errors, non-EP stays lexical). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/executor-core/src/module_resolver.rs | 31 +++- crates/executor-core/tests/modules.rs | 149 +++++++++++++++++++- crates/manager-core/src/module_source.rs | 96 +++++++++++-- crates/shared/src/lib.rs | 4 +- crates/shared/src/modules.rs | 35 +++++ 5 files changed, 299 insertions(+), 16 deletions(-) diff --git a/crates/executor-core/src/module_resolver.rs b/crates/executor-core/src/module_resolver.rs index 53dcb8b..9a316c1 100644 --- a/crates/executor-core/src/module_resolver.rs +++ b/crates/executor-core/src/module_resolver.rs @@ -377,17 +377,40 @@ impl ModuleResolver for PicloudModuleResolver { let origin = importer_source .and_then(decode_origin) .unwrap_or(self.default_origin); - let lookup_result: Result, ModuleSourceError> = - tokio::task::block_in_place(|| handle.block_on(self.source.resolve(origin, path))); + // §5.5 policy resolution: the source decides lexical (seal to `origin`) + // vs dynamic (an extension point resolves against the inheriting app, + // `cx.app_id`). The result is already the concrete module to bind, or a + // terminal NoProvider/NotFound. + let lookup_result: Result = + tokio::task::block_in_place(|| { + handle.block_on(self.source.resolve_policy(origin, self.cx.app_id, path)) + }); let module_row = match lookup_result { - Ok(Some(m)) => m, - Ok(None) => { + Ok(picloud_shared::ModuleResolution::Module(m)) => m, + Ok(picloud_shared::ModuleResolution::NotFound) => { return Err(Box::new(EvalAltResult::ErrorModuleNotFound( path.to_string(), pos, ))); } + Ok(picloud_shared::ModuleResolution::NoProvider) => { + // §5.5: an extension point with no provider for this app is a + // hard failure (the app must supply the module or a default + // body must exist up-chain). Distinct, actionable message. + return Err(Box::new(EvalAltResult::ErrorInModule( + path.to_string(), + Box::new(EvalAltResult::ErrorRuntime( + format!( + "extension point {path:?} has no provider for this app — \ + define a module named {path:?} or a default body up-chain" + ) + .into(), + pos, + )), + pos, + ))); + } Err(e) => { // v1.1.4 §10a: redact the backend error before it // reaches a script. In public-HTTP context (principal: diff --git a/crates/executor-core/tests/modules.rs b/crates/executor-core/tests/modules.rs index 8054dad..ea7319a 100644 --- a/crates/executor-core/tests/modules.rs +++ b/crates/executor-core/tests/modules.rs @@ -8,7 +8,7 @@ //! verify the same code path the `picloud` binary runs at request //! time. -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; @@ -736,3 +736,150 @@ async fn lexical_entry_origin_selects_app_module() { .expect("should execute"); assert_eq!(resp.body, serde_json::json!(999)); } + +// --------------------------------------------------------------------------- +// §5.5 extension points — resolver handling of the policy outcomes. +// +// `PolicyModuleSource` is a flat fake that overrides `resolve_policy`: an EP +// name resolves dynamically against the inheriting app; a non-EP name resolves +// lexically from the importing origin. This verifies the resolver maps +// Module/NoProvider/NotFound correctly. The full chain semantics (default body +// up-chain, nearest-declaration tie) are covered by the CLI journey tests. +// --------------------------------------------------------------------------- + +#[derive(Default)] +struct PolicyModuleSource { + modules: Mutex>, + eps: Mutex>, +} + +impl PolicyModuleSource { + fn new() -> Arc { + Arc::new(Self::default()) + } + async fn put(self: &Arc, owner: ScriptOwner, name: &str, source: &str) { + let (app_id, group_id) = match owner { + ScriptOwner::App(a) => (Some(a), None), + ScriptOwner::Group(g) => (None, Some(g)), + }; + self.modules.lock().await.insert( + (owner, name.to_string()), + ModuleScript { + script_id: ScriptId::new(), + app_id, + group_id, + name: name.to_string(), + source: source.to_string(), + updated_at: Utc::now(), + }, + ); + } + async fn mark_ep(self: &Arc, name: &str) { + self.eps.lock().await.insert(name.to_string()); + } +} + +#[async_trait] +impl ModuleSource for PolicyModuleSource { + async fn resolve( + &self, + origin: ScriptOwner, + name: &str, + ) -> Result, ModuleSourceError> { + Ok(self + .modules + .lock() + .await + .get(&(origin, name.to_string())) + .cloned()) + } + + async fn resolve_policy( + &self, + origin: ScriptOwner, + inheriting_app: AppId, + name: &str, + ) -> Result { + use picloud_shared::ModuleResolution; + if self.eps.lock().await.contains(name) { + // Extension point → dynamic, resolved against the inheriting app. + return Ok( + match self.resolve(ScriptOwner::App(inheriting_app), name).await? { + Some(m) => ModuleResolution::Module(m), + None => ModuleResolution::NoProvider, + }, + ); + } + Ok(match self.resolve(origin, name).await? { + Some(m) => ModuleResolution::Module(m), + None => ModuleResolution::NotFound, + }) + } +} + +/// An extension-point import binds the inheriting APP's module even though the +/// importing script's defining node is the group (dynamic override — the +/// inverse of the Phase 4b sealed/lexical import). +#[tokio::test(flavor = "multi_thread")] +async fn extension_point_resolves_app_override() { + let source = PolicyModuleSource::new(); + let app = AppId::new(); + let group = GroupId::new(); + source.mark_ep("theme").await; + // App provides its own `theme`; group has none. + source + .put(ScriptOwner::App(app), "theme", r#"fn color() { "red" }"#) + .await; + + let engine = engine_with(source.clone()); + // Entry runs as the inherited GROUP endpoint importing the EP `theme`. + let resp = engine + .execute( + r#"import "theme" as t; t::color()"#, + req_with_owner(app, ScriptOwner::Group(group)), + ) + .expect("should execute"); + assert_eq!(resp.body, serde_json::json!("red")); +} + +/// An extension point with no provider for the app is a hard error. +#[tokio::test(flavor = "multi_thread")] +async fn extension_point_without_provider_errors() { + let source = PolicyModuleSource::new(); + let app = AppId::new(); + let group = GroupId::new(); + source.mark_ep("theme").await; // declared, but no module anywhere. + + let engine = engine_with(source.clone()); + let err = engine + .execute( + r#"import "theme" as t; t::color()"#, + req_with_owner(app, ScriptOwner::Group(group)), + ) + .expect_err("missing provider must error"); + let msg = format!("{err:?}").to_lowercase(); + assert!( + msg.contains("no provider") || msg.contains("extension point"), + "expected a no-provider error, got {msg}" + ); +} + +/// A non-extension-point name still resolves lexically from the origin. +#[tokio::test(flavor = "multi_thread")] +async fn non_extension_point_stays_lexical() { + let source = PolicyModuleSource::new(); + let app = AppId::new(); + let group = GroupId::new(); + source + .put(ScriptOwner::Group(group), "util", r#"fn v() { 7 }"#) + .await; + + let engine = engine_with(source.clone()); + let resp = engine + .execute( + r#"import "util" as u; u::v()"#, + req_with_owner(app, ScriptOwner::Group(group)), + ) + .expect("should execute"); + assert_eq!(resp.body, serde_json::json!(7)); +} diff --git a/crates/manager-core/src/module_source.rs b/crates/manager-core/src/module_source.rs index b353efc..f4ed534 100644 --- a/crates/manager-core/src/module_source.rs +++ b/crates/manager-core/src/module_source.rs @@ -1,18 +1,25 @@ //! `PostgresModuleSource` — the Postgres-backed `ModuleSource` impl. //! -//! Resolution is **lexical** (§5.5): a module is looked up by walking the -//! group chain rooted at the **importing script's defining node** -//! (`origin`), nearest-owner-wins — *not* the inheriting app's effective -//! view. An `App` origin walks `app → its group → ancestors` (reusing -//! [`CHAIN_LEVELS_CTE`]); a `Group` origin walks `group → ancestors` -//! ([`GROUP_CHAIN_LEVELS_CTE`]) and never sees app-owned modules below it -//! (the trust boundary). The resolver lives in `executor-core` and consumes -//! this trait through the `Services` bundle, so manager-core stays the only -//! crate that touches Postgres. +//! `resolve` is **lexical** (Phase 4b): a module is looked up by walking the +//! group chain rooted at the **importing script's defining node** (`origin`), +//! nearest-owner-wins — *not* the inheriting app's effective view. An `App` +//! origin walks `app → its group → ancestors` (reusing [`CHAIN_LEVELS_CTE`]); +//! a `Group` origin walks `group → ancestors` ([`GROUP_CHAIN_LEVELS_CTE`]) and +//! never sees app-owned modules below it (the trust boundary). +//! +//! `resolve_policy` adds §5.5 **extension points**: the nearest declaration of +//! a name on `origin`'s chain decides lexical (concrete module) vs **dynamic** +//! (an extension-point marker → resolve against the inheriting app so it can +//! override, falling back to a default body up-chain). EP wins a depth tie. +//! +//! The resolver lives in `executor-core` and consumes this trait through the +//! `Services` bundle, so manager-core stays the only crate that touches Postgres. use async_trait::async_trait; use chrono::{DateTime, Utc}; -use picloud_shared::{ModuleScript, ModuleSource, ModuleSourceError, ScriptOwner}; +use picloud_shared::{ + AppId, ModuleResolution, ModuleScript, ModuleSource, ModuleSourceError, ScriptOwner, +}; use sqlx::PgPool; use crate::config_resolver::{CHAIN_LEVELS_CTE, GROUP_CHAIN_LEVELS_CTE}; @@ -96,4 +103,73 @@ impl ModuleSource for PostgresModuleSource { .map_err(|e| ModuleSourceError::Backend(e.to_string()))?; Ok(row.map(Into::into)) } + + async fn resolve_policy( + &self, + origin: ScriptOwner, + inheriting_app: AppId, + name: &str, + ) -> Result { + // §5.5 nearest-declaration-kind-wins. One round trip: the min chain + // depth at which `name` is declared as an extension-point marker vs a + // concrete module, walking up `origin`'s chain. The EP wins a tie (a + // marker + its co-located default body live at the same node). + let query = match origin { + ScriptOwner::App(_) => format!( + "{CHAIN_LEVELS_CTE} \ + SELECT \ + (SELECT MIN(c.depth) FROM extension_points e \ + JOIN chain c ON (e.app_id = c.app_owner OR e.group_id = c.group_owner) \ + WHERE LOWER(e.name) = LOWER($2)) AS ep_depth, \ + (SELECT MIN(c.depth) FROM scripts s \ + JOIN chain c ON (s.app_id = c.app_owner OR s.group_id = c.group_owner) \ + WHERE s.kind = 'module' AND LOWER(s.name) = LOWER($2)) AS mod_depth", + ), + ScriptOwner::Group(_) => format!( + "{GROUP_CHAIN_LEVELS_CTE} \ + SELECT \ + (SELECT MIN(c.depth) FROM extension_points e \ + JOIN chain c ON e.group_id = c.group_owner \ + WHERE LOWER(e.name) = LOWER($2)) AS ep_depth, \ + (SELECT MIN(c.depth) FROM scripts s \ + JOIN chain c ON s.group_id = c.group_owner \ + WHERE s.kind = 'module' AND LOWER(s.name) = LOWER($2)) AS mod_depth", + ), + }; + let root = match origin { + ScriptOwner::App(a) => a.into_inner(), + ScriptOwner::Group(g) => g.into_inner(), + }; + let (ep_depth, mod_depth): (Option, Option) = sqlx::query_as(&query) + .bind(root) + .bind(name) + .fetch_one(&self.pool) + .await + .map_err(|e| ModuleSourceError::Backend(e.to_string()))?; + + // EP wins when it is declared and is no farther than the nearest + // concrete module (tie → EP). Then resolve dynamically against the + // inheriting app (its own override, else the default body up-chain). + let ep_wins = match (ep_depth, mod_depth) { + (Some(ep), Some(m)) => ep <= m, + (Some(_), None) => true, + _ => false, + }; + if ep_wins { + return Ok( + match self.resolve(ScriptOwner::App(inheriting_app), name).await? { + Some(m) => ModuleResolution::Module(m), + None => ModuleResolution::NoProvider, + }, + ); + } + if mod_depth.is_some() { + // Concrete module nearest → lexical, exactly as Phase 4b. + return Ok(match self.resolve(origin, name).await? { + Some(m) => ModuleResolution::Module(m), + None => ModuleResolution::NotFound, + }); + } + Ok(ModuleResolution::NotFound) + } } diff --git a/crates/shared/src/lib.rs b/crates/shared/src/lib.rs index d8ee317..fcff7fd 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::{ + ModuleResolution, 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..1dbb074 100644 --- a/crates/shared/src/modules.rs +++ b/crates/shared/src/modules.rs @@ -62,6 +62,19 @@ impl ModuleScript { } } +/// Outcome of [`ModuleSource::resolve_policy`] — the §5.5 lexical-vs-dynamic +/// decision the resolver acts on. +#[derive(Debug)] +pub enum ModuleResolution { + /// Bind this module (lexical, or an extension point's resolved provider). + Module(ModuleScript), + /// The name is an extension point, but no provider exists for the + /// inheriting app (no app override and no default body) — a hard error. + NoProvider, + /// No declaration of the name anywhere on the chain → module-not-found. + NotFound, +} + /// 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 +93,28 @@ pub trait ModuleSource: Send + Sync { origin: ScriptOwner, name: &str, ) -> Result, ModuleSourceError>; + + /// §5.5 extension-point-aware resolution. Decides lexical vs dynamic by + /// the **nearest declaration** of `name` walking up `origin`'s chain: a + /// concrete module → lexical (`resolve(origin, …)`); an extension-point + /// marker → dynamic, resolved against the **inheriting app** + /// (`resolve(App(inheriting_app), …)`) so the app can override, falling + /// back to the default body up-chain; the EP wins on a depth tie. + /// + /// The default impl is lexical-only (no extension points) — the Postgres + /// source overrides it. Test fakes that don't exercise EPs inherit this. + async fn resolve_policy( + &self, + origin: ScriptOwner, + inheriting_app: AppId, + name: &str, + ) -> Result { + let _ = inheriting_app; + Ok(match self.resolve(origin, name).await? { + Some(m) => ModuleResolution::Module(m), + None => ModuleResolution::NotFound, + }) + } } /// Failure modes surfaced from `ModuleSource::resolve`. "Not found" is