feat(cli): kind-aware shared-collection reconcile + string-or-table manifest (§11.6 docs C4)

Make the declarative collection surface carry a `kind` so docs (and future
kinds) are declarable. Back-compatible: the shipped `collections = ["catalog"]`
form still means kv.

- manifest: `collections` entries are now `CollectionDecl` — an untagged enum of
  a bare string (kv shorthand) or a `{ name, kind }` table (kind ∈ kv/docs).
  `Manifest::collections()` normalizes to `(name, kind)` pairs; build_bundle
  emits `[{name, kind}]`. Still `[group]`-only. Unit test covers the
  string-or-table mix + app rejection.
- apply: `Bundle.collections: Vec<CollectionSpec>` ({name, kind=default "kv"});
  `CurrentState.collections: Vec<(name,kind)>` via list_all_for_owner;
  `diff_collections` keys each change by name with `detail = kind` and identity
  `(LOWER(name), kind)` (so a kv and a docs collection of the same name are
  distinct); reconcile reads the kind from `detail`; state_token folds
  `coll|{kind}|{name}`; validate_bundle rejects unknown kinds + dups by
  (name,kind); `collection_report`/CollectionInfo + `pic collections ls` gain a
  kind column.

Existing kv journeys + nearest-wins still green (the bare-string form normalizes
to kind=kv end to end).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-30 19:04:43 +02:00
parent 78a468de83
commit 5efb068b9f
5 changed files with 181 additions and 63 deletions

View File

