agentmux_srv/migrations/

m0021_backfill_agent_bundles.rs

// Copyright 2026, AgentMux Corp.
// SPDX-License-Identifier: Apache-2.0

//! Backfill a dedicated ABF bundle for every existing agent definition that
//! doesn't have one — the data-migration half of
//! `docs/specs/ARCHITECTURE_MANDATORY_ABF_RETHINK_2026_08_14.md` §3/§7:
//! "every agent must have an ABF," and, at the time, that ABF carried its
//! own provider/model (readonly once set by a bundle-level guard). Since
//! SPEC_AGENT_BUNDLE_FORMAT_V0_3_2026_10_05.md §3 the bundle carries no
//! harness: the agent owns its provider, locked by
//! `app_api::check_provider_unchanged`, and `m0035` reconciles agents with
//! the bundles this migration wrote.
//!
//! For every definition with an empty `memory_id`: create a fresh
//! `db_bundles` row (empty instructions/context — a legitimate starting
//! state, not an error; `bundle.validate` already treats an empty bundle as
//! valid) with the definition's OWN `provider` (harness) carried onto the
//! bundle, and `model` defaulted from that provider's first declared
//! `supported_vendors` entry (same derivation
//! `Store::bundle_provision_for_new_agent` uses for brand-new agents) —
//! and point the definition's `memory_id` at it.
//!
//! P0 fix (2026-08-15, ReAgent review on PR #2587): this used to hardcode
//! `provider='claude'`, `model='anthropic'` for every backfilled agent
//! regardless of `def.provider`, reasoning from §7.3's operator-confirmed
//! "every agent on this instance is Claude Code + OAuth Anthropic." That's
//! true today, but baking it into the migration is unsafe: combined with
//! the then bundle-level read-only lock and `agent_open.rs`'s
//! spawn-time bundle-provider preference, hardcoding here would silently
//! reassign any non-Claude agent (codex, gemini, ...) — present now or
//! added before this migration runs on a given install — to spawn as
//! Claude afterward, permanently (readonly-once-set). Deriving from the
//! definition's own already-known `provider` costs nothing extra and has
//! no such failure mode; falls back to `"claude"`/`"anthropic"` only when
//! `def.provider` is itself empty (a definition with no harness set at
//! all — degenerate case, but still needs *some* value to backfill to).
//!
//! **Local-channel only, deliberately** — same pattern this migration's own
//! module doc for `agent_def_set_memory_id_if_empty` documents: a
//! cross-channel (global-registry-only) definition can't be reached today
//! because `DefinitionRecordV1` doesn't carry `memory_id` yet (same
//! accepted gap as `model_vendor_base_url`). The global registry is still
//! attached (mirrors `m0020`'s pattern) so this migration at least SEES
//! every agent and can log which ones it had to skip, rather than silently
//! processing only whatever happens to be in local SQLite.
//!
//! **The bundle itself, though, is NOT channel-local** — it's written into
//! the effective identity/memory store (shared store when resolvable,
//! same as `AppState.id_store` at runtime), never the channel's own
//! `objects.db`. P1 fix (2026-08-15, Codex review on PR #2587): this used
//! to write bundles straight into `mstore` (the channel store this
//! migration already has open for the agent-definition side) — every real
//! bundle-read path (`listmemories`/`getmemory`/the Memory editor/the
//! bundle-summary panel) reads through `id_store` instead, so a
//! channel-local bundle would be silently invisible everywhere else, the
//! exact same bug the runtime provisioning-path fix
//! (`agent_def_provision_and_bind_bundle`) addresses for new agents.
//!
//! Idempotent: re-running only ever touches definitions still at
//! `memory_id=''`; already-backfilled or already-bound rows are untouched
//! (both by the `agent_def_list()` filter here and by
//! `agent_def_set_memory_id_if_empty`'s own `WHERE memory_id = ''` guard).

use std::sync::Arc;

use crate::backend::storage::store::{Bundle, Store};
use crate::registry::{resolve_shared_definitions_dir, DefinitionStore};

use super::{Migration, MigrationContext, MigrationError, MigrationScope};

