fix: apply provider modalities after tool injection

[bugkeep]

Summary

• move provider modality filtering until after built-in tool injection so models configured without tool_use do not receive late-added tools
• keep context sanitization aligned with the final provider/tool state
• add a regression test covering cron/proactive tools being stripped for text-only providers

Verification

• confirmed the new regression test fails on current upstream before the fix
• uv run pytest tests/unit/test_astr_main_agent.py -q (80 passed)
• uv run ruff format .
• uv run ruff check .

Related to #6857.

Summary by Sourcery

Adjust main agent build flow so provider modality filtering and context sanitization occur after all built-in tools are injected, ensuring text-only providers do not receive unsupported tools.

Bug Fixes:

• Ensure late-injected tools such as cron/proactive tools are stripped when the selected provider does not support tool use.

Tests:

• Add a regression test verifying that providers configured as text-only do not receive function tools after built-in tool injection.

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• The reordering of _modalities_fix and _sanitize_context_by_modalities to occur after tool injection also changes their relative ordering with respect to llm_safety_mode; consider confirming whether safety-related adjustments should conceptually happen before or after modality-based filtering and, if intentional, adding a brief comment to document that invariant.
• Since _modalities_fix and _sanitize_context_by_modalities now run later in build_main_agent, it may be worth double-checking that they are still safe for all existing early-return paths and error cases (e.g., where a provider might be missing or partially initialized) and, if needed, guarding them accordingly.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The reordering of `_modalities_fix` and `_sanitize_context_by_modalities` to occur after tool injection also changes their relative ordering with respect to `llm_safety_mode`; consider confirming whether safety-related adjustments should conceptually happen before or after modality-based filtering and, if intentional, adding a brief comment to document that invariant.
- Since `_modalities_fix` and `_sanitize_context_by_modalities` now run later in `build_main_agent`, it may be worth double-checking that they are still safe for all existing early-return paths and error cases (e.g., where a provider might be missing or partially initialized) and, if needed, guarding them accordingly.

## Individual Comments

### Comment 1
<location path="tests/unit/test_astr_main_agent.py" line_range="970-979" />
<code_context>
+        assert result is not None
+        assert result.provider_request.func_tool is None
+
     @pytest.mark.asyncio
     async def test_build_main_agent_no_provider(self, mock_event, mock_context):
         """Test building main agent when no provider is available."""
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a complementary test for providers that *do* support tool use to ensure tools are retained after the refactor.

To complement the negative case, please add a positive test where `mock_provider.provider_config["modalities"]` includes tool support and `add_cron_tools=True`. That test should assert that the function tools remain on `result.provider_request` after `_modalities_fix` and `_sanitize_context_by_modalities`, so we catch any future regressions that strip tools for tool-capable providers.
</issue_to_address>

### Comment 2
<location path="tests/unit/test_astr_main_agent.py" line_range="1005-1006" />
<code_context>
+                config=module.MainAgentBuildConfig(
+                    tool_call_timeout=60,
+                    computer_use_runtime="none",
+                    add_cron_tools=True,
+                ),
+            )
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the regression assertion by checking that no tools were attached at all, not just `func_tool` being `None`.

Since the regression is currently checked only via `assert result.provider_request.func_tool is None`, please also assert that any other tool-related collections on `provider_request` (e.g., lists of tools or `tool_specs`) are empty. This will keep the test robust if tools are later attached via different fields or `func_tool` is refactored away.

```suggestion
        assert result is not None

        provider_request = result.provider_request
        assert provider_request.func_tool is None

        # Strengthen regression: ensure no other tools were attached at all
        for attr in ("tools", "tool_specs", "tool_choice"):
            if hasattr(provider_request, attr):
                value = getattr(provider_request, attr)
                # Treat both None and empty collections as "no tools"
                assert not value, (
                    f"Expected provider_request.{attr} to be empty/None when "
                    "computer_use_runtime='none' and add_cron_tools=True"
                )
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (testing): Consider adding a complementary test for providers that do support tool use to ensure tools are retained after the refactor.

To complement the negative case, please add a positive test where mock_provider.provider_config["modalities"] includes tool support and add_cron_tools=True. That test should assert that the function tools remain on result.provider_request after _modalities_fix and _sanitize_context_by_modalities, so we catch any future regressions that strip tools for tool-capable providers.

[sourcery-ai]

suggestion (testing): Strengthen the regression assertion by checking that no tools were attached at all, not just func_tool being None.

Since the regression is currently checked only via assert result.provider_request.func_tool is None, please also assert that any other tool-related collections on provider_request (e.g., lists of tools or tool_specs) are empty. This will keep the test robust if tools are later attached via different fields or func_tool is refactored away.

Suggested change

	assert result is not None
	assert result.provider_request.func_tool is None
	assert result is not None
	provider_request = result.provider_request
	assert provider_request.func_tool is None
	# Strengthen regression: ensure no other tools were attached at all
	for attr in ("tools", "tool_specs", "tool_choice"):
	if hasattr(provider_request, attr):
	value = getattr(provider_request, attr)
	# Treat both None and empty collections as "no tools"
	assert not value, (
	f"Expected provider_request.{attr} to be empty/None when "
	"computer_use_runtime='none' and add_cron_tools=True"
	)

[gemini-code-assist]

Code Review

This pull request moves the modality-related sanitization logic in build_main_agent to a later stage in the agent construction process. This change ensures that tools added by plugins or web search are correctly filtered based on the provider's supported modalities. Additionally, a unit test has been included to verify that tools are stripped when the provider is restricted to text-only modalities. I have no feedback to provide.

[Soulter]

I put all modalities fix logic into tool loop agent runner in #7506 and fixed this issue