diff --git a/AGENTS.md b/AGENTS.md index 60e1d7f27..4b29df7c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,6 +101,7 @@ Per-hive (host + in-container): Wire types + socket plumbing: - **`hive-types/`** — leaf newtypes, chiefly the validated `Ident`. +- **`hive-template/`** — the one template language (minijinja, strict undefined): `render`, `variables`. - **`hive-sh4re/`** — shared payload types (`Message`, …). - **`hive-host-sock/`** (hivectl ↔ c0re), **`hive-priv-sock/`** (c0re ↔ priv), **`hive-core-agent-sock/`** (harness ↔ c0re, `/run/hive/mcp.sock`) diff --git a/Cargo.lock b/Cargo.lock index 8013bd1ab..44c2964d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2106,6 +2106,15 @@ dependencies = [ "uuid", ] +[[package]] +name = "hive-template" +version = "0.1.0" +dependencies = [ + "minijinja", + "serde", + "serde_json", +] + [[package]] name = "hive-types" version = "0.1.0" @@ -3176,6 +3185,12 @@ version = "2.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98" +[[package]] +name = "memo-map" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5449c8c750f1a07ea702bbd212bd999fceece9b3d1508b17023b3e174583124b" + [[package]] name = "mime" version = "0.3.18" @@ -3198,6 +3213,16 @@ dependencies = [ "unicase", ] +[[package]] +name = "minijinja" +version = "2.24.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86886cf6dbf4e614b19c9a1eec9775f021869d7eadde0fc73921a81b90c9b4c9" +dependencies = [ + "memo-map", + "serde", +] + [[package]] name = "miniz_oxide" version = "0.9.1" @@ -5254,6 +5279,7 @@ dependencies = [ "async-nats", "futures-util", "hive-log", + "hive-template", "opentelemetry", "opentelemetry_sdk", "serde", @@ -5338,6 +5364,7 @@ dependencies = [ name = "swarm-wire" version = "0.1.0" dependencies = [ + "hive-template", "hive-types", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 7c00b1ce7..a243805e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ members = [ "hive-sh4re", "hive-sock-client", "hive-subagent-mcp", + "hive-template", "hive-types", "hivectl", "swarm-agent-verify", @@ -115,6 +116,7 @@ hive-host-sock = { path = "hive-host-sock" } hive-priv-sock = { path = "hive-priv-sock" } hive-runtime = { path = "hive-runtime" } hive-sock-client = { path = "hive-sock-client" } +hive-template = { path = "hive-template" } hive-types = { path = "hive-types" } swarm-agent-verify = { path = "swarm-agent-verify" } swarm-authelia-bridge-sock = { path = "swarm-authelia-bridge-sock" } diff --git a/docs/2-handbook/swarm/ui.md b/docs/2-handbook/swarm/ui.md index 5dfc43b41..7c15d8d3a 100644 --- a/docs/2-handbook/swarm/ui.md +++ b/docs/2-handbook/swarm/ui.md @@ -403,7 +403,7 @@ the ends. Pick a node to edit it in the panel beside the canvas: - a **match** lists conditions, each an event field and the value it must equal; - a **todo** says who gets it (the agents an event field names, or one agent, picked from the agents the swarm has), whether it sets or clears a todo, the todo's key, and its text. The key and the - text can use `{field}` placeholders; + text are Jinja templates over the event's fields, such as `{{ repo }}#{{ number }}`; - a **request_reviewer** names the forge login the router asks to review the pull request. It ends a branch, like a todo, and the row of kinds above the canvas only offers it when the controller does. diff --git a/docs/4-implementation/swarm/notify-router.md b/docs/4-implementation/swarm/notify-router.md index 2c9945576..780d123d0 100644 --- a/docs/4-implementation/swarm/notify-router.md +++ b/docs/4-implementation/swarm/notify-router.md @@ -100,8 +100,8 @@ switched off, and the custom flows. Only the controller writes it, through compare-and-set. The controller answers 400 unless every flow passes the tree check above and reads only what exists: a source event from `issues`, `issue_assign`, `pull_request`, `pull_request_assign`, `pull_request_sync` or one of the -three `pull_request_review_*` events, `workflow_run_failure` or `workflow_run_cancelled`, and condition and recipient fields from the event's field list. The same goes for the `{field}` -placeholders in a todo's key and text. A `PUT` must carry the `revision` it read, `0` for no +three `pull_request_review_*` events, `workflow_run_failure` or `workflow_run_cancelled`, and condition and recipient fields from the event's field list. The same goes for the `{{ field }}` +variables in a todo's key and text, which are Jinja templates (see [`hive-template`](../../../hive-template/README.md)). A `PUT` must carry the `revision` it read, `0` for no document, and gets 409 when the stored revision is another. Swarm UI's [notification router card on `/swarm`](../../2-handbook/swarm/ui.md#notification-flows) shows the document and switches built-ins. @@ -112,7 +112,7 @@ accepts: - `source_events`: `[{name, description}]`, the source events. - `fields`: `[{name, description, values, values_from?, closed}]`, the condition - and recipient fields, and the `{field}` placeholders. `values` lists what the + and recipient fields, and the `{{ field }}` template variables. `values` lists what the field holds, as far as the controller knows, as `{name, description}` pairs, such as the `issue` and `PR` of `kind`. - `node_kinds`: `[{kind, description, fields}]` for `source`, `match`, `todo` @@ -197,10 +197,10 @@ anything else, such as a push to a branch with no PR, names no PR and the router ignores it. The `author` field is the account that opened the issue or PR, so a flow can send to it: -| node | setting | -| ------ | ------------------------------------------------------------- | -| source | `workflow_run_failure` | -| todo | `to` = field `author`, key `forge:{repo}#{number}:ci`, upsert | +| node | setting | +| ------ | --------------------------------------------------------------------- | +| source | `workflow_run_failure` | +| todo | `to` = field `author`, key `forge:{{ repo }}#{{ number }}:ci`, upsert | A clear of one key, such as `ci-cleared`'s, leaves the agent in the issue's holders, because it may still hold the assignment todo that `closed` must find. @@ -225,7 +225,10 @@ leaves it out, and runs the rest. | a document it can't read | the flows it ran before | | a read that fails | the flows it ran before | -"Can't read" means not JSON, not a flows document, or a `version` other than 1. +"Can't read" means not JSON, not a flows document, or a `version` other than 1 or 2. +A version 1 document, whose todo keys and texts used `{field}` placeholders, reads +as version 2 with each one converted to `{{ field }}`; the next save writes version 2, +and a router that predates version 2 keeps its flows until you upgrade it. Before the router has read any document, "before" means the built-ins, so a bad document never leaves it with no flows. The controller refuses to overwrite a document of a version it doesn't know. diff --git a/hive-template/Cargo.toml b/hive-template/Cargo.toml new file mode 100644 index 000000000..e54e2c52e --- /dev/null +++ b/hive-template/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "hive-template" +edition.workspace = true +version.workspace = true +readme = "README.md" + +[lints] +workspace = true + +[dependencies] +minijinja = { version = "2", default-features = false, features = [ + "builtins", + "serde", +] } +serde.workspace = true + +[dev-dependencies] +serde_json.workspace = true diff --git a/hive-template/README.md b/hive-template/README.md new file mode 100644 index 000000000..f56111250 --- /dev/null +++ b/hive-template/README.md @@ -0,0 +1,14 @@ +# hive-template + +The one template language the hyperhive workspace renders with: Jinja, through +[`minijinja`](https://crates.io/crates/minijinja). A value is `{{ name }}`, a +block is `{% if cond %}...{% endif %}`, and a lone `{` or `}` stays plain text. + +- **`render(template, &context)`**: the text, or a message when the template + does not parse or names a variable the context lacks. An unknown variable is + an error, never an empty string. +- **`variables(template)`**: the variables the template reads, so a caller can + refuse an unknown field when the template is saved instead of when it renders. + +Used by the notification router's flow templates (`swarm-notify-router`, +`swarm-wire`). diff --git a/hive-template/src/lib.rs b/hive-template/src/lib.rs new file mode 100644 index 000000000..3faaab9f2 --- /dev/null +++ b/hive-template/src/lib.rs @@ -0,0 +1,89 @@ +//! The one template language the workspace renders with: Jinja, through +//! `minijinja`, with an unknown variable an error rather than an empty string. +//! +//! A template is `{{ name }}` for a value and `{% if cond %}...{% endif %}` for +//! a block; a lone `{` or `}` is plain text. Callers pass their context as any +//! `Serialize` value and get back the text or a message naming what is wrong. + +use std::collections::BTreeSet; + +use minijinja::{Environment, UndefinedBehavior}; +use serde::Serialize; + +/// An environment that refuses a name the context does not hold, and keeps the +/// template's trailing newline. +fn environment() -> Environment<'static> { + let mut env = Environment::new(); + env.set_undefined_behavior(UndefinedBehavior::Strict); + env.set_keep_trailing_newline(true); + env +} + +/// `template` rendered against `context`. +/// +/// # Errors +/// +/// The template does not parse, or it names a variable `context` does not hold. +pub fn render(template: &str, context: &impl Serialize) -> Result { + environment() + .render_str(template, context) + .map_err(|e| e.to_string()) +} + +/// The variables `template` reads that it does not define itself, such as `repo` +/// for `{{ repo }}`. Lets a caller refuse a template that names an unknown field +/// before any event renders it. +/// +/// # Errors +/// +/// The template does not parse. +pub fn variables(template: &str) -> Result, String> { + let env = environment(); + let parsed = env.template_from_str(template).map_err(|e| e.to_string())?; + Ok(parsed.undeclared_variables(false).into_iter().collect()) +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn a_value_and_a_block_render() { + let ctx = json!({ "repo": "o/r", "role": "agent" }); + let out = render( + "{{ repo }}{% if role == \"agent\" %} (agent){% endif %}\n", + &ctx, + ); + assert_eq!(out.as_deref(), Ok("o/r (agent)\n")); + } + + #[test] + fn single_braces_are_plain_text() { + assert_eq!( + render("{not a name} and { open", &json!({})).as_deref(), + Ok("{not a name} and { open") + ); + } + + #[test] + fn an_unknown_variable_is_an_error() { + assert!(render("{{ numbr }}", &json!({ "number": "7" })).is_err()); + } + + #[test] + fn variables_lists_what_the_template_reads_not_what_it_defines() { + let found = variables("{{ repo }}#{{ number }}{% set x = 1 %}{{ x }}").expect("parses"); + assert_eq!( + found.into_iter().collect::>(), + ["number", "repo"].map(String::from) + ); + } + + #[test] + fn a_template_that_does_not_parse_is_refused() { + assert!(variables("{{ repo ").is_err()); + assert!(render("{% if %}", &json!({})).is_err()); + } +} diff --git a/swarm-controller/src/notify_flows.rs b/swarm-controller/src/notify_flows.rs index f003782c1..e0ddbaacb 100644 --- a/swarm-controller/src/notify_flows.rs +++ b/swarm-controller/src/notify_flows.rs @@ -375,24 +375,24 @@ mod tests { store .put( notify_flows::KEY, - br#"{"version":2,"flows":[]}"#.to_vec().into(), + br#"{"version":3,"flows":[]}"#.to_vec().into(), ) .await .expect("seed"); let got = get_flows(State(state.clone())) .await - .expect_err("version 2"); + .expect_err("version 3"); assert_eq!(got.status, Some(StatusCode::CONFLICT), "{got:?}"); let refused = put_flows(State(state.clone()), Ok(body(0, &[], vec![]))) .await - .expect_err("version 2 is kept"); + .expect_err("version 3 is kept"); assert_eq!(refused.status, Some(StatusCode::CONFLICT), "{refused:?}"); let kept = store .get(notify_flows::KEY) .await .expect("read") .expect("stored"); - assert_eq!(&kept[..], br#"{"version":2,"flows":[]}"#); + assert_eq!(&kept[..], br#"{"version":3,"flows":[]}"#); // Control: garbage that names no version is replaced by a save. let garbage = store @@ -454,11 +454,11 @@ mod tests { let mut typo = custom("typo"); if let NodeKind::Todo(sink) = &mut typo.nodes[1].kind { - sink.key = "forge:{repo}#{numbr}:triage".to_owned(); + sink.key = "forge:{{ repo }}#{{ numbr }}:triage".to_owned(); } let refused = put_flows(State(state.clone()), Ok(body(0, &[], vec![typo]))) .await - .expect_err("{numbr} is no field"); + .expect_err("numbr is no field"); assert_eq!(refused.status, Some(StatusCode::BAD_REQUEST), "{refused:?}"); // Control: the same flow spelled right is stored. let _ = put_flows(State(state), Ok(body(0, &[], vec![custom("typo")]))) diff --git a/swarm-controller/src/notify_router_api.rs b/swarm-controller/src/notify_router_api.rs index 2922df9e0..7f33ad2d3 100644 --- a/swarm-controller/src/notify_router_api.rs +++ b/swarm-controller/src/notify_router_api.rs @@ -377,11 +377,11 @@ mod tests { store .put( notify_flows::KEY, - br#"{"version":2,"flows":[]}"#.to_vec().into(), + br#"{"version":3,"flows":[]}"#.to_vec().into(), ) .await .expect("seed"); - let unreadable = get_dry_run(State(state)).await.expect_err("version 2"); + let unreadable = get_dry_run(State(state)).await.expect_err("version 3"); assert_eq!(unreadable.status, Some(StatusCode::CONFLICT)); } diff --git a/swarm-notify-router/Cargo.toml b/swarm-notify-router/Cargo.toml index 57fcef82a..daa2672bd 100644 --- a/swarm-notify-router/Cargo.toml +++ b/swarm-notify-router/Cargo.toml @@ -15,6 +15,8 @@ async-nats = { workspace = true, features = ["kv"] } # StreamExt / TryStreamExt on the pulled batch and the subject listing. futures-util.workspace = true hive-log.workspace = true +# The flow templates a todo node renders. +hive-template.workspace = true # The metrics the router exports (`metrics`), pushed through the OTLP exporter # `swarm-secret-client`'s `meter_provider` builds. opentelemetry.workspace = true diff --git a/swarm-notify-router/src/flow.rs b/swarm-notify-router/src/flow.rs index 9cd4bb3af..4b7e91f08 100644 --- a/swarm-notify-router/src/flow.rs +++ b/swarm-notify-router/src/flow.rs @@ -175,28 +175,20 @@ fn holds(condition: &Condition, fields: &Fields) -> bool { } } -/// `template` with each `{field}` replaced by that field's value, a list -/// joined with `, `. A name that is no field is an error naming it. +/// `template`, a Jinja template, rendered over the event's fields; a list field +/// reads as its items joined with `, `. A name that is no field is an error. pub fn expand(template: &str, fields: &Fields) -> Result { - let mut out = String::with_capacity(template.len()); - let mut rest = template; - while let Some(open) = rest.find('{') { - out.push_str(&rest[..open]); - let after = &rest[open + 1..]; - let Some(close) = after.find('}') else { - out.push_str(&rest[open..]); - return Ok(out); - }; - let name = &after[..close]; - match fields.get(name) { - Some(Value::Text(v)) => out.push_str(v), - Some(Value::List(items)) => out.push_str(&items.join(", ")), - None => return Err(format!("{{{name}}} is no event field")), - } - rest = &after[close + 1..]; - } - out.push_str(rest); - Ok(out) + let context: BTreeMap<&str, String> = fields + .iter() + .map(|(name, value)| { + let text = match value { + Value::Text(v) => v.clone(), + Value::List(items) => items.join(", "), + }; + (*name, text) + }) + .collect(); + hive_template::render(template, &context) } #[cfg(test)] @@ -387,9 +379,9 @@ mod tests { f.nodes[2].kind = NodeKind::Todo(TodoSink { to: Recipients::Agent("triage".to_owned()), op: Op::Upsert, - key: "forge:{repo}#{number}:triage".to_owned(), + key: "forge:{{ repo }}#{{ number }}:triage".to_owned(), prefix: false, - text: "triage {repo}#{number}".to_owned(), + text: "triage {{ repo }}#{{ number }}".to_owned(), }); let got = evaluate( &f, @@ -502,13 +494,10 @@ mod tests { fn a_placeholder_expands_to_its_field_and_an_unknown_one_is_an_error() { let fields = event("assigned", &["a", "b"], &[], &[]); assert_eq!( - expand("{repo}#{number} {assignees_added} {", &fields), + expand("{{ repo }}#{{ number }} {{ assignees_added }} {", &fields), Ok("hyperhive/hyperhive#7 a, b {".to_owned()) ); - assert_eq!( - expand("x {nope}", &fields), - Err("{nope} is no event field".to_owned()) - ); + assert!(expand("x {{ nope }}", &fields).is_err()); } /// A sink naming a placeholder the event lacks sends nothing, rather @@ -519,7 +508,7 @@ mod tests { f.nodes[2].kind = NodeKind::Todo(TodoSink { to: Recipients::Agent("triage".to_owned()), op: Op::Upsert, - key: "forge:{repo}#{numbr}:triage".to_owned(), + key: "forge:{{ repo }}#{{ numbr }}:triage".to_owned(), prefix: false, text: "t".to_owned(), }); @@ -527,7 +516,7 @@ mod tests { assert!(evaluate(&f, "issue_assign", &fields).is_empty()); // Control: spelled right, the same flow emits. if let NodeKind::Todo(s) = &mut f.nodes[2].kind { - s.key = "forge:{repo}#{number}:triage".to_owned(); + s.key = "forge:{{ repo }}#{{ number }}:triage".to_owned(); } assert_eq!(evaluate(&f, "issue_assign", &fields).len(), 1); } diff --git a/swarm-notify-router/src/stored.rs b/swarm-notify-router/src/stored.rs index ec8bbc386..904354a43 100644 --- a/swarm-notify-router/src/stored.rs +++ b/swarm-notify-router/src/stored.rs @@ -236,7 +236,7 @@ mod tests { ); for bad in [ junk(2, b"not json"), - junk(3, br#"{"version":2,"flows":[]}"#), + junk(3, br#"{"version":3,"flows":[]}"#), junk(4, br#"{"flows":[{"id":"x"}]}"#), Read::Failed("timeout".to_owned()), ] { diff --git a/swarm-wire/Cargo.toml b/swarm-wire/Cargo.toml index cfb3a5a4c..4b5345861 100644 --- a/swarm-wire/Cargo.toml +++ b/swarm-wire/Cargo.toml @@ -9,6 +9,7 @@ edition.workspace = true schema = ["dep:utoipa"] [dependencies] +hive-template.workspace = true hive-types.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/swarm-wire/src/notify_flows.rs b/swarm-wire/src/notify_flows.rs index 40c451371..b64840fba 100644 --- a/swarm-wire/src/notify_flows.rs +++ b/swarm-wire/src/notify_flows.rs @@ -27,8 +27,14 @@ pub const BUCKET: &str = "notify-flows"; /// The one key in [`BUCKET`]. pub const KEY: &str = "all"; -/// The document layout this build reads and writes. -pub const VERSION: u32 = 1; +/// The document layout this build reads and writes. Version 1 wrote a todo's +/// key and text with `{field}` placeholders; version 2 writes them as Jinja +/// templates (`{{ field }}`). [`Document::decode`] converts a version 1 +/// document, and the next save writes version 2. +pub const VERSION: u32 = 2; + +/// The layout [`Document::decode`] still reads and converts. +const LEGACY_VERSION: u32 = 1; /// The most nodes one flow may have. pub const MAX_NODES: usize = 64; @@ -85,7 +91,7 @@ pub const SOURCE_EVENTS: &[(&str, &str)] = &[ ]; /// The fields of an event a condition or a recipient can name, with what each -/// holds. A todo's key and text read them as `{field}` placeholders. +/// holds. A todo's key and text read them as `{{ field }}` in a template. pub const FIELDS: &[(&str, &str)] = &[ ("event", "The event type, one of the source events"), ("action", "What happened, such as opened, closed, assigned"), @@ -237,12 +243,12 @@ pub struct TodoSink { pub to: Recipients, #[cfg_attr(feature = "schema", schema(value_type = String))] pub op: Op, - /// The todo key, with `{field}` placeholders. + /// The todo key, a template over the event's [`FIELDS`]. pub key: String, /// For a clear: `key` is a prefix. #[serde(default)] pub prefix: bool, - /// The todo text, with `{field}` placeholders. Empty for a clear. + /// The todo text, a template over the event's [`FIELDS`]. Empty for a clear. #[serde(default)] pub text: String, } @@ -261,7 +267,7 @@ pub enum Recipients { /// Why [`Document::decode`] could not read a document. #[derive(Clone, Debug, PartialEq, Eq)] pub enum DecodeError { - /// A version other than [`VERSION`], such as a newer build's. + /// A version this build does not read, such as a newer build's. UnknownVersion(String), /// Not JSON, or not the shape of a [`Document`]. Malformed(String), @@ -271,7 +277,10 @@ impl std::fmt::Display for DecodeError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::UnknownVersion(v) => { - write!(f, "version {v}, and this build reads version {VERSION}") + write!( + f, + "version {v}, and this build reads versions {LEGACY_VERSION} and {VERSION}" + ) } Self::Malformed(why) => f.write_str(why), } @@ -284,14 +293,18 @@ impl Document { /// # Errors /// /// Not the expected shape, or a [`Document::version`] this build does not - /// know. The reader keeps what it had. + /// know. The reader keeps what it had. A version 1 document is + /// converted to [`VERSION`]. pub fn decode(bytes: &[u8]) -> Result { - let value: serde_json::Value = serde_json::from_slice(bytes) + let mut value: serde_json::Value = serde_json::from_slice(bytes) .map_err(|e| DecodeError::Malformed(format!("not JSON: {e}")))?; if let Some(found) = value.get("version") && found != VERSION { - return Err(DecodeError::UnknownVersion(found.to_string())); + if found != LEGACY_VERSION { + return Err(DecodeError::UnknownVersion(found.to_string())); + } + migrate_legacy(&mut value); } serde_json::from_value(value) .map_err(|e| DecodeError::Malformed(format!("not a flows document: {e}"))) @@ -573,11 +586,11 @@ pub fn schema() -> Schema { fields: vec![ to, op, - kind_field("key", "The todo key, with {field} placeholders", true), + kind_field("key", "The todo key, a template: {{ field }}", true), kind_field("prefix", "For a clear: key is a prefix", false), kind_field( "text", - "The todo text, with {field} placeholders; required for an upsert, empty for a clear", + "The todo text, a template: {{ field }}; required for an upsert, empty for a clear", false, ), ], @@ -596,19 +609,44 @@ pub fn schema() -> Schema { } } -/// The `{name}` placeholders in `template`, in order. A `{` with no `}` after -/// it is plain text. -#[must_use] -pub fn placeholders(template: &str) -> Vec<&str> { - let mut names = Vec::new(); +/// A version 1 document with its todo templates rewritten as version 2 ones and +/// its version set to [`VERSION`]. +fn migrate_legacy(document: &mut serde_json::Value) { + document["version"] = VERSION.into(); + let nodes = document["flows"] + .as_array_mut() + .into_iter() + .flatten() + .flat_map(|flow| flow["nodes"].as_array_mut().into_iter().flatten()); + for node in nodes.filter(|n| n["kind"] == "todo") { + for field in ["key", "text"] { + if let Some(template) = node[field].as_str() { + node[field] = jinja_of_legacy(template).into(); + } + } + } +} + +/// A version 1 template as a Jinja one: each `{name}` becomes `{{ name }}`. A `{` +/// with no `}` after it was plain text, and is kept so by writing it as a string. +fn jinja_of_legacy(template: &str) -> String { + let mut out = String::with_capacity(template.len()); let mut rest = template; while let Some(open) = rest.find('{') { + out.push_str(&rest[..open]); let after = &rest[open + 1..]; - let Some(close) = after.find('}') else { break }; - names.push(&after[..close]); + let Some(close) = after.find('}') else { + out.push_str("{{ \"{\" }}"); + rest = after; + continue; + }; + out.push_str("{{ "); + out.push_str(&after[..close]); + out.push_str(" }}"); rest = &after[close + 1..]; } - names + out.push_str(rest); + out } impl Flow { @@ -769,8 +807,10 @@ impl TodoSink { return Err(format!("text is longer than {MAX_TEXT} bytes")); } for (what, template) in [("key", &self.key), ("text", &self.text)] { - if let Some(bad) = placeholders(template).into_iter().find(|n| !is_field(n)) { - return Err(format!("{what} has {{{bad}}}, which is no event field")); + let read = hive_template::variables(template) + .map_err(|why| format!("{what} is not a valid template: {why}"))?; + if let Some(bad) = read.iter().find(|n| !is_field(n)) { + return Err(format!("{what} reads {bad}, which is no event field")); } } match self.op { @@ -800,7 +840,7 @@ impl TodoSink { /// replaces the old one. #[must_use] pub fn builtins() -> Vec { - let assignment_key = "forge:{repo}#{number}:assignment"; + let assignment_key = "forge:{{ repo }}#{{ number }}:assignment"; let mut flows = vec![ chain( "assigned", @@ -811,7 +851,7 @@ pub fn builtins() -> Vec { op: Op::Upsert, key: assignment_key.to_owned(), prefix: false, - text: "you are assigned to {kind} {repo}#{number}: {title} ({url})".to_owned(), + text: "you are assigned to {{ kind }} {{ repo }}#{{ number }}: {{ title }} ({{ url }})".to_owned(), }, ), chain( @@ -823,7 +863,7 @@ pub fn builtins() -> Vec { op: Op::Upsert, key: assignment_key.to_owned(), prefix: false, - text: "you are not assigned to {kind} {repo}#{number} anymore: {title} ({url})" + text: "you are not assigned to {{ kind }} {{ repo }}#{{ number }} anymore: {{ title }} ({{ url }})" .to_owned(), }, ), @@ -837,7 +877,7 @@ pub fn builtins() -> Vec { /// The flows that tell a PR's author about its CI: the todo for a failed run, /// the same key rewritten for a cancelled one, and the clear a push makes. fn ci_flows() -> Vec { - let ci_key = "forge:{repo}#{number}:ci"; + let ci_key = "forge:{{ repo }}#{{ number }}:ci"; let to_author = |op, text: &str| TodoSink { to: Recipients::Field("author".to_owned()), op, @@ -852,7 +892,7 @@ fn ci_flows() -> Vec { "failure", to_author( Op::Upsert, - "CI failed on {kind} {repo}#{number}: {title} ({url})", + "CI failed on {{ kind }} {{ repo }}#{{ number }}: {{ title }} ({{ url }})", ), ), chain( @@ -861,7 +901,7 @@ fn ci_flows() -> Vec { "cancelled", to_author( Op::Upsert, - "CI was cancelled on {kind} {repo}#{number}: {title} ({url})", + "CI was cancelled on {{ kind }} {{ repo }}#{{ number }}: {{ title }} ({{ url }})", ), ), chain( @@ -882,7 +922,7 @@ fn closed() -> Flow { TodoSink { to: Recipients::Field("todo_holders".to_owned()), op: Op::Clear, - key: "forge:{repo}#{number}:".to_owned(), + key: "forge:{{ repo }}#{{ number }}:".to_owned(), prefix: true, text: String::new(), }, @@ -892,9 +932,10 @@ fn closed() -> Flow { kind: NodeKind::Todo(TodoSink { to: Recipients::Field("todo_holders".to_owned()), op: Op::Upsert, - key: "forge:{repo}#{number}:closed".to_owned(), + key: "forge:{{ repo }}#{{ number }}:closed".to_owned(), prefix: false, - text: "{kind} {repo}#{number} was closed: {title} ({url})".to_owned(), + text: "{{ kind }} {{ repo }}#{{ number }} was closed: {{ title }} ({{ url }})" + .to_owned(), }), }); flow.edges.push(Edge { @@ -1022,9 +1063,9 @@ mod tests { f.nodes[2].kind = NodeKind::Todo(TodoSink { to: Recipients::Agent("triage".to_owned()), op: Op::Upsert, - key: "forge:{repo}#{number}:triage".to_owned(), + key: "forge:{{ repo }}#{{ number }}:triage".to_owned(), prefix: false, - text: "triage {repo}#{number}".to_owned(), + text: "triage {{ repo }}#{{ number }}".to_owned(), }); f } @@ -1371,19 +1412,47 @@ mod tests { f.validate() }; assert_eq!( - with("forge:{repo}#{number}:triage", "{title} {url}"), + with( + "forge:{{ repo }}#{{ number }}:triage", + "{{ title }} {{ url }}" + ), Ok(()) ); assert!( - with("forge:{repo}#{numbr}:triage", "t") + with("forge:{{ repo }}#{{ numbr }}:triage", "t") .unwrap_err() .contains("numbr") ); - assert!(with("k", "see {ulr}").unwrap_err().contains("ulr")); - assert!(with("k{}", "t").is_err(), "an empty name"); - // An unclosed brace is text. + assert!(with("k", "see {{ ulr }}").unwrap_err().contains("ulr")); + assert!(with("k{{ }}", "t").is_err(), "an empty name"); + assert!(with("k{{ repo", "t").is_err(), "an unclosed tag"); + // A lone brace is text. assert_eq!(with("k", "set {x"), Ok(())); - assert_eq!(placeholders("a{b}c{d} {e"), ["b", "d"]); + } + + #[test] + fn a_version_1_document_reads_with_its_templates_converted() { + let v1 = serde_json::json!({ + "version": 1, + "disabled_builtins": [], + "flows": [{ + "id": "c", "name": "c", "enabled": true, + "nodes": [ + {"id": "source", "kind": "source", "events": ["issues"]}, + {"id": "t", "kind": "todo", "to": {"agent": "triage"}, "op": "upsert", + "key": "forge:{repo}#{number}:x", "text": "{title} {url} set {x"}, + ], + "edges": [{"from": "source", "to": "t"}], + }], + }); + let doc = Document::decode(v1.to_string().as_bytes()).expect("reads"); + assert_eq!(doc.version, VERSION); + let NodeKind::Todo(sink) = &doc.flows[0].nodes[1].kind else { + panic!("a todo node"); + }; + assert_eq!(sink.key, "forge:{{ repo }}#{{ number }}:x"); + assert_eq!(sink.text, "{{ title }} {{ url }} set {{ \"{\" }}x"); + assert_eq!(doc.check(), Ok(())); } #[test] @@ -1434,7 +1503,7 @@ mod tests { let many = (0..=MAX_FLOWS).map(|i| custom(&format!("f{i}"))).collect(); assert!(doc(&[], many).check().is_err(), "flow count"); let mut old = doc(&[], vec![]); - old.version = 2; + old.version = 3; assert!(old.check().is_err(), "version"); } @@ -1492,8 +1561,8 @@ mod tests { assert_eq!(Document::decode(&bytes), Ok(doc)); assert_eq!(Document::decode(b"{}"), Ok(Document::default())); assert_eq!( - Document::decode(br#"{"version":2}"#), - Err(DecodeError::UnknownVersion("2".to_owned())) + Document::decode(br#"{"version":3}"#), + Err(DecodeError::UnknownVersion("3".to_owned())) ); assert!(Document::decode(b"nope").is_err()); assert!(Document::decode(br#"{"flows":[{"id":"x"}]}"#).is_err()); @@ -1510,7 +1579,7 @@ mod tests { "kind": "todo", "to": { "field": "todo_holders" }, "op": "clear", - "key": "forge:{repo}#{number}:", + "key": "forge:{{ repo }}#{{ number }}:", "prefix": true, "text": "", })