From 82b457931728fd81216734eeb1148aff5683612e Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Mon, 29 Jun 2026 20:31:14 +0200 Subject: [PATCH] =?UTF-8?q?feat(modules):=20no-provider=20plan=20check=20+?= =?UTF-8?q?=20read-only=20extension-points=20ls=20(=C2=A75.5=20C4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - apply: `check_extension_points_provided` (app nodes) — every EP visible on the app's chain (its own + inherited) must have a provider: a same-apply module, or one resolvable up-chain (override or default body). Else a clean `pic plan` error instead of a runtime failure. Single-node only (the tree path leans on the runtime backstop, like the dangling-import check). - read-only surface: `extension_point_report` + `ExtensionPointInfo`; `GET /{apps|groups}/{id}/extension-points` (AppRead / GroupScriptsRead); `pic extension-points ls --app|--group` showing declared-vs-inherited + the resolved provider (`app override` / `inherited default` / `unset`). - `pic pull` round-trips an app's OWN declared EPs (filtered `declared_here`), so pull→plan stays idempotent. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/apply_api.rs | 49 ++++++- crates/manager-core/src/apply_service.rs | 132 +++++++++++++++++- .../manager-core/src/extension_point_repo.rs | 20 ++- crates/picloud-cli/src/client.rs | 29 ++++ .../picloud-cli/src/cmds/extension_points.rs | 48 +++++++ crates/picloud-cli/src/cmds/mod.rs | 1 + crates/picloud-cli/src/cmds/pull.rs | 12 +- crates/picloud-cli/src/main.rs | 24 ++++ 8 files changed, 307 insertions(+), 8 deletions(-) create mode 100644 crates/picloud-cli/src/cmds/extension_points.rs diff --git a/crates/manager-core/src/apply_api.rs b/crates/manager-core/src/apply_api.rs index 6033f43..aa4c657 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, ExtensionPointInfo, + NodeKind, PlanResult, TreeBundle, TreePlanResult, }; use crate::authz::{require, AuthzDenied, Capability}; use crate::group_repo::GroupRepository; @@ -32,9 +32,52 @@ pub fn apply_router(service: ApplyService) -> Router { .route("/groups/{id}/apply", post(group_apply_handler)) .route("/tree/plan", post(tree_plan_handler)) .route("/tree/apply", post(tree_apply_handler)) + .route( + "/apps/{id}/extension-points", + get(app_extension_points_handler), + ) + .route( + "/groups/{id}/extension-points", + get(group_extension_points_handler), + ) .with_state(service) } +/// Read-only §5.5 extension-point report for an app: every EP visible on its +/// chain, each with `declared_here` + resolved `provider`. Viewer-tier read. +async fn app_extension_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)?; + let report = svc.extension_point_report(ApplyOwner::App(app_id)).await?; + Ok(Json(report)) +} + +/// Read-only §5.5 extension-point report for a group: its own declared names. +async fn group_extension_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)?; + let report = svc + .extension_point_report(ApplyOwner::Group(group_id)) + .await?; + Ok(Json(report)) +} + #[derive(Deserialize)] pub struct ApplyRequest { pub bundle: Bundle, diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index f4fcbca..b86f61f 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -32,8 +32,8 @@ use std::sync::Arc; use picloud_orchestrator_core::routing::{pattern, RouteTable}; use picloud_shared::{ AdminUserId, AppId, DispatchMode, DocsEventOp, FilesEventOp, GroupId, HostKind, KvEventOp, - MasterKey, PathKind, Route, Script, ScriptId, ScriptKind, ScriptSandbox, ScriptValidator, - TriggerId, + MasterKey, PathKind, Route, Script, ScriptId, ScriptKind, ScriptOwner, ScriptSandbox, + ScriptValidator, TriggerId, }; use serde::{Deserialize, Serialize}; use sqlx::PgPool; @@ -415,6 +415,17 @@ pub struct CurrentState { pub extension_point_names: Vec, } +/// One row of the read-only extension-point report (§5.5). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ExtensionPointInfo { + pub name: String, + /// True when this node owns the marker (vs inheriting it from an ancestor). + pub declared_here: bool, + /// Resolved provider for an app (`app override` / `inherited default`), or + /// `None` when unset (no provider — a plan error) or for a group node. + pub provider: Option, +} + // ---------------------------------------------------------------------------- // Errors // ---------------------------------------------------------------------------- @@ -499,6 +510,7 @@ impl ApplyService { 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?; + self.check_extension_points_provided(owner, bundle).await?; if let ApplyOwner::App(app_id) = owner { self.validate_route_hosts(app_id, bundle).await?; } @@ -926,6 +938,7 @@ impl ApplyService { 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?; + self.check_extension_points_provided(owner, bundle).await?; if let ApplyOwner::App(app_id) = owner { self.validate_route_hosts(app_id, bundle).await?; } @@ -1638,6 +1651,121 @@ impl ApplyService { Ok(()) } + /// Read-only extension-point report for a node (§5.5). For an **app**: + /// every EP visible on its chain, each flagged `declared_here` (owned by + /// the app, vs inherited) with its resolved `provider` (`app override` / + /// `inherited default` / `null` = unset). For a **group**: its own declared + /// names (no single app to resolve a provider against). Backs the read-only + /// `extension-points ls` and the `pull` round-trip. + pub async fn extension_point_report( + &self, + owner: ApplyOwner, + ) -> Result, ApplyError> { + let pool = &self.pool; + match owner { + ApplyOwner::Group(g) => { + let names = + crate::extension_point_repo::list_for_owner(pool, ScriptOwner::Group(g)) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + Ok(names + .into_iter() + .map(|name| ExtensionPointInfo { + name, + declared_here: true, + provider: None, + }) + .collect()) + } + ApplyOwner::App(a) => { + let own: HashSet = + crate::extension_point_repo::list_for_owner(pool, ScriptOwner::App(a)) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))? + .into_iter() + .map(|n| n.to_lowercase()) + .collect(); + let visible = crate::extension_point_repo::list_on_app_chain(pool, a) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + let mut out = Vec::with_capacity(visible.len()); + for name in visible { + let provider = match self + .modules + .resolve(ScriptOwner::App(a), &name) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))? + .and_then(|m| m.owner()) + { + Some(ScriptOwner::App(_)) => Some("app override".to_string()), + Some(ScriptOwner::Group(_)) => Some("inherited default".to_string()), + None => None, + }; + out.push(ExtensionPointInfo { + declared_here: own.contains(&name.to_lowercase()), + name, + provider, + }); + } + Ok(out) + } + } + } + + /// §5.5 no-provider plan check (app nodes only). Every extension point + /// visible to the app — declared in this bundle or inherited from an + /// ancestor — must have a provider: a module of that name the app provides + /// (in this bundle, or resolvable up its chain as an override or a default + /// body). Otherwise the import would fail at runtime, so refuse at plan. + /// Group nodes have no single inheriting app, so the check is per-app. + /// Single-node only (the tree path leans on the runtime backstop). + async fn check_extension_points_provided( + &self, + owner: ApplyOwner, + bundle: &Bundle, + ) -> Result<(), ApplyError> { + let ApplyOwner::App(app_id) = owner else { + return Ok(()); + }; + // A same-apply app module provides its EP without being committed yet. + let local: HashSet = bundle + .scripts + .iter() + .filter(|s| s.kind == ScriptKind::Module) + .map(|s| s.name.to_lowercase()) + .collect(); + // EP names visible to the app: its own (this bundle) ∪ inherited. + let mut names: HashSet = bundle + .extension_points + .iter() + .map(|n| n.to_lowercase()) + .collect(); + for n in crate::extension_point_repo::list_on_app_chain(&self.pool, app_id) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))? + { + names.insert(n.to_lowercase()); + } + for name in names { + if local.contains(&name) { + continue; + } + let found = self + .modules + .resolve(picloud_shared::ScriptOwner::App(app_id), &name) + .await + .map_err(|e| ApplyError::Backend(e.to_string()))?; + if found.is_none() { + return Err(ApplyError::Invalid(format!( + "extension point `{name}` has no provider for this app — \ + declare a module named `{name}` here or ensure a default \ + body exists up-chain" + ))); + } + } + 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 diff --git a/crates/manager-core/src/extension_point_repo.rs b/crates/manager-core/src/extension_point_repo.rs index 21f9a10..8c0f4e2 100644 --- a/crates/manager-core/src/extension_point_repo.rs +++ b/crates/manager-core/src/extension_point_repo.rs @@ -7,9 +7,11 @@ //! helpers; it mirrors the var/secret tx-function style (free functions over a //! `&PgPool` / `&mut Transaction`), keyed by the shared [`ScriptOwner`]. -use picloud_shared::ScriptOwner; +use picloud_shared::{AppId, ScriptOwner}; use sqlx::{PgPool, Postgres, Transaction}; +use crate::config_resolver::CHAIN_LEVELS_CTE; + /// List the EP names declared **directly at** `owner` (not inherited), /// case-insensitively sorted. Used by `load_current` (apply diff), the /// no-provider check, and the read-only `extension-points ls`. @@ -35,6 +37,22 @@ pub async fn list_for_owner(pool: &PgPool, owner: ScriptOwner) -> Result Result, sqlx::Error> { + let rows: Vec<(String,)> = sqlx::query_as(&format!( + "{CHAIN_LEVELS_CTE} \ + SELECT DISTINCT e.name FROM extension_points e \ + JOIN chain c ON (e.app_id = c.app_owner OR e.group_id = c.group_owner) \ + ORDER BY e.name", + )) + .bind(app_id.into_inner()) + .fetch_all(pool) + .await?; + Ok(rows.into_iter().map(|(n,)| n).collect()) +} + /// Insert an EP marker at `owner`, in the apply transaction. Idempotent: a /// re-apply of an already-declared name is a no-op (`ON CONFLICT DO NOTHING`), /// so the marker survives without a spurious version bump. diff --git a/crates/picloud-cli/src/client.rs b/crates/picloud-cli/src/client.rs index 95ae06b..b495b91 100644 --- a/crates/picloud-cli/src/client.rs +++ b/crates/picloud-cli/src/client.rs @@ -1305,6 +1305,35 @@ impl NodeKind { } } +/// One row of the §5.5 extension-point report. +#[derive(Debug, Deserialize)] +pub struct ExtensionPointInfoDto { + pub name: String, + #[serde(default)] + pub declared_here: bool, + #[serde(default)] + pub provider: Option, +} + +impl Client { + /// `GET /api/v1/admin/{apps|groups}/{ident}/extension-points`. + pub async fn extension_points_list( + &self, + kind: NodeKind, + ident: &str, + ) -> Result> { + let ident = seg(ident); + let resp = self + .request( + Method::GET, + &format!("/api/v1/admin/{}/{ident}/extension-points", kind.path()), + ) + .send() + .await?; + decode(resp).await + } +} + // ---------- DTOs (CLI-local, wire-shape-matched) ---------- /// Response of `POST .../plan`: per-resource diffs grouped by kind. diff --git a/crates/picloud-cli/src/cmds/extension_points.rs b/crates/picloud-cli/src/cmds/extension_points.rs new file mode 100644 index 0000000..1c806d9 --- /dev/null +++ b/crates/picloud-cli/src/cmds/extension_points.rs @@ -0,0 +1,48 @@ +//! `pic extension-points ls --app|--group` — read-only view of a node's +//! §5.5 extension points. For an app, each EP visible on its chain is shown +//! with its resolved provider (`app override` / `inherited default` / `unset`); +//! for a group, its own declared names. Authoring is declarative only (the +//! manifest `extension_points = [...]`). + +use anyhow::{bail, Result}; + +use crate::client::{Client, NodeKind}; +use crate::config; +use crate::output::{OutputMode, Table}; + +pub async fn ls(app: Option<&str>, group: Option<&str>, mode: OutputMode) -> Result<()> { + let (kind, ident) = match (app, group) { + (Some(a), None) => (NodeKind::App, a), + (None, Some(g)) => (NodeKind::Group, g), + _ => bail!("`extension-points ls` requires exactly one of --app or --group"), + }; + let creds = config::resolve()?; + let client = Client::from_creds(&creds)?; + let items = client.extension_points_list(kind, ident).await?; + + match kind { + NodeKind::App => { + let mut table = Table::new(["name", "source", "provider"]); + for ep in &items { + table.row([ + ep.name.clone(), + if ep.declared_here { + "declared".into() + } else { + "inherited".into() + }, + ep.provider.clone().unwrap_or_else(|| "unset".into()), + ]); + } + table.print(mode); + } + NodeKind::Group => { + let mut table = Table::new(["name"]); + for ep in &items { + table.row([ep.name.clone()]); + } + table.print(mode); + } + } + Ok(()) +} diff --git a/crates/picloud-cli/src/cmds/mod.rs b/crates/picloud-cli/src/cmds/mod.rs index b4d4629..b11d72e 100644 --- a/crates/picloud-cli/src/cmds/mod.rs +++ b/crates/picloud-cli/src/cmds/mod.rs @@ -5,6 +5,7 @@ pub mod apps; pub mod apps_domains; pub mod config; pub mod dead_letters; +pub mod extension_points; pub mod files; pub mod groups; pub mod init; diff --git a/crates/picloud-cli/src/cmds/pull.rs b/crates/picloud-cli/src/cmds/pull.rs index fc19e3d..007950d 100644 --- a/crates/picloud-cli/src/cmds/pull.rs +++ b/crates/picloud-cli/src/cmds/pull.rs @@ -49,6 +49,15 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> .secrets_list(VarOwnerArg::App(app_ident), None) .await? .secrets; + // The app's OWN declared extension points (§5.5) — inherited ones belong to + // the ancestor group's manifest, so filter to `declared_here`. + let extension_points: Vec = client + .extension_points_list(crate::client::NodeKind::App, app_ident) + .await? + .into_iter() + .filter(|ep| ep.declared_here) + .map(|ep| ep.name) + .collect(); // App's OWN env-agnostic vars become the manifest `[vars]` block. Inherited // group vars and tombstones are excluded (pull exports own rows only, §4.6); // a value TOML can't represent (e.g. JSON null) is skipped with a warning. @@ -224,8 +233,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, - // Wired to the read endpoint in C4 so pull→plan round-trips EPs. - extension_points: Vec::new(), + extension_points, }; std::fs::write(&manifest_path, manifest.to_toml()?) diff --git a/crates/picloud-cli/src/main.rs b/crates/picloud-cli/src/main.rs index 5023883..7253a80 100644 --- a/crates/picloud-cli/src/main.rs +++ b/crates/picloud-cli/src/main.rs @@ -153,6 +153,15 @@ enum Cmd { cmd: VarsCmd, }, + /// Extension points (§5.5) — read-only view of the module names a node + /// marks as overridable. Authored declaratively via the manifest + /// `extension_points = [...]`; this lists them + (for an app) each one's + /// resolved provider. + ExtensionPoints { + #[command(subcommand)] + cmd: ExtensionPointsCmd, + }, + /// Files inspection — list a collection's blobs, download bytes, or /// delete a file. Read + delete only; writes go through scripts. Files { @@ -537,6 +546,18 @@ enum DomainsCmd { Rm { app: String, domain_id: String }, } +#[derive(Subcommand)] +enum ExtensionPointsCmd { + /// List a node's extension points. `--app` shows every EP visible on the + /// app's chain with its resolved provider; `--group` shows the group's own. + Ls { + #[arg(long)] + app: Option, + #[arg(long, conflicts_with = "app")] + group: Option, + }, +} + #[derive(Subcommand)] enum ScriptsCmd { /// List scripts. With `--app`, scoped to one app; with `--group`, a @@ -1961,6 +1982,9 @@ async fn main() -> ExitCode { env, }, } => cmds::vars::rm(group.as_deref(), app.as_deref(), &key, env.as_deref()).await, + Cmd::ExtensionPoints { + cmd: ExtensionPointsCmd::Ls { app, group }, + } => cmds::extension_points::ls(app.as_deref(), group.as_deref(), mode).await, Cmd::Files { cmd: FilesCmd::Ls {