From da03fda1d698b4a2c17c8c30e7ca51e344d895b0 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Thu, 25 Jun 2026 21:57:58 +0200 Subject: [PATCH] feat(cli): [group] manifest + `pic plan`/`apply` for a group node (Phase 5 C2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 5 C2. A `picloud.toml` can now declare a `[group]` node (instead of `[app]`): the group's own scripts + `[vars]` (+ secret names). `pic plan` / `pic apply` reconcile it via the C1 group endpoints — the same plan/apply/ bound-token flow as an app, routed by node kind. * `Manifest` is now app-XOR-group: `app`/`group` are both optional with an exactly-one check in `parse`, plus a group-node guard that rejects `[[routes]]`/`[[triggers]]` (those bind to an app). Accessors `slug()` / `is_group()` replace the hardcoded `manifest.app.slug`. * `client`: `plan_node`/`apply_node` take a `NodeKind { App | Group }` that selects the `apps` vs `groups` API path; the old `plan`/`apply` wrappers are gone (callers pass the kind). * `pic plan`/`apply` detect the node kind from the manifest and label output `group`/`app`; `pic config --effective` cleanly rejects a group manifest (effective config is an app's inherited view). Live-validated: a `[group]` manifest with a script + var → `pic plan` (script + var creates) → `pic apply` (group-owned) → idempotent re-plan noop → `pic scripts ls --group` shows it. Manifest unit test for group parse + app-only-block rejection; init/pull/plan/apply/overlay journeys all green. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/picloud-cli/src/client.rs | 51 ++++++++--- crates/picloud-cli/src/cmds/apply.rs | 26 +++--- crates/picloud-cli/src/cmds/config.rs | 8 +- crates/picloud-cli/src/cmds/init.rs | 5 +- crates/picloud-cli/src/cmds/plan.rs | 15 ++-- crates/picloud-cli/src/cmds/pull.rs | 7 +- crates/picloud-cli/src/manifest.rs | 122 ++++++++++++++++++++++---- 7 files changed, 183 insertions(+), 51 deletions(-) diff --git a/crates/picloud-cli/src/client.rs b/crates/picloud-cli/src/client.rs index 2b610ea..f20afcd 100644 --- a/crates/picloud-cli/src/client.rs +++ b/crates/picloud-cli/src/client.rs @@ -1168,33 +1168,47 @@ impl Client { /// `POST /api/v1/admin/apps/{id_or_slug}/plan` — diff a desired-state /// bundle against the app's live state. Read-only. - pub async fn plan(&self, app: &str, bundle: &serde_json::Value) -> Result { - let app = seg(app); + /// `POST /api/v1/admin/{apps|groups}/{id_or_slug}/plan` — diff a bundle + /// against an app OR group node (Phase 5). + pub async fn plan_node( + &self, + kind: NodeKind, + slug: &str, + bundle: &serde_json::Value, + ) -> Result { + let slug = seg(slug); let resp = self - .request(Method::POST, &format!("/api/v1/admin/apps/{app}/plan")) + .request( + Method::POST, + &format!("/api/v1/admin/{}/{slug}/plan", kind.path()), + ) .json(bundle) .send() .await?; decode(resp).await } - /// `POST /api/v1/admin/apps/{id_or_slug}/apply` — reconcile the live - /// app to the bundle in one transaction. - pub async fn apply( + /// `POST /api/v1/admin/{apps|groups}/{id_or_slug}/apply` — reconcile an + /// app OR group node in one transaction (Phase 5). + pub async fn apply_node( &self, - app: &str, + kind: NodeKind, + slug: &str, bundle: &serde_json::Value, prune: bool, expected_token: Option<&str>, ) -> Result { - let app = seg(app); + let slug = seg(slug); let body = serde_json::json!({ "bundle": bundle, "prune": prune, "expected_token": expected_token, }); let resp = self - .request(Method::POST, &format!("/api/v1/admin/apps/{app}/apply")) + .request( + Method::POST, + &format!("/api/v1/admin/{}/{slug}/apply", kind.path()), + ) .json(&body) .send() .await?; @@ -1206,7 +1220,7 @@ impl Client { let body = resp.text().await.unwrap_or_default(); let msg = parse_error_body(&body).unwrap_or(body); return Err(anyhow!( - "{msg}\nThe app changed since your last `pic plan`. Re-run \ + "{msg}\nThe live state changed since your last `pic plan`. Re-run \ `pic plan` to review the new diff, then `pic apply` — or \ `pic apply --force` to apply without re-reviewing." )); @@ -1235,6 +1249,23 @@ pub async fn auth_login(url: &str, username: &str, password: &str) -> Result &'static str { + match self { + Self::App => "apps", + Self::Group => "groups", + } + } +} + // ---------- DTOs (CLI-local, wire-shape-matched) ---------- /// Response of `POST .../plan`: per-resource diffs grouped by kind. diff --git a/crates/picloud-cli/src/cmds/apply.rs b/crates/picloud-cli/src/cmds/apply.rs index 5974aec..c1940d1 100644 --- a/crates/picloud-cli/src/cmds/apply.rs +++ b/crates/picloud-cli/src/cmds/apply.rs @@ -8,7 +8,7 @@ use std::path::Path; use anyhow::{Context, Result}; -use crate::client::Client; +use crate::client::{Client, NodeKind}; use crate::cmds::plan::build_bundle; use crate::config; use crate::manifest::Manifest; @@ -28,36 +28,38 @@ pub async fn run( let manifest = Manifest::load_with_env(manifest_path, env)?; let base_dir = manifest_path.parent().unwrap_or_else(|| Path::new(".")); let bundle = build_bundle(&manifest, base_dir)?; + let kind = if manifest.is_group() { + NodeKind::Group + } else { + NodeKind::App + }; + let slug = manifest.slug().to_string(); if prune && !yes { - confirm_prune(&manifest.app.slug)?; + confirm_prune(&slug)?; } // Bound-plan check: replay the token from the last `pic plan` (for this - // app) so the server refuses if the app changed since it was reviewed. + // node) so the server refuses if it changed since it was reviewed. // `--force` skips it; no recorded plan means no check (apply still works // standalone). The token is single-use — cleared after a successful apply. let expected_token = if force { None } else { crate::linkstate::read_plan(base_dir) - .filter(|l| l.app == manifest.app.slug) + .filter(|l| l.app == slug) .map(|l| l.state_token) }; let report = client - .apply( - &manifest.app.slug, - &bundle, - prune, - expected_token.as_deref(), - ) + .apply_node(kind, &slug, &bundle, prune, expected_token.as_deref()) .await?; - crate::linkstate::clear_plan(base_dir, &manifest.app.slug); + crate::linkstate::clear_plan(base_dir, &slug); + let label = if manifest.is_group() { "group" } else { "app" }; let mut block = KvBlock::new(); block - .field("app", manifest.app.slug.clone()) + .field(label, slug.clone()) .field( "scripts", format!( diff --git a/crates/picloud-cli/src/cmds/config.rs b/crates/picloud-cli/src/cmds/config.rs index d92cc36..06fa73d 100644 --- a/crates/picloud-cli/src/cmds/config.rs +++ b/crates/picloud-cli/src/cmds/config.rs @@ -45,8 +45,14 @@ pub async fn run( // masked report targets the app those commands would act on — not the // base slug — when an overlay re-points slug/secrets. let manifest = Manifest::load_with_env(manifest_path, env)?; + if manifest.is_group() { + bail!( + "`pic config --effective` resolves an APP's inherited config; \ + a [group] manifest has no single effective view" + ); + } - let eff = client.config_effective(&manifest.app.slug).await?; + let eff = client.config_effective(manifest.slug()).await?; // --- vars: the resolved view, with winning owner + provenance. --- let mut vars_table = Table::new(["key", "value", "owner", "scope"]); diff --git a/crates/picloud-cli/src/cmds/init.rs b/crates/picloud-cli/src/cmds/init.rs index d2e85c0..dec46ca 100644 --- a/crates/picloud-cli/src/cmds/init.rs +++ b/crates/picloud-cli/src/cmds/init.rs @@ -109,11 +109,12 @@ pub fn run( /// The minimal working project: one `hello` endpoint bound to `GET /hello`. fn scaffold_manifest(slug: &str, name: &str) -> Manifest { Manifest { - app: ManifestApp { + app: Some(ManifestApp { slug: slug.to_string(), name: name.to_string(), description: None, - }, + }), + group: None, scripts: vec![ManifestScript { name: "hello".into(), file: "scripts/hello.rhai".into(), diff --git a/crates/picloud-cli/src/cmds/plan.rs b/crates/picloud-cli/src/cmds/plan.rs index 648445a..01d8806 100644 --- a/crates/picloud-cli/src/cmds/plan.rs +++ b/crates/picloud-cli/src/cmds/plan.rs @@ -9,7 +9,7 @@ use anyhow::{Context, Result}; use serde::Serialize; use serde_json::{json, Map, Value}; -use crate::client::{ChangeDto, Client, PlanDto}; +use crate::client::{ChangeDto, Client, NodeKind, PlanDto}; use crate::config; use crate::manifest::Manifest; use crate::output::{OutputMode, Table}; @@ -21,15 +21,18 @@ pub async fn run(manifest_path: &Path, env: Option<&str>, mode: OutputMode) -> R let manifest = Manifest::load_with_env(manifest_path, env)?; let base_dir = manifest_path.parent().unwrap_or_else(|| Path::new(".")); let bundle = build_bundle(&manifest, base_dir)?; + let kind = if manifest.is_group() { + NodeKind::Group + } else { + NodeKind::App + }; - let plan = client.plan(&manifest.app.slug, &bundle).await?; + let plan = client.plan_node(kind, manifest.slug(), &bundle).await?; // Record the bound-plan token so a subsequent `pic apply` can detect the - // app changing underneath the reviewed plan (best-effort — a read-only + // node changing underneath the reviewed plan (best-effort — a read-only // plan still succeeds if the project dir isn't writable). if !plan.state_token.is_empty() { - if let Err(e) = - crate::linkstate::write_plan(base_dir, &manifest.app.slug, &plan.state_token) - { + if let Err(e) = crate::linkstate::write_plan(base_dir, manifest.slug(), &plan.state_token) { eprintln!("warning: could not record plan state for `pic apply`: {e}"); } } diff --git a/crates/picloud-cli/src/cmds/pull.rs b/crates/picloud-cli/src/cmds/pull.rs index bf2124f..d8a7320 100644 --- a/crates/picloud-cli/src/cmds/pull.rs +++ b/crates/picloud-cli/src/cmds/pull.rs @@ -211,11 +211,12 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> } let manifest = Manifest { - app: ManifestApp { + app: Some(ManifestApp { slug: app.app.slug.clone(), name: app.app.name.clone(), description: app.app.description.clone(), - }, + }), + group: None, scripts: manifest_scripts, routes, triggers: manifest_triggers, @@ -231,7 +232,7 @@ pub async fn run(app_ident: &str, dir: &Path, force: bool, mode: OutputMode) -> let mut block = KvBlock::new(); block .field("manifest", manifest_path.display().to_string()) - .field("app", manifest.app.slug.clone()) + .field("app", manifest.slug().to_string()) .field("scripts", manifest.scripts.len().to_string()) .field("routes", manifest.routes.len().to_string()) .field("triggers", trigger_count(&manifest.triggers).to_string()) diff --git a/crates/picloud-cli/src/manifest.rs b/crates/picloud-cli/src/manifest.rs index f80d7f1..84cd2a9 100644 --- a/crates/picloud-cli/src/manifest.rs +++ b/crates/picloud-cli/src/manifest.rs @@ -30,7 +30,12 @@ pub const MANIFEST_FILE: &str = "picloud.toml"; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Manifest { - pub app: ManifestApp, + /// An app node declares `[app]`; a group node declares `[group]` (Phase 5). + /// Exactly one is present (enforced by [`Manifest::parse`]). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub app: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub group: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub scripts: Vec, #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -47,9 +52,51 @@ pub struct Manifest { } impl Manifest { - /// Parse a manifest from TOML text. + /// Parse a manifest from TOML text. Enforces the app-XOR-group invariant. pub fn parse(text: &str) -> Result { - toml::from_str(text).context("parsing manifest TOML") + let m: Self = toml::from_str(text).context("parsing manifest TOML")?; + match (&m.app, &m.group) { + (Some(_), None) | (None, Some(_)) => {} + (Some(_), Some(_)) => { + anyhow::bail!( + "manifest declares both [app] and [group]; a node is one or the other" + ) + } + (None, None) => { + anyhow::bail!("manifest declares neither [app] nor [group]") + } + } + // A group node owns only scripts + vars (+ secret names) — routes and + // triggers are app concerns. Reject them early with a clear message. + if m.group.is_some() { + if !m.routes.is_empty() { + anyhow::bail!( + "a [group] manifest cannot declare [[routes]] — routes bind to an app" + ); + } + if !m.triggers.is_empty() { + anyhow::bail!( + "a [group] manifest cannot declare [[triggers]] — triggers belong to an app" + ); + } + } + Ok(m) + } + + /// This node's slug (app or group). + #[must_use] + pub fn slug(&self) -> &str { + match (&self.app, &self.group) { + (Some(a), _) => &a.slug, + (_, Some(g)) => &g.slug, + _ => "", + } + } + + /// True iff this manifest declares a `[group]` node (Phase 5). + #[must_use] + pub fn is_group(&self) -> bool { + self.group.is_some() } /// Load and parse the manifest at `path`. @@ -90,11 +137,13 @@ impl Manifest { /// Merge a sparse overlay onto this manifest: overlay `app.slug`/`name` /// replace the base's; overlay secret names union into the base set. fn apply_overlay(&mut self, overlay: ManifestOverlay) { - if let Some(slug) = overlay.app.slug { - self.app.slug = slug; - } - if let Some(name) = overlay.app.name { - self.app.name = name; + if let Some(app) = &mut self.app { + if let Some(slug) = overlay.app.slug { + app.slug = slug; + } + if let Some(name) = overlay.app.name { + app.name = name; + } } for n in overlay.secrets.names { if !self.secrets.names.contains(&n) { @@ -156,6 +205,18 @@ pub struct ManifestApp { pub description: Option, } +/// A `[group]` node (Phase 5): a group's own declarative content — its scripts +/// and `[vars]`. The group must already exist on the server (created with +/// `pic groups create`); the manifest reconciles its content, not the tree +/// shape. In a nested project the parent is inferred from the directory tree. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ManifestGroup { + pub slug: String, + pub name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub description: Option, +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ManifestScript { pub name: String, @@ -361,11 +422,12 @@ mod tests { fn sample() -> Manifest { Manifest { - app: ManifestApp { + app: Some(ManifestApp { slug: "blog".into(), name: "My Blog".into(), description: Some("demo".into()), - }, + }), + group: None, scripts: vec![ ManifestScript { name: "create-post".into(), @@ -471,11 +533,9 @@ mod tests { ) .unwrap(); m.apply_overlay(overlay); - assert_eq!(m.app.slug, "blog-staging", "overlay slug wins"); - assert_eq!( - m.app.name, "My Blog", - "base name kept when overlay omits it" - ); + let app = m.app.as_ref().unwrap(); + assert_eq!(app.slug, "blog-staging", "overlay slug wins"); + assert_eq!(app.name, "My Blog", "base name kept when overlay omits it"); assert_eq!( m.secrets.names, vec!["STRIPE_KEY".to_string(), "STAGING_ONLY".to_string()], @@ -520,11 +580,12 @@ mod tests { #[test] fn empty_optional_sections_omitted() { let m = Manifest { - app: ManifestApp { + app: Some(ManifestApp { slug: "x".into(), name: "X".into(), description: None, - }, + }), + group: None, scripts: vec![], routes: vec![], triggers: ManifestTriggers::default(), @@ -540,6 +601,33 @@ mod tests { assert_eq!(m, Manifest::parse(&text).unwrap()); } + #[test] + fn group_manifest_parses_and_rejects_app_only_blocks() { + // A [group] node: scripts + vars, no [app]. + let m = Manifest::parse( + "[group]\nslug = \"acme\"\nname = \"ACME\"\n\n\ + [[scripts]]\nname = \"shared\"\nfile = \"scripts/shared.rhai\"\n\n\ + [vars]\nregion = \"eu\"\n", + ) + .expect("group manifest parses"); + assert!(m.is_group()); + assert_eq!(m.slug(), "acme"); + assert_eq!(m.scripts.len(), 1); + + // A group cannot carry routes/triggers. + let err = Manifest::parse( + "[group]\nslug = \"acme\"\nname = \"ACME\"\n\n\ + [[routes]]\nscript = \"shared\"\nhost_kind = \"any\"\npath_kind = \"exact\"\npath = \"/x\"\n", + ) + .expect_err("group with routes is rejected"); + assert!(err.to_string().contains("routes"), "got: {err}"); + + // Neither / both is rejected. + Manifest::parse("[vars]\nx = 1\n").expect_err("no [app] or [group]"); + Manifest::parse("[app]\nslug=\"a\"\nname=\"A\"\n[group]\nslug=\"g\"\nname=\"G\"\n") + .expect_err("both [app] and [group]"); + } + #[test] fn overlay_vars_override_base_per_key() { let mut base = sample();