test(core): Add test coverage for tool_registry.py

[brilliantkid87]

Summary

The framework/runner/tool_registry.py (13KB, 388 lines) has no dedicated test coverage. This module handles critical tool discovery and registration functionality.

Current State

• tool_registry.py - 388 lines, NO TESTS
• Contains ToolRegistry class and @tool decorator
• Handles tool discovery from modules, MCP servers, and manual registration

Key Components to Test

ToolRegistry Class

• register() - Register tool with executor
• register_function() - Auto-generate Tool from function
• discover_from_module() - Load tools from Python file
• get_tools() - Return all registered tools
• get_executor() - Get unified executor function
• has_tool() - Check if tool exists
• cleanup() - Clean up MCP connections

@tool Decorator

• Basic decoration
• Custom name/description

Acceptance Criteria

• Tests in core/tests/test_tool_registry.py
• All tests pass with pytest tests/test_tool_registry.py -v
• Ruff linting passes

---

I'd like to work on this!

[Ramez-sherif]

Can I take it

[Ajzel]

I would like to work on this issue

[AasthaParmar]

Hi! I’d like to work on this.

Research

• Reviewed core/framework/runner/tool_registry.py and confirmed it currently has no dedicated tests.

Plan

• Add core/tests/test_tool_registry.py with focused unit tests for:
  • register() / register_function()
  • get_executor() (unknown tool + success + exception)
  • discover_from_module() (TOOLS dict + @tool-decorated functions)
  • basic coverage for get_tools() / has_tool() / cleanup()
• Verify with pytest tests/test_tool_registry.py -v and ruff check.

Could you please assign to me? @bryanadenhq

[AasthaParmar]

Hey, saw the issue was open and decided to try it out. I see a few others are interested in tackling this, would be happy to collaborate or delete if someone is further along. Thanks!

[brilliantkid87]

Hi @AasthaParmar, I will tackle this issue. thank you

[Hundao]

Hi @brilliantkid87, thanks for creating this issue and identifying the test coverage gap!

@AasthaParmar already submitted a complete implementation in PR #2028 that passes all tests locally. To avoid duplicate effort, I'll assign this to them so they can fix a minor formatting issue and reopen the PR. Thanks!

[AasthaParmar]

@Hundao The PR is ready to be merged #2028

[raiigauravv]

Hi, I'd like to contribute additional test coverage here. I'm an ML engineer with experience in Python, FastAPI, and agent frameworks.

I've opened PR #6818 which adds 19 new tests that complement the existing coverage — covering register_function(), @tool decorator discovery, session/execution context injection via set_session_context() and set_execution_context(), _convert_mcp_tool_to_framework_tool() CONTEXT_PARAMS stripping, both load_mcp_config() JSON formats, and resync_mcp_servers_if_needed() no-op paths.

Coverage: 47% → 69% (+22 pp), 31/31 tests passing, ruff clean.

Happy to adjust based on maintainer feedback.

[raiigauravv]

Happy to close this if #2028 gets merged alternatively, since my PR covers additional paths (session context, execution context, MCP config formats) that #2028 may not, I'm open to having the maintainers cherry-pick what's useful. Either way, no problem.