fix(templates): a template stopped accepting edits once it minted an agent

`upsert_builtin` replaced the role set with DELETE + reinsert. That looks
equivalent to an upsert and is not: `agent_template_link` carries a plain FK
on (template_id, role_slot), so the delete is rejected as soon as one agent
has been minted from the template, rolling back the whole transaction.

The failure mode was silent and self-targeting. The loader logs the error and
continues, so the on-disk TOML and the DB drifted apart — and only for the
templates someone had actually used. Running the smoke mission against
insight_research is what put it on the boot log:

    failed to load insight_research.toml: violates foreign key constraint
    "agent_template_link_template_id_role_slot_fkey"

which also means that template never received the skill-name fix.

- Upsert each role in place via ON CONFLICT (template_id, slot), the table's
  primary key.
- Prune only slots the TOML dropped, and skip a slot still referenced by a
  live agent with a log line. Keeping one stale role row is a smaller failure
  than discarding every edit to the template.
- Regression test drives the real sequence — upsert, mint an agent, link it,
  upsert again — and asserts both the prompt and skill edits land. Verified to
  fail without the fix with the same 23503 the server logged.

Co-Authored-By: Claude Opus 5 <[email protected]>

crates/cm-db/src/repo/team_templates.rs

@@ -125,17 +125,26 @@ pub async fn upsert_builtin(pool: &PgPool, b: UpsertBuiltin<'_>) -> Result<Uuid,
     .execute(&mut *tx)
     .await?;
 
-    // Replace-in-place role set. Roles that get removed from the TOML
+    // Upsert each role in place, then prune only the slots the TOML dropped.
-    // disappear from the DB; keeps the on-disk source authoritative.
+    //
-    sqlx::query("DELETE FROM template_roles WHERE template_id = $1")
+    // This was `DELETE FROM template_roles` + reinsert, which looks equivalent
-        .bind(id)
+    // and is not: `agent_template_link` carries a plain FK on
-        .execute(&mut *tx)
+    // (template_id, role_slot), so once a template has minted a single agent
-        .await?;
+    // the delete is rejected and the whole upsert transaction rolls back. The
+    // effect was that **a template stopped accepting edits the moment it was
+    // first used** — the loader logged a foreign-key error and moved on, so
+    // the on-disk TOML and the DB drifted apart silently, and only for the
+    // templates anyone actually ran.
     for r in &b.roles {
         sqlx::query(
             "INSERT INTO template_roles
                (template_id, slot, order_idx, system_prompt, skills, brain_seed)
-             VALUES ($1,$2,$3,$4,$5,$6)",
+             VALUES ($1,$2,$3,$4,$5,$6)
+             ON CONFLICT (template_id, slot) DO UPDATE SET
+                 order_idx     = EXCLUDED.order_idx,
+                 system_prompt = EXCLUDED.system_prompt,
+                 skills        = EXCLUDED.skills,
+                 brain_seed    = EXCLUDED.brain_seed",
         )
         .bind(id)
         .bind(r.slot)
@@ -147,6 +156,43 @@ pub async fn upsert_builtin(pool: &PgPool, b: UpsertBuiltin<'_>) -> Result<Uuid,
         .await?;
     }
 
+    // Prune removed slots, but never at the cost of the whole upsert: a slot
+    // still referenced by a live agent is left in place and reported. Losing
+    // one stale role row is a smaller failure than losing every edit to the
+    // template.
+    let slots: Vec<String> = b.roles.iter().map(|r| r.slot.to_string()).collect();
+    let stale: Vec<String> = sqlx::query_scalar(
+        "SELECT slot FROM template_roles
+          WHERE template_id = $1 AND slot <> ALL($2)",
+    )
+    .bind(id)
+    .bind(&slots)
+    .fetch_all(&mut *tx)
+    .await?;
+    for slot in stale {
+        let referenced: i64 = sqlx::query_scalar(
+            "SELECT count(*) FROM agent_template_link
+              WHERE template_id = $1 AND role_slot = $2",
+        )
+        .bind(id)
+        .bind(&slot)
+        .fetch_one(&mut *tx)
+        .await?;
+        if referenced > 0 {
+            eprintln!(
+                "team_templates: role {}.{slot} was removed from the TOML but {referenced} \
+                 agent(s) still reference it — keeping the row so the upsert can commit",
+                b.key
+            );
+            continue;
+        }
+        sqlx::query("DELETE FROM template_roles WHERE template_id = $1 AND slot = $2")
+            .bind(id)
+            .bind(&slot)
+            .execute(&mut *tx)
+            .await?;
+    }
+
     tx.commit().await?;
     Ok(id)
 }

crates/cm-db/tests/repos.rs

@@ -1,6 +1,6 @@
 use std::str::FromStr;
 
-use cm_db::repo::{agents, audit, credits, users, workspaces};
+use cm_db::repo::{agent_template_link, agents, audit, credits, team_templates, users, workspaces};
 use cm_db::DbError;
 use cm_domain::{
     AccessPolicy, Agent, AgentId, AgentScope, AgentStatus, HumanScope, Role, User, UserId,
@@ -229,3 +229,80 @@ async fn audit_log_appends_and_rejects_mutation() {
         .await;
     assert!(delete.is_err());
 }
+
+/// A template must stay editable after it has minted agents.
+///
+/// `agent_template_link` holds a plain FK on (template_id, role_slot), so the
+/// old delete-then-reinsert upsert was rejected the moment a template had been
+/// used — and because the loader logs and continues, the on-disk TOML and the
+/// DB drifted apart silently, for exactly the templates anyone actually ran.
+#[tokio::test]
+async fn a_template_with_live_agents_still_accepts_edits() {
+    let pool = cm_testkit::test_pool().await;
+    let ws = workspace();
+    workspaces::insert(&pool, &ws).await.unwrap();
+    let owner = user_in(&ws, Role::Owner);
+    users::insert(&pool, &owner).await.unwrap();
+
+    let id = uuid::Uuid::now_v7();
+    let build = |prompt: &'static str, extra: Vec<String>| team_templates::UpsertBuiltin {
+        id,
+        key: "fixture_team",
+        name: "Fixture",
+        stack: vec!["rust".into()],
+        default_topology: "pipeline",
+        risk_profile: "medium",
+        mcp_bundles: vec![],
+        version: 1,
+        description: None,
+        config: serde_json::json!({}),
+        category: "development",
+        roles: vec![team_templates::UpsertBuiltinRole {
+            slot: "coder",
+            order_idx: 0,
+            system_prompt: prompt,
+            skills: extra,
+            brain_seed: None,
+        }],
+    };
+    team_templates::upsert_builtin(&pool, build("first", vec![]))
+        .await
+        .unwrap();
+
+    // Mint an agent against the template — this is what a mission does.
+    let agent = Agent {
+        id: AgentId::new(),
+        workspace_id: ws.id,
+        name: "Fixture · coder".into(),
+        job_title: "coder".into(),
+        system_prompt: "first".into(),
+        avatar: String::new(),
+        accent: "#fff".into(),
+        wallpaper: String::new(),
+        managed_by: owner.id,
+        status: AgentStatus::Online,
+    };
+    agents::insert(&pool, &agent, &AccessPolicy::default())
+        .await
+        .unwrap();
+    agent_template_link::upsert(&pool, agent.id.as_uuid(), id, 1, "coder")
+        .await
+        .unwrap();
+
+    // The edit that used to fail with a foreign-key violation.
+    team_templates::upsert_builtin(
+        &pool,
+        build("second", vec!["write-rust-current-edition".into()]),
+    )
+    .await
+    .expect("a used template must still accept edits");
+
+    let detail = team_templates::get(&pool, id).await.unwrap().unwrap();
+    let role = detail.roles.iter().find(|r| r.slot == "coder").unwrap();
+    assert_eq!(role.system_prompt, "second", "the prompt edit applied");
+    assert_eq!(
+        role.skills,
+        vec!["write-rust-current-edition".to_string()],
+        "the skill edit applied",
+    );
+}