@@ -88,14 +88,30 @@ pub struct Bundle {
/// optional default body is a co-located `kind = module` script. /// optional default body is a co-located `kind = module` script.
#[serde(default)] #[serde(default)]
pub extension_points: Vec<String>, pub extension_points: Vec<String>,
/// Declared shared group-collection *names* (§11.6) — KV collections this /// Declared shared group collections (§11.6) — `(name, kind)` markers this
/// node offers as cross-app-shared. Name-only, like `extension_points`; the /// node offers as cross-app-shared (`kind` ∈ `kv`/`docs`). The data lives in
/// data lives in `group_kv_entries`. Authored on `[group]` nodes only (the /// the per-kind store (`group_kv_entries` / `group_docs`). Authored on
/// CLI rejects it on `[app]`); the apply engine stays owner-generic. /// `[group]` nodes only (the CLI rejects it on `[app]`).
#[serde(default)] #[serde(default)]
pub collections: Vec<String>, pub collections: Vec<CollectionSpec>,
} }
/// One declared shared-collection marker on the wire: a name + its store kind.
#[derive(Debug, Clone, Deserialize)]
pub struct CollectionSpec {
pub name: String,
/// Storage kind; defaults to `kv` for back-compat with the name-only form.
#[serde(default = "default_collection_kind")]
pub kind: String,
}
fn default_collection_kind() -> String {
"kv".to_string()
}
/// The shared-collection kinds the apply engine accepts.
const COLLECTION_KINDS: &[&str] = &["kv", "docs"];
#[derive(Debug, Clone, Deserialize)] #[derive(Debug, Clone, Deserialize)]
pub struct BundleScript { pub struct BundleScript {
pub name: String, pub name: String,
@@ -422,8 +438,9 @@ pub struct CurrentState {
pub vars: Vec<(String, serde_json::Value)>, pub vars: Vec<(String, serde_json::Value)>,
/// Extension-point marker names declared directly at this node (§5.5). /// Extension-point marker names declared directly at this node (§5.5).
pub extension_point_names: Vec<String>, pub extension_point_names: Vec<String>,
/// Shared group-collection names declared directly at this node (§11.6). /// Shared group collections declared directly at this node (§11.6), as
pub collection_names: Vec<String>, /// `(name, kind)` pairs.
pub collections: Vec<(String, String)>,
} }
/// One row of the read-only extension-point report (§5.5). /// One row of the read-only extension-point report (§5.5).
@@ -441,6 +458,7 @@ pub struct ExtensionPointInfo {
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
pub struct CollectionInfo { pub struct CollectionInfo {
pub name: String, pub name: String,
pub kind: String,
} }
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------
@@ -846,11 +864,12 @@ impl ApplyService {
// (that dies only with the owning group). // (that dies only with the owning group).
for ch in &plan.collections { for ch in &plan.collections {
if ch.op == Op::Create { if ch.op == Op::Create {
let kind = ch.detail.as_deref().unwrap_or("kv");
crate::group_collection_repo::insert_collection_tx( crate::group_collection_repo::insert_collection_tx(
&mut *tx, &mut *tx,
owner.as_script_owner(), owner.as_script_owner(),
&ch.key, &ch.key,
"kv", kind,
) )
.await .await
.map_err(|e| ApplyError::Backend(e.to_string()))?; .map_err(|e| ApplyError::Backend(e.to_string()))?;
@@ -953,11 +972,12 @@ impl ApplyService {
// owning group is deleted. // owning group is deleted.
for ch in &plan.collections { for ch in &plan.collections {
if ch.op == Op::Delete { if ch.op == Op::Delete {
let kind = ch.detail.as_deref().unwrap_or("kv");
crate::group_collection_repo::delete_collection_tx( crate::group_collection_repo::delete_collection_tx(
&mut *tx, &mut *tx,
owner.as_script_owner(), owner.as_script_owner(),
&ch.key, &ch.key,
"kv", kind,
) )
.await .await
.map_err(|e| ApplyError::Backend(e.to_string()))?; .map_err(|e| ApplyError::Backend(e.to_string()))?;
@@ -1793,21 +1813,21 @@ impl ApplyService {
} }
} }
/// Read-only §11.6 shared-collection report for a node: the shared KV /// Read-only §11.6 shared-collection report for a node: the shared
/// collection names declared directly at it. Group-only in practice (the /// collections (`name` + `kind`) declared directly at it. Group-only in
/// CLI rejects app-declared collections); an app node returns its own /// practice (the CLI rejects app-declared collections). Backs
/// (degenerate) declarations, if any. Backs `pic collections ls`. /// `pic collections ls`.
pub async fn collection_report( pub async fn collection_report(
&self, &self,
owner: ApplyOwner, owner: ApplyOwner,
) -> Result<Vec<CollectionInfo>, ApplyError> { ) -> Result<Vec<CollectionInfo>, ApplyError> {
let names = let rows =
crate::group_collection_repo::list_for_owner(&self.pool, owner.as_script_owner(), "kv") crate::group_collection_repo::list_all_for_owner(&self.pool, owner.as_script_owner())
.await .await
.map_err(|e| ApplyError::Backend(e.to_string()))?; .map_err(|e| ApplyError::Backend(e.to_string()))?;
Ok(names Ok(rows
.into_iter() .into_iter()
.map(|name| CollectionInfo { name }) .map(|(name, kind)| CollectionInfo { name, kind })
.collect()) .collect())
} }
@@ -2008,19 +2028,26 @@ impl ApplyService {
} }
} }
// Shared group collections (§11.6): unique names. No reserved-name // Shared group collections (§11.6): a known kind + unique (name, kind).
// guard — a collection name is data namespace, not an importable // No reserved-name guard — a collection name is a data namespace, not an
// module, so it can't shadow an SDK namespace. // importable module, so it can't shadow an SDK namespace.
let mut collection_names: HashSet<String> = HashSet::new(); let mut seen_collections: HashSet<(String, &str)> = HashSet::new();
for name in &bundle.collections { for c in &bundle.collections {
if name.is_empty() { if c.name.is_empty() {
return Err(ApplyError::Invalid( return Err(ApplyError::Invalid(
"shared collection name must not be empty".into(), "shared collection name must not be empty".into(),
)); ));
} }
if !collection_names.insert(name.to_lowercase()) { if !COLLECTION_KINDS.contains(&c.kind.as_str()) {
return Err(ApplyError::Invalid(format!( return Err(ApplyError::Invalid(format!(
"duplicate shared collection `{name}`" "shared collection `{}`: unknown kind `{}` (want one of {COLLECTION_KINDS:?})",
c.name, c.kind
)));
}
if !seen_collections.insert((c.name.to_lowercase(), c.kind.as_str())) {
return Err(ApplyError::Invalid(format!(
"duplicate shared collection `{}` (kind `{}`)",
c.name, c.kind
))); )));
} }
} }
@@ -2076,9 +2103,10 @@ impl ApplyService {
crate::extension_point_repo::list_for_owner(&self.pool, owner.as_script_owner()) crate::extension_point_repo::list_for_owner(&self.pool, owner.as_script_owner())
.await .await
.map_err(|e| ApplyError::Backend(e.to_string()))?; .map_err(|e| ApplyError::Backend(e.to_string()))?;
// Shared group-collection markers declared directly at this node (§11.6). // Shared group-collection markers declared directly at this node
let collection_names = // (§11.6), all kinds.
crate::group_collection_repo::list_for_owner(&self.pool, owner.as_script_owner(), "kv") let collections =
crate::group_collection_repo::list_all_for_owner(&self.pool, owner.as_script_owner())
.await .await
.map_err(|e| ApplyError::Backend(e.to_string()))?; .map_err(|e| ApplyError::Backend(e.to_string()))?;
Ok(CurrentState { Ok(CurrentState {
@@ -2088,7 +2116,7 @@ impl ApplyService {
secret_names, secret_names,
vars, vars,
extension_point_names, extension_point_names,
collection_names, collections,
}) })
} }
@@ -2669,36 +2697,42 @@ fn diff_extension_points(current: &CurrentState, bundle: &Bundle) -> Vec<Resourc
out out
} }
/// Diff shared group-collection markers by name (§11.6). Same shape as /// Diff shared group-collection markers by `(name, kind)` (§11.6). A
/// `diff_extension_points`: a declared-but-absent name is a Create, a /// declared-but-absent marker is a Create, a live-but-undeclared one is a
/// live-but-undeclared name is a Delete (pruned under `--prune`). Pruning a /// Delete (pruned under `--prune`). Each change carries `key = name`,
/// marker hides the store but never drops its `group_kv_entries` data. /// `detail = Some(kind)` so the reconcile knows which store to touch and
/// `pic plan` renders the kind. Identity is `(LOWER(name), kind)`, so the same
/// name as both a `kv` and a `docs` collection are distinct markers.
fn diff_collections(current: &CurrentState, bundle: &Bundle) -> Vec<ResourceChange> { fn diff_collections(current: &CurrentState, bundle: &Bundle) -> Vec<ResourceChange> {
let live: HashSet<&str> = current let live: HashSet<(String, &str)> = current
.collection_names .collections
.iter() .iter()
.map(String::as_str) .map(|(n, k)| (n.to_lowercase(), k.as_str()))
.collect();
let declared: HashSet<(String, &str)> = bundle
.collections
.iter()
.map(|c| (c.name.to_lowercase(), c.kind.as_str()))
.collect(); .collect();
let declared: HashSet<&str> = bundle.collections.iter().map(String::as_str).collect();
let mut out = Vec::new(); let mut out = Vec::new();
for name in &bundle.collections { for c in &bundle.collections {
out.push(ResourceChange { out.push(ResourceChange {
op: if live.contains(name.as_str()) { op: if live.contains(&(c.name.to_lowercase(), c.kind.as_str())) {
Op::NoOp Op::NoOp
} else { } else {
Op::Create Op::Create
}, },
key: name.clone(), key: c.name.clone(),
detail: None, detail: Some(c.kind.clone()),
}); });
} }
for name in &current.collection_names { for (name, kind) in &current.collections {
if !declared.contains(name.as_str()) { if !declared.contains(&(name.to_lowercase(), kind.as_str())) {
out.push(ResourceChange { out.push(ResourceChange {
op: Op::Delete, op: Op::Delete,
key: name.clone(), key: name.clone(),
detail: Some("on server, not declared".into()), detail: Some(kind.clone()),
}); });
} }
} }
@@ -3170,7 +3204,7 @@ fn state_token_with_names(
+ current.secret_names.len() + current.secret_names.len()
+ current.vars.len() + current.vars.len()
+ current.extension_point_names.len() + current.extension_point_names.len()
+ current.collection_names.len(), + current.collections.len(),
); );
for s in &current.scripts { for s in &current.scripts {
parts.push(format!( parts.push(format!(
@@ -3229,9 +3263,10 @@ fn state_token_with_names(
for n in &current.extension_point_names { for n in &current.extension_point_names {
parts.push(format!("ep|{n}")); parts.push(format!("ep|{n}"));
} }
// Shared group-collection markers are name-only too (§11.6). // Shared group-collection markers carry a kind (§11.6), so a kind change
for n in &current.collection_names { // moves the token.
parts.push(format!("coll|{n}")); for (name, kind) in &current.collections {
parts.push(format!("coll|{kind}|{name}"));
} }
// Order-independent: sort the per-resource tokens before hashing. // Order-independent: sort the per-resource tokens before hashing.
parts.sort_unstable(); parts.sort_unstable();

View File

@@ -1352,6 +1352,8 @@ impl Client {
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct CollectionInfoDto { pub struct CollectionInfoDto {
pub name: String, pub name: String,
#[serde(default)]
pub kind: String,
} }
// ---------- DTOs (CLI-local, wire-shape-matched) ---------- // ---------- DTOs (CLI-local, wire-shape-matched) ----------

View File

@@ -14,9 +14,9 @@ pub async fn ls(group: &str, mode: OutputMode) -> Result<()> {
let client = Client::from_creds(&creds)?; let client = Client::from_creds(&creds)?;
let items = client.collections_list(group).await?; let items = client.collections_list(group).await?;
let mut table = Table::new(["name"]); let mut table = Table::new(["name", "kind"]);
for c in &items { for c in &items {
table.row([c.name.clone()]); table.row([c.name.clone(), c.kind.clone()]);
} }
table.print(mode); table.print(mode);
Ok(()) Ok(())

View File

@@ -163,7 +163,11 @@ pub fn build_bundle(manifest: &Manifest, base_dir: &Path) -> Result<Value> {
"secrets": manifest.secrets.names, "secrets": manifest.secrets.names,
"vars": vars, "vars": vars,
"extension_points": manifest.extension_points(), "extension_points": manifest.extension_points(),
"collections": manifest.collections(), "collections": manifest
.collections()
.into_iter()
.map(|(name, kind)| json!({ "name": name, "kind": kind }))
.collect::<Vec<_>>(),
})) }))
} }

View File

@@ -111,14 +111,19 @@ impl Manifest {
} }
} }
/// This node's declared shared group-collection names (§11.6). Authored on /// This node's declared shared group collections (§11.6), normalized to
/// `[group]` nodes only — an `[app]` manifest carrying `collections` is a /// `(name, kind)` pairs. Authored on `[group]` nodes only — an `[app]`
/// hard parse error (`ManifestApp` has no such field + `deny_unknown_fields`). /// manifest carrying `collections` is a hard parse error (`ManifestApp` has
/// no such field + `deny_unknown_fields`).
#[must_use] #[must_use]
pub fn collections(&self) -> &[String] { pub fn collections(&self) -> Vec<(String, String)> {
match &self.group { match &self.group {
Some(g) => &g.collections, Some(g) => g
None => &[], .collections
.iter()
.map(CollectionDecl::normalized)
.collect(),
None => Vec::new(),
} }
} }
@@ -249,13 +254,60 @@ pub struct ManifestGroup {
/// See [`ManifestApp::extension_points`]. A key of the `[group]` table. /// See [`ManifestApp::extension_points`]. A key of the `[group]` table.
#[serde(default, skip_serializing_if = "Vec::is_empty")] #[serde(default, skip_serializing_if = "Vec::is_empty")]
pub extension_points: Vec<String>, pub extension_points: Vec<String>,
/// `collections = ["catalog", …]` (§11.6) — shared KV collection names this /// `collections = [...]` (§11.6) — shared collections this group offers as
/// group offers as cross-app-shared (read by any app in the subtree, /// cross-app-shared (read by any app in the subtree, written by an
/// written by an authenticated editor+). Group-only: there is no /// authenticated editor+). Each entry is either a bare string (a `kv`
/// `collections` key on `[app]`. The data lives in `group_kv_entries`; /// collection) or a `{ name, kind }` table (`kind` ∈ `kv`/`docs`).
/// pruning a name removes the declaration, not the data. /// Group-only: there is no `collections` key on `[app]`.
#[serde(default, skip_serializing_if = "Vec::is_empty")] #[serde(default, skip_serializing_if = "Vec::is_empty")]
pub collections: Vec<String>, pub collections: Vec<CollectionDecl>,
}
/// The store kind of a shared collection (§11.6).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
pub enum CollectionKind {
Kv,
Docs,
}
impl CollectionKind {
#[must_use]
pub fn as_str(self) -> &'static str {
match self {
Self::Kv => "kv",
Self::Docs => "docs",
}
}
}
/// One `collections` entry: a bare string (`kv` shorthand) or a `{ name, kind }`
/// table. Untagged so the shipped `collections = ["catalog"]` form keeps working
/// alongside `{ name = "articles", kind = "docs" }`.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum CollectionDecl {
Name(String),
Full {
name: String,
#[serde(default = "kv_kind")]
kind: CollectionKind,
},
}
fn kv_kind() -> CollectionKind {
CollectionKind::Kv
}
impl CollectionDecl {
/// `(name, kind-as-string)` — a bare string normalizes to `kind = "kv"`.
#[must_use]
pub fn normalized(&self) -> (String, String) {
match self {
Self::Name(n) => (n.clone(), "kv".to_string()),
Self::Full { name, kind } => (name.clone(), kind.as_str().to_string()),
}
}
} }
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -668,6 +720,31 @@ mod tests {
assert_eq!(m.extension_points(), ["theme"]); assert_eq!(m.extension_points(), ["theme"]);
} }
#[test]
fn collections_string_or_table_normalizes_kind() {
// §11.6: a bare string is a `kv` collection (the shipped form); a
// `{ name, kind }` table sets an explicit kind. Both coexist.
let m = Manifest::parse(
"[group]\nslug = \"acme\"\nname = \"ACME\"\n\n\
collections = [\"catalog\", { name = \"articles\", kind = \"docs\" }]\n",
)
.expect("string-or-table collections must parse");
assert_eq!(
m.collections(),
vec![
("catalog".to_string(), "kv".to_string()),
("articles".to_string(), "docs".to_string()),
]
);
// An app manifest carrying `collections` is rejected (no such field +
// deny_unknown_fields).
let app = "[app]\nslug = \"x\"\nname = \"X\"\ncollections = [\"catalog\"]\n";
assert!(
Manifest::parse(app).is_err(),
"collections on [app] must be rejected"
);
}
#[test] #[test]
fn group_manifest_parses_and_rejects_app_only_blocks() { fn group_manifest_parses_and_rejects_app_only_blocks() {
// A [group] node: scripts + vars, no [app]. // A [group] node: scripts + vars, no [app].