Phase 4 of the plan, plus the PLAN_COMPLETE decision and the gitea_forge
cleanup from Phase 5.
THE REVIEW UI
mission_plan and mission_roster have been complete and reachable by curl
since they shipped, with zero frontend. That matters more than a missing
screen usually would: the decide step is not a convenience, it IS the
safety mechanism. Approving a plan replaces the mission's phases; approving
a roster flips it to the composed engine. A gate nobody can reach is a gate
that is always open or always shut.
MissionProposalDrawer, modelled on LevelUpDrawer which already does
load → review → decide. Reached from a mission's SETUP tab. Verified end to
end against the live backend, not just compiled: a model proposed a roster,
approval flipped the mission to `composed`, and approval on a non-draft
mission was refused.
The plan view shows each phase's done_when, and says plainly when one is
absent — a phase without a completion condition is never judged and reports
completed whatever it did, so its absence is the thing worth seeing.
AND THE DEFECT BUILDING IT FOUND
Every refusal path computed a precise reason — "the mission is running, not
a draft", "no node can boot that backend any more" — logged it to stderr,
and returned a bare {"error":"bad request"}. The person who needed the
sentence was the one clicking Approve; they got two words, and the reason
went to a server log they cannot read.
ApiError::Refused(String) carries it now. Same argument ApiError::Unavailable
was added for ("a 500 with 'internal error' sent them looking for a bug that
was not there"), one status code down. Live: the 400 now reads "this mission
is completed — a roster can only be approved while it is a draft, because
approving one rewrites how the mission will run".
PLAN_COMPLETE, decided
The Skill-Use measurement found that int-xx-marker-protocol documents
PLAN_COMPLETE and task_card_parser never implemented it, so an agent
following the skill exactly was silently ignored. Implemented rather than
removed from the skill: the planner needs a way to say it is done
specifying, and agents already emit it.
Marker ids are now strictly INT-<digits>. `starts_with("INT-")` accepted the
range form `INT-01..02` — observed live — which parsed into an id matching
no real item, so a task card appeared for something that did not exist while
the two items it covered stayed open. Rejecting is right: an ignored marker
is visible, a plausible row is not.
GITEA_FORGE, REMOVED
Named in nine places, defined in none. Harmless while provision_claw ignored
the bundle list; once the list was honoured, an undefined name became a
capability an agent is told it has and does not. Removed from seven team
templates, a workflow recipe, the auto-provision path, and a dropdown a user
could pick it from.
A new test asserts every bundle a template names is defined in the runtime
config — and it immediately found `web_fetch` in two templates I had missed
removing by hand. Same shape as the skill-binding test, one layer up.
Agents reach the forge through git over HTTPS with the ambient GITEA_TOKEN,
which is why nothing ever broke.
Full workspace suite green (106 binaries); frontend builds clean.
Co-Authored-By: Claude Opus 5 <[email protected]>
436 lines
16 KiB
Rust
436 lines
16 KiB
Rust
//! Load builtin team template TOML recipes from disk into
|
|
//! `team_templates` + `template_roles` at server boot.
|
|
//!
|
|
//! Templates ship under `templates/teams/*.toml` (repo-root path,
|
|
//! bundled into the server container image). The loader is idempotent:
|
|
//! re-upserts every boot so template edits go live on the next
|
|
//! deploy without needing a manual migration.
|
|
|
|
use serde::Deserialize;
|
|
use sha2::{Digest, Sha256};
|
|
use sqlx::PgPool;
|
|
use std::path::PathBuf;
|
|
|
|
use cm_db::repo::team_templates::{upsert_builtin, UpsertBuiltin, UpsertBuiltinRole};
|
|
|
|
/// Deterministic id per builtin template key. Rolling to sha256 of a
|
|
/// stable namespace + the key gives us a v4-shaped id that never
|
|
/// changes across boots (the uuid crate's v5 feature isn't enabled in
|
|
/// the workspace and we didn't want to bump it just for this).
|
|
fn builtin_id(key: &str) -> uuid::Uuid {
|
|
let mut h = Sha256::new();
|
|
h.update(b"clawmates.builtin.team_template\x00");
|
|
h.update(key.as_bytes());
|
|
let digest = h.finalize();
|
|
let mut bytes = [0u8; 16];
|
|
bytes.copy_from_slice(&digest[..16]);
|
|
// Force the v4 layout so the id passes any downstream v4-shaped
|
|
// checks (version nibble = 4, variant bits = 10xx).
|
|
bytes[6] = (bytes[6] & 0x0f) | 0x40;
|
|
bytes[8] = (bytes[8] & 0x3f) | 0x80;
|
|
uuid::Uuid::from_bytes(bytes)
|
|
}
|
|
|
|
#[derive(Debug, Deserialize)]
|
|
struct TemplateFile {
|
|
key: String,
|
|
name: String,
|
|
#[serde(default)]
|
|
stack: Vec<String>,
|
|
#[serde(default = "default_category")]
|
|
category: String,
|
|
default_topology: String,
|
|
risk_profile: String,
|
|
#[serde(default)]
|
|
mcp_bundles: Vec<String>,
|
|
#[serde(default = "default_version")]
|
|
version: i32,
|
|
#[serde(default)]
|
|
description: Option<String>,
|
|
#[serde(default)]
|
|
config: serde_json::Value,
|
|
#[serde(default)]
|
|
roles: Vec<TemplateRoleFile>,
|
|
}
|
|
fn default_version() -> i32 {
|
|
1
|
|
}
|
|
|
|
fn default_category() -> String {
|
|
"development".to_string()
|
|
}
|
|
|
|
#[derive(Debug, Deserialize)]
|
|
struct TemplateRoleFile {
|
|
slot: String,
|
|
order_idx: i32,
|
|
system_prompt: String,
|
|
#[serde(default)]
|
|
skills: Vec<String>,
|
|
#[serde(default)]
|
|
brain_seed: Option<String>,
|
|
/// Which model this role's claw runs on. Omitted means the mint's default,
|
|
/// which is what every authored template does today — so adding the field
|
|
/// changes nothing until a template uses it.
|
|
#[serde(default)]
|
|
model: Option<String>,
|
|
}
|
|
|
|
fn templates_dir() -> PathBuf {
|
|
if let Ok(d) = std::env::var("CLAWMATES_TEAM_TEMPLATES_DIR") {
|
|
return PathBuf::from(d);
|
|
}
|
|
// Container default — Dockerfile copies templates/ to /etc/clawmates/templates.
|
|
let container = PathBuf::from("/etc/clawmates/templates/teams");
|
|
if container.exists() {
|
|
return container;
|
|
}
|
|
// Dev fallback — repo-relative.
|
|
PathBuf::from("templates/teams")
|
|
}
|
|
|
|
/// Read every `*.toml` under the templates dir and upsert. Skips files
|
|
/// that fail to parse but logs the reason so a broken template can't
|
|
/// block server boot entirely.
|
|
pub async fn load_builtins(pool: &PgPool) -> usize {
|
|
let dir = templates_dir();
|
|
let entries = match std::fs::read_dir(&dir) {
|
|
Ok(r) => r,
|
|
Err(e) => {
|
|
eprintln!(
|
|
"team_template_loader: templates dir {} not readable: {e} — skipping builtin seed",
|
|
dir.display()
|
|
);
|
|
return 0;
|
|
}
|
|
};
|
|
let mut loaded = 0usize;
|
|
for entry in entries.flatten() {
|
|
let path = entry.path();
|
|
if path.extension().and_then(|s| s.to_str()) != Some("toml") {
|
|
continue;
|
|
}
|
|
match load_one(pool, &path).await {
|
|
Ok(key) => {
|
|
loaded += 1;
|
|
eprintln!("team_template_loader: upserted builtin {key}");
|
|
}
|
|
Err(e) => {
|
|
eprintln!(
|
|
"team_template_loader: failed to load {}: {e}",
|
|
path.display()
|
|
);
|
|
}
|
|
}
|
|
}
|
|
loaded
|
|
}
|
|
|
|
async fn load_one(pool: &PgPool, path: &std::path::Path) -> Result<String, String> {
|
|
let text =
|
|
std::fs::read_to_string(path).map_err(|e| format!("read {}: {e}", path.display()))?;
|
|
let file: TemplateFile =
|
|
toml::from_str(&text).map_err(|e| format!("parse {}: {e}", path.display()))?;
|
|
let key = file.key.clone();
|
|
|
|
let roles: Vec<UpsertBuiltinRole<'_>> = file
|
|
.roles
|
|
.iter()
|
|
.map(|r| UpsertBuiltinRole {
|
|
slot: &r.slot,
|
|
order_idx: r.order_idx,
|
|
system_prompt: &r.system_prompt,
|
|
skills: r.skills.clone(),
|
|
brain_seed: r.brain_seed.as_deref(),
|
|
model: r.model.as_deref(),
|
|
})
|
|
.collect();
|
|
|
|
let builtin = UpsertBuiltin {
|
|
id: builtin_id(&file.key),
|
|
key: &file.key,
|
|
name: &file.name,
|
|
stack: file.stack.clone(),
|
|
default_topology: &file.default_topology,
|
|
risk_profile: &file.risk_profile,
|
|
mcp_bundles: file.mcp_bundles.clone(),
|
|
version: file.version,
|
|
description: file.description.as_deref(),
|
|
config: file.config.clone(),
|
|
category: &file.category,
|
|
roles,
|
|
};
|
|
let template_id = upsert_builtin(pool, builtin)
|
|
.await
|
|
.map_err(|e| format!("upsert {key}: {e}"))?;
|
|
|
|
// Bind skills to template roles (Slice 3.5d): replace the entire
|
|
// set so a TOML edit that removes a skill from a role's list also
|
|
// removes the DB binding. Skills the loader can't find by name
|
|
// are skipped with a log — usually means the skill file hasn't
|
|
// been authored yet (or the name is a typo).
|
|
if let Err(e) = cm_db::repo::skills_catalog::clear_template_role_skills(pool, template_id).await
|
|
{
|
|
eprintln!("team_template_loader: clear_template_role_skills({key}) failed: {e}");
|
|
}
|
|
// Unresolved names are aggregated into one line per template rather than
|
|
// logged individually: the per-name spam (128 lines at last count) scrolled
|
|
// past unread for long enough that every template's skill bindings were
|
|
// silently empty, because the TOMLs used snake_case slugs while the authored
|
|
// skills in `skills/**/*.md` use kebab-case names. A count is noticeable.
|
|
let mut unresolved: Vec<String> = Vec::new();
|
|
let mut bound = 0usize;
|
|
for role in &file.roles {
|
|
for (idx, skill_name) in role.skills.iter().enumerate() {
|
|
match cm_db::repo::skills_catalog::get_by_name(pool, None, skill_name).await {
|
|
Ok(Some(skill)) => {
|
|
// Pin the foundation + first-N-per-role skills for
|
|
// now; the finer policy (per-skill pin flag) lives
|
|
// in the skill's own metadata in a later slice.
|
|
let pin = idx < 2 || skill.tags.iter().any(|t| t == "foundation");
|
|
let bind = cm_db::repo::skills_catalog::AttachRoleSkill {
|
|
template_id,
|
|
slot: &role.slot,
|
|
skill_id: skill.id,
|
|
pin_in_context: pin,
|
|
order_idx: idx as i32,
|
|
};
|
|
if let Err(e) = cm_db::repo::skills_catalog::attach_role_skill(pool, bind).await
|
|
{
|
|
eprintln!(
|
|
"team_template_loader: attach skill {skill_name} → {key}.{}: {e}",
|
|
role.slot
|
|
);
|
|
} else {
|
|
bound += 1;
|
|
}
|
|
}
|
|
Ok(None) => unresolved.push(format!("{}.{skill_name}", role.slot)),
|
|
Err(e) => {
|
|
eprintln!("team_template_loader: lookup skill '{skill_name}' failed: {e}");
|
|
}
|
|
}
|
|
}
|
|
}
|
|
if unresolved.is_empty() {
|
|
eprintln!("team_template_loader: {key} — {bound} role skills bound");
|
|
} else {
|
|
eprintln!(
|
|
"team_template_loader: {key} — {bound} role skills bound, {} unresolved (no such skill authored under skills/): {}",
|
|
unresolved.len(),
|
|
unresolved.join(", "),
|
|
);
|
|
}
|
|
|
|
Ok(key)
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use std::collections::HashSet;
|
|
use std::path::{Path, PathBuf};
|
|
|
|
fn repo_root() -> PathBuf {
|
|
// crates/cm-api → repo root
|
|
Path::new(env!("CARGO_MANIFEST_DIR"))
|
|
.ancestors()
|
|
.nth(2)
|
|
.expect("repo root above crates/cm-api")
|
|
.to_path_buf()
|
|
}
|
|
|
|
fn authored_skill_names(dir: &Path, out: &mut HashSet<String>) {
|
|
let Ok(entries) = std::fs::read_dir(dir) else {
|
|
return;
|
|
};
|
|
for e in entries.flatten() {
|
|
let p = e.path();
|
|
if p.is_dir() {
|
|
authored_skill_names(&p, out);
|
|
} else if p.extension().is_some_and(|x| x == "md") {
|
|
let body = std::fs::read_to_string(&p).unwrap_or_default();
|
|
if let Some(name) = body
|
|
.lines()
|
|
.find_map(|l| l.strip_prefix("name:").map(str::trim))
|
|
{
|
|
out.insert(name.to_string());
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
fn referenced_skill_names() -> HashSet<String> {
|
|
let mut refs = HashSet::new();
|
|
let dir = repo_root().join("templates/teams");
|
|
for e in std::fs::read_dir(&dir)
|
|
.expect("templates/teams readable")
|
|
.flatten()
|
|
{
|
|
let body = std::fs::read_to_string(e.path()).unwrap_or_default();
|
|
let parsed: toml::Value = match body.parse() {
|
|
Ok(v) => v,
|
|
Err(e) => panic!("{:?} is not valid TOML: {e}", e),
|
|
};
|
|
if let Some(roles) = parsed.get("roles").and_then(|r| r.as_array()) {
|
|
for role in roles {
|
|
if let Some(skills) = role.get("skills").and_then(|s| s.as_array()) {
|
|
refs.extend(skills.iter().filter_map(|s| s.as_str()).map(str::to_string));
|
|
}
|
|
}
|
|
}
|
|
}
|
|
refs
|
|
}
|
|
|
|
/// Every skill authored under `skills/**/*.md` must be reachable by at
|
|
/// least one team role.
|
|
///
|
|
/// This is the half of the naming drift that was invisible: the TOMLs used
|
|
/// snake_case slugs (`write_rust`) while the authored skills use kebab-case
|
|
/// names (`write-rust-current-edition`), so `get_by_name` missed on every
|
|
/// lookup — no role got any skill, and ten authored skills were reachable
|
|
/// by nobody. Both halves are silent at runtime; only a test catches them.
|
|
#[test]
|
|
fn every_authored_skill_is_referenced_by_some_role() {
|
|
let mut authored = HashSet::new();
|
|
authored_skill_names(&repo_root().join("skills"), &mut authored);
|
|
assert!(
|
|
!authored.is_empty(),
|
|
"no authored skills found — check the skills/ path"
|
|
);
|
|
let referenced = referenced_skill_names();
|
|
let orphans: Vec<_> = authored.difference(&referenced).cloned().collect();
|
|
assert!(
|
|
orphans.is_empty(),
|
|
"authored skills no team role references (they can never reach an agent): {orphans:?}"
|
|
);
|
|
}
|
|
|
|
/// EVERY referenced name must resolve to an authored skill.
|
|
///
|
|
/// The other direction, and the one that was missing. Both existing tests
|
|
/// assert `authored ⊆ referenced` — true of all 30 authored skills, so both
|
|
/// passed while 55 of 85 bindings resolved to nothing and ten roles ran with
|
|
/// an empty context bundle.
|
|
///
|
|
/// The old comment on the test below called the gap "deliberately
|
|
/// aspirational". An aspirational binding is indistinguishable at runtime
|
|
/// from a typo: `get_by_name` returns Ok(None), the loader logs a line
|
|
/// nobody reads, and the role ships without the instructions its prompt
|
|
/// assumes it has. If a skill is worth naming it is worth authoring, and if
|
|
/// it is not, the name should not be in the template.
|
|
#[test]
|
|
fn every_referenced_skill_resolves_to_an_authored_one() {
|
|
let mut authored = HashSet::new();
|
|
authored_skill_names(&repo_root().join("skills"), &mut authored);
|
|
let referenced = referenced_skill_names();
|
|
let mut missing: Vec<_> = referenced.difference(&authored).cloned().collect();
|
|
missing.sort();
|
|
assert!(
|
|
missing.is_empty(),
|
|
"{} referenced skill(s) bind to nothing — the role gets no instructions \
|
|
and nothing errors: {missing:#?}",
|
|
missing.len()
|
|
);
|
|
}
|
|
|
|
/// A referenced name that matches no authored skill binds to nothing. Some
|
|
/// are deliberately aspirational, so this asserts the *resolvable* ones
|
|
/// stay resolvable rather than demanding every name exist.
|
|
#[test]
|
|
fn referenced_skills_that_exist_use_the_authored_spelling() {
|
|
let mut authored = HashSet::new();
|
|
authored_skill_names(&repo_root().join("skills"), &mut authored);
|
|
let referenced = referenced_skill_names();
|
|
let resolvable = referenced.intersection(&authored).count();
|
|
assert_eq!(
|
|
resolvable,
|
|
authored.len(),
|
|
"every authored skill should be referenced by its exact name",
|
|
);
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod bundle_tests {
|
|
use std::collections::HashSet;
|
|
|
|
fn repo() -> std::path::PathBuf {
|
|
std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
|
|
.join("../..")
|
|
.canonicalize()
|
|
.expect("repo root")
|
|
}
|
|
|
|
/// Every `mcp_bundles` name a template asks for must be one the runtime
|
|
/// config actually defines.
|
|
///
|
|
/// This was harmless while `provision_claw` wrote a constant bundle list
|
|
/// and ignored the templates. It is not harmless now that the list is
|
|
/// honoured: an undefined name is a capability the agent is told it has and
|
|
/// does not, which is the same failure as an unresolved skill binding one
|
|
/// layer down. `gitea_forge` was named by seven team templates, one
|
|
/// workflow recipe, the auto-provision path and a user-selectable dropdown,
|
|
/// and defined nowhere.
|
|
#[test]
|
|
fn every_named_mcp_bundle_is_defined_by_the_runtime_config() {
|
|
let cfg = std::fs::read_to_string(
|
|
repo().join("deploy/clawmates-runtime/agent.config.example.toml"),
|
|
)
|
|
.expect("runtime config");
|
|
let defined: HashSet<String> = cfg
|
|
.lines()
|
|
.filter_map(|l| l.trim().strip_prefix("[mcp_bundles."))
|
|
.filter_map(|r| r.strip_suffix(']'))
|
|
.map(|s| s.to_string())
|
|
.collect();
|
|
assert!(
|
|
defined.contains("clawmates_door"),
|
|
"parsed no bundles from the runtime config — the parser, not the \
|
|
templates, is what broke"
|
|
);
|
|
|
|
let mut missing: Vec<String> = Vec::new();
|
|
for dir in ["templates/teams", "templates/workflows"] {
|
|
for entry in std::fs::read_dir(repo().join(dir)).expect("template dir") {
|
|
let path = entry.expect("entry").path();
|
|
if path.extension().and_then(|e| e.to_str()) != Some("toml") {
|
|
continue;
|
|
}
|
|
let body = std::fs::read_to_string(&path).expect("read template");
|
|
for line in body.lines() {
|
|
let t = line.trim();
|
|
// Skip comments: several deliberately NAME a bundle while
|
|
// explaining that it is not delivered.
|
|
if t.starts_with('#') || !t.starts_with("mcp_bundles") {
|
|
continue;
|
|
}
|
|
let Some(inner) = t.split_once('[').and_then(|(_, r)| r.rsplit_once(']'))
|
|
else {
|
|
continue;
|
|
};
|
|
for name in inner.0.split(',') {
|
|
let name = name.trim().trim_matches('"');
|
|
if !name.is_empty() && !defined.contains(name) {
|
|
missing.push(format!(
|
|
"{}: {name}",
|
|
path.file_name().unwrap().to_string_lossy()
|
|
));
|
|
}
|
|
}
|
|
}
|
|
}
|
|
}
|
|
missing.sort();
|
|
missing.dedup();
|
|
assert!(
|
|
missing.is_empty(),
|
|
"{} template(s) name an MCP bundle the runtime does not define, so \
|
|
the agent is provisioned with a capability that resolves to \
|
|
nothing:\n {}",
|
|
missing.len(),
|
|
missing.join("\n ")
|
|
);
|
|
}
|
|
}
|