fix: reject duplicate tool registration and add collision test

[Ishita-190]

📋 Summary

This PR makes duplicate tool registration explicit by rejecting tools with names that are already registered. Previously, ToolRegistry::register_with_source would silently overwrite an existing tool and its associated source. This behavior has been changed to return an error instead.

A unit test has also been added to ensure duplicate registration is rejected and registry state remains unchanged.

🔗 Related Issues

Closes #736

---

🧠 Context

register_with_source previously allowed silent overwriting:

• If a tool with the same name was already registered,
• The new tool replaced the existing one,
• The original ToolSource metadata was overwritten,
• No error was returned.

This behavior was unsafe and inconsistent with hot_reload_plugin, which already rejects duplicate tool names and rolls back on conflict.

---

🛠️ Changes

Duplicate registrations now return:

AgentError::RegistrationFailed("Tool '<name>' is already registered")

Updated implementation:

pub fn register_with_source(
    &mut self,
    tool: Arc<dyn DynTool>,
    source: ToolSource,
) -> AgentResult<()> {
    let name = tool.name().to_string();

    if self.tools.contains_key(&name) {
        return Err(AgentError::RegistrationFailed(format!(
            "Tool '{name}' is already registered",
        )));
    }

    self.sources.insert(name.clone(), source);
    self.tools.insert(name, tool);
    Ok(())
}

The added test now verifies:

• Second registration with same name fails
• Registry count remains unchanged
• Original source remains intact

#[tokio::test]
async fn register_with_source_rejects_duplicate_tool_names() {
    let mut registry = ToolRegistry::new();

    registry
        .register_with_source(
            TestTool::new("dup_tool").into_dynamic(),
            ToolSource::Builtin,
        )
        .unwrap();

    let err = registry
        .register_with_source(
            TestTool::new("dup_tool").into_dynamic(),
            ToolSource::Dynamic,
        )
        .expect_err("duplicate registration should fail");

    assert!(matches!(err, AgentError::RegistrationFailed(_)));
    assert!(err.to_string().contains("already registered"));

    assert_eq!(registry.count(), 1);
    assert!(matches!(
        registry.get_source("dup_tool"),
        Some(ToolSource::Builtin)
    ));
}

---

🧪 How you Tested

The new behavior and test were verified locally:

cargo test -p mofa-foundation register_with_source_rejects_duplicate_tool_names

---

📸 Screenshots / Logs (if applicable)

---

⚠️ Breaking Changes

• No breaking changes
• Breaking change (describe below)

---

🧹 Checklist

Code Quality

• Code follows Rust idioms and project conventions
• cargo fmt run
• cargo clippy passes without warnings

Testing

• Tests added/updated
• cargo test passes locally without any error

Documentation

• Public APIs documented
• README / docs updated (if needed)

PR Hygiene

• PR is small and focused (one logical change)
• Branch is up to date with main
• No unrelated commits
• Commit messages explain why, not only what

---

[Ishita-190]

@lijingrs @BH3GEI please take a look!

[Ishita-190]

@doroteaMonaco thank you for merging this! Actually I have some open PRs in studio as well,( 54 46 ), whenever you're free please take a look at them!

[doroteaMonaco]

@Ishita-190 I'll review them as soon as possible. Thanks for contributing!