feat(modules): owner-aware module lookup — lexical chain walk (Phase 4b C1)
Make the module-loading contract origin-aware so group-owned modules become resolvable and imports can resolve lexically (§5.5). - `ModuleScript` gains a polymorphic owner (`app_id`/`group_id` + `owner()`), mirroring `Script`. - `ModuleSource::lookup(&cx, name)` -> `resolve(origin: ScriptOwner, name)`: resolution walks the group chain rooted at the importing script's defining node (nearest-owner-wins), not the inheriting app's view. - `PostgresModuleSource::resolve` branches on origin: app-rooted (`CHAIN_LEVELS_CTE`) or a new group-rooted `GROUP_CHAIN_LEVELS_CTE`, joining `kind='module'` rows. The executor resolver passes a temporary `App(cx.app_id)` origin (behaviour- preserving for app scripts; true lexical origin lands in C3). Group scripts still can't carry imports until C4, so no trust-inversion window opens here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -319,8 +319,19 @@ impl ModuleResolver for PicloudModuleResolver {
|
||||
))
|
||||
})?;
|
||||
|
||||
// C1 (Phase 4b): resolve via the owner-aware `resolve`. Until C3
|
||||
// threads the importing script's true defining node through, the
|
||||
// origin is the calling app — behaviour-preserving for app-owned
|
||||
// scripts (group scripts can't yet carry imports). The app-rooted
|
||||
// chain walk additionally lets an app import an ancestor group's
|
||||
// modules, which is the intended Phase 4b capability.
|
||||
let lookup_result: Result<Option<picloud_shared::ModuleScript>, ModuleSourceError> =
|
||||
tokio::task::block_in_place(|| handle.block_on(self.source.lookup(&self.cx, path)));
|
||||
tokio::task::block_in_place(|| {
|
||||
handle.block_on(
|
||||
self.source
|
||||
.resolve(picloud_shared::ScriptOwner::App(self.cx.app_id), path),
|
||||
)
|
||||
});
|
||||
|
||||
let module_row = match lookup_result {
|
||||
Ok(Some(m)) => m,
|
||||
|
||||
@@ -16,7 +16,7 @@ use picloud_executor_core::{Engine, ExecRequest, InvocationType, Limits};
|
||||
use picloud_shared::{
|
||||
AppId, ExecutionId, ModuleScript, ModuleSource, ModuleSourceError, NoopDeadLetterService,
|
||||
NoopDocsService, NoopEventEmitter, NoopHttpService, NoopKvService, RequestId, ScriptId,
|
||||
ScriptSandbox, SdkCallCx, Services,
|
||||
ScriptOwner, ScriptSandbox, Services,
|
||||
};
|
||||
use serde_json::Value;
|
||||
use tracing_subscriber::fmt::MakeWriter;
|
||||
@@ -27,9 +27,9 @@ struct FailingSource;
|
||||
|
||||
#[async_trait]
|
||||
impl ModuleSource for FailingSource {
|
||||
async fn lookup(
|
||||
async fn resolve(
|
||||
&self,
|
||||
_cx: &SdkCallCx,
|
||||
_origin: ScriptOwner,
|
||||
_name: &str,
|
||||
) -> Result<Option<ModuleScript>, ModuleSourceError> {
|
||||
Err(ModuleSourceError::Backend(SENTINEL.to_string()))
|
||||
|
||||
@@ -18,7 +18,7 @@ use picloud_executor_core::{Engine, ExecRequest, InvocationType, Limits};
|
||||
use picloud_shared::{
|
||||
AppId, ExecutionId, ModuleScript, ModuleSource, ModuleSourceError, NoopDeadLetterService,
|
||||
NoopDocsService, NoopEventEmitter, NoopHttpService, NoopKvService, RequestId, ScriptId,
|
||||
ScriptSandbox, SdkCallCx, Services,
|
||||
ScriptOwner, ScriptSandbox, Services,
|
||||
};
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
@@ -55,7 +55,8 @@ impl CountingModuleSource {
|
||||
(app_id, name.to_string()),
|
||||
ModuleScript {
|
||||
script_id,
|
||||
app_id,
|
||||
app_id: Some(app_id),
|
||||
group_id: None,
|
||||
name: name.to_string(),
|
||||
source: source.to_string(),
|
||||
updated_at,
|
||||
@@ -71,20 +72,27 @@ impl CountingModuleSource {
|
||||
|
||||
#[async_trait]
|
||||
impl ModuleSource for CountingModuleSource {
|
||||
async fn lookup(
|
||||
async fn resolve(
|
||||
&self,
|
||||
cx: &SdkCallCx,
|
||||
origin: ScriptOwner,
|
||||
name: &str,
|
||||
) -> Result<Option<ModuleScript>, ModuleSourceError> {
|
||||
self.lookups.fetch_add(1, Ordering::SeqCst);
|
||||
if let Some(err) = self.fail_with.lock().await.as_ref() {
|
||||
return Err(ModuleSourceError::Backend(err.clone()));
|
||||
}
|
||||
// This fake is flat/app-scoped — the inheritance + lexical
|
||||
// resolution semantics are covered by the CLI journey tests
|
||||
// against real Postgres. A group origin has no entries here.
|
||||
let app_id = match origin {
|
||||
ScriptOwner::App(a) => a,
|
||||
ScriptOwner::Group(_) => return Ok(None),
|
||||
};
|
||||
Ok(self
|
||||
.table
|
||||
.lock()
|
||||
.await
|
||||
.get(&(cx.app_id, name.to_string()))
|
||||
.get(&(app_id, name.to_string()))
|
||||
.cloned())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -42,6 +42,22 @@ pub(crate) const CHAIN_LEVELS_CTE: &str = "\
|
||||
WHERE c.depth < 64 \
|
||||
)";
|
||||
|
||||
/// Group-rooted variant of [`CHAIN_LEVELS_CTE`]: the chain starts at a
|
||||
/// **group** (depth 0 = the group itself, `group_owner` set), then walks
|
||||
/// its ancestor groups nearest-first. There is no `app_owner` level — a
|
||||
/// group origin never sees app-owned rows below it (the §5.5 trust
|
||||
/// boundary). Bind `$1 = group_id`. Used for the lexical module resolver
|
||||
/// when the importing script's defining node is a group.
|
||||
pub(crate) const GROUP_CHAIN_LEVELS_CTE: &str = "\
|
||||
WITH RECURSIVE chain AS ( \
|
||||
SELECT g.id AS group_owner, g.parent_id AS next_group, 0 AS depth \
|
||||
FROM groups g WHERE g.id = $1 \
|
||||
UNION ALL \
|
||||
SELECT g.id, g.parent_id, c.depth + 1 \
|
||||
FROM groups g JOIN chain c ON g.id = c.next_group \
|
||||
WHERE c.depth < 64 \
|
||||
)";
|
||||
|
||||
/// Owner kind of a resolved value, for `--explain` provenance.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum OwnerKind {
|
||||
|
||||
@@ -1,18 +1,22 @@
|
||||
//! `PostgresModuleSource` — the Postgres-backed `ModuleSource` impl.
|
||||
//!
|
||||
//! Mirrors the structure of [`crate::kv_repo::PostgresKvRepo`] /
|
||||
//! [`crate::docs_repo::PostgresDocsRepo`]: thin wrapper around a
|
||||
//! `PgPool` that owns a single statement returning the module by
|
||||
//! `(cx.app_id, name, kind = 'module')`. The resolver lives in
|
||||
//! `executor-core` and consumes this trait through the `Services`
|
||||
//! bundle, so manager-core stays the only crate that touches
|
||||
//! Postgres.
|
||||
//! 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.
|
||||
|
||||
use async_trait::async_trait;
|
||||
use chrono::{DateTime, Utc};
|
||||
use picloud_shared::{ModuleScript, ModuleSource, ModuleSourceError, SdkCallCx};
|
||||
use picloud_shared::{ModuleScript, ModuleSource, ModuleSourceError, ScriptOwner};
|
||||
use sqlx::PgPool;
|
||||
|
||||
use crate::config_resolver::{CHAIN_LEVELS_CTE, GROUP_CHAIN_LEVELS_CTE};
|
||||
|
||||
pub struct PostgresModuleSource {
|
||||
pool: PgPool,
|
||||
}
|
||||
@@ -27,7 +31,8 @@ impl PostgresModuleSource {
|
||||
#[derive(sqlx::FromRow)]
|
||||
struct ModuleRow {
|
||||
id: uuid::Uuid,
|
||||
app_id: uuid::Uuid,
|
||||
app_id: Option<uuid::Uuid>,
|
||||
group_id: Option<uuid::Uuid>,
|
||||
name: String,
|
||||
source: String,
|
||||
updated_at: DateTime<Utc>,
|
||||
@@ -37,7 +42,8 @@ impl From<ModuleRow> for ModuleScript {
|
||||
fn from(r: ModuleRow) -> Self {
|
||||
Self {
|
||||
script_id: r.id.into(),
|
||||
app_id: r.app_id.into(),
|
||||
app_id: r.app_id.map(Into::into),
|
||||
group_id: r.group_id.map(Into::into),
|
||||
name: r.name,
|
||||
source: r.source,
|
||||
updated_at: r.updated_at,
|
||||
@@ -47,28 +53,47 @@ impl From<ModuleRow> for ModuleScript {
|
||||
|
||||
#[async_trait]
|
||||
impl ModuleSource for PostgresModuleSource {
|
||||
async fn lookup(
|
||||
async fn resolve(
|
||||
&self,
|
||||
cx: &SdkCallCx,
|
||||
origin: ScriptOwner,
|
||||
name: &str,
|
||||
) -> Result<Option<ModuleScript>, ModuleSourceError> {
|
||||
// The query is the cross-app isolation boundary: app_id comes
|
||||
// from cx (never from the script-passed argument), and the
|
||||
// CHECK constraint `kind IN ('endpoint','module')` plus the
|
||||
// `kind = 'module'` filter together guarantee endpoint scripts
|
||||
// are never importable. The `(app_id, kind)` index from
|
||||
// migration 0015 makes this an index scan returning at most
|
||||
// one row (per-app uniqueness on `name`).
|
||||
let row: Option<ModuleRow> = sqlx::query_as(
|
||||
"SELECT id, app_id, name, source, updated_at \
|
||||
FROM scripts \
|
||||
WHERE app_id = $1 AND kind = 'module' AND name = $2",
|
||||
)
|
||||
.bind(cx.app_id.into_inner())
|
||||
.bind(name)
|
||||
.fetch_optional(&self.pool)
|
||||
.await
|
||||
.map_err(|e| ModuleSourceError::Backend(e.to_string()))?;
|
||||
// Lexical lookup: JOIN module scripts against the chain rooted at
|
||||
// `origin`, take the nearest level (lowest depth). The `kind =
|
||||
// 'module'` filter + the CHECK constraint guarantee endpoint
|
||||
// scripts are never importable. Case-insensitive to match the
|
||||
// per-owner `LOWER(name)` partial-unique indexes (0050). `origin`
|
||||
// is a trusted dispatch-derived value, so the chain it roots is
|
||||
// the script's true ancestry — the cross-app isolation boundary
|
||||
// is preserved (a group origin can't reach an app's modules).
|
||||
let query = match origin {
|
||||
ScriptOwner::App(_) => format!(
|
||||
"{CHAIN_LEVELS_CTE} \
|
||||
SELECT s.id, s.app_id, s.group_id, s.name, s.source, s.updated_at \
|
||||
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) \
|
||||
ORDER BY c.depth ASC LIMIT 1",
|
||||
),
|
||||
ScriptOwner::Group(_) => format!(
|
||||
"{GROUP_CHAIN_LEVELS_CTE} \
|
||||
SELECT s.id, s.app_id, s.group_id, s.name, s.source, s.updated_at \
|
||||
FROM scripts s \
|
||||
JOIN chain c ON s.group_id = c.group_owner \
|
||||
WHERE s.kind = 'module' AND LOWER(s.name) = LOWER($2) \
|
||||
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<ModuleRow> = sqlx::query_as(&query)
|
||||
.bind(root)
|
||||
.bind(name)
|
||||
.fetch_optional(&self.pool)
|
||||
.await
|
||||
.map_err(|e| ModuleSourceError::Backend(e.to_string()))?;
|
||||
Ok(row.map(Into::into))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,51 +1,88 @@
|
||||
//! `ModuleSource` — the v1.1.3 Rhai module-loading contract.
|
||||
//! `ModuleSource` — the Rhai module-loading contract (v1.1.3; made
|
||||
//! origin-aware in v1.2 Phase 4b).
|
||||
//!
|
||||
//! The executor-core `PicloudModuleResolver` calls into this trait to
|
||||
//! load `kind = 'module'` scripts referenced by `import "<name>" as <alias>;`
|
||||
//! statements. The Postgres impl in `manager-core` reads from the
|
||||
//! `scripts` table; tests pin in-memory fakes.
|
||||
//!
|
||||
//! Implementations MUST derive `app_id` from `cx.app_id` and pass it
|
||||
//! to every backend query. The `name` argument carries only the
|
||||
//! script's name (the literal between the import quotes); the trait
|
||||
//! has no way to express a cross-app lookup. That asymmetry is the
|
||||
//! load-bearing cross-app isolation boundary — see `docs/sdk-shape.md`.
|
||||
//! **Resolution is lexical (§5.5).** A module is resolved against the
|
||||
//! module set visible at the **importing script's own defining node**
|
||||
//! (`origin`), walking up the group chain from there — *not* the
|
||||
//! inheriting app's effective view. So `resolve` is keyed by a
|
||||
//! [`ScriptOwner`] origin, not by `cx.app_id`. This deliberately differs
|
||||
//! from the SDK-call isolation boundary: a group script's `import`
|
||||
//! resolves from the **group** (sealed from below — a leaf app cannot
|
||||
//! shadow it), while every SDK call *inside* that module still scopes to
|
||||
//! the inheriting app via `cx.app_id`. The `origin` argument is a trusted
|
||||
//! dispatch-derived value (the resolved script's owner), never a
|
||||
//! script-passed argument, so the cross-app isolation boundary holds.
|
||||
|
||||
use async_trait::async_trait;
|
||||
use chrono::{DateTime, Utc};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use thiserror::Error;
|
||||
|
||||
use crate::{AppId, ScriptId, SdkCallCx};
|
||||
use crate::{AppId, GroupId, ScriptId, ScriptOwner};
|
||||
|
||||
/// A module script as returned by `ModuleSource::lookup`. Carries only
|
||||
/// the fields the resolver needs: the id (for diagnostics), the source
|
||||
/// (to compile), and `updated_at` (the cache-staleness comparator).
|
||||
/// A module script as returned by `ModuleSource::resolve`. Carries the
|
||||
/// fields the resolver needs: the id (cache key + diagnostics), the
|
||||
/// resolved module's **owner** (so nested imports inside it resolve from
|
||||
/// *its* defining node), the source (to compile), and `updated_at` (the
|
||||
/// cache-staleness comparator).
|
||||
///
|
||||
/// Ownership is polymorphic (Phase 4): exactly one of `app_id` / `group_id`
|
||||
/// is set — mirroring [`crate::Script`] and the DB CHECK.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct ModuleScript {
|
||||
pub script_id: ScriptId,
|
||||
pub app_id: AppId,
|
||||
/// Owning app, when app-owned. Mutually exclusive with `group_id`.
|
||||
#[serde(default)]
|
||||
pub app_id: Option<AppId>,
|
||||
/// Owning group, when group-owned (Phase 4b group modules).
|
||||
#[serde(default)]
|
||||
pub group_id: Option<GroupId>,
|
||||
pub name: String,
|
||||
pub source: String,
|
||||
pub updated_at: DateTime<Utc>,
|
||||
}
|
||||
|
||||
/// Lookup contract used by `PicloudModuleResolver`. `lookup` MUST
|
||||
/// scope by `cx.app_id`; cross-app reads must be unreachable.
|
||||
impl ModuleScript {
|
||||
/// The resolved module's defining node, reconstructed from the
|
||||
/// polymorphic columns. Used by the resolver to seal *this* module's
|
||||
/// own nested imports to its node (lexical chaining). Prefers the app
|
||||
/// if a corrupt row sets both, so the hot path never panics.
|
||||
#[must_use]
|
||||
pub fn owner(&self) -> Option<ScriptOwner> {
|
||||
match (self.app_id, self.group_id) {
|
||||
(Some(a), _) => Some(ScriptOwner::App(a)),
|
||||
(None, Some(g)) => Some(ScriptOwner::Group(g)),
|
||||
(None, None) => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// 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
|
||||
/// (dispatch-derived); the `name` is the literal between the import
|
||||
/// quotes. See the module docs for why this is lexical, not `cx`-scoped.
|
||||
#[async_trait]
|
||||
pub trait ModuleSource: Send + Sync {
|
||||
/// Resolve a module script by `(cx.app_id, name)`. Returns `None`
|
||||
/// when no row exists, or when a row exists but its `kind` is
|
||||
/// `'endpoint'` (endpoints are never importable). The resolver
|
||||
/// surfaces `None` as `ErrorModuleNotFound` to Rhai.
|
||||
async fn lookup(
|
||||
/// Resolve a `kind = 'module'` script named `name`, visible from the
|
||||
/// `origin` node walking up its group chain (nearest depth wins).
|
||||
/// Returns `None` when no module of that name exists anywhere on the
|
||||
/// chain (or a row exists but its `kind` is `'endpoint'` — endpoints
|
||||
/// are never importable). The resolver surfaces `None` as
|
||||
/// `ErrorModuleNotFound` to Rhai.
|
||||
async fn resolve(
|
||||
&self,
|
||||
cx: &SdkCallCx,
|
||||
origin: ScriptOwner,
|
||||
name: &str,
|
||||
) -> Result<Option<ModuleScript>, ModuleSourceError>;
|
||||
}
|
||||
|
||||
/// Failure modes surfaced from `ModuleSource::lookup`. "Not found" is
|
||||
/// Failure modes surfaced from `ModuleSource::resolve`. "Not found" is
|
||||
/// not exceptional — it's `Ok(None)`.
|
||||
#[derive(Debug, Error)]
|
||||
pub enum ModuleSourceError {
|
||||
@@ -57,7 +94,7 @@ pub enum ModuleSourceError {
|
||||
}
|
||||
|
||||
/// Stub used by the executor-core test harness so engine integration
|
||||
/// tests don't need a real DB-backed source. Every lookup returns
|
||||
/// tests don't need a real DB-backed source. Every resolve returns
|
||||
/// `Ok(None)` — `import "x"` always errors as "module not found"
|
||||
/// under this impl.
|
||||
#[derive(Debug, Default, Clone, Copy)]
|
||||
@@ -65,9 +102,9 @@ pub struct NoopModuleSource;
|
||||
|
||||
#[async_trait]
|
||||
impl ModuleSource for NoopModuleSource {
|
||||
async fn lookup(
|
||||
async fn resolve(
|
||||
&self,
|
||||
_cx: &SdkCallCx,
|
||||
_origin: ScriptOwner,
|
||||
_name: &str,
|
||||
) -> Result<Option<ModuleScript>, ModuleSourceError> {
|
||||
Ok(None)
|
||||
|
||||
Reference in New Issue
Block a user