From b8eced9d91dc4b925692592c04625fd508007aba Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Tue, 7 Jul 2026 20:23:10 +0200 Subject: [PATCH] =?UTF-8?q?test/docs(apply):=20=C2=A76=20M2=20structural-d?= =?UTF-8?q?ivergence=20journey=20+=20design=20note?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/structural_divergence.rs`: a repo that nests a group under a different parent than the server → `pic plan` shows it `diverged`; a bare apply is refused (422); `--force-local-structure` reparents to the manifest shape; `--adopt-server-structure` keeps the server placement. Design doc §6 records M2 shipped, leaving only per-env approval gating deferred. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/picloud-cli/tests/cli.rs | 1 + .../tests/structural_divergence.rs | 167 ++++++++++++++++++ docs/design/groups-and-project-tool.md | 18 +- 3 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 crates/picloud-cli/tests/structural_divergence.rs diff --git a/crates/picloud-cli/tests/cli.rs b/crates/picloud-cli/tests/cli.rs index 5e55536..30234cf 100644 --- a/crates/picloud-cli/tests/cli.rs +++ b/crates/picloud-cli/tests/cli.rs @@ -44,6 +44,7 @@ mod roles; mod routes; mod scripts; mod sealed; +mod structural_divergence; mod secrets; mod shared_queues; mod shared_topics; diff --git a/crates/picloud-cli/tests/structural_divergence.rs b/crates/picloud-cli/tests/structural_divergence.rs new file mode 100644 index 0000000..01248cd --- /dev/null +++ b/crates/picloud-cli/tests/structural_divergence.rs @@ -0,0 +1,167 @@ +//! §6 M2 — structural-divergence detection + declarative reparent. +//! +//! Once the manifest declares a group's parent (by directory nesting), a +//! `pic apply --dir` compares it to the server's actual parent. A divergence is +//! REFUSED by default (Terraform detect-and-refuse); `--force-local-structure` +//! reparents the group to match the manifest, `--adopt-server-structure` keeps +//! the server's placement and reconciles content in place. `pic plan` previews +//! the divergence first. + +use std::fs; +use std::path::Path; + +use tempfile::TempDir; + +use crate::common; +use crate::common::cleanup::GroupGuard; + +/// A repo that declares `mover` NESTED under `alt` (so its manifest parent is +/// `alt`), with `alt` at the attach point `base`. +fn diverging_repo(dir: &Path, project: &str, base: &str, alt: &str, mover: &str) { + fs::write( + dir.join("picloud.toml"), + format!( + "[project]\nslug = \"{project}\"\nparent_group = \"{base}\"\n\n\ + [group]\nslug = \"{alt}\"\nname = \"Alt\"\n" + ), + ) + .unwrap(); + let sub = dir.join("mover"); + fs::create_dir_all(&sub).unwrap(); + fs::write( + sub.join("picloud.toml"), + format!("[group]\nslug = \"{mover}\"\nname = \"Mover\"\n"), + ) + .unwrap(); +} + +fn server_parent(env: &common::TestEnv, group: &str) -> String { + let ls = common::pic_as(env) + .args(["groups", "ls"]) + .output() + .expect("groups ls"); + let table = String::from_utf8(ls.stdout).unwrap(); + table + .lines() + .map(common::cells) + .find(|c| c.first() == Some(&group)) + .and_then(|c| c.get(2).map(|s| (*s).to_string())) + .unwrap_or_else(|| panic!("group `{group}` not in groups ls:\n{table}")) +} + +/// Pre-create base → {alt, mover} (both directly under base), so a repo that +/// nests `mover` under `alt` diverges. +fn seed(env: &common::TestEnv, base: &str, alt: &str, mover: &str) { + common::pic_as(env) + .args(["groups", "create", base]) + .assert() + .success(); + common::pic_as(env) + .args(["groups", "create", alt, "--parent", base]) + .assert() + .success(); + common::pic_as(env) + .args(["groups", "create", mover, "--parent", base]) + .assert() + .success(); +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn divergence_refused_by_default_then_force_local_reparents() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let base = common::unique_slug("sd-base"); + let alt = common::unique_slug("sd-alt"); + let mover = common::unique_slug("sd-mover"); + let project = common::unique_slug("sd-proj"); + let _b = GroupGuard::new(&env.url, &env.token, &base); + let _a = GroupGuard::new(&env.url, &env.token, &alt); + let _m = GroupGuard::new(&env.url, &env.token, &mover); + seed(&env, &base, &alt, &mover); + + let dir = TempDir::new().unwrap(); + diverging_repo(dir.path(), &project, &base, &alt, &mover); + + // Plan previews the divergence (server parent `base`, manifest parent `alt`). + let plan = String::from_utf8( + common::pic_as(&env) + .args(["plan", "--dir"]) + .arg(dir.path()) + .output() + .unwrap() + .stdout, + ) + .unwrap(); + assert!( + plan.contains("diverged") && plan.contains(&base) && plan.contains(&alt), + "plan should show `mover` diverged (server={base}, manifest={alt}):\n{plan}" + ); + + // A bare apply is REFUSED on the divergence (422). + let out = common::pic_as(&env) + .args(["apply", "--dir"]) + .arg(dir.path()) + .output() + .expect("apply"); + assert!(!out.status.success(), "a divergence must refuse by default"); + let err = String::from_utf8_lossy(&out.stderr).to_lowercase(); + assert!( + err.contains("structural") || err.contains("force-local") || err.contains("different parent"), + "the refusal must name the resolution flags:\n{err}" + ); + assert_eq!( + server_parent(&env, &mover), + base, + "a refused apply must not move the group" + ); + + // `--force-local-structure` reparents `mover` to `alt` (the manifest shape). + common::pic_as(&env) + .args(["apply", "--dir"]) + .arg(dir.path()) + .arg("--force-local-structure") + .assert() + .success(); + assert_eq!( + server_parent(&env, &mover), + alt, + "--force-local-structure must reparent to the manifest's parent" + ); +} + +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[test] +fn adopt_server_structure_keeps_the_server_placement() { + let Some(fx) = common::fixture_or_skip() else { + return; + }; + let env = common::admin_env(fx); + let base = common::unique_slug("sda-base"); + let alt = common::unique_slug("sda-alt"); + let mover = common::unique_slug("sda-mover"); + let project = common::unique_slug("sda-proj"); + let _b = GroupGuard::new(&env.url, &env.token, &base); + let _a = GroupGuard::new(&env.url, &env.token, &alt); + let _m = GroupGuard::new(&env.url, &env.token, &mover); + seed(&env, &base, &alt, &mover); + + let dir = TempDir::new().unwrap(); + diverging_repo(dir.path(), &project, &base, &alt, &mover); + + // `--adopt-server-structure` proceeds without reparenting — `mover` stays + // under `base` (its server placement), and content reconciles in place. + common::pic_as(&env) + .args(["apply", "--dir"]) + .arg(dir.path()) + .arg("--adopt-server-structure") + .assert() + .success(); + assert_eq!( + server_parent(&env, &mover), + base, + "--adopt-server-structure must keep the server's parent" + ); +} diff --git a/docs/design/groups-and-project-tool.md b/docs/design/groups-and-project-tool.md index a047bdb..76c56d3 100644 --- a/docs/design/groups-and-project-tool.md +++ b/docs/design/groups-and-project-tool.md @@ -810,8 +810,22 @@ caller-supplied `resolved_ids` map (existing + just-created) and returns a `to_c previews (empty current → a full-create plan; ownership → `claim`) — a to-create group's token part is identical to its post-create part, so plan/apply tokens match across a create. `authz_tree` skips a to-create group (its authorization is the service-side create gate). Reparent of an EXISTING diverged group + the detect-and-refuse -flags are M2. Pinned by `tests/group_create.rs`. **Still deferred:** §6 structural-divergence detection + M2 -reparent; per-env approval gating. +flags are M2. Pinned by `tests/group_create.rs`. + +**§6 group-tree M2 shipped — structural-divergence detection + declarative reparent.** With the declared parent now +on the wire, `apply_tree` compares each EXISTING group node's server parent against the manifest's (directory- +nesting) parent. A divergence is resolved by a request `StructureMode` (default `Refuse`, so a pre-M2 CLI never +reshapes the tree): `Refuse` → `ApplyError::StructuralDivergence` (422, naming the resolution flags); `ForceLocal` +→ reparent to the declared parent IN the apply tx via the extracted `group_repo::reparent_group_tx` (cycle guard + +`structure_version` bump), §5.6-gated (`GroupAdmin` on the node + source + destination) and attach-ceilinged; +`AdoptServer` → keep the server shape, reconcile content in place. M1's `create_missing_groups_tx` generalized to +`reconcile_group_structure_tx` (create + reparent share the coarse-lock / attach / RBAC path; both are recorded as +`structurally_changed` so the pool-based attach re-check skips their uncommitted rows). `pic plan` previews the +divergence (`divergence_preview` → a `structure` row: `diverged`, server + manifest parents). CLI: +`--force-local-structure` / `--adopt-server-structure` (mutually exclusive) → the `structure_mode` wire field; the +422 surfaces verbatim. A concurrent `pic groups reparent` between plan and apply still trips `StateMoved` (the tree +token folds `structure_version`). Pinned by `tests/structural_divergence.rs`. **Still deferred:** per-env approval +gating (`[project.environments]`). ---