fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(modules): enable group modules + dangling-import plan check (Phase 4b C4)

Browse Source 
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 "<m>"` 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) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-26 07:29:18 +02:00
parent b53eabb786
commit c8acac1d20
3 changed files with 73 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
crates/manager-core/src/apply_service.rs
View File

@@ -449,6 +449,9 @@ pub struct ApplyService {
pub apps: Arc<dyn AppRepository>,
/// Group lookups (Phase 5): resolve a group slug→id for group/tree apply.
pub groups: Arc<dyn crate::group_repo::GroupRepository>,
/// Module resolver (Phase 4b): lexical, origin-aware module lookup used by
/// the single-node dangling-import plan check (§5.5).
pub modules: Arc<dyn picloud_shared::ModuleSource>,
/// App domain claims — validates a route's host belongs to the app.
pub domains: Arc<dyn AppDomainRepository>,
pub authz: Arc<dyn AuthzRepo>,
@@ -485,6 +488,7 @@ impl ApplyService {
) -> Result<PlanResult, ApplyError> {
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<ApplyReport, ApplyError> {
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 "<m>"` 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<String> = 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<String> = 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",

21
crates/manager-core/src/group_scripts_api.rs
View File

@@ -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)))