Skip to main content

agentmux_srv/server/app_api/
agent_define.rs

1use super::*;
2
3pub fn register(engine: &Arc<WshRpcEngine>, state: &AppState) {
4    register_agent_define(engine, state);
5}
6
7/// Rejects a non-empty `model_vendor_base_url` unless `provider_id`'s
8/// `ProviderConfig` declares support for redirection via
9/// `base_url_env_var`. Pure — no I/O — so it's directly unit-testable
10/// without the async `Store`/`Broker` harness `agent_define_core` needs.
11/// An empty `base_url` is always fine — that's "use the harness's default
12/// vendor endpoint," never rejected regardless of provider.
13///
14/// `pub(crate)` (not `pub(super)`) so `server::agent_handlers::template`'s
15/// `agentdefcreatefromtemplate` and `server::agent_handlers::core`'s
16/// `updateagent` handlers can reuse it too, instead of duplicating this
17/// check for the human-facing creation/edit paths.
18pub(crate) fn validate_vendor_base_url(provider_id: &str, base_url: &str) -> Result<(), String> {
19    if base_url.is_empty() {
20        return Ok(());
21    }
22    match providers::get_provider(provider_id) {
23        Some(p) if p.base_url_env_var.is_some() => Ok(()),
24        _ => Err(format!(
25            "agent.define: provider '{provider_id}' does not support a custom model vendor base URL"
26        )),
27    }
28}
29
30/// An agent's provider is read-only once set: its sessions, linked accounts
31/// and native memory all belong to that harness, so switching it would
32/// strand them. Run the same setup on another harness by forking the agent.
33/// The lock moved here from the agent's bundle
34/// (SPEC_AGENT_BUNDLE_FORMAT_V0_3_2026_10_05.md §3.1). An empty stored
35/// provider can still be set once.
36pub(crate) fn check_provider_unchanged(agent_id: &str, existing: &str, incoming: &str) -> Result<(), String> {
37    if existing.is_empty() || existing == incoming {
38        return Ok(());
39    }
40    Err(format!(
41        "FORBIDDEN: agent {agent_id} provider is readonly once set (has '{existing}', got '{incoming}'); fork the agent to run it on another provider"
42    ))
43}
44
45/// Infer a provider slug from a model name prefix.
46/// Only maps prefixes that correspond to a registered provider slug.
47/// Callers must still validate the result via `providers::get_provider`.
48pub(super) fn infer_provider_from_model(model: &str) -> String {
49    let m = model.to_lowercase();
50    if m.starts_with("claude") {
51        "claude".to_string()
52    } else if m.starts_with("gemini") {
53        "gemini".to_string()
54    } else if m.starts_with("codex") {
55        "codex".to_string()
56    } else if m.starts_with("qwen") {
57        "qwen".to_string()
58    } else if m.starts_with("kimi") {
59        "kimi".to_string()
60    } else {
61        // Unknown prefix — return as-is; get_provider will reject it with
62        // a "cannot infer provider" error so callers know to set provider explicitly.
63        model.to_string()
64    }
65}
66
67/// Returns `(agent_row_id, newly_inserted)`. In the consolidated model the
68/// definition row IS the agent, so the "stub instance" that makes a defined
69/// agent visible in My Agents is that same row carrying an `instance_name`;
70/// `instance_create` folds into it. `newly_inserted = false` when the row
71/// already carries a name (the stub already existed); callers use this to
72/// avoid broadcasting `agents:changed` on no-op calls. The returned id is
73/// the row's id, which is what `instance_create` reports back.
74pub(super) fn make_stub_idempotent(
75    mstore: &crate::backend::storage::store::Store,
76    def_id: &str,
77    name: &str,
78    now: i64,
79) -> Result<(String, bool), String> {
80    if let Some(existing) = mstore
81        .instance_get(def_id)
82        .map_err(|e| format!("agent.define: read agent row: {e}"))?
83    {
84        if !existing.instance_name.is_empty() {
85            return Ok((existing.id, false));
86        }
87    }
88    let stub_id = format!("si-{}", def_id.replace('-', ""));
89    let inst = AgentInstance {
90        id: stub_id.clone(),
91        definition_id: def_id.to_string(),
92        parent_instance_id: String::new(),
93        block_id: String::new(),
94        session_id: String::new(),
95        status: "stopped".to_string(),
96        github_context: String::new(),
97        started_at: now,
98        ended_at: 0,
99        created_at: now,
100        identity_id: String::new(),
101        memory_id: String::new(),
102        instance_name: name.to_string(),
103        working_directory: String::new(),
104        display_hidden: false,
105    };
106    match mstore.instance_create(&inst) {
107        Ok(canonical) => Ok((canonical.id, true)),
108        Err(e) => Err(format!("agent.define: create stub instance: {e}")),
109    }
110}
111
112/// Core logic for the `agent.define` command, shared by the WebSocket RPC
113/// handler and the HTTP service dispatch (`("agent", "define")` in service.rs).
114/// Persist `system_prompt` and `env` content blobs for a freshly created or
115/// updated agent definition.  Errors are logged but not propagated — the
116/// definition row is already committed and the caller has already published
117/// `agents:changed`, so a content-write failure must not abort the response.
118pub(super) fn persist_define_content(
119    mstore: &Store,
120    agent_id: &str,
121    cmd: &CommandAgentDefineData,
122    now: i64,
123) {
124    if let Some(prompt) = &cmd.system_prompt {
125        if !prompt.is_empty() {
126            if let Err(e) = mstore.agent_content_set(&AgentContent {
127                agent_id: agent_id.to_string(),
128                content_type: "agentmd".to_string(),
129                content: prompt.clone(),
130                updated_at: now,
131            }) {
132                tracing::warn!(agent_id, err = %e, "agent.define: failed to persist system_prompt (non-fatal)");
133            }
134        }
135    }
136    if let Some(env_map) = &cmd.env {
137        if !env_map.is_empty() {
138            let content = env_map.iter()
139                .map(|(k, v)| format!("{}={}", k, v))
140                .collect::<Vec<_>>()
141                .join("\n");
142            if let Err(e) = mstore.agent_content_set(&AgentContent {
143                agent_id: agent_id.to_string(),
144                content_type: "env".to_string(),
145                content,
146                updated_at: now,
147            }) {
148                tracing::warn!(agent_id, err = %e, "agent.define: failed to persist env (non-fatal)");
149            }
150        }
151    }
152}
153
154fn register_agent_define(engine: &Arc<WshRpcEngine>, state: &AppState) {
155    let mstore = state.mstore.clone();
156    let id_store = state.id_store.clone();
157    let broker = state.broker.clone();
158
159    engine.register_typed(COMMAND_AGENT_DEFINE, move |cmd: CommandAgentDefineData, _ctx| {
160        let mstore = mstore.clone();
161        let id_store = id_store.clone();
162        let broker = broker.clone();
163        async move { agent_define_core(mstore, id_store, broker, cmd).await }
164    });
165}
166
167#[cfg(test)]
168mod tests {
169    use super::*;
170
171    #[test]
172    fn empty_base_url_is_always_valid() {
173        // Even for a provider with no base_url_env_var — "unset" never needs
174        // the capability to exist.
175        assert!(validate_vendor_base_url("codex", "").is_ok());
176        assert!(validate_vendor_base_url("claude", "").is_ok());
177        assert!(validate_vendor_base_url("nonexistent-provider", "").is_ok());
178    }
179
180    #[test]
181    fn non_empty_base_url_accepted_for_a_supporting_provider() {
182        assert!(validate_vendor_base_url("claude", "https://my-proxy.example.com").is_ok());
183    }
184
185    #[test]
186    fn non_empty_base_url_rejected_for_a_non_supporting_provider() {
187        let err = validate_vendor_base_url("codex", "https://my-proxy.example.com").unwrap_err();
188        assert!(err.contains("codex"));
189        assert!(err.contains("does not support"));
190    }
191
192    #[test]
193    fn non_empty_base_url_rejected_for_an_unknown_provider() {
194        assert!(validate_vendor_base_url("not-a-real-provider", "https://x").is_err());
195    }
196}
197
198// reagent P1 on PR #2505: agent.define's update path had no way to distinguish
199// "don't touch model_vendor_base_url" from "clear it" (a plain empty String
200// always meant "don't touch"). Combined with the authoritative validation
201// re-running on every update against whatever is currently stored, a provider
202// change away from a vendor-capable provider while an old override sat there
203// permanently blocked every future agent.define call for that agent — there
204// was no way to ever un-set the stale value. Fixed via Option<String>: `None`
205// = don't touch, `Some("")` = explicit clear.
206#[cfg(test)]
207mod agent_define_core_vendor_tests {
208    use super::*;
209    use crate::server::tests::test_state;
210
211    fn cmd(value: serde_json::Value) -> CommandAgentDefineData {
212        serde_json::from_value(value).unwrap()
213    }
214
215    #[tokio::test]
216    async fn omitting_the_field_on_update_does_not_clear_a_previously_set_override() {
217        let state = test_state();
218
219        let created = agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
220            "name": "vendor-test-agent",
221            "provider": "claude",
222            "model_vendor_base_url": "https://my-proxy.example.com",
223        }))).await.unwrap();
224        let def = state.mstore.agent_def_get(&created.definition_id).unwrap().unwrap();
225        assert_eq!(def.model_vendor_base_url, "https://my-proxy.example.com");
226
227        // Update that touches an unrelated field and omits model_vendor_base_url.
228        agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
229            "name": "vendor-test-agent",
230            "if_exists": "update",
231            "description": "just touching something else",
232        }))).await.unwrap();
233        let def = state.mstore.agent_def_get(&created.definition_id).unwrap().unwrap();
234        assert_eq!(
235            def.model_vendor_base_url, "https://my-proxy.example.com",
236            "an omitted field must not clear the stored override"
237        );
238    }
239
240    #[tokio::test]
241    async fn explicit_empty_string_clears_a_previously_set_override() {
242        let state = test_state();
243
244        let created = agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
245            "name": "vendor-clear-test",
246            "provider": "claude",
247            "model_vendor_base_url": "https://my-proxy.example.com",
248        }))).await.unwrap();
249
250        agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
251            "name": "vendor-clear-test",
252            "if_exists": "update",
253            "model_vendor_base_url": "",
254        }))).await.unwrap();
255        let def = state.mstore.agent_def_get(&created.definition_id).unwrap().unwrap();
256        assert_eq!(def.model_vendor_base_url, "", "explicit empty string must clear the override");
257    }
258
259    // The provider is read-only once set (SPEC_AGENT_BUNDLE_FORMAT_V0_3_2026_10_05.md
260    // §3.1), so the old "change provider and clear a stale override in the
261    // same call" path is now a refusal; the override itself still updates.
262    #[tokio::test]
263    async fn an_update_cannot_change_the_provider_but_can_change_the_rest() {
264        let state = test_state();
265
266        let created = agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
267            "name": "provider-lock-test",
268            "provider": "claude",
269            "model_vendor_base_url": "https://my-proxy.example.com",
270        }))).await.unwrap();
271
272        for change in [json!({ "provider": "codex" }), json!({ "model": "gemini-2.5-pro" })] {
273            let mut req = json!({ "name": "provider-lock-test", "if_exists": "update" });
274            req.as_object_mut().unwrap().extend(change.as_object().unwrap().clone());
275            let err = agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(req))
276                .await
277                .unwrap_err();
278            assert!(err.contains("readonly once set"), "{err}");
279        }
280
281        agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
282            "name": "provider-lock-test",
283            "if_exists": "update",
284            "provider": "claude",
285            "model_vendor_base_url": "",
286            "description": "updated",
287        }))).await.unwrap();
288        let def = state.mstore.agent_def_get(&created.definition_id).unwrap().unwrap();
289        assert_eq!(def.provider, "claude");
290        assert_eq!(def.model_vendor_base_url, "");
291        assert_eq!(def.description, "updated");
292    }
293
294    #[test]
295    fn check_provider_unchanged_allows_setting_an_empty_provider_once() {
296        assert!(check_provider_unchanged("a", "", "codex").is_ok());
297        assert!(check_provider_unchanged("a", "codex", "codex").is_ok());
298        assert!(check_provider_unchanged("a", "codex", "claude").is_err());
299    }
300}
301
302// Mandatory ABF (ARCHITECTURE_MANDATORY_ABF_RETHINK_2026_08_14.md §3.2):
303// a fresh `agent.define` call provisions its own dedicated bundle.
304#[cfg(test)]
305mod agent_define_core_bundle_provisioning_tests {
306    use super::*;
307    use crate::server::tests::test_state;
308
309    fn cmd(value: serde_json::Value) -> CommandAgentDefineData {
310        serde_json::from_value(value).unwrap()
311    }
312
313    #[tokio::test]
314    async fn fresh_insert_provisions_its_own_bundle_and_the_agent_keeps_the_provider() {
315        let state = test_state();
316
317        let created = agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
318            "name": "define-bundle-test",
319            "provider": "gemini",
320        }))).await.unwrap();
321
322        let def = state.mstore.agent_def_get(&created.definition_id).unwrap().unwrap();
323        assert!(!def.memory_id.is_empty(), "fresh agent.define insert must bind a bundle");
324        let bundle = state.mstore.bundle_get(&def.memory_id).unwrap().unwrap();
325        assert!(!bundle.is_blank);
326        assert_eq!(def.provider, "gemini");
327        assert_eq!((bundle.provider.as_str(), bundle.model.as_str()), ("", ""), "the bundle carries no harness");
328    }
329
330    #[tokio::test]
331    async fn skip_of_an_existing_definition_does_not_leak_a_stray_bundle() {
332        let state = test_state();
333
334        let first = agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
335            "name": "skip-no-leak-test",
336            "provider": "claude",
337            "if_exists": "skip",
338        }))).await.unwrap();
339        assert_eq!(first.action, "created");
340        let bundles_after_first = state.mstore.bundle_list().unwrap().len();
341
342        // Same name again, if_exists=skip — must resolve to the SAME
343        // definition without creating (and leaking) a second bundle. The
344        // bundle-provision step only runs on agent_define_core's
345        // genuinely-fresh-insert path, not the atomic find-or-insert's
346        // own lookup, precisely so idempotent callers don't leak one
347        // unbound bundle per repeated call.
348        let second = agent_define_core(state.mstore.clone(), state.id_store.clone(), state.broker.clone(), cmd(json!({
349            "name": "skip-no-leak-test",
350            "provider": "claude",
351            "if_exists": "skip",
352        }))).await.unwrap();
353        assert_eq!(second.action, "skipped");
354        assert_eq!(second.definition_id, first.definition_id);
355        let bundles_after_second = state.mstore.bundle_list().unwrap().len();
356        assert_eq!(bundles_after_first, bundles_after_second, "skip path must not leak an extra bundle");
357    }
358}
359
360/// Atomically allocate an agent working directory.
361///
362/// Tries to atomically create `desired` via `std::fs::create_dir`. If
363/// that fails because the directory already exists, tries `<desired>-1`,
364/// `<desired>-2`, …, up to `-99`. The atomic `create_dir` (NOT
365/// `create_dir_all` for the leaf) is the reservation mechanism: two
366/// concurrent callers competing for the same path race on the OS
367/// `mkdir` syscall and one wins; the loser sees `AlreadyExists` and
368/// moves on.
369///
370/// Caller is responsible for distinguishing auto-generated paths from
371/// user-specified ones — this function rewrites the path on collision,
372/// which would clobber a user's intent if they pointed an agent at
373/// `~/projects/myrepo` and that already had a `CLAUDE.md`.
374pub fn allocate_agent_workdir(desired: &str) -> Result<String, String> {
375    let p = std::path::Path::new(desired);
376    if let Some(parent) = p.parent() {
377        if !parent.as_os_str().is_empty() {
378            std::fs::create_dir_all(parent)
379                .map_err(|e| format!("allocate_agent_workdir: parent {}: {e}", parent.display()))?;
380        }
381    }
382    match std::fs::create_dir(p) {
383        Ok(()) => return Ok(desired.to_string()),
384        Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {}
385        Err(e) => return Err(format!("allocate_agent_workdir: create_dir({}): {e}", desired)),
386    }
387    for n in 1..=99u32 {
388        let candidate = format!("{desired}-{n}");
389        match std::fs::create_dir(std::path::Path::new(&candidate)) {
390            Ok(()) => return Ok(candidate),
391            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
392            Err(e) => return Err(format!("allocate_agent_workdir: create_dir({candidate}): {e}")),
393        }
394    }
395    Err(format!(
396        "allocate_agent_workdir: too many collisions (>99) under {desired}-N — clean up old runs"
397    ))
398}
399
400pub(crate) async fn agent_define_core(
401    mstore: Arc<Store>,
402    id_store: Arc<Store>,
403    broker: Arc<crate::backend::mps::Broker>,
404    cmd: CommandAgentDefineData,
405) -> Result<AgentDefineResult, String> {
406    if cmd.name.trim().is_empty() {
407        return Err("agent.define: name is required".to_string());
408    }
409
410    // Validate if_exists early so a typo is caught even for new definitions,
411    // not only when a matching definition already exists.
412    let if_exists = cmd.if_exists.as_deref().unwrap_or("skip");
413    if !matches!(if_exists, "skip" | "update" | "error") {
414        return Err(format!(
415            "agent.define: unknown if_exists value '{if_exists}'; valid: skip, update, error"
416        ));
417    }
418
419    // Resolve provider: explicit `provider` wins; fall back to inference from
420    // `model` prefix; default to "claude" when neither is supplied.
421    let provider = if !cmd.provider.is_empty() {
422        if providers::get_provider(&cmd.provider).is_none() {
423            return Err(format!(
424                "agent.define: unknown provider '{}'; valid: claude, codex, gemini, qwen, kimi, openclaw, pi, copilot",
425                cmd.provider
426            ));
427        }
428        cmd.provider.clone()
429    } else if !cmd.model.is_empty() {
430        let inferred = agent_define::infer_provider_from_model(&cmd.model);
431        if providers::get_provider(&inferred).is_none() {
432            return Err(format!(
433                "agent.define: cannot infer provider from model '{}'; set provider explicitly",
434                cmd.model
435            ));
436        }
437        inferred
438    } else {
439        "claude".to_string()
440    };
441
442    let create_stub = cmd.create_instance_stub.unwrap_or(true);
443
444    let now = agentmux_common::time::now_ms();
445
446    // Gates the fresh-insert path below (the "no existing match" case, where
447    // `def` — built from `provider` — is what actually gets written). The
448    // "update" branch further down re-validates against the EXISTING
449    // agent's actual provider (not this possibly-defaulted `provider`,
450    // which may not reflect an unspecified `cmd.provider` on an update
451    // call) right before its own write — this check doesn't gate that path.
452    let cmd_model_vendor_base_url = cmd.model_vendor_base_url.clone().unwrap_or_default();
453    agent_define::validate_vendor_base_url(&provider, &cmd_model_vendor_base_url)?;
454
455    // Build the new definition struct up-front so agent_def_find_or_insert
456    // can use it as both the lookup key and the insert payload.
457    // agent_def_find_or_insert holds a single mutex guard for the check +
458    // conditional insert — closing the TOCTOU window between list and insert.
459    let mut def = AgentDefinition {
460        id: uuid::Uuid::new_v4().to_string(),
461        slug: String::new(), // resolved by agent_def_find_or_insert
462        name: cmd.name.clone(),
463        icon: cmd.icon.clone(),
464        provider: provider.clone(),
465        description: cmd.description.clone(),
466        working_directory: cmd.working_directory.clone(),
467        shell: cmd.shell.clone(),
468        environment: cmd.environment.clone(),
469        // Persist the requested model as a CLI flag so the agent launches
470        // with the specified model rather than the provider default.
471        provider_flags: if cmd.model.is_empty() {
472            String::new()
473        } else {
474            format!("--model {}", cmd.model)
475        },
476        auto_start: 0,
477        restart_on_crash: 0,
478        idle_timeout_minutes: 0,
479        created_at: now,
480        agent_type: cmd.agent_type.clone(),
481        agent_bus_id: String::new(),
482        is_seeded: 0,
483        accounts: String::new(),
484        parent_id: String::new(),
485        branch_label: String::new(),
486        updated_at: now,
487        user_hidden: 0,
488        container_image: cmd.container_image.clone(),
489        container_volumes: cmd.container_volumes.clone(),
490        container_name: String::new(), // assigned by ContainerManager on first spawn
491        use_ambient_login: 0,
492        model_vendor_base_url: cmd_model_vendor_base_url.clone(),
493        auto_continue_enabled: 0,
494        memory_id: String::new(),
495        conversation_visibility: crate::backend::storage::agents::default_conversation_visibility(),
496    };
497
498    // Atomic check-then-insert.
499    // Returns Some(existing) if a row matched by name/slug already exists;
500    // None if the row was freshly inserted (def.slug now holds resolved slug).
501    let existing_opt = mstore.agent_def_find_or_insert(&mut def)
502        .map_err(|e| format!("agent.define: find_or_insert: {e}"))?;
503
504    if let Some(existing) = existing_opt {
505        // A definition with this name/slug already exists — apply if_exists policy.
506        match if_exists {
507            "skip" => {
508                // Honor create_instance_stub even on skip: a definition that was
509                // created with create_instance_stub=false (or imported via another
510                // path) might not have a stub yet; a subsequent idempotent call
511                // with create_instance_stub=true should make it visible in My Agents.
512                // Only fire agents:changed when the stub was actually newly inserted.
513                let (stub_id, stub_new) = if create_stub {
514                    match agent_define::make_stub_idempotent(&mstore, &existing.id, &existing.name, now) {
515                        Ok((id, new)) => (Some(id), new),
516                        Err(e) => {
517                            tracing::warn!(id = %existing.id, err = %e, "agent.define: skip stub failed (non-fatal)");
518                            (None, false)
519                        }
520                    }
521                } else {
522                    (None, false)
523                };
524                if stub_new {
525                    broker.publish_event::<crate::backend::mps_events::AgentsChanged>(&(), vec![], 0);
526                }
527                tracing::info!(id = %existing.id, slug = %existing.slug, stub = stub_id.is_some(), "agent.define: skipped (exists)");
528                return Ok(AgentDefineResult {
529                    definition_id: existing.id.clone(),
530                    slug: existing.slug.clone(),
531                    action: "skipped".to_string(),
532                    instance_stub_id: stub_id,
533                });
534            }
535            "error" => {
536                return Err(format!(
537                    "agent.define: definition '{}' already exists (if_exists=error)",
538                    cmd.name.trim()
539                ));
540            }
541            "update" => {
542                let mut updated = existing.clone();
543                // provider was already validated/defaulted above; only
544                // overwrite if the caller explicitly supplied a provider or model.
545                if !cmd.provider.is_empty() || !cmd.model.is_empty() {
546                    agent_define::check_provider_unchanged(&existing.id, &existing.provider, &provider)?;
547                    updated.provider = provider.clone();
548                }
549                // Persist the model as a CLI flag so the agent launches with
550                // the requested model rather than the provider default.
551                // If the provider changes but no model is supplied, clear stale
552                // flags from the old provider so the new provider's default is used.
553                if !cmd.model.is_empty() {
554                    updated.provider_flags = format!("--model {}", cmd.model);
555                } else if !cmd.provider.is_empty() {
556                    updated.provider_flags = String::new();
557                }
558                if !cmd.icon.is_empty()     { updated.icon = cmd.icon.clone(); }
559                if !cmd.description.is_empty() { updated.description = cmd.description.clone(); }
560                if !cmd.working_directory.is_empty() { updated.working_directory = cmd.working_directory.clone(); }
561                if !cmd.shell.is_empty()    { updated.shell = cmd.shell.clone(); }
562                // `None` = don't touch; `Some(_)` (including `Some("")`) sets
563                // it explicitly — the caller MUST be able to pass `Some("")`
564                // to clear a stale override, or a provider change away from
565                // a vendor-capable provider (see validation below) would
566                // permanently block every future agent.define call for this
567                // agent, since there'd be no way to ever un-set the old value.
568                if let Some(url) = &cmd.model_vendor_base_url { updated.model_vendor_base_url = url.clone(); }
569                // Authoritative check for this write: validates the FINAL
570                // effective (provider, override) pair — catches both a
571                // freshly-supplied override against the real provider, and a
572                // provider change that leaves a stale override from before
573                // now invalid (the caller must clear it explicitly rather
574                // than silently carrying an inconsistent combination).
575                agent_define::validate_vendor_base_url(&updated.provider, &updated.model_vendor_base_url)?;
576                if !cmd.environment.is_empty() { updated.environment = cmd.environment.clone(); }
577                // name update intentionally omitted — the slug is immutable;
578                // renaming would create a slug mismatch. Use updateagent for renames.
579                let did_update = mstore.agent_def_update(&mut updated)
580                    .map_err(|e| format!("agent.define: update: {e}"))?;
581                if !did_update {
582                    return Err("agent.define: update: row was deleted between find and update".to_string());
583                }
584                agent_define::persist_define_content(&mstore, &updated.id, &cmd, now);
585                let stub_id = if create_stub {
586                    match agent_define::make_stub_idempotent(&mstore, &updated.id, &updated.name, now) {
587                        Ok((id, _new)) => Some(id),
588                        Err(e) => {
589                            tracing::warn!(id = %updated.id, err = %e, "agent.define: update stub failed (non-fatal)");
590                            None
591                        }
592                    }
593                } else {
594                    None
595                };
596                broker.publish_event::<crate::backend::mps_events::AgentsChanged>(&(), vec![], 0);
597                tracing::info!(id = %updated.id, slug = %updated.slug, stub = stub_id.is_some(), "agent.define: updated");
598                return Ok(AgentDefineResult {
599                    definition_id: updated.id.clone(),
600                    slug: updated.slug.clone(),
601                    action: "updated".to_string(),
602                    instance_stub_id: stub_id,
603                });
604            }
605            other => {
606                return Err(format!("agent.define: unknown if_exists value '{other}'"));
607            }
608        }
609    }
610
611    // Fresh insert — def.slug is now set by agent_def_find_or_insert.
612    // Every agent gets its own dedicated ABF bundle
613    // (ARCHITECTURE_MANDATORY_ABF_RETHINK_2026_08_14.md §3.2). Done here,
614    // after the atomic find-or-insert has confirmed this is a genuinely
615    // NEW definition — not before, or every idempotent `if_exists=skip`/
616    // `update` call against an existing name would leak an unbound bundle
617    // (see `agent_def_provision_and_bind_bundle`'s own doc comment).
618    mstore.agent_def_provision_and_bind_bundle(&id_store, &mut def, now);
619    // Create the stub first so that listeners handling agents:changed can
620    // immediately find the new agent via ListRecentSessionsCommand. The
621    // definition is already committed; a stub failure is non-fatal (log +
622    // continue) and we still broadcast so callers see the new definition.
623    let stub_id = if create_stub {
624        match agent_define::make_stub_idempotent(&mstore, &def.id, &def.name, now) {
625            Ok((id, _new)) => Some(id),
626            Err(e) => {
627                tracing::warn!(id = %def.id, err = %e, "agent.define: stub failed (definition committed, non-fatal)");
628                None
629            }
630        }
631    } else {
632        None
633    };
634    broker.publish_event::<crate::backend::mps_events::AgentsChanged>(&(), vec![], 0);
635    agent_define::persist_define_content(&mstore, &def.id, &cmd, now);
636
637    tracing::info!(
638        id = %def.id,
639        slug = %def.slug,
640        stub = stub_id.is_some(),
641        "agent.define: created"
642    );
643
644    Ok(AgentDefineResult {
645        definition_id: def.id.clone(),
646        slug: def.slug.clone(),
647        action: "created".to_string(),
648        instance_stub_id: stub_id,
649    })
650}