From c8acac1d20f2cd86d92bb922815d4c2dc57aeb28 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 26 Jun 2026 07:29:18 +0200 Subject: [PATCH] feat(modules): enable group modules + dangling-import plan check (Phase 4b C4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lift the Phase-4-lite restrictions now that import resolution is lexical: - `create_group_script` (group_scripts_api): allow `kind = module` and group scripts with `import`s; record the import edges. Module-shape and sandbox-ceiling validation stay. - The declarative apply path already reconciles group modules generically (`BundleScript.kind` + `owner.script_owner()` flow through `reconcile_node_tx`); no change needed beyond stale-comment fixes. Adds the §5.5 dangling-import plan check (single-node plan/apply): `check_imports_resolve` rejects an `import ""` that resolves to neither a bundle-local module nor a module reachable lexically up the node's chain — caught at plan time instead of as a runtime 404. Roots resolution at the node's owner (an app node reaches ancestor group modules; a group node walks its own ancestry). ApplyService gains an injected `ModuleSource`. The tree path opts out (a node may import a module created by another node in the same uncommitted tx); the runtime check is the backstop there. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/apply_service.rs | 72 ++++++++++++++++++-- crates/manager-core/src/group_scripts_api.rs | 21 ++---- crates/picloud/src/lib.rs | 3 +- 3 files changed, 73 insertions(+), 23 deletions(-) diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index 2415a96..b284dd2 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -449,6 +449,9 @@ pub struct ApplyService { pub apps: Arc, /// Group lookups (Phase 5): resolve a group slug→id for group/tree apply. pub groups: Arc, + /// Module resolver (Phase 4b): lexical, origin-aware module lookup used by + /// the single-node dangling-import plan check (§5.5). + pub modules: Arc, /// App domain claims — validates a route's host belongs to the app. pub domains: Arc, pub authz: Arc, @@ -485,6 +488,7 @@ impl ApplyService { ) -> Result { let inherited = self.resolve_inherited_targets_for(owner, bundle).await?; self.validate_bundle_for(owner, bundle, &keys_set(&inherited))?; + self.check_imports_resolve(owner, bundle).await?; if let ApplyOwner::App(app_id) = owner { self.validate_route_hosts(app_id, bundle).await?; } @@ -882,6 +886,7 @@ impl ApplyService { ) -> Result { let inherited = self.resolve_inherited_targets_for(owner, bundle).await?; self.validate_bundle_for(owner, bundle, &keys_set(&inherited))?; + self.check_imports_resolve(owner, bundle).await?; if let ApplyOwner::App(app_id) = owner { self.validate_route_hosts(app_id, bundle).await?; } @@ -1496,10 +1501,11 @@ impl ApplyService { .await .map_err(map_repo)? { - // Only genuinely-inherited group endpoints qualify. A depth-0 - // app-own match is already covered by `name_to_id`; skip it so - // the manifest keeps precedence. Group modules don't exist in - // Phase 4-lite, but guard kind anyway. + // Only genuinely-inherited group endpoints qualify as + // route/trigger targets. A depth-0 app-own match is already + // covered by `name_to_id`; skip it so the manifest keeps + // precedence. Group modules (Phase 4b) exist but are never + // bindable targets — the kind guard excludes them. if s.group_id.is_some() && s.kind == ScriptKind::Endpoint { out.insert(name, s.id); } @@ -1540,6 +1546,59 @@ impl ApplyService { Ok(out) } + /// §5.5 dangling-import plan check (Phase 4b, single-node path). Every + /// `import ""` in a bundle script must resolve to a module declared in + /// the same bundle OR reachable lexically up the node's chain — otherwise + /// the apply would write a script that 404s its import at runtime. Lexical: + /// the node's owner IS the defining node for its own scripts, so resolution + /// roots at `owner` (an app node reaches ancestor group modules; a group + /// node walks its own ancestry, sealed from apps below). + /// + /// Not run on the tree path: a tree node may import a module declared by + /// another node created in the same transaction (not yet committed), so the + /// in-tree set would have to be threaded through; the runtime check is the + /// backstop there. + async fn check_imports_resolve( + &self, + owner: ApplyOwner, + bundle: &Bundle, + ) -> Result<(), ApplyError> { + // Modules this bundle declares (lowercased) satisfy their own imports. + let local: HashSet = bundle + .scripts + .iter() + .filter(|s| s.kind == ScriptKind::Module) + .map(|s| s.name.to_lowercase()) + .collect(); + let origin = match owner { + ApplyOwner::App(a) => picloud_shared::ScriptOwner::App(a), + ApplyOwner::Group(g) => picloud_shared::ScriptOwner::Group(g), + }; + // Resolve each distinct imported name once. + let mut checked: HashSet = HashSet::new(); + for s in &bundle.scripts { + for imp in self.script_imports(s)? { + let key = imp.to_lowercase(); + if local.contains(&key) || !checked.insert(key) { + continue; + } + let found = self + .modules + .resolve(origin, &imp) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + if found.is_none() { + return Err(ApplyError::Invalid(format!( + "script `{}` imports unknown module `{imp}` \ + (not declared here and not inherited from an ancestor)", + s.name + ))); + } + } + } + Ok(()) + } + /// `inherited_endpoints` (lowercased names) are group-owned endpoint /// 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 @@ -1612,8 +1671,9 @@ impl ApplyService { r.script ))); } - // Inherited targets are endpoints by construction (group modules - // don't exist in Phase 4-lite); only check kind for app-own names. + // Inherited targets are resolved as endpoints by construction + // (the inherited-name resolver excludes group modules); only + // check kind for app-own names. if !inherited && !endpoints.contains(r.script.as_str()) { return Err(ApplyError::Invalid(format!( "route binds to `{}` which is a module, not an endpoint", diff --git a/crates/manager-core/src/group_scripts_api.rs b/crates/manager-core/src/group_scripts_api.rs index 2394eb4..b8d606b 100644 --- a/crates/manager-core/src/group_scripts_api.rs +++ b/crates/manager-core/src/group_scripts_api.rs @@ -97,20 +97,10 @@ async fn create_group_script( ) .await?; - // Phase 4-lite: endpoint-only, self-contained. - if input.kind == ScriptKind::Module { - return Err(GroupScriptsApiError::Invalid( - "group modules are not supported yet (Phase 4b); create an endpoint script".into(), - )); - } + // Phase 4b: group modules + imports are allowed. Imports resolve + // lexically from this group's chain at runtime (§5.5); the recorded + // edges feed the declarative dangling-import plan check. let validated = s.validator.validate(&input.source)?; - if !validated.imports.is_empty() { - return Err(GroupScriptsApiError::Invalid(format!( - "group scripts must be self-contained in Phase 4-lite; \ - remove the import(s): {}", - validated.imports.join(", ") - ))); - } s.sandbox_ceiling .check(&input.sandbox) .map_err(|e| GroupScriptsApiError::Invalid(e.to_string()))?; @@ -123,7 +113,7 @@ async fn create_group_script( name: input.name, description: input.description, source: input.source, - kind: ScriptKind::Endpoint, + kind: input.kind, timeout_seconds: input.timeout_seconds, memory_limit_mb: input.memory_limit_mb, sandbox: if input.sandbox.is_empty() { @@ -132,8 +122,7 @@ async fn create_group_script( Some(input.sandbox) }, enabled: true, - // Self-contained — no import edges (enforced above). - imports: Vec::new(), + imports: validated.imports, }) .await?; Ok((StatusCode::CREATED, Json(created))) diff --git a/crates/picloud/src/lib.rs b/crates/picloud/src/lib.rs index af2cd1c..b01ca25 100644 --- a/crates/picloud/src/lib.rs +++ b/crates/picloud/src/lib.rs @@ -330,7 +330,7 @@ pub async fn build_app( docs, dl_service.clone(), events, - modules, + modules.clone(), http, files, pubsub, @@ -474,6 +474,7 @@ pub async fn build_app( vars: Arc::new(PostgresVarsRepo::new(pool.clone())), apps: apps_repo.clone(), groups: groups_repo.clone(), + modules: modules.clone(), domains: domains_repo.clone(), authz: authz.clone(), validator: engine.clone(),