From 2caa0d7f159c3c0d9c483bd63b2579d33f1986ff Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Fri, 31 Jul 2026 01:03:43 +0200 Subject: [PATCH 1/3] Add experimental read-only plan inspection --- README.md | 46 + docs/vrs/spec.md | 17 + examples/plans/agent.kdl | 13 + examples/plans/review-follow-up.md | 3 + examples/plans/ship-remote-approvals/plan.kdl | 9 + .../ship-remote-approvals/versions/0000.md | 3 + .../ship-remote-approvals/versions/0001.md | 3 + src/lib.rs | 1 + src/main.rs | 199 ++++ src/plans.rs | 934 ++++++++++++++++++ tests/plans.rs | 131 +++ 11 files changed, 1359 insertions(+) create mode 100644 examples/plans/agent.kdl create mode 100644 examples/plans/review-follow-up.md create mode 100644 examples/plans/ship-remote-approvals/plan.kdl create mode 100644 examples/plans/ship-remote-approvals/versions/0000.md create mode 100644 examples/plans/ship-remote-approvals/versions/0001.md create mode 100644 src/plans.rs create mode 100644 tests/plans.rs diff --git a/README.md b/README.md index aa2c63d1..e33470a8 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,52 @@ agent "" { } ``` +## Experimental read-only plans + +The experimental `st2 plan` surface implements only the inspectable file model from the +[st2 plans sketch at revision `5c1d142`](https://gist.github.com/myobie/d5ecfac24cd3965e095a5031cd2e00cb/5c1d1427c0556d95d13890e5c5086cd85b25d994). +It parses either a top-level `plan.kdl`: + +```kdl +plan "ship-remote-approvals" { + owner "app-web" + version "0000" resource="file:versions/0000.md" + version "0001" resource="file:versions/0001.md" { + parent "0000" + why "Browser proof exposed an approval race." + } +} +``` + +or the equivalent agent-local form: + +```kdl +agent "app-web" { + plan "review-follow-up" { + version "0000" resource="file:review-follow-up.md" + } +} +``` + +An agent links an external declaration with `plan-ref "file:relative/plan.kdl"`. Plan identity is +the explicit KDL value, never the directory. Inline ownership comes from the containing agent; +external plans require one `owner`. Every version has one relative `file:` resource. Revisions have +one or more immutable `parent` links and a non-empty `why`; root versions have no parent. The +frontier is derived as every version with no child, so concurrent siblings remain visible. +References resolve from the KDL file that declares them and must stay inside the selected catalog. + +```sh +st2 plan validate --catalog examples/plans +st2 plan list --catalog examples/plans +st2 plan show ship-remote-approvals --catalog examples/plans +st2 plan inspect ship-remote-approvals --catalog examples/plans --json +``` + +All four commands are read-only. They do not select a current version, write progress, emit events, +execute a plan, reconcile agents, schedule work, interpret claims, or require CAS. Direct KDL and +direct human-to-agent planning remain complete workflows; this experiment must earn any larger +runtime. + st2 provides `CATALOG`, flat native `ST_ROOT`, local `PTY_ROOT`, `ST_AGENT`, and `ST_HOOKS` to the task. The complete st2-managed overlay is also persisted in PTY metadata, so a manual `pty restart` retains those values. Declarations should not contain machine-specific install paths. diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index 50498cda..89792da8 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -208,6 +208,23 @@ atomic inbox file → DING attempt → agent reads → archive receipt - **R10:** Fleet identities are agents. General-purpose identity kinds are unsupported. +### Experimental read-only plan inspection + +`st2 plan validate|list|show|inspect` is a non-executing probe for the +[source plan sketch at revision `5c1d142`](https://gist.github.com/myobie/d5ecfac24cd3965e095a5031cd2e00cb/5c1d1427c0556d95d13890e5c5086cd85b25d994). +It normalizes top-level external plans and agent-local inline plans into one +model with explicit plan identity, owner, immutable version/parent links, +mandatory reasons on revisions, source-relative `file:` resources, and a +derived frontier that retains concurrent siblings. External `plan-ref` and +version resources resolve from the declaring KDL file and remain within the +selected catalog. + +The parser is deliberately deny-by-default for plan fields beyond that model. +It has no current pointer, controller, execution, scheduling, step graph, +retry, claim, receipt, event, reconciliation, CAS, or mutation behavior. It +does not yet fulfill R08 plan-progress observability or resolve DQ3; direct KDL +and direct human-to-agent planning remain first-class. + The owner updates this spec whenever implementation changes. Changing [vision.md](./vision.md) or [requirements.md](./requirements.md) requires Nathan's explicit approval. diff --git a/examples/plans/agent.kdl b/examples/plans/agent.kdl new file mode 100644 index 00000000..f440e4f1 --- /dev/null +++ b/examples/plans/agent.kdl @@ -0,0 +1,13 @@ +agent "app-web" { + host "example" + command "true" + plan-ref "file:ship-remote-approvals/plan.kdl" +} + +agent "reviewer" { + host "example" + command "true" + plan "review-follow-up" { + version "0000" resource="file:review-follow-up.md" + } +} diff --git a/examples/plans/review-follow-up.md b/examples/plans/review-follow-up.md new file mode 100644 index 00000000..9d49d4e0 --- /dev/null +++ b/examples/plans/review-follow-up.md @@ -0,0 +1,3 @@ +# Review follow-up + +Done means the accepted review corrections are present and independently verified. diff --git a/examples/plans/ship-remote-approvals/plan.kdl b/examples/plans/ship-remote-approvals/plan.kdl new file mode 100644 index 00000000..26f75a01 --- /dev/null +++ b/examples/plans/ship-remote-approvals/plan.kdl @@ -0,0 +1,9 @@ +plan "ship-remote-approvals" { + owner "app-web" + + version "0000" resource="file:versions/0000.md" + version "0001" resource="file:versions/0001.md" { + parent "0000" + why "Browser proof exposed an approval race." + } +} diff --git a/examples/plans/ship-remote-approvals/versions/0000.md b/examples/plans/ship-remote-approvals/versions/0000.md new file mode 100644 index 00000000..a9ee69ad --- /dev/null +++ b/examples/plans/ship-remote-approvals/versions/0000.md @@ -0,0 +1,3 @@ +# Ship remote approvals + +Done means an authorized reviewer can approve the exact remote change. diff --git a/examples/plans/ship-remote-approvals/versions/0001.md b/examples/plans/ship-remote-approvals/versions/0001.md new file mode 100644 index 00000000..8dc287fd --- /dev/null +++ b/examples/plans/ship-remote-approvals/versions/0001.md @@ -0,0 +1,3 @@ +# Ship remote approvals + +Done means an authorized reviewer can approve the exact remote change without racing stale state. diff --git a/src/lib.rs b/src/lib.rs index ce2f7c57..9d5b4747 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ pub mod host_lock; pub mod isolate; pub mod materialize; pub mod message; +pub mod plans; pub mod pretrust; pub mod reconcile; pub mod resource; diff --git a/src/main.rs b/src/main.rs index 907bceb6..85aefb45 100644 --- a/src/main.rs +++ b/src/main.rs @@ -269,6 +269,9 @@ enum Command { #[command(flatten)] ctx: MsgCtx, }, + /// EXPERIMENTAL, READ-ONLY: parse, validate, and inspect versioned catalog plans. + #[command(subcommand)] + Plan(PlanCmd), /// Print a shell completion script for `st2` to stdout (`st2 completions `). /// Generated from the live command tree, so it never drifts from the actual flags. Completions { @@ -339,6 +342,48 @@ enum HooksCmd { VerifyOwn, } +#[derive(Subcommand)] +enum PlanCmd { + /// Validate every external and inline plan without executing or writing anything. + Validate { + /// Catalog folder or KDL file. Prefer --catalog; defaults to the selected catalog. + #[arg(conflicts_with = "catalog_path")] + root: Option, + /// Emit a machine-readable validation receipt. + #[arg(long)] + json: bool, + }, + /// List normalized plan identity, owner, and derived frontier. + List { + /// Catalog folder or KDL file. Prefer --catalog; defaults to the selected catalog. + #[arg(conflicts_with = "catalog_path")] + root: Option, + /// Emit a machine-readable array. + #[arg(long)] + json: bool, + }, + /// Show normalized intent for one explicit plan identity. + Show { + identity: String, + /// Catalog folder or KDL file. Prefer --catalog; defaults to the selected catalog. + #[arg(conflicts_with = "catalog_path")] + root: Option, + /// Emit machine-readable normalized intent. + #[arg(long)] + json: bool, + }, + /// Inspect one plan with source provenance, resolved file paths, and agent references. + Inspect { + identity: String, + /// Catalog folder or KDL file. Prefer --catalog; defaults to the selected catalog. + #[arg(conflicts_with = "catalog_path")] + root: Option, + /// Emit the complete machine-readable inspection record. + #[arg(long)] + json: bool, + }, +} + #[derive(Subcommand)] enum ResourceCmd { /// Link a resource (a URL you produced or reference) into your resource list. @@ -571,6 +616,7 @@ fn main() -> Result<()> { enrich, ctx, } => agents_cmd(catalog, status, json, enrich, ctx), + Command::Plan(command) => plan_cmd(command), Command::CompileAgent { catalog, identity, @@ -638,6 +684,159 @@ fn main() -> Result<()> { } } +fn plan_cmd(command: PlanCmd) -> Result<()> { + match command { + PlanCmd::Validate { root, json } => { + let root = catalog_arg(root)?; + match st2::plans::load(&root) { + Ok(catalog) => { + if json { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "result": "valid", + "plans": catalog.plans.len(), + "errors": 0, + }))? + ); + } else { + println!( + "valid: {} plan{}; read-only (no execution or writes)", + catalog.plans.len(), + plural(catalog.plans.len()) + ); + } + Ok(()) + } + Err(error) => { + if json { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "result": "invalid", + "code": error.code(), + "path": error.path(), + "error": error.to_string(), + }))? + ); + } + Err(error.into()) + } + } + } + PlanCmd::List { root, json } => { + let catalog = st2::plans::load(&catalog_arg(root)?)?; + if json { + let rows = catalog + .plans + .iter() + .map(|plan| { + serde_json::json!({ + "identity": plan.identity, + "owner": plan.owner, + "frontier": plan.frontier, + }) + }) + .collect::>(); + println!("{}", serde_json::to_string_pretty(&rows)?); + } else { + for plan in catalog.plans { + println!( + "{}\towner={}\tfrontier={}", + plan.identity, + plan.owner, + plan.frontier.join(",") + ); + } + } + Ok(()) + } + PlanCmd::Show { + identity, + root, + json, + } => { + let catalog = st2::plans::load(&catalog_arg(root)?)?; + let plan = exact_plan(&catalog, &identity)?; + let intent = serde_json::json!({ + "identity": plan.identity, + "owner": plan.owner, + "versions": plan.versions.iter().map(|version| serde_json::json!({ + "identity": version.identity, + "parents": version.parents, + "why": version.why, + "resource": version.resource, + })).collect::>(), + "frontier": plan.frontier, + }); + if json { + println!("{}", serde_json::to_string_pretty(&intent)?); + } else { + println!("plan {} owner={}", plan.identity, plan.owner); + for version in &plan.versions { + let marker = if plan.frontier.contains(&version.identity) { + " [frontier]" + } else { + "" + }; + println!( + " version {}{marker} resource={}", + version.identity, version.resource + ); + if !version.parents.is_empty() { + println!(" parents: {}", version.parents.join(", ")); + } + if let Some(why) = &version.why { + println!(" why: {why}"); + } + } + } + Ok(()) + } + PlanCmd::Inspect { + identity, + root, + json, + } => { + let catalog = st2::plans::load(&catalog_arg(root)?)?; + let plan = exact_plan(&catalog, &identity)?; + if json { + println!("{}", serde_json::to_string_pretty(plan)?); + } else { + println!( + "plan {} owner={} kind={:?}\n source: {}\n referenced-by: {}\n frontier: {}", + plan.identity, + plan.owner, + plan.source_kind, + plan.source.display(), + plan.referenced_by.join(","), + plan.frontier.join(",") + ); + for version in &plan.versions { + println!( + " {}: {} -> {}", + version.identity, + version.resource, + version.resolved_resource.display() + ); + } + } + Ok(()) + } + } +} + +fn exact_plan<'a>( + catalog: &'a st2::plans::PlanCatalog, + identity: &str, +) -> Result<&'a st2::plans::Plan> { + catalog + .plans + .iter() + .find(|plan| plan.identity == identity) + .with_context(|| format!("no plan '{identity}' found")) +} + #[allow(clippy::too_many_arguments)] fn compile_agent_cmd( catalog: &Path, diff --git a/src/plans.rs b/src/plans.rs new file mode 100644 index 00000000..a8b4261f --- /dev/null +++ b/src/plans.rs @@ -0,0 +1,934 @@ +//! Experimental, read-only plan discovery and inspection. +//! +//! Plans remain ordinary KDL and referenced files. This module parses intent, validates immutable +//! version links, and derives the frontier; it never executes, schedules, reconciles, or writes. + +use std::collections::{BTreeMap, BTreeSet}; +use std::fmt; +use std::fs; +use std::path::{Path, PathBuf}; + +use kdl::{KdlDocument, KdlEntry, KdlNode}; +use serde::Serialize; + +/// A normalized set of plans selected from one catalog folder or KDL file. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct PlanCatalog { + pub plans: Vec, +} + +/// One explicit plan, independent of its directory name. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct Plan { + pub identity: String, + pub owner: String, + pub versions: Vec, + pub frontier: Vec, + pub source: PathBuf, + pub source_kind: PlanSourceKind, + pub referenced_by: Vec, +} + +/// Whether a plan is a top-level declaration or nested in an agent declaration. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum PlanSourceKind { + External, + Inline, +} + +/// One immutable intent revision. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct PlanVersion { + pub identity: String, + pub parents: Vec, + pub why: Option, + pub resource: String, + pub resolved_resource: PathBuf, +} + +/// A classified plan parse or validation error. +#[derive(Debug)] +pub struct PlanError { + code: &'static str, + path: PathBuf, + message: String, +} + +impl PlanError { + fn new(code: &'static str, path: &Path, message: impl Into) -> Self { + Self { + code, + path: path.to_path_buf(), + message: message.into(), + } + } + + pub fn code(&self) -> &'static str { + self.code + } + + pub fn path(&self) -> &Path { + &self.path + } +} + +impl fmt::Display for PlanError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "{}: {}", self.path.display(), self.message) + } +} + +impl std::error::Error for PlanError {} + +#[derive(Debug)] +struct PlanReference { + owner: String, + target: PathBuf, +} + +/// Parse, normalize, and validate plans without mutating the selected files. +pub fn load(selected: &Path) -> Result { + let selected = selected.canonicalize().map_err(|error| { + PlanError::new( + "selection-read-failed", + selected, + format!("reading selected plan input: {error}"), + ) + })?; + let boundary = if selected.is_dir() { + selected.clone() + } else { + selected + .parent() + .map(Path::to_path_buf) + .ok_or_else(|| PlanError::new("invalid-selection", &selected, "input has no parent"))? + }; + let files = collect_kdl_files(&selected, &boundary)?; + let mut plans = Vec::new(); + let mut external_by_source: BTreeMap> = BTreeMap::new(); + let mut references = Vec::new(); + + for path in files { + parse_seed( + &path, + &boundary, + &mut plans, + &mut external_by_source, + &mut references, + )?; + } + + for reference in references { + if !external_by_source.contains_key(&reference.target) { + parse_external_file( + &reference.target, + &boundary, + &mut plans, + &mut external_by_source, + )?; + } + let matches = &external_by_source[&reference.target]; + if matches.len() != 1 { + return Err(PlanError::new( + "ambiguous-plan-reference", + &reference.target, + "a plan-ref target must contain exactly one top-level plan", + )); + } + plans[matches[0]].referenced_by.push(reference.owner); + } + + let mut identities = BTreeMap::new(); + for (index, plan) in plans.iter().enumerate() { + if let Some(previous) = identities.insert(plan.identity.clone(), index) { + return Err(PlanError::new( + "duplicate-plan-identity", + &plan.source, + format!( + "plan '{}' is also declared in {}", + plan.identity, + plans[previous].source.display() + ), + )); + } + } + for plan in &mut plans { + plan.referenced_by.sort(); + plan.referenced_by.dedup(); + } + plans.sort_by(|left, right| left.identity.cmp(&right.identity)); + Ok(PlanCatalog { plans }) +} + +fn collect_kdl_files(selected: &Path, boundary: &Path) -> Result, PlanError> { + if selected.is_file() { + if selected.extension().and_then(|value| value.to_str()) != Some("kdl") { + return Err(PlanError::new( + "unsupported-plan-input", + selected, + "plan input must be a catalog folder or KDL file", + )); + } + return Ok(vec![selected.to_path_buf()]); + } + if !selected.is_dir() { + return Err(PlanError::new( + "invalid-selection", + selected, + "plan input is neither a folder nor a file", + )); + } + + fn walk(root: &Path, directory: &Path, out: &mut Vec) -> Result<(), PlanError> { + let entries = fs::read_dir(directory).map_err(|error| { + PlanError::new( + "selection-read-failed", + directory, + format!("reading plan input folder: {error}"), + ) + })?; + for entry in entries { + let entry = entry.map_err(|error| { + PlanError::new( + "selection-read-failed", + directory, + format!("reading plan input entry: {error}"), + ) + })?; + let path = entry.path(); + if !crate::discovery::is_catalog_path(root, &path) { + continue; + } + let kind = entry.file_type().map_err(|error| { + PlanError::new( + "selection-read-failed", + &path, + format!("reading plan input type: {error}"), + ) + })?; + if kind.is_dir() { + walk(root, &path, out)?; + } else if kind.is_file() + && path.extension().and_then(|value| value.to_str()) == Some("kdl") + { + out.push(path); + } + } + Ok(()) + } + + let mut files = Vec::new(); + walk(boundary, selected, &mut files)?; + files.sort(); + Ok(files) +} + +fn parse_seed( + path: &Path, + boundary: &Path, + plans: &mut Vec, + external_by_source: &mut BTreeMap>, + references: &mut Vec, +) -> Result<(), PlanError> { + let document = read_document(path)?; + for node in document.nodes() { + match node.name().value() { + "plan" => { + let plan = parse_plan(node, path, boundary, PlanSourceKind::External, None)?; + let index = plans.len(); + plans.push(plan); + external_by_source + .entry(path.to_path_buf()) + .or_default() + .push(index); + } + "agent" => parse_agent_plans(node, path, boundary, plans, references)?, + _ => {} + } + } + Ok(()) +} + +fn parse_external_file( + path: &Path, + boundary: &Path, + plans: &mut Vec, + external_by_source: &mut BTreeMap>, +) -> Result<(), PlanError> { + let document = read_document(path)?; + for node in document.nodes() { + if node.name().value() == "plan" { + let plan = parse_plan(node, path, boundary, PlanSourceKind::External, None)?; + let index = plans.len(); + plans.push(plan); + external_by_source + .entry(path.to_path_buf()) + .or_default() + .push(index); + } + } + if !external_by_source.contains_key(path) { + return Err(PlanError::new( + "missing-plan", + path, + "plan-ref target contains no top-level plan", + )); + } + Ok(()) +} + +fn read_document(path: &Path) -> Result { + let text = fs::read_to_string(path).map_err(|error| { + PlanError::new("plan-read-failed", path, format!("reading KDL: {error}")) + })?; + KdlDocument::parse(&text).map_err(|error| { + PlanError::new("malformed-plan-kdl", path, format!("parsing KDL: {error}")) + }) +} + +fn parse_agent_plans( + agent: &KdlNode, + source: &Path, + boundary: &Path, + plans: &mut Vec, + references: &mut Vec, +) -> Result<(), PlanError> { + let Some(children) = agent.children() else { + return Ok(()); + }; + let owner = explicit_agent_identity(agent); + for child in children.nodes() { + match child.name().value() { + "plan" => plans.push(parse_plan( + child, + source, + boundary, + PlanSourceKind::Inline, + Some(required_inline_owner(owner.as_deref(), source)?), + )?), + "plan-ref" => { + let target = exact_string_node(child, source, "plan-ref")?; + let target = resolve_file_reference(source, boundary, &target, "plan-ref")?; + if target.extension().and_then(|value| value.to_str()) != Some("kdl") { + return Err(PlanError::new( + "invalid-plan-reference", + source, + "plan-ref must resolve to a KDL file", + )); + } + references.push(PlanReference { + owner: required_inline_owner(owner.as_deref(), source)?, + target, + }); + } + _ => {} + } + } + Ok(()) +} + +fn required_inline_owner(owner: Option<&str>, source: &Path) -> Result { + owner.map(str::to_string).ok_or_else(|| { + PlanError::new( + "inline-owner-required", + source, + "an agent with inline plans or plan-ref must declare an explicit identity", + ) + }) +} + +fn explicit_agent_identity(agent: &KdlNode) -> Option { + let mut identity = positional_string(agent); + if let Some(children) = agent.children() { + for child in children.nodes() { + if child.name().value() == "identity" { + identity = positional_string(child).or(identity); + } + } + } + identity.filter(|value| valid_text(value)) +} + +fn parse_plan( + node: &KdlNode, + source: &Path, + boundary: &Path, + source_kind: PlanSourceKind, + inline_owner: Option, +) -> Result { + let identity = exact_positional_with_properties(node, source, "plan", &[])? + .0 + .ok_or_else(|| { + PlanError::new("plan-identity-required", source, "plan needs an identity") + })?; + if !valid_text(&identity) { + return Err(PlanError::new( + "invalid-plan-identity", + source, + "plan identity must be non-empty and contain no control characters", + )); + } + let children = node.children().ok_or_else(|| { + PlanError::new( + "plan-body-required", + source, + format!("plan '{identity}' needs a body"), + ) + })?; + let mut owner = inline_owner; + let mut versions = Vec::new(); + for child in children.nodes() { + match child.name().value() { + "owner" if source_kind == PlanSourceKind::External => { + if owner.is_some() { + return Err(PlanError::new( + "duplicate-plan-owner", + source, + format!("plan '{identity}' declares owner more than once"), + )); + } + owner = Some(exact_string_node(child, source, "owner")?); + } + "owner" => { + return Err(PlanError::new( + "inline-owner-is-derived", + source, + format!("inline plan '{identity}' derives owner from its containing agent"), + )); + } + "version" => versions.push(parse_version(child, source, boundary, &identity)?), + other => { + return Err(PlanError::new( + "unsupported-plan-field", + source, + format!( + "plan '{identity}' contains unsupported '{other}'; execution, current pointers, steps, retries, claims, and schedules are outside this experiment" + ), + )); + } + } + } + let owner = owner.ok_or_else(|| { + PlanError::new( + "plan-owner-required", + source, + format!("external plan '{identity}' needs one owner"), + ) + })?; + if !valid_text(&owner) { + return Err(PlanError::new( + "invalid-plan-owner", + source, + format!("plan '{identity}' owner must be non-empty"), + )); + } + let frontier = validate_versions(source, &identity, &mut versions)?; + Ok(Plan { + identity, + owner, + versions, + frontier, + source: source.to_path_buf(), + source_kind, + referenced_by: Vec::new(), + }) +} + +fn parse_version( + node: &KdlNode, + source: &Path, + boundary: &Path, + plan: &str, +) -> Result { + let (identity, properties) = + exact_positional_with_properties(node, source, "version", &["resource"])?; + let identity = identity.ok_or_else(|| { + PlanError::new( + "version-identity-required", + source, + format!("plan '{plan}' has a version without an identity"), + ) + })?; + if !valid_text(&identity) { + return Err(PlanError::new( + "invalid-version-identity", + source, + format!("plan '{plan}' has an invalid version identity"), + )); + } + let resource = properties.get("resource").cloned().ok_or_else(|| { + PlanError::new( + "version-resource-required", + source, + format!("plan '{plan}' version '{identity}' needs resource=\"file:...\""), + ) + })?; + let resolved_resource = + resolve_file_reference(source, boundary, &resource, "version resource")?; + let mut parents = Vec::new(); + let mut why = None; + if let Some(children) = node.children() { + for child in children.nodes() { + match child.name().value() { + "parent" => parents.push(exact_string_node(child, source, "parent")?), + "why" => { + if why.is_some() { + return Err(PlanError::new( + "duplicate-version-reason", + source, + format!( + "plan '{plan}' version '{identity}' declares why more than once" + ), + )); + } + why = Some(exact_string_node(child, source, "why")?); + } + other => { + return Err(PlanError::new( + "unsupported-version-field", + source, + format!( + "plan '{plan}' version '{identity}' contains unsupported '{other}'" + ), + )); + } + } + } + } + parents.sort(); + if parents.windows(2).any(|pair| pair[0] == pair[1]) { + return Err(PlanError::new( + "duplicate-version-parent", + source, + format!("plan '{plan}' version '{identity}' repeats a parent"), + )); + } + if !parents.is_empty() && why.as_ref().is_none_or(|value| !valid_text(value)) { + return Err(PlanError::new( + "revision-reason-required", + source, + format!("plan '{plan}' revision '{identity}' needs a non-empty why"), + )); + } + Ok(PlanVersion { + identity, + parents, + why, + resource, + resolved_resource, + }) +} + +fn validate_versions( + source: &Path, + plan: &str, + versions: &mut [PlanVersion], +) -> Result, PlanError> { + if versions.is_empty() { + return Err(PlanError::new( + "plan-version-required", + source, + format!("plan '{plan}' needs at least one version"), + )); + } + let mut indices = BTreeMap::new(); + for (index, version) in versions.iter().enumerate() { + if indices.insert(version.identity.clone(), index).is_some() { + return Err(PlanError::new( + "duplicate-version-identity", + source, + format!( + "plan '{plan}' declares version '{}' more than once", + version.identity + ), + )); + } + } + let mut referenced = BTreeSet::new(); + for version in versions.iter() { + for parent in &version.parents { + if !indices.contains_key(parent) { + return Err(PlanError::new( + "unknown-version-parent", + source, + format!( + "plan '{plan}' version '{}' has unknown parent '{parent}'", + version.identity + ), + )); + } + referenced.insert(parent.clone()); + } + } + + fn visit( + index: usize, + versions: &[PlanVersion], + indices: &BTreeMap, + marks: &mut [u8], + ) -> bool { + if marks[index] == 1 { + return false; + } + if marks[index] == 2 { + return true; + } + marks[index] = 1; + for parent in &versions[index].parents { + if !visit(indices[parent], versions, indices, marks) { + return false; + } + } + marks[index] = 2; + true + } + let mut marks = vec![0; versions.len()]; + for index in 0..versions.len() { + if !visit(index, versions, &indices, &mut marks) { + return Err(PlanError::new( + "version-cycle", + source, + format!("plan '{plan}' version parents contain a cycle"), + )); + } + } + + let frontier = indices + .keys() + .filter(|identity| !referenced.contains(*identity)) + .cloned() + .collect(); + versions.sort_by(|left, right| left.identity.cmp(&right.identity)); + Ok(frontier) +} + +fn exact_string_node(node: &KdlNode, source: &Path, field: &str) -> Result { + let (value, properties) = exact_positional_with_properties(node, source, field, &[])?; + if !properties.is_empty() || node.children().is_some() { + return Err(PlanError::new( + "invalid-plan-field", + source, + format!("{field} must be one string without properties or children"), + )); + } + value.filter(|value| valid_text(value)).ok_or_else(|| { + PlanError::new( + "invalid-plan-field", + source, + format!("{field} must be one non-empty string"), + ) + }) +} + +fn exact_positional_with_properties( + node: &KdlNode, + source: &Path, + field: &str, + allowed_properties: &[&str], +) -> Result<(Option, BTreeMap), PlanError> { + let mut positional = None; + let mut properties = BTreeMap::new(); + for entry in node.entries() { + match entry.name() { + None if positional.is_none() => { + positional = Some(entry_string(entry).ok_or_else(|| { + PlanError::new( + "invalid-plan-field", + source, + format!("{field} positional value must be a string"), + ) + })?) + } + None => { + return Err(PlanError::new( + "invalid-plan-field", + source, + format!("{field} accepts exactly one positional string"), + )); + } + Some(name) if allowed_properties.contains(&name.value()) => { + let name = name.value().to_string(); + let value = entry_string(entry).ok_or_else(|| { + PlanError::new( + "invalid-plan-field", + source, + format!("{field} property '{name}' must be a string"), + ) + })?; + if properties.insert(name.clone(), value).is_some() { + return Err(PlanError::new( + "invalid-plan-field", + source, + format!("{field} property '{name}' is duplicated"), + )); + } + } + Some(name) => { + return Err(PlanError::new( + "unsupported-plan-property", + source, + format!("{field} contains unsupported property '{}'", name.value()), + )); + } + } + } + Ok((positional, properties)) +} + +fn entry_string(entry: &KdlEntry) -> Option { + entry.value().as_string().map(str::to_string) +} + +fn positional_string(node: &KdlNode) -> Option { + node.entries() + .iter() + .find(|entry| entry.name().is_none()) + .and_then(entry_string) +} + +fn valid_text(value: &str) -> bool { + !value.is_empty() && value.trim() == value && !value.chars().any(char::is_control) +} + +fn resolve_file_reference( + source: &Path, + boundary: &Path, + reference: &str, + field: &str, +) -> Result { + let relative = reference.strip_prefix("file:").ok_or_else(|| { + PlanError::new( + "unsupported-plan-reference", + source, + format!("{field} must use a relative file: reference"), + ) + })?; + if relative.is_empty() || relative.starts_with("//") || Path::new(relative).is_absolute() { + return Err(PlanError::new( + "nonrelative-plan-reference", + source, + format!("{field} must be relative to its declaring KDL file"), + )); + } + let candidate = source + .parent() + .unwrap_or(boundary) + .join(relative) + .canonicalize() + .map_err(|error| { + PlanError::new( + "missing-plan-resource", + source, + format!("resolving {field} '{reference}': {error}"), + ) + })?; + if !candidate.starts_with(boundary) { + return Err(PlanError::new( + "plan-reference-escape", + source, + format!("{field} '{reference}' resolves outside the selected catalog"), + )); + } + if !candidate.is_file() { + return Err(PlanError::new( + "invalid-plan-resource", + source, + format!("{field} '{reference}' does not resolve to a regular file"), + )); + } + Ok(candidate) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write(root: &Path, relative: &str, contents: &str) { + let path = root.join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); + } + + #[test] + fn external_and_inline_forms_normalize_and_preserve_concurrent_frontier_heads() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "agent.kdl", + r#" +agent "worker" { + plan-ref "file:plans/shared/plan.kdl" + plan "local" { + version "v0" resource="file:plans/local-v0.md" + } +} +"#, + ); + write(root, "plans/local-v0.md", "# Local\n"); + write(root, "plans/shared/v0.md", "# v0\n"); + write(root, "plans/shared/v1.md", "# v1\n"); + write(root, "plans/shared/v2.md", "# v2\n"); + write( + root, + "plans/shared/plan.kdl", + r#" +plan "shared" { + owner "cos" + version "v2" resource="file:v2.md" { + parent "v0" + why "Second concurrent direction." + } + version "v0" resource="file:v0.md" + version "v1" resource="file:v1.md" { + parent "v0" + why "First concurrent direction." + } +} +"#, + ); + + let loaded = load(root).unwrap(); + assert_eq!(loaded.plans.len(), 2); + let local = &loaded.plans[0]; + assert_eq!(local.identity, "local"); + assert_eq!(local.owner, "worker"); + assert_eq!(local.frontier, ["v0"]); + assert_eq!(local.source_kind, PlanSourceKind::Inline); + + let shared = &loaded.plans[1]; + assert_eq!(shared.owner, "cos"); + assert_eq!(shared.frontier, ["v1", "v2"]); + assert_eq!(shared.referenced_by, ["worker"]); + assert_eq!( + shared + .versions + .iter() + .map(|version| version.identity.as_str()) + .collect::>(), + ["v0", "v1", "v2"] + ); + } + + #[test] + fn validation_rejects_mutable_current_execution_fields_and_bad_revision_graphs() { + let temporary = tempfile::tempdir().unwrap(); + write(temporary.path(), "v0.md", "# v0\n"); + write( + temporary.path(), + "plan.kdl", + r#" +plan "bad" { + owner "cos" + current "v0" + version "v0" resource="file:v0.md" +} +"#, + ); + assert_eq!( + load(temporary.path()).unwrap_err().code(), + "unsupported-plan-field" + ); + + write( + temporary.path(), + "plan.kdl", + r#" +plan "bad" { + owner "cos" + version "v1" resource="file:v0.md" { + parent "missing" + } +} +"#, + ); + assert_eq!( + load(temporary.path()).unwrap_err().code(), + "revision-reason-required" + ); + + write( + temporary.path(), + "plan.kdl", + r#" +plan "bad" { + owner "cos" + version "v1" resource="file:v0.md" { + parent "missing" + why "Revision." + } +} +"#, + ); + assert_eq!( + load(temporary.path()).unwrap_err().code(), + "unknown-version-parent" + ); + + write( + temporary.path(), + "plan.kdl", + r#" +plan "bad" { + owner "cos" + version "v0" resource="file:v0.md" { + parent "v1" + why "First half." + } + version "v1" resource="file:v0.md" { + parent "v0" + why "Second half." + } +} +"#, + ); + assert_eq!(load(temporary.path()).unwrap_err().code(), "version-cycle"); + } + + #[test] + fn references_are_source_relative_bounded_and_read_only() { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "nested/agent.kdl", + r#" +agent "worker" { + plan "local" { + version "v0" resource="file:body.md" + } +} +"#, + ); + write(root, "nested/body.md", "unchanged\n"); + let before = fs::read(root.join("nested/agent.kdl")).unwrap(); + let loaded = load(root).unwrap(); + assert_eq!( + loaded.plans[0].versions[0].resolved_resource, + root.join("nested/body.md").canonicalize().unwrap() + ); + assert_eq!(fs::read(root.join("nested/agent.kdl")).unwrap(), before); + assert_eq!( + fs::read_to_string(root.join("nested/body.md")).unwrap(), + "unchanged\n" + ); + + let bounded = root.join("bounded"); + fs::create_dir(&bounded).unwrap(); + fs::write(root.join("outside.md"), "outside\n").unwrap(); + write( + &bounded, + "plan.kdl", + r#" +plan "escape" { + owner "worker" + version "v0" resource="file:../outside.md" +} +"#, + ); + assert_eq!(load(&bounded).unwrap_err().code(), "plan-reference-escape"); + } +} diff --git a/tests/plans.rs b/tests/plans.rs new file mode 100644 index 00000000..a27aeaec --- /dev/null +++ b/tests/plans.rs @@ -0,0 +1,131 @@ +use std::fs; +use std::path::Path; +use std::process::{Command, Output}; + +fn write(root: &Path, relative: &str, contents: &str) { + let path = root.join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, contents).unwrap(); +} + +fn fixture() -> tempfile::TempDir { + let temporary = tempfile::tempdir().unwrap(); + let root = temporary.path(); + write( + root, + "agent.kdl", + r#" +agent "worker" { + plan-ref "file:plans/shared/plan.kdl" + plan "local" { + version "0000" resource="file:plans/local.md" + } +} +"#, + ); + write(root, "plans/local.md", "# Local\n"); + write(root, "plans/shared/0000.md", "# Initial\n"); + write(root, "plans/shared/0001.md", "# Left\n"); + write(root, "plans/shared/0002.md", "# Right\n"); + write( + root, + "plans/shared/plan.kdl", + r#" +plan "shared" { + owner "cos" + version "0000" resource="file:0000.md" + version "0001" resource="file:0001.md" { + parent "0000" + why "Left branch." + } + version "0002" resource="file:0002.md" { + parent "0000" + why "Right branch." + } +} +"#, + ); + temporary +} + +fn run(root: &Path, args: &[&str]) -> Output { + Command::new(env!("CARGO_BIN_EXE_st2")) + .arg("--catalog") + .arg(root) + .args(args) + .output() + .unwrap() +} + +fn success_json(root: &Path, args: &[&str]) -> serde_json::Value { + let output = run(root, args); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice(&output.stdout).unwrap() +} + +#[test] +fn cli_validates_lists_shows_and_inspects_the_same_read_only_plan_model() { + let temporary = fixture(); + let root = temporary.path(); + let agent_before = fs::read(root.join("agent.kdl")).unwrap(); + let plan_before = fs::read(root.join("plans/shared/plan.kdl")).unwrap(); + + let valid = success_json(root, &["plan", "validate", "--json"]); + assert_eq!(valid["result"], "valid"); + assert_eq!(valid["plans"], 2); + + let listed = success_json(root, &["plan", "list", "--json"]); + assert_eq!(listed[0]["identity"], "local"); + assert_eq!(listed[1]["identity"], "shared"); + assert_eq!(listed[1]["frontier"], serde_json::json!(["0001", "0002"])); + + let shown = success_json(root, &["plan", "show", "shared", "--json"]); + assert_eq!(shown["owner"], "cos"); + assert_eq!(shown["frontier"], serde_json::json!(["0001", "0002"])); + assert!(shown.get("source").is_none()); + assert!(shown["versions"][0].get("resolvedResource").is_none()); + + let inspected = success_json(root, &["plan", "inspect", "shared", "--json"]); + assert_eq!(inspected["sourceKind"], "external"); + assert_eq!(inspected["referencedBy"], serde_json::json!(["worker"])); + assert!( + inspected["versions"][0]["resolvedResource"] + .as_str() + .unwrap() + .ends_with("/plans/shared/0000.md") + ); + + assert_eq!(fs::read(root.join("agent.kdl")).unwrap(), agent_before); + assert_eq!( + fs::read(root.join("plans/shared/plan.kdl")).unwrap(), + plan_before + ); + assert!(!root.join(".st2").exists()); +} + +#[test] +fn cli_validation_is_nonzero_and_classified_for_out_of_scope_plan_fields() { + let temporary = tempfile::tempdir().unwrap(); + write(temporary.path(), "v0.md", "# Initial\n"); + write( + temporary.path(), + "plan.kdl", + r#" +plan "not-read-only" { + owner "cos" + current "0000" + version "0000" resource="file:v0.md" +} +"#, + ); + + let output = run(temporary.path(), &["plan", "validate", "--json"]); + assert!(!output.status.success()); + let receipt: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(receipt["result"], "invalid"); + assert_eq!(receipt["code"], "unsupported-plan-field"); +} From 60d48bae5b7ac3a83c8d2c3324b61680bd6404dd Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Fri, 31 Jul 2026 23:18:20 +0200 Subject: [PATCH 2/3] Correct plan history claims --- README.md | 8 +++++--- docs/vrs/spec.md | 5 +++-- src/plans.rs | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index db204f71..87b79517 100644 --- a/README.md +++ b/README.md @@ -216,9 +216,11 @@ agent "app-web" { An agent links an external declaration with `plan-ref "file:relative/plan.kdl"`. Plan identity is the explicit KDL value, never the directory. Inline ownership comes from the containing agent; -external plans require one `owner`. Every version has one relative `file:` resource. Revisions have -one or more immutable `parent` links and a non-empty `why`; root versions have no parent. The -frontier is derived as every version with no child, so concurrent siblings remain visible. +external plans require one `owner`. Every version has one relative `file:` resource. Revisions +declare one or more `parent` links and a non-empty `why`; root versions have no parent. The frontier +is derived as every version with no child, so concurrent siblings remain visible. The experiment +stores no content digest or history. It cannot prove that an earlier declaration or resource file +stayed unchanged. References resolve from the KDL file that declares them and must stay inside the selected catalog. ```sh diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index 0a11f388..4dff12f1 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -239,11 +239,12 @@ atomic inbox file → DING attempt → agent reads → archive receipt `st2 plan validate|list|show|inspect` is a non-executing probe for the [source plan sketch at revision `5c1d142`](https://gist.github.com/myobie/d5ecfac24cd3965e095a5031cd2e00cb/5c1d1427c0556d95d13890e5c5086cd85b25d994). It normalizes top-level external plans and agent-local inline plans into one -model with explicit plan identity, owner, immutable version/parent links, +model with explicit plan identity, owner, declared version and parent links, mandatory reasons on revisions, source-relative `file:` resources, and a derived frontier that retains concurrent siblings. External `plan-ref` and version resources resolve from the declaring KDL file and remain within the -selected catalog. +selected catalog. The experiment stores no content digest or history. It +cannot prove that an earlier declaration or resource file stayed unchanged. The parser is deliberately deny-by-default for plan fields beyond that model. It has no current pointer, controller, execution, scheduling, step graph, diff --git a/src/plans.rs b/src/plans.rs index a8b4261f..f1b5ea41 100644 --- a/src/plans.rs +++ b/src/plans.rs @@ -1,6 +1,6 @@ //! Experimental, read-only plan discovery and inspection. //! -//! Plans remain ordinary KDL and referenced files. This module parses intent, validates immutable +//! Plans remain ordinary KDL and referenced files. This module parses intent, validates declared //! version links, and derives the frontier; it never executes, schedules, reconciles, or writes. use std::collections::{BTreeMap, BTreeSet}; @@ -38,7 +38,7 @@ pub enum PlanSourceKind { Inline, } -/// One immutable intent revision. +/// One declared intent revision. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] #[serde(rename_all = "camelCase")] pub struct PlanVersion { From 8a76b6e71355140e5b89cd9313fcfd88c82b5cad Mon Sep 17 00:00:00 2001 From: Nathan Herald Date: Mon, 3 Aug 2026 19:10:07 +0200 Subject: [PATCH 3/3] Link agents to plan-owned intent resources --- README.md | 48 ++- docs/vrs/spec.md | 18 +- examples/plans/agent.kdl | 6 +- examples/plans/review-follow-up.md | 3 - examples/plans/review-follow-up/plan.kdl | 11 + examples/plans/ship-remote-approvals/plan.kdl | 4 +- src/main.rs | 56 ++- src/plans.rs | 351 ++++++++++++++---- tests/plans.rs | 71 +++- 9 files changed, 430 insertions(+), 138 deletions(-) delete mode 100644 examples/plans/review-follow-up.md create mode 100644 examples/plans/review-follow-up/plan.kdl diff --git a/README.md b/README.md index c298ab19..3eb91249 100644 --- a/README.md +++ b/README.md @@ -252,37 +252,55 @@ agent "" { The experimental `st2 plan` surface implements only the inspectable file model from the [st2 plans sketch at revision `5c1d142`](https://gist.github.com/myobie/d5ecfac24cd3965e095a5031cd2e00cb/5c1d1427c0556d95d13890e5c5086cd85b25d994). -It parses either a top-level `plan.kdl`: +The implemented agent integration uses the existing Resource envelope; plan truth stays only in the +linked `plan.kdl`. +An agent links a plan with the existing Agent Spec Resource envelope: + +```kdl +agent "app-web" { + resource "receipt-report" _tag="plan" uri="file:plans/receipt-report/plan.kdl" +} +``` + +The Resource name is the agent-local role. The `_tag` selects the experimental plan reader. The +`uri` resolves from the agent KDL file and must remain inside the selected catalog. The Resource +has no children and owns no plan fields. It may add the agent identity to `referencedBy`, but the +target `plan.kdl` remains the only plan authority. + +A plan can keep version content in referenced Markdown files: ```kdl plan "ship-remote-approvals" { owner "app-web" - version "0000" resource="file:versions/0000.md" - version "0001" resource="file:versions/0001.md" { + version "0000" content="file:versions/0000.md" + version "0001" content="file:versions/0001.md" { parent "0000" why "Browser proof exposed an approval race." } } ``` -or the equivalent agent-local form: +Or a small plan can keep the complete version intent inline in `plan.kdl`: ```kdl -agent "app-web" { - plan "review-follow-up" { - version "0000" resource="file:review-follow-up.md" +plan "review-follow-up" { + owner "reviewer" + version "0000" { + intent #""" +Review the accepted corrections. + +Done means the corrections are present and independently verified. +"""# } } ``` -An agent links an external declaration with `plan-ref "file:relative/plan.kdl"`. Plan identity is -the explicit KDL value, never the directory. Inline ownership comes from the containing agent; -external plans require one `owner`. Every version has one relative `file:` resource. Revisions -declare one or more `parent` links and a non-empty `why`; root versions have no parent. The frontier -is derived as every version with no child, so concurrent siblings remain visible. The experiment -stores no content digest or history. It cannot prove that an earlier declaration or resource file -stayed unchanged. -References resolve from the KDL file that declares them and must stay inside the selected catalog. +Plan identity and owner are explicit in `plan.kdl`, never derived from a Resource name, directory, +or referring agent. Each version has exactly one `content="file:..."` property or one inline +`intent` child. Content references resolve from `plan.kdl` and remain inside the selected catalog. +Revisions declare one or more `parent` links and a non-empty `why`; root versions have no parent. +The frontier is every version with no child, so concurrent siblings remain visible. The experiment +stores no digest or immutable history proof. ```sh st2 plan validate --catalog examples/plans diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index 84a623cc..61f14d34 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -553,13 +553,17 @@ atomic inbox file → DING attempt → agent reads → archive receipt `st2 plan validate|list|show|inspect` is a non-executing probe for the [source plan sketch at revision `5c1d142`](https://gist.github.com/myobie/d5ecfac24cd3965e095a5031cd2e00cb/5c1d1427c0556d95d13890e5c5086cd85b25d994). -It normalizes top-level external plans and agent-local inline plans into one -model with explicit plan identity, owner, declared version and parent links, -mandatory reasons on revisions, source-relative `file:` resources, and a -derived frontier that retains concurrent siblings. External `plan-ref` and -version resources resolve from the declaring KDL file and remain within the -selected catalog. The experiment stores no content digest or history. It -cannot prove that an earlier declaration or resource file stayed unchanged. +The implemented agent integration uses the existing Resource envelope to link +to plan-owned truth. +It reads standalone plans and plans linked through a childless Agent Spec +Resource whose `_tag` is `plan`. The Resource name is an agent-local role. The +Resource can add `referencedBy`; it does not own plan identity, owner, version, +parent, reason, or intent. Those fields remain in the referenced `plan.kdl`. +Each version contains either source-relative Markdown `content` or complete +inline `intent`. Resource URIs resolve from the agent KDL file. Content URIs +resolve from `plan.kdl`. Both stay within the selected catalog. A derived +frontier retains concurrent siblings. The experiment stores no content digest +or immutable history proof. The parser is deliberately deny-by-default for plan fields beyond that model. It has no current pointer, controller, execution, scheduling, step graph, diff --git a/examples/plans/agent.kdl b/examples/plans/agent.kdl index f440e4f1..fa8e27fd 100644 --- a/examples/plans/agent.kdl +++ b/examples/plans/agent.kdl @@ -1,13 +1,11 @@ agent "app-web" { host "example" command "true" - plan-ref "file:ship-remote-approvals/plan.kdl" + resource "release-plan" _tag="plan" uri="file:ship-remote-approvals/plan.kdl" } agent "reviewer" { host "example" command "true" - plan "review-follow-up" { - version "0000" resource="file:review-follow-up.md" - } + resource "review-plan" _tag="plan" uri="file:review-follow-up/plan.kdl" } diff --git a/examples/plans/review-follow-up.md b/examples/plans/review-follow-up.md deleted file mode 100644 index 9d49d4e0..00000000 --- a/examples/plans/review-follow-up.md +++ /dev/null @@ -1,3 +0,0 @@ -# Review follow-up - -Done means the accepted review corrections are present and independently verified. diff --git a/examples/plans/review-follow-up/plan.kdl b/examples/plans/review-follow-up/plan.kdl new file mode 100644 index 00000000..aebeb2d1 --- /dev/null +++ b/examples/plans/review-follow-up/plan.kdl @@ -0,0 +1,11 @@ +plan "review-follow-up" { + owner "reviewer" + + version "0000" { + intent #""" +Review the accepted corrections. + +Done means the corrections are present and independently verified. +"""# + } +} diff --git a/examples/plans/ship-remote-approvals/plan.kdl b/examples/plans/ship-remote-approvals/plan.kdl index 26f75a01..1810346d 100644 --- a/examples/plans/ship-remote-approvals/plan.kdl +++ b/examples/plans/ship-remote-approvals/plan.kdl @@ -1,8 +1,8 @@ plan "ship-remote-approvals" { owner "app-web" - version "0000" resource="file:versions/0000.md" - version "0001" resource="file:versions/0001.md" { + version "0000" content="file:versions/0000.md" + version "0001" content="file:versions/0001.md" { parent "0000" why "Browser proof exposed an approval race." } diff --git a/src/main.rs b/src/main.rs index 192d405e..7e1a7053 100644 --- a/src/main.rs +++ b/src/main.rs @@ -480,7 +480,7 @@ enum HooksCmd { #[derive(Subcommand)] enum PlanCmd { - /// Validate every external and inline plan without executing or writing anything. + /// Validate every Resource-linked or standalone plan without executing or writing anything. Validate { /// Catalog folder or KDL file. Prefer --catalog; defaults to the selected catalog. #[arg(conflicts_with = "catalog_path")] @@ -1041,12 +1041,20 @@ fn plan_cmd(command: PlanCmd) -> Result<()> { let intent = serde_json::json!({ "identity": plan.identity, "owner": plan.owner, - "versions": plan.versions.iter().map(|version| serde_json::json!({ - "identity": version.identity, - "parents": version.parents, - "why": version.why, - "resource": version.resource, - })).collect::>(), + "versions": plan.versions.iter().map(|version| { + let mut row = serde_json::json!({ + "identity": version.identity, + "parents": version.parents, + "why": version.why, + }); + if let Some(content) = &version.content { + row["content"] = serde_json::json!(content); + } + if let Some(intent) = &version.intent { + row["intent"] = serde_json::json!(intent); + } + row + }).collect::>(), "frontier": plan.frontier, }); if json { @@ -1059,10 +1067,15 @@ fn plan_cmd(command: PlanCmd) -> Result<()> { } else { "" }; - println!( - " version {}{marker} resource={}", - version.identity, version.resource - ); + if let Some(content) = &version.content { + println!(" version {}{marker} content={content}", version.identity); + } else if let Some(intent) = &version.intent { + println!( + " version {}{marker} intent={}", + version.identity, + serde_json::to_string(intent)? + ); + } if !version.parents.is_empty() { println!(" parents: {}", version.parents.join(", ")); } @@ -1093,12 +1106,21 @@ fn plan_cmd(command: PlanCmd) -> Result<()> { plan.frontier.join(",") ); for version in &plan.versions { - println!( - " {}: {} -> {}", - version.identity, - version.resource, - version.resolved_resource.display() - ); + if let (Some(content), Some(resolved)) = + (&version.content, &version.resolved_content) + { + println!( + " {}: {content} -> {}", + version.identity, + resolved.display() + ); + } else if let Some(intent) = &version.intent { + println!( + " {}: inline intent={}", + version.identity, + serde_json::to_string(intent)? + ); + } } } Ok(()) diff --git a/src/plans.rs b/src/plans.rs index f1b5ea41..d2852718 100644 --- a/src/plans.rs +++ b/src/plans.rs @@ -30,12 +30,11 @@ pub struct Plan { pub referenced_by: Vec, } -/// Whether a plan is a top-level declaration or nested in an agent declaration. +/// A plan is always declared in its own plan KDL document. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "lowercase")] pub enum PlanSourceKind { External, - Inline, } /// One declared intent revision. @@ -45,8 +44,12 @@ pub struct PlanVersion { pub identity: String, pub parents: Vec, pub why: Option, - pub resource: String, - pub resolved_resource: PathBuf, + #[serde(skip_serializing_if = "Option::is_none")] + pub content: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub resolved_content: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub intent: Option, } /// A classified plan parse or validation error. @@ -85,7 +88,7 @@ impl std::error::Error for PlanError {} #[derive(Debug)] struct PlanReference { - owner: String, + agent: String, target: PathBuf, } @@ -135,10 +138,10 @@ pub fn load(selected: &Path) -> Result { return Err(PlanError::new( "ambiguous-plan-reference", &reference.target, - "a plan-ref target must contain exactly one top-level plan", + "a plan Resource target must contain exactly one top-level plan", )); } - plans[matches[0]].referenced_by.push(reference.owner); + plans[matches[0]].referenced_by.push(reference.agent); } let mut identities = BTreeMap::new(); @@ -237,7 +240,7 @@ fn parse_seed( for node in document.nodes() { match node.name().value() { "plan" => { - let plan = parse_plan(node, path, boundary, PlanSourceKind::External, None)?; + let plan = parse_plan(node, path, boundary)?; let index = plans.len(); plans.push(plan); external_by_source @@ -245,7 +248,7 @@ fn parse_seed( .or_default() .push(index); } - "agent" => parse_agent_plans(node, path, boundary, plans, references)?, + "agent" => parse_agent_plan_references(node, path, boundary, references)?, _ => {} } } @@ -261,7 +264,7 @@ fn parse_external_file( let document = read_document(path)?; for node in document.nodes() { if node.name().value() == "plan" { - let plan = parse_plan(node, path, boundary, PlanSourceKind::External, None)?; + let plan = parse_plan(node, path, boundary)?; let index = plans.len(); plans.push(plan); external_by_source @@ -274,7 +277,7 @@ fn parse_external_file( return Err(PlanError::new( "missing-plan", path, - "plan-ref target contains no top-level plan", + "plan Resource target contains no top-level plan", )); } Ok(()) @@ -289,38 +292,37 @@ fn read_document(path: &Path) -> Result { }) } -fn parse_agent_plans( +fn parse_agent_plan_references( agent: &KdlNode, source: &Path, boundary: &Path, - plans: &mut Vec, references: &mut Vec, ) -> Result<(), PlanError> { let Some(children) = agent.children() else { return Ok(()); }; - let owner = explicit_agent_identity(agent); + let agent_identity = explicit_agent_identity(agent); for child in children.nodes() { match child.name().value() { - "plan" => plans.push(parse_plan( - child, - source, - boundary, - PlanSourceKind::Inline, - Some(required_inline_owner(owner.as_deref(), source)?), - )?), - "plan-ref" => { - let target = exact_string_node(child, source, "plan-ref")?; - let target = resolve_file_reference(source, boundary, &target, "plan-ref")?; + "plan" | "plan-ref" => { + return Err(PlanError::new( + "unsupported-agent-plan-form", + source, + "agent plans must use a childless Resource binding tagged 'plan'", + )); + } + "resource" if is_declared_plan_resource(child) => { + let (_name, uri) = parse_plan_resource_binding(child, source)?; + let target = resolve_file_reference(source, boundary, &uri, "plan Resource uri")?; if target.extension().and_then(|value| value.to_str()) != Some("kdl") { return Err(PlanError::new( "invalid-plan-reference", source, - "plan-ref must resolve to a KDL file", + "a plan Resource must resolve to a KDL file", )); } references.push(PlanReference { - owner: required_inline_owner(owner.as_deref(), source)?, + agent: required_referencing_agent(agent_identity.as_deref(), source)?, target, }); } @@ -330,16 +332,64 @@ fn parse_agent_plans( Ok(()) } -fn required_inline_owner(owner: Option<&str>, source: &Path) -> Result { - owner.map(str::to_string).ok_or_else(|| { +fn required_referencing_agent(identity: Option<&str>, source: &Path) -> Result { + identity.map(str::to_string).ok_or_else(|| { PlanError::new( - "inline-owner-required", + "plan-reference-agent-required", source, - "an agent with inline plans or plan-ref must declare an explicit identity", + "an agent with a plan Resource must declare an explicit identity", ) }) } +fn is_declared_plan_resource(node: &KdlNode) -> bool { + node.entries().iter().any(|entry| { + entry.name().is_some_and(|name| name.value() == "_tag") + && entry.value().as_string() == Some("plan") + }) +} + +fn parse_plan_resource_binding( + node: &KdlNode, + source: &Path, +) -> Result<(String, String), PlanError> { + if node.children().is_some() { + return Err(PlanError::new( + "invalid-plan-resource-binding", + source, + "a plan Resource binding cannot have children", + )); + } + let (name, properties) = + exact_positional_with_properties(node, source, "plan Resource", &["_tag", "uri"])?; + let name = name.filter(|value| valid_text(value)).ok_or_else(|| { + PlanError::new( + "invalid-plan-resource-binding", + source, + "a plan Resource needs one non-empty local name", + ) + })?; + if properties.get("_tag").map(String::as_str) != Some("plan") { + return Err(PlanError::new( + "invalid-plan-resource-binding", + source, + format!("plan Resource '{name}' needs _tag=\"plan\""), + )); + } + let uri = properties + .get("uri") + .filter(|value| valid_text(value)) + .cloned() + .ok_or_else(|| { + PlanError::new( + "invalid-plan-resource-binding", + source, + format!("plan Resource '{name}' needs a non-empty uri"), + ) + })?; + Ok((name, uri)) +} + fn explicit_agent_identity(agent: &KdlNode) -> Option { let mut identity = positional_string(agent); if let Some(children) = agent.children() { @@ -352,13 +402,7 @@ fn explicit_agent_identity(agent: &KdlNode) -> Option { identity.filter(|value| valid_text(value)) } -fn parse_plan( - node: &KdlNode, - source: &Path, - boundary: &Path, - source_kind: PlanSourceKind, - inline_owner: Option, -) -> Result { +fn parse_plan(node: &KdlNode, source: &Path, boundary: &Path) -> Result { let identity = exact_positional_with_properties(node, source, "plan", &[])? .0 .ok_or_else(|| { @@ -378,11 +422,11 @@ fn parse_plan( format!("plan '{identity}' needs a body"), ) })?; - let mut owner = inline_owner; + let mut owner = None; let mut versions = Vec::new(); for child in children.nodes() { match child.name().value() { - "owner" if source_kind == PlanSourceKind::External => { + "owner" => { if owner.is_some() { return Err(PlanError::new( "duplicate-plan-owner", @@ -392,13 +436,6 @@ fn parse_plan( } owner = Some(exact_string_node(child, source, "owner")?); } - "owner" => { - return Err(PlanError::new( - "inline-owner-is-derived", - source, - format!("inline plan '{identity}' derives owner from its containing agent"), - )); - } "version" => versions.push(parse_version(child, source, boundary, &identity)?), other => { return Err(PlanError::new( @@ -432,7 +469,7 @@ fn parse_plan( versions, frontier, source: source.to_path_buf(), - source_kind, + source_kind: PlanSourceKind::External, referenced_by: Vec::new(), }) } @@ -444,7 +481,7 @@ fn parse_version( plan: &str, ) -> Result { let (identity, properties) = - exact_positional_with_properties(node, source, "version", &["resource"])?; + exact_positional_with_properties(node, source, "version", &["content"])?; let identity = identity.ok_or_else(|| { PlanError::new( "version-identity-required", @@ -459,17 +496,14 @@ fn parse_version( format!("plan '{plan}' has an invalid version identity"), )); } - let resource = properties.get("resource").cloned().ok_or_else(|| { - PlanError::new( - "version-resource-required", - source, - format!("plan '{plan}' version '{identity}' needs resource=\"file:...\""), - ) - })?; - let resolved_resource = - resolve_file_reference(source, boundary, &resource, "version resource")?; + let content = properties.get("content").cloned(); + let resolved_content = content + .as_deref() + .map(|reference| resolve_file_reference(source, boundary, reference, "version content")) + .transpose()?; let mut parents = Vec::new(); let mut why = None; + let mut intent = None; if let Some(children) = node.children() { for child in children.nodes() { match child.name().value() { @@ -486,6 +520,28 @@ fn parse_version( } why = Some(exact_string_node(child, source, "why")?); } + "intent" => { + if intent.is_some() { + return Err(PlanError::new( + "duplicate-version-intent", + source, + format!( + "plan '{plan}' version '{identity}' declares intent more than once" + ), + )); + } + let value = exact_intent_node(child, source)?; + if !valid_intent(&value) { + return Err(PlanError::new( + "invalid-version-intent", + source, + format!( + "plan '{plan}' version '{identity}' needs non-empty inline intent" + ), + )); + } + intent = Some(value); + } other => { return Err(PlanError::new( "unsupported-version-field", @@ -498,6 +554,27 @@ fn parse_version( } } } + match (content.is_some(), intent.is_some()) { + (true, true) => { + return Err(PlanError::new( + "ambiguous-version-intent", + source, + format!( + "plan '{plan}' version '{identity}' must use content or inline intent, not both" + ), + )); + } + (false, false) => { + return Err(PlanError::new( + "version-intent-required", + source, + format!( + "plan '{plan}' version '{identity}' needs content=\"file:...\" or inline intent" + ), + )); + } + _ => {} + } parents.sort(); if parents.windows(2).any(|pair| pair[0] == pair[1]) { return Err(PlanError::new( @@ -517,8 +594,9 @@ fn parse_version( identity, parents, why, - resource, - resolved_resource, + content, + resolved_content, + intent, }) } @@ -623,6 +701,24 @@ fn exact_string_node(node: &KdlNode, source: &Path, field: &str) -> Result Result { + let (value, properties) = exact_positional_with_properties(node, source, "intent", &[])?; + if !properties.is_empty() || node.children().is_some() { + return Err(PlanError::new( + "invalid-version-intent", + source, + "intent must be one string without properties or children", + )); + } + value.ok_or_else(|| { + PlanError::new( + "invalid-version-intent", + source, + "intent must be one string", + ) + }) +} + fn exact_positional_with_properties( node: &KdlNode, source: &Path, @@ -693,6 +789,13 @@ fn valid_text(value: &str) -> bool { !value.is_empty() && value.trim() == value && !value.chars().any(char::is_control) } +fn valid_intent(value: &str) -> bool { + !value.trim().is_empty() + && !value + .chars() + .any(|character| character.is_control() && !matches!(character, '\n' | '\r' | '\t')) +} + fn resolve_file_reference( source: &Path, boundary: &Path, @@ -753,7 +856,7 @@ mod tests { } #[test] - fn external_and_inline_forms_normalize_and_preserve_concurrent_frontier_heads() { + fn resource_links_discover_both_content_forms_and_preserve_frontier_heads() { let temporary = tempfile::tempdir().unwrap(); let root = temporary.path(); write( @@ -761,14 +864,23 @@ mod tests { "agent.kdl", r#" agent "worker" { - plan-ref "file:plans/shared/plan.kdl" - plan "local" { - version "v0" resource="file:plans/local-v0.md" + resource "shared-role" _tag="plan" uri="file:plans/shared/plan.kdl" + resource "local-role" _tag="plan" uri="file:plans/local/plan.kdl" +} +"#, + ); + write( + root, + "plans/local/plan.kdl", + r#" +plan "local" { + owner "cos" + version "v0" { + intent "Keep the complete local intent in plan.kdl." } } "#, ); - write(root, "plans/local-v0.md", "# Local\n"); write(root, "plans/shared/v0.md", "# v0\n"); write(root, "plans/shared/v1.md", "# v1\n"); write(root, "plans/shared/v2.md", "# v2\n"); @@ -778,12 +890,12 @@ agent "worker" { r#" plan "shared" { owner "cos" - version "v2" resource="file:v2.md" { + version "v2" content="file:v2.md" { parent "v0" why "Second concurrent direction." } - version "v0" resource="file:v0.md" - version "v1" resource="file:v1.md" { + version "v0" content="file:v0.md" + version "v1" content="file:v1.md" { parent "v0" why "First concurrent direction." } @@ -795,14 +907,20 @@ plan "shared" { assert_eq!(loaded.plans.len(), 2); let local = &loaded.plans[0]; assert_eq!(local.identity, "local"); - assert_eq!(local.owner, "worker"); + assert_eq!(local.owner, "cos"); assert_eq!(local.frontier, ["v0"]); - assert_eq!(local.source_kind, PlanSourceKind::Inline); + assert_eq!(local.source_kind, PlanSourceKind::External); + assert_eq!( + local.versions[0].intent.as_deref(), + Some("Keep the complete local intent in plan.kdl.") + ); + assert_eq!(local.referenced_by, ["worker"]); let shared = &loaded.plans[1]; assert_eq!(shared.owner, "cos"); assert_eq!(shared.frontier, ["v1", "v2"]); assert_eq!(shared.referenced_by, ["worker"]); + assert_eq!(shared.versions[0].content.as_deref(), Some("file:v0.md")); assert_eq!( shared .versions @@ -824,7 +942,7 @@ plan "shared" { plan "bad" { owner "cos" current "v0" - version "v0" resource="file:v0.md" + version "v0" content="file:v0.md" } "#, ); @@ -839,7 +957,7 @@ plan "bad" { r#" plan "bad" { owner "cos" - version "v1" resource="file:v0.md" { + version "v1" content="file:v0.md" { parent "missing" } } @@ -856,7 +974,7 @@ plan "bad" { r#" plan "bad" { owner "cos" - version "v1" resource="file:v0.md" { + version "v1" content="file:v0.md" { parent "missing" why "Revision." } @@ -874,11 +992,11 @@ plan "bad" { r#" plan "bad" { owner "cos" - version "v0" resource="file:v0.md" { + version "v0" content="file:v0.md" { parent "v1" why "First half." } - version "v1" resource="file:v0.md" { + version "v1" content="file:v0.md" { parent "v0" why "Second half." } @@ -897,19 +1015,31 @@ plan "bad" { "nested/agent.kdl", r#" agent "worker" { - plan "local" { - version "v0" resource="file:body.md" - } + resource "local-role" _tag="plan" uri="file:plan.kdl" } "#, ); write(root, "nested/body.md", "unchanged\n"); + write( + root, + "nested/plan.kdl", + r#" +plan "local" { + owner "cos" + version "v0" content="file:body.md" +} +"#, + ); let before = fs::read(root.join("nested/agent.kdl")).unwrap(); let loaded = load(root).unwrap(); assert_eq!( - loaded.plans[0].versions[0].resolved_resource, - root.join("nested/body.md").canonicalize().unwrap() + loaded.plans[0].versions[0] + .resolved_content + .as_ref() + .unwrap(), + &root.join("nested/body.md").canonicalize().unwrap() ); + assert_eq!(loaded.plans[0].referenced_by, ["worker"]); assert_eq!(fs::read(root.join("nested/agent.kdl")).unwrap(), before); assert_eq!( fs::read_to_string(root.join("nested/body.md")).unwrap(), @@ -925,10 +1055,71 @@ agent "worker" { r#" plan "escape" { owner "worker" - version "v0" resource="file:../outside.md" + version "v0" content="file:../outside.md" } "#, ); assert_eq!(load(&bounded).unwrap_err().code(), "plan-reference-escape"); } + + #[test] + fn agent_plan_truth_and_ambiguous_version_intent_are_rejected() { + let temporary = tempfile::tempdir().unwrap(); + write( + temporary.path(), + "agent.kdl", + r#" +agent "worker" { + plan-ref "file:plan.kdl" +} +"#, + ); + assert_eq!( + load(temporary.path()).unwrap_err().code(), + "unsupported-agent-plan-form" + ); + + write( + temporary.path(), + "agent.kdl", + r#" +agent "worker" { + resource "local-role" _tag="plan" uri="file:plan.kdl" { + owner "forbidden" + } +} +"#, + ); + assert_eq!( + load(temporary.path()).unwrap_err().code(), + "invalid-plan-resource-binding" + ); + + write( + temporary.path(), + "agent.kdl", + r#" +agent "worker" { + resource "local-role" _tag="plan" uri="file:plan.kdl" +} +"#, + ); + write(temporary.path(), "body.md", "body\n"); + write( + temporary.path(), + "plan.kdl", + r#" +plan "ambiguous" { + owner "cos" + version "v0" content="file:body.md" { + intent "This second source of intent is forbidden." + } +} +"#, + ); + assert_eq!( + load(temporary.path()).unwrap_err().code(), + "ambiguous-version-intent" + ); + } } diff --git a/tests/plans.rs b/tests/plans.rs index a27aeaec..0ebf49a7 100644 --- a/tests/plans.rs +++ b/tests/plans.rs @@ -16,14 +16,23 @@ fn fixture() -> tempfile::TempDir { "agent.kdl", r#" agent "worker" { - plan-ref "file:plans/shared/plan.kdl" - plan "local" { - version "0000" resource="file:plans/local.md" + resource "shared-role" _tag="plan" uri="file:plans/shared/plan.kdl" + resource "local-role" _tag="plan" uri="file:plans/local/plan.kdl" +} +"#, + ); + write( + root, + "plans/local/plan.kdl", + r#" +plan "local" { + owner "cos" + version "0000" { + intent "Keep the complete local intent in plan.kdl." } } "#, ); - write(root, "plans/local.md", "# Local\n"); write(root, "plans/shared/0000.md", "# Initial\n"); write(root, "plans/shared/0001.md", "# Left\n"); write(root, "plans/shared/0002.md", "# Right\n"); @@ -33,12 +42,12 @@ agent "worker" { r#" plan "shared" { owner "cos" - version "0000" resource="file:0000.md" - version "0001" resource="file:0001.md" { + version "0000" content="file:0000.md" + version "0001" content="file:0001.md" { parent "0000" why "Left branch." } - version "0002" resource="file:0002.md" { + version "0002" content="file:0002.md" { parent "0000" why "Right branch." } @@ -87,13 +96,21 @@ fn cli_validates_lists_shows_and_inspects_the_same_read_only_plan_model() { assert_eq!(shown["owner"], "cos"); assert_eq!(shown["frontier"], serde_json::json!(["0001", "0002"])); assert!(shown.get("source").is_none()); - assert!(shown["versions"][0].get("resolvedResource").is_none()); + assert_eq!(shown["versions"][0]["content"], "file:0000.md"); + assert!(shown["versions"][0].get("resolvedContent").is_none()); + + let inline = success_json(root, &["plan", "show", "local", "--json"]); + assert_eq!( + inline["versions"][0]["intent"], + "Keep the complete local intent in plan.kdl." + ); + assert!(inline["versions"][0].get("content").is_none()); let inspected = success_json(root, &["plan", "inspect", "shared", "--json"]); assert_eq!(inspected["sourceKind"], "external"); assert_eq!(inspected["referencedBy"], serde_json::json!(["worker"])); assert!( - inspected["versions"][0]["resolvedResource"] + inspected["versions"][0]["resolvedContent"] .as_str() .unwrap() .ends_with("/plans/shared/0000.md") @@ -118,7 +135,7 @@ fn cli_validation_is_nonzero_and_classified_for_out_of_scope_plan_fields() { plan "not-read-only" { owner "cos" current "0000" - version "0000" resource="file:v0.md" + version "0000" content="file:v0.md" } "#, ); @@ -129,3 +146,37 @@ plan "not-read-only" { assert_eq!(receipt["result"], "invalid"); assert_eq!(receipt["code"], "unsupported-plan-field"); } + +#[test] +fn cli_rejects_agent_owned_plan_truth_and_childful_plan_resources() { + let temporary = tempfile::tempdir().unwrap(); + write( + temporary.path(), + "agent.kdl", + r#" +agent "worker" { + plan-ref "file:plan.kdl" +} +"#, + ); + let old_form = run(temporary.path(), &["plan", "validate", "--json"]); + assert!(!old_form.status.success()); + let receipt: serde_json::Value = serde_json::from_slice(&old_form.stdout).unwrap(); + assert_eq!(receipt["code"], "unsupported-agent-plan-form"); + + write( + temporary.path(), + "agent.kdl", + r#" +agent "worker" { + resource "local-role" _tag="plan" uri="file:plan.kdl" { + owner "forbidden" + } +} +"#, + ); + let childful = run(temporary.path(), &["plan", "validate", "--json"]); + assert!(!childful.status.success()); + let receipt: serde_json::Value = serde_json::from_slice(&childful.stdout).unwrap(); + assert_eq!(receipt["code"], "invalid-plan-resource-binding"); +}