pub struct M0021BackfillAgentBundles;

/// Fallback provider/model ONLY for the degenerate case of a definition
/// whose own `provider` column is itself empty — see the module doc's P0
/// fix note. Every normal backfill derives from `def.provider` instead;
/// this is not the common path.
const FALLBACK_PROVIDER: &str = "claude";
const FALLBACK_MODEL: &str = "anthropic";

/// Same derivation `Store::bundle_provision_for_new_agent`
/// (`backend/storage/agents/definition.rs`) uses for brand-new agents — delegates to
/// `Store::resolve_effective_vendor` so both paths agree on the
/// `model_vendor_base_url` → `"custom"` rule (P2 fix, Codex review on PR
/// #2587) rather than maintaining two copies of the same logic. Falls
/// back to `FALLBACK_PROVIDER`/`FALLBACK_MODEL` when `provider` is empty
/// — the one case `resolve_effective_vendor` can't help with, since it
/// still needs a real provider id to look up.
fn resolve_backfill_provider_and_model(provider: &str, model_vendor_base_url: &str) -> (String, String) {
    if provider.is_empty() {
        return (FALLBACK_PROVIDER.to_string(), FALLBACK_MODEL.to_string());
    }
    let vendor = Store::resolve_effective_vendor(provider, model_vendor_base_url);
    (provider.to_string(), vendor)
}

/// Resolve the store bundles must be written into — mirrors
/// `AppState.id_store`'s own fallback (shared store when resolvable, else
/// the channel store) so a migration-time bundle always lands exactly
/// where the runtime RPC layer will look for it. Best-effort or degrade,
/// never hard-fail: an unusable shared store means "not today," same
/// posture the global-registry attach above already has.
pub(super) fn resolve_bundle_store(ctx: &MigrationContext, mstore: &Arc<Store>) -> Arc<Store> {
    match Store::open_shared(&ctx.shared_store_path) {
        Ok(shared) => Arc::new(shared),
        Err(e) => {
            tracing::warn!(
                error = %e,
                path = %ctx.shared_store_path.display(),
                "backfill_agent_bundles: shared store unavailable, falling back to channel store for bundle writes"
            );
            mstore.clone()
        }
    }
}

impl Migration for M0021BackfillAgentBundles {
    fn id(&self) -> &'static str { "0021_backfill_agent_bundles" }
    fn scope(&self) -> MigrationScope { MigrationScope::Channel }
    fn description(&self) -> &'static str {
        "Provision a dedicated ABF bundle for every agent definition lacking one"
    }

    fn up(&self, ctx: &MigrationContext) -> Result<(), MigrationError> {
        if !ctx.channel_store_path.exists() {
            return Ok(());
        }
        let mstore = Arc::new(
            Store::open(&ctx.channel_store_path)
                .map_err(|e| MigrationError(format!("backfill_agent_bundles: open mstore: {}", e)))?,
        );
        // Attach the global registry (see module doc) purely for VISIBILITY
        // — this migration cannot write memory_id for a global-only
        // definition (DefinitionRecordV1 doesn't carry the field), but
        // attaching it means agent_def_list() surfaces those agents so we
        // can at least log which ones were skipped, matching m0020's
        // best-effort handling if the registry can't be resolved/opened.
        if let Some(def_dir) = resolve_shared_definitions_dir() {
            match DefinitionStore::open(def_dir) {
                Ok(def_store) => mstore.set_def_registry(Arc::new(def_store)),
                Err(e) => tracing::warn!(error = %e, "backfill_agent_bundles: failed to open global def registry, backfilling local-only"),
            }
        } else {
            tracing::warn!("backfill_agent_bundles: could not resolve global def registry dir, backfilling local-only");
        }

        let bundle_store = resolve_bundle_store(ctx, &mstore);
        // The agent's bundle recorded by another channel
        // (SPEC_MEMORY_FOLLOWS_THE_AGENT_2026_09_24.md §2.1.6): bound here too
        // instead of a new one per channel. Best-effort: without the global
        // store every channel still gets a bundle, as before.
        let sidecar = crate::backend::agent_bundle_sidecar::open_global();

        let defs = mstore
            .agent_def_list()
            .map_err(|e| MigrationError(format!("backfill_agent_bundles: list defs: {}", e)))?;
        let now = std::time::SystemTime::now()
            .duration_since(std::time::UNIX_EPOCH)
            .map(|d| d.as_millis() as i64)
            .unwrap_or(0);

        for def in defs {
            if !def.memory_id.is_empty() {
                continue;
            }
            // Only in the global registry: there's no row here to bind a
            // bundle to, so a bundle minted now would be orphaned. Every
            // fresh channel used to mint one per such agent into the shared
            // store ("AgentA — ABF (11)" on one host). The agent's bundle is
            // provisioned in a channel that has its row.
            let local = mstore
                .agent_def_has_local_row(&def.id)
                .map_err(|e| MigrationError(format!("backfill_agent_bundles: look up {}: {}", def.id, e)))?;
            if !local {
                tracing::debug!(agent_id = %def.id, "backfill_agent_bundles: definition only in the global registry; nothing here to bind a bundle to");
                continue;
            }
            use crate::backend::agent_bundle_sidecar as sc;
            let recorded = sidecar.as_ref().and_then(|fs| sc::get(fs, &def.id).ok().flatten());
            if let Some(existing) = recorded.filter(|id| matches!(bundle_store.bundle_get(id), Ok(Some(_)))) {
                mstore
                    .agent_def_set_memory_id_if_empty(&def.id, &existing)
                    .map_err(|e| MigrationError(format!("backfill_agent_bundles: bind {}: {}", def.id, e)))?;
                continue;
            }
            let bundle_id = uuid::Uuid::new_v4().to_string();
            let (provider, model) = resolve_backfill_provider_and_model(&def.provider, &def.model_vendor_base_url);
            // P1 fix (ReAgent review on PR #2587 round 4): db_bundles.name
            // is UNIQUE but only AgentDefinition.slug is guaranteed
            // unique, not the display name — a naive "{name} — ABF" on
            // two same-named pre-existing agents used to abort this
            // ENTIRE backfill loop via the `?` below on the very first
            // collision, permanently stalling backfill for every agent
            // processed after it. resolve_unique_bundle_name (shared with
            // the runtime provisioning path) disambiguates instead.
            let name = bundle_store
                .resolve_unique_bundle_name(&format!("{} — ABF", def.name))
                .map_err(|e| MigrationError(format!("backfill_agent_bundles: resolve unique bundle name for {}: {}", def.id, e)))?;
            let bundle = Bundle {
                id: bundle_id.clone(),
                name,
                description: String::new(),
                is_blank: false,
                is_global: false,
                provider,
                model,
                instructions: String::new(),
                instructions_by_provider: "{}".to_string(),
                context_files: "[]".to_string(),
                mcp_servers: "[]".to_string(),
                skills: "[]".to_string(),
                sort_order: 0,
                created_at: now,
                updated_at: now,
                is_system: false,
            };
            bundle_store
                .bundle_upsert(&bundle)
                .map_err(|e| MigrationError(format!("backfill_agent_bundles: create bundle for {}: {}", def.id, e)))?;
            // Record it, unless another channel just recorded one first — then
            // that one is the agent's, and this one goes.
            let mut bundle_id = bundle_id;
            if let Some(fs) = sidecar.as_ref() {
                // A recorded bundle this channel can't find is replaced, not
                // obeyed (see `claim_unless_live`).
                let live = |id: &str| matches!(bundle_store.bundle_get(id), Ok(Some(_)));
                match sc::claim_unless_live(fs, &def.id, &bundle_id, &live) {
                    Ok(winner) if winner != bundle_id && matches!(bundle_store.bundle_get(&winner), Ok(Some(_))) => {
                        if let Err(e) = bundle_store.bundle_delete(&bundle_id) {
                            tracing::warn!(
                                agent_id = %def.id,
                                bundle_id = %bundle_id,
                                error = %e,
                                "backfill_agent_bundles: another channel's bundle won; this duplicate could not be deleted and is left unbound"
                            );
                        }
                        bundle_id = winner;
                    }
                    Ok(_) => {}
                    Err(e) => tracing::warn!(agent_id = %def.id, error = %e, "backfill_agent_bundles: bundle sidecar not recorded"),
                }
            }

            let applied = mstore
                .agent_def_set_memory_id_if_empty(&def.id, &bundle_id)
                .map_err(|e| MigrationError(format!("backfill_agent_bundles: bind {}: {}", def.id, e)))?;
            if !applied {
                // No local row for this id — it only resolved via the
                // global registry overlay. Known gap (see module doc); the
                // freshly-created bundle is orphaned (nothing points at it
                // yet) rather than left dangling silently — log it so it's
                // discoverable, matching this codebase's "no silent gaps"
                // convention elsewhere in the migration set.
                tracing::warn!(
                    agent_id = %def.id,
                    bundle_id = %bundle_id,
                    "backfill_agent_bundles: definition only exists in the global registry — \
                     cannot bind memory_id there yet (DefinitionRecordV1 gap); bundle created but unbound"
                );
            }
        }
        Ok(())
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::backend::storage::agents::AgentDefinition;

    // Same isolation discipline as m0020_agent_color_backfill's test module
    // (see its own doc comment for the full rationale): every test here
    // calls `up()`, which always tries to resolve+attach the global
    // registry via the process-global `AGENTMUX_HOME_OVERRIDE` env var.
    // Left unset, that would merge in every real agent on the developer's
    // machine. Uses the CRATE-WIDE `test_support::ISOLATED_AUTH_ENV_LOCK`
    // (see its own doc comment), not a mutex local to this module — see
    // m0020_agent_color_backfill's identical comment for why a per-file
    // mutex isn't sufficient (confirmed live, 2026-08-15: these two
    // modules' tests raced each other, and would still race
    // registry/paths.rs's and friends', under any mutex narrower than the
    // crate-wide one every other consumer of this env var already uses).
    use crate::test_support::ISOLATED_AUTH_ENV_LOCK as ENV_GUARD;

    fn with_isolated_home<T>(f: impl FnOnce(&std::path::Path) -> T) -> T {
        let _guard = ENV_GUARD.lock().unwrap_or_else(|e| e.into_inner());
        let home_tmp = tempfile::tempdir().unwrap();
        std::env::set_var("AGENTMUX_HOME_OVERRIDE", home_tmp.path().to_str().unwrap());
        let result = f(home_tmp.path());
        std::env::remove_var("AGENTMUX_HOME_OVERRIDE");
        result
    }

    // `shared_path` MUST be a fresh per-test tempfile, never a shared
    // constant path — the migration now genuinely writes bundles there
    // (P1 fix, Codex/ReAgent review on PR #2587), so a shared literal path
    // across tests would leak real bundle rows between parallel test runs
    // and across repeated `cargo test` invocations.
    fn ctx_for(channel_path: &std::path::Path, shared_path: &std::path::Path) -> MigrationContext {
        MigrationContext {
            home: std::env::temp_dir(),
            data_dir: std::env::temp_dir(),
            shared_store_path: shared_path.to_path_buf(),
            channel_store_path: channel_path.to_path_buf(),
        }
    }

    fn insert_def(mstore: &Store, name: &str) -> String {
        let mut def = AgentDefinition {
            conversation_visibility: crate::backend::storage::agents::default_conversation_visibility(),
            id: format!("test-{name}"),
            slug: String::new(),
            name: name.to_string(),
            icon: String::new(),
            provider: "claude".to_string(),
            description: String::new(),
            working_directory: String::new(),
            shell: String::new(),
            provider_flags: String::new(),
            auto_start: 0,
            restart_on_crash: 0,
            idle_timeout_minutes: 0,
            created_at: 1,
            agent_type: "host".to_string(),
            environment: String::new(),
            agent_bus_id: String::new(),
            is_seeded: 0,
            accounts: String::new(),
            parent_id: String::new(),
            branch_label: String::new(),
            updated_at: 1,
            user_hidden: 0,
            container_image: String::new(),
            container_volumes: "[]".to_string(),
            container_name: String::new(),
            use_ambient_login: 0,
            auto_continue_enabled: 0,
            model_vendor_base_url: String::new(),
            memory_id: String::new(),
        };
        mstore.agent_def_insert(&mut def).unwrap();
        def.id
    }

    fn insert_def_with_provider(mstore: &Store, name: &str, provider: &str) -> String {
        let mut def = AgentDefinition {
            conversation_visibility: crate::backend::storage::agents::default_conversation_visibility(),
            id: format!("test-{name}"),
            slug: String::new(),
            name: name.to_string(),
            icon: String::new(),
            provider: provider.to_string(),
            description: String::new(),
            working_directory: String::new(),
            shell: String::new(),
            provider_flags: String::new(),
            auto_start: 0,
            restart_on_crash: 0,
            idle_timeout_minutes: 0,
            created_at: 1,
            agent_type: "host".to_string(),
            environment: String::new(),
            agent_bus_id: String::new(),
            is_seeded: 0,
            accounts: String::new(),
            parent_id: String::new(),
            branch_label: String::new(),
            updated_at: 1,
            user_hidden: 0,
            container_image: String::new(),
            container_volumes: "[]".to_string(),
            container_name: String::new(),
            use_ambient_login: 0,
            auto_continue_enabled: 0,
            model_vendor_base_url: String::new(),
            memory_id: String::new(),
        };
        mstore.agent_def_insert(&mut def).unwrap();
        def.id
    }

    /// SPEC_MEMORY_FOLLOWS_THE_AGENT_2026_09_24.md §2.1.6: the agent's
    /// second channel binds the bundle its first recorded, instead of minting
    /// "Name — ABF (2)" in the same shared store.
    #[test]
    fn a_second_channel_binds_the_bundle_the_first_recorded() {
        with_isolated_home(|_home| {
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let (ch1, ch2) = (tempfile::NamedTempFile::new().unwrap(), tempfile::NamedTempFile::new().unwrap());
            let (s1, s2) = (Store::open(ch1.path()).unwrap(), Store::open(ch2.path()).unwrap());
            let id = insert_def(&s1, "RoamingAgent");
            assert_eq!(insert_def(&s2, "RoamingAgent"), id, "same agent in both channels");

            M0021BackfillAgentBundles.up(&ctx_for(ch1.path(), shared_tmp.path())).unwrap();
            M0021BackfillAgentBundles.up(&ctx_for(ch2.path(), shared_tmp.path())).unwrap();

            let b1 = s1.agent_def_get(&id).unwrap().unwrap().memory_id;
            let b2 = s2.agent_def_get(&id).unwrap().unwrap().memory_id;
            assert!(!b1.is_empty());
            assert_eq!(b1, b2, "both channels bind one bundle");
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let abf: Vec<String> =
                shared.bundle_list().unwrap().into_iter().map(|b| b.name).filter(|n| n.contains("RoamingAgent")).collect();
            assert_eq!(abf, vec!["RoamingAgent — ABF".to_string()], "no second bundle minted");
        });
    }

    /// A definition that only resolves through the global registry gets no
    /// bundle here: nothing could point at it.
    #[test]
    fn a_global_only_definition_gets_no_orphaned_bundle() {
        with_isolated_home(|home| {
            use crate::registry::{DefinitionRecord, DefinitionRecordV1, DEF_MAX_SUPPORTED_SCHEMA};
            let def_dir = home.join("shared").join("agents").join("definitions");
            crate::registry::DefinitionStore::open(def_dir)
                .unwrap()
                .upsert(&DefinitionRecord {
                    schema_version: DEF_MAX_SUPPORTED_SCHEMA,
                    data: DefinitionRecordV1 {
                        id: "test-GlobalOnly".to_string(),
                        name: "GlobalOnly".to_string(),
                        provider: "claude".to_string(),
                        ..Default::default()
                    },
                })
                .unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let ch = tempfile::NamedTempFile::new().unwrap();
            let s = Store::open(ch.path()).unwrap();
            assert!(!s.agent_def_has_local_row("test-GlobalOnly").unwrap());

            M0021BackfillAgentBundles.up(&ctx_for(ch.path(), shared_tmp.path())).unwrap();

            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let abf: Vec<String> =
                shared.bundle_list().unwrap().into_iter().map(|b| b.name).filter(|n| n.contains("GlobalOnly")).collect();
            assert!(abf.is_empty(), "minted an orphan: {abf:?}");
        });
    }

    /// The sidecar recorded a bundle this channel can't see (it lived in an
    /// isolated channel's store): the channel's new bundle replaces the
    /// record, and the next channel binds that one instead of minting again.
    #[test]
    fn a_recorded_bundle_missing_here_is_replaced_then_reused() {
        with_isolated_home(|_home| {
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let (ch1, ch2) = (tempfile::NamedTempFile::new().unwrap(), tempfile::NamedTempFile::new().unwrap());
            let (s1, s2) = (Store::open(ch1.path()).unwrap(), Store::open(ch2.path()).unwrap());
            let id = insert_def(&s1, "PinnedAgent");
            insert_def(&s2, "PinnedAgent");
            let global = crate::backend::agent_bundle_sidecar::open_global().unwrap();
            crate::backend::agent_bundle_sidecar::claim(&global, &id, "bundle-in-an-isolated-store").unwrap();

            M0021BackfillAgentBundles.up(&ctx_for(ch1.path(), shared_tmp.path())).unwrap();
            let b1 = s1.agent_def_get(&id).unwrap().unwrap().memory_id;
            assert!(!b1.is_empty() && b1 != "bundle-in-an-isolated-store");
            assert_eq!(crate::backend::agent_bundle_sidecar::get(&global, &id).unwrap().as_deref(), Some(b1.as_str()));

            M0021BackfillAgentBundles.up(&ctx_for(ch2.path(), shared_tmp.path())).unwrap();
            assert_eq!(s2.agent_def_get(&id).unwrap().unwrap().memory_id, b1, "the second channel reuses it");
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let abf = shared.bundle_list().unwrap().into_iter().filter(|b| b.name.contains("PinnedAgent")).count();
            assert_eq!(abf, 1, "one bundle, not one per channel");
        });
    }

    #[test]
    fn backfills_a_definition_with_no_bundle() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def(&mstore, "Plain Agent");

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let def = mstore.agent_def_get(&id).unwrap().unwrap();
            assert!(!def.memory_id.is_empty(), "expected memory_id to be backfilled");
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let bundle = shared.bundle_get(&def.memory_id).unwrap().unwrap();
            // insert_def hardcodes provider="claude" — derived, not the
            // fallback path (see resolve_backfill_provider_and_model).
            assert_eq!(bundle.provider, "claude");
            assert_eq!(bundle.model, "anthropic");
            assert!(bundle.name.contains("Plain Agent"));
            assert!(!bundle.is_blank);
        });
    }

    // P1 regression test (Codex + ReAgent review on PR #2587): the bundle
    // must land in the EFFECTIVE identity/memory store (shared store,
    // mirroring AppState.id_store), never the channel-local store the
    // agent definition itself lives in — a channel-local bundle is
    // invisible to bundle.list/bundle.get/the Memory editor.
    #[test]
    fn backfilled_bundle_lands_in_the_shared_store_not_the_channel_store() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def(&mstore, "Shared Store Agent");

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let def = mstore.agent_def_get(&id).unwrap().unwrap();
            assert!(!def.memory_id.is_empty());

            // Absent from the channel store — Store::open always seeds the
            // "blank" singleton, so a fresh channel store already has 1
            // bundle; the backfilled one must NOT also land here.
            let channel_bundle_count = mstore.bundle_list().unwrap().iter().filter(|b| !b.is_blank).count();
            assert_eq!(channel_bundle_count, 0, "bundle must not be written into the channel store");

            // Present in the shared store.
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            assert!(shared.bundle_get(&def.memory_id).unwrap().is_some(), "bundle must be reachable via the shared store");
        });
    }

    // P0 regression test (ReAgent review on PR #2587): the migration must
    // derive each bundle's provider from THAT definition's own
    // `provider`, never a hardcoded value — a hardcoded claude/anthropic
    // default would silently reassign a non-Claude agent's harness on
    // backfill, permanently, while the bundle's provider was read-only and
    // preferred over the definition's own at spawn (as it was until
    // SPEC_AGENT_BUNDLE_FORMAT_V0_3_2026_10_05.md §3; `m0035` relies on
    // these values to reconcile agents).
    #[test]
    fn backfill_derives_provider_and_model_from_the_definitions_own_provider_not_a_hardcoded_default() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def_with_provider(&mstore, "Codex Agent", "codex");

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let def = mstore.agent_def_get(&id).unwrap().unwrap();
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let bundle = shared.bundle_get(&def.memory_id).unwrap().unwrap();
            assert_eq!(bundle.provider, "codex", "must carry the agent's OWN provider, not claude");
            assert_eq!(bundle.model, "openai", "vendor derived from codex's supported_vendors[0]");
        });
    }

    // P2 regression test (Codex review on PR #2587): a definition with a
    // custom model_vendor_base_url override must backfill to model="custom",
    // matching Store::resolve_effective_vendor / the frontend's
    // resolveEffectiveVendor — not the provider's bare default vendor.
    #[test]
    fn backfill_uses_custom_vendor_when_the_definition_has_a_model_vendor_base_url_override() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def_with_provider(&mstore, "Custom Vendor Agent", "claude");
            {
                let mut def = mstore.agent_def_get(&id).unwrap().unwrap();
                def.model_vendor_base_url = "https://my-proxy.example.com".to_string();
                mstore.agent_def_update(&mut def).unwrap();
            }

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let def = mstore.agent_def_get(&id).unwrap().unwrap();
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let bundle = shared.bundle_get(&def.memory_id).unwrap().unwrap();
            assert_eq!(bundle.provider, "claude");
            assert_eq!(bundle.model, "custom", "a vendor override must backfill to \"custom\", not the provider default");
        });
    }

    #[test]
    fn backfill_falls_back_to_claude_anthropic_only_when_the_definition_has_no_provider_at_all() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def_with_provider(&mstore, "No Provider Agent", "");

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let def = mstore.agent_def_get(&id).unwrap().unwrap();
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let bundle = shared.bundle_get(&def.memory_id).unwrap().unwrap();
            assert_eq!(bundle.provider, FALLBACK_PROVIDER);
            assert_eq!(bundle.model, FALLBACK_MODEL);
        });
    }

    #[test]
    fn leaves_an_already_bound_definition_untouched() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def(&mstore, "Already Bound");
            mstore.agent_def_set_memory_id_if_empty(&id, "pre-existing-bundle").unwrap();

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let def = mstore.agent_def_get(&id).unwrap().unwrap();
            assert_eq!(def.memory_id, "pre-existing-bundle", "must not overwrite an existing binding");
        });
    }

    #[test]
    fn rerun_is_idempotent_no_duplicate_bundles() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def(&mstore, "Rerun Agent");

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();
            let first_bundle_id = mstore.agent_def_get(&id).unwrap().unwrap().memory_id;

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();
            let second_bundle_id = mstore.agent_def_get(&id).unwrap().unwrap().memory_id;

            assert_eq!(first_bundle_id, second_bundle_id, "rerun must not replace the bound bundle");
            // Store::open_shared also seeds the "blank" singleton (see
            // bundles.rs), so a fresh shared store already has 1
            // bundle before this migration ever runs — count non-blank
            // bundles in the store bundles actually land in now.
            let shared = Store::open_shared(shared_tmp.path()).unwrap();
            let non_blank_count = shared
                .bundle_list()
                .unwrap()
                .iter()
                .filter(|b| !b.is_blank)
                .count();
            assert_eq!(non_blank_count, 1, "rerun must not create a second orphaned bundle");
        });
    }

    #[test]
    fn backfills_multiple_definitions_with_distinct_bundles() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let a = insert_def(&mstore, "Agent A");
            let b = insert_def(&mstore, "Agent B");

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let bundle_a = mstore.agent_def_get(&a).unwrap().unwrap().memory_id;
            let bundle_b = mstore.agent_def_get(&b).unwrap().unwrap().memory_id;
            assert!(!bundle_a.is_empty());
            assert!(!bundle_b.is_empty());
            assert_ne!(bundle_a, bundle_b, "each agent must get its OWN dedicated bundle");
        });
    }

    // P1 regression test (ReAgent review on PR #2587 round 4): two
    // pre-existing agent definitions sharing the same display name (only
    // `slug` is guaranteed unique, not `name`) used to abort this ENTIRE
    // migration on the second agent's bundle-name collision — every
    // definition processed after the collision stayed unbound
    // permanently, on every subsequent boot, until manually resolved.
    #[test]
    fn backfills_both_agents_when_two_definitions_share_a_display_name() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let mut def_a = AgentDefinition {
                conversation_visibility: crate::backend::storage::agents::default_conversation_visibility(),
                id: "test-twin-a".to_string(),
                slug: String::new(),
                name: "Twin".to_string(),
                icon: String::new(),
                provider: "claude".to_string(),
                description: String::new(),
                working_directory: String::new(),
                shell: String::new(),
                provider_flags: String::new(),
                auto_start: 0,
                restart_on_crash: 0,
                idle_timeout_minutes: 0,
                created_at: 1,
                agent_type: "host".to_string(),
                environment: String::new(),
                agent_bus_id: String::new(),
                is_seeded: 0,
                accounts: String::new(),
                parent_id: String::new(),
                branch_label: String::new(),
                updated_at: 1,
                user_hidden: 0,
                container_image: String::new(),
                container_volumes: "[]".to_string(),
                container_name: String::new(),
                use_ambient_login: 0,
                auto_continue_enabled: 0,
                model_vendor_base_url: String::new(),
                memory_id: String::new(),
            };
            let mut def_b = def_a.clone();
            def_b.id = "test-twin-b".to_string();
            mstore.agent_def_insert(&mut def_a).unwrap();
            mstore.agent_def_insert(&mut def_b).unwrap();

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), shared_tmp.path())).unwrap();

            let bound_a = mstore.agent_def_get(&def_a.id).unwrap().unwrap().memory_id;
            let bound_b = mstore.agent_def_get(&def_b.id).unwrap().unwrap().memory_id;
            assert!(!bound_a.is_empty(), "first same-named agent must be backfilled");
            assert!(!bound_b.is_empty(), "second same-named agent must NOT be left unbound by a collision");
            assert_ne!(bound_a, bound_b, "each agent still gets its own distinct bundle");
        });
    }

    #[test]
    fn missing_channel_store_is_a_noop() {
        with_isolated_home(|_home| {
            let shared_tmp = tempfile::NamedTempFile::new().unwrap();
            let ctx = ctx_for(std::path::Path::new("Z:/does/not/exist/objects.db"), shared_tmp.path());
            M0021BackfillAgentBundles.up(&ctx).unwrap();
        });
    }

    // Fallback regression test: when the shared store path itself is
    // unusable (parent directory doesn't exist), the migration must still
    // succeed by falling back to the channel store — mirroring
    // AppState.id_store's own degrade-to-mstore behavior — rather than
    // hard-failing the whole migration.
    #[test]
    fn falls_back_to_the_channel_store_when_the_shared_store_path_is_unusable() {
        with_isolated_home(|_home| {
            let tmp = tempfile::NamedTempFile::new().unwrap();
            let mstore = Store::open(tmp.path()).unwrap();
            let id = insert_def(&mstore, "Fallback Agent");
            let bad_shared_path = std::path::Path::new("Z:/does/not/exist/shared-store.db");

            M0021BackfillAgentBundles.up(&ctx_for(tmp.path(), bad_shared_path)).unwrap();

            let def = mstore.agent_def_get(&id).unwrap().unwrap();
            assert!(!def.memory_id.is_empty(), "must still backfill even when the shared store is unreachable");
            assert!(mstore.bundle_get(&def.memory_id).unwrap().is_some(), "falls back to writing the bundle in the channel store");
        });
    }
}