From 7f1d148778399c1922e5db94b472ac5a68e18141 Mon Sep 17 00:00:00 2001 From: ian-at-strix Date: Wed, 7 Oct 2026 16:11:58 -0400 Subject: [PATCH] feat(llm): keep images away from text-only models (#1465) * feat(llm): keep images away from text-only models Leave view_image out of the tools, replace MCP image blocks with a text note, and swap agent_browser's screenshot section for text-only guidance when the run's model does not accept images. * refactor(llm): disable view_image via is_enabled and gate skill image guidance by modality * refactor(skills): render opt-in Jinja skills instead of comment markers * Tweaked skill --- strix/agents/factory.py | 11 +++++++- strix/agents/prompt.py | 3 ++- strix/core/runner.py | 10 ++++++++ strix/skills/__init__.py | 17 +++++++++++-- strix/skills/tooling/agent_browser.md | 21 +++++++++++----- strix/tools/load_skill/tool.py | 4 +-- strix/tools/mcp/agent_tools.py | 2 ++ strix/tools/mcp/client.py | 14 +++++++++-- strix/tools/mcp/session.py | 2 ++ tests/test_agent_factory_shell.py | 12 +++++++++ tests/test_mcp_client.py | 36 ++++++++++++++++++++++++++- tests/test_runner_root_prompt.py | 9 +++++++ tests/test_skill_dir_extension.py | 11 ++++++++ 13 files changed, 137 insertions(+), 15 deletions(-) diff --git a/strix/agents/factory.py b/strix/agents/factory.py index 6622d3c0..3b9dd616 100644 --- a/strix/agents/factory.py +++ b/strix/agents/factory.py @@ -375,11 +375,15 @@ def _configure_filesystem_tools( ) -def _make_filesystem_configurator(*, chat_completions: bool, strict_schemas: bool) -> Any: +def _make_filesystem_configurator( + *, chat_completions: bool, strict_schemas: bool, supports_images: bool = True +) -> Any: def configure(toolset: Any) -> None: _configure_filesystem_tools( toolset, chat_completions=chat_completions, strict_schemas=strict_schemas ) + if not supports_images: + toolset.view_image.is_enabled = False return configure @@ -670,6 +674,7 @@ def build_strix_agent( system_prompt_context: dict[str, Any] | None = None, extra_tools: Sequence[Tool] | None = None, instructions_override: str | None = None, + supports_images: bool = True, ) -> SandboxAgent[Any]: """Build a SandboxAgent for either root or child use. @@ -694,6 +699,7 @@ def build_strix_agent( is_diff_scoped=is_diff_scoped, interactive=interactive, system_prompt_context=system_prompt_context, + supports_images=supports_images, ) agent_tools = [*_EXTRA_TOOLS, *(extra_tools or [])] @@ -733,6 +739,7 @@ def build_strix_agent( configure_tools=_make_filesystem_configurator( chat_completions=chat_completions_tools, strict_schemas=strict_tool_schemas, + supports_images=supports_images, ), ), Shell( @@ -754,6 +761,7 @@ def make_child_factory( chat_completions_tools: bool = False, strict_tool_schemas: bool = True, system_prompt_context: dict[str, Any] | None = None, + supports_images: bool = True, ) -> Any: """Return the runner-owned builder used by ``spawn_child_agent``. @@ -774,6 +782,7 @@ def make_child_factory( chat_completions_tools=chat_completions_tools, strict_tool_schemas=strict_tool_schemas, system_prompt_context=system_prompt_context, + supports_images=supports_images, ) return _factory diff --git a/strix/agents/prompt.py b/strix/agents/prompt.py index cf53ca67..c45831aa 100644 --- a/strix/agents/prompt.py +++ b/strix/agents/prompt.py @@ -85,6 +85,7 @@ def render_system_prompt( interactive: bool = False, system_prompt_context: dict[str, Any] | None = None, include_scope: bool = True, + supports_images: bool = True, ) -> str: """Render the system prompt. Returns empty string on template failure. @@ -119,7 +120,7 @@ def render_system_prompt( is_root=is_root, is_diff_scoped=is_diff_scoped, ) - skill_content = load_skills(skills_to_load) + skill_content = load_skills(skills_to_load, supports_images=supports_images) env.globals["get_skill"] = lambda name: skill_content.get(name, "") # Skills every agent of this kind loads come first, so siblings share them diff --git a/strix/core/runner.py b/strix/core/runner.py index f43b1d7f..55784d21 100644 --- a/strix/core/runner.py +++ b/strix/core/runner.py @@ -23,6 +23,7 @@ from strix.config.models import ( StrixProvider, configure_sdk_api_route, configure_sdk_model_defaults, + model_supports_images, supports_strict_tool_schemas, uses_chat_completions_tool_schema, ) @@ -153,6 +154,7 @@ def _compose_root_instructions_override( is_diff_scoped: bool, interactive: bool, system_prompt_context: dict[str, Any], + supports_images: bool, ) -> str | None: if root_instructions_override is None: return None @@ -166,6 +168,7 @@ def _compose_root_instructions_override( interactive=interactive, system_prompt_context=system_prompt_context, include_scope=False, + supports_images=supports_images, ) return ( f"{base_instructions}\n\n" @@ -265,6 +268,9 @@ async def run_strix_scan( strict_tool_schemas = supports_strict_tool_schemas(resolved_model) if not strict_tool_schemas: logger.info("Sending non-strict tool schemas: %s caps strict tools", resolved_model) + supports_images = model_supports_images(resolved_model) + if not supports_images: + logger.info("Leaving out image tools: %s does not accept images", resolved_model) if budget_policy not in ("stop", "pause"): raise ValueError(f"unknown budget_policy: {budget_policy!r}") @@ -468,6 +474,7 @@ async def run_strix_scan( is_diff_scoped=is_diff_scoped, interactive=interactive, system_prompt_context=root_context, + supports_images=supports_images, ) root_agent = build_strix_agent( @@ -482,6 +489,7 @@ async def run_strix_scan( strict_tool_schemas=strict_tool_schemas, system_prompt_context=root_context, instructions_override=root_instructions, + supports_images=supports_images, ) if not is_resume: @@ -501,6 +509,7 @@ async def run_strix_scan( chat_completions_tools=chat_completions_tools, strict_tool_schemas=strict_tool_schemas, system_prompt_context=scope_context, + supports_images=supports_images, ) async def spawn_child_agent(**kwargs: Any) -> dict[str, Any]: @@ -528,6 +537,7 @@ async def run_strix_scan( "spawn_child_agent": spawn_child_agent, "scan_targets": build_scan_targets(scan_config), "max_context_images": settings.runtime.max_context_images, + "supports_images": supports_images, } root_session = open_agent_session(root_id, agents_db) diff --git a/strix/skills/__init__.py b/strix/skills/__init__.py index db5ae3e4..bc2d3b40 100644 --- a/strix/skills/__init__.py +++ b/strix/skills/__init__.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import TypeGuard import yaml +from jinja2 import Environment, StrictUndefined from strix.utils.resource_paths import get_strix_resource_path @@ -13,6 +14,14 @@ from strix.utils.resource_paths import get_strix_resource_path logger = logging.getLogger(__name__) _FRONTMATTER_PATTERN = re.compile(r"^---\s*\n(?P.*?)\n---\s*\n", re.DOTALL) +# Only skills with `template: jinja` in their frontmatter are rendered; the rest hold +# literal `{{ }}` payloads. +_SKILL_TEMPLATES = Environment( + autoescape=False, # noqa: S701 - prompts, not HTML + undefined=StrictUndefined, + trim_blocks=True, + lstrip_blocks=True, +) _INTERNAL_SKILL_CATEGORIES: frozenset[str] = frozenset({"scan_modes", "coordination", "analysis"}) _ROOT_SKILL_CATEGORY = "root" @@ -261,7 +270,7 @@ def _candidate_skill_files(skill_name: str) -> list[Path]: return _bare_skill_files(skill_name) -def load_skills(skill_names: list[str]) -> dict[str, str]: +def load_skills(skill_names: list[str], *, supports_images: bool = True) -> dict[str, str]: """Load skill markdown bodies (frontmatter stripped) by name. Skill files live at ``strix/skills//.md`` (or any @@ -291,7 +300,11 @@ def load_skills(skill_names: list[str]) -> dict[str, str]: continue var_name = skill_name.split("/")[-1] - _, skill_body = _parse_skill_content(content, file_path) + metadata, skill_body = _parse_skill_content(content, file_path) + if metadata.get("template") == "jinja": + skill_body = _SKILL_TEMPLATES.from_string(skill_body).render( + supports_images=supports_images + ) skill_content[var_name] = skill_body logger.debug("Loaded skill: %s -> %s", skill_name, var_name) _track_skill_loaded(var_name, file_path) diff --git a/strix/skills/tooling/agent_browser.md b/strix/skills/tooling/agent_browser.md index a254bfaf..efc59cdd 100644 --- a/strix/skills/tooling/agent_browser.md +++ b/strix/skills/tooling/agent_browser.md @@ -1,6 +1,7 @@ --- name: agent_browser description: agent-browser CLI for headless Chrome via shell. Snapshot-and-ref workflow, click/fill/extract, screenshots, multi-tab, multi-session, network mocking. Pre-installed in the sandbox; invoke via exec_command. +template: jinja --- @@ -260,8 +261,9 @@ only for simple expressions. ### Screenshot -`agent-browser screenshot` writes a PNG to disk in the sandbox. The -shell command alone does **not** put the image into your context — +`agent-browser screenshot` writes a PNG to disk in the sandbox. +{% if supports_images %} +The shell command alone does **not** put the image into your context — chain it with the SDK ``view_image`` tool to actually see it: ```bash @@ -275,6 +277,12 @@ the full path on stdout — pass that to ``view_image``). If you need a specific filename, keep it inside that directory or a sibling hidden dir under ``/workspace``. Never write screenshots to ``/tmp`` — ``view_image`` rejects anything outside the workspace root. +{% else %} +You are running on a text-only model and cannot view images. Screenshots +are only useful to you as a saved file (for example, login evidence). +Drive the page from `snapshot -i` refs, `eval` for any DOM/JS state you +need to read, and `text @ref` / `get text` for content extraction. +{% endif %} ```bash agent-browser screenshot # path printed on stdout @@ -283,6 +291,7 @@ agent-browser screenshot --full # full scroll height agent-browser screenshot --annotate # numbered labels + legend keyed to snapshot refs ``` +{% if supports_images %} `--annotate` is designed for multimodal models: each label `[N]` maps to ref `@eN`. Take the annotated screenshot, then ``view_image`` it, and you can correlate visual layout with snapshot refs. @@ -294,10 +303,10 @@ layout questions, captchas, custom widgets where the accessibility tree is incomplete). If ``view_image`` errors back at you (rejected image, "vision not -supported", or similar), you are running on a text-only model — stop -calling it and stop taking screenshots. Drive the page entirely from -`snapshot -i` refs, `eval` for any DOM/JS state you need to read, and -`text @ref` / `get text` for content extraction. +supported", or similar), you can drive the page from `snapshot -i` +refs, `eval` for any DOM/JS state you need to read, and `text @ref` +or `get text` for content extraction. +{% endif %} ### Handle multiple pages via tabs diff --git a/strix/tools/load_skill/tool.py b/strix/tools/load_skill/tool.py index 3ef6f9ec..83f70f8d 100644 --- a/strix/tools/load_skill/tool.py +++ b/strix/tools/load_skill/tool.py @@ -24,12 +24,12 @@ async def load_skill(ctx: RunContextWrapper, skills: list[str]) -> str: Max 5. Names match the bare files under ``strix/skills//.md``. """ - del ctx requested = list(skills or []) err = validate_requested_skills(requested) if err: return f"load_skill: {err}" - contents = load_skills(requested) + context = ctx.context if isinstance(ctx.context, dict) else {} + contents = load_skills(requested, supports_images=context.get("supports_images", True)) if not contents: return "load_skill: no content loaded for requested skills." sections = [f"## Skill: {name}\n\n{body}" for name, body in contents.items()] diff --git a/strix/tools/mcp/agent_tools.py b/strix/tools/mcp/agent_tools.py index ab068c64..7fa2c392 100644 --- a/strix/tools/mcp/agent_tools.py +++ b/strix/tools/mcp/agent_tools.py @@ -288,9 +288,11 @@ async def call_mcp( "Call search_mcp_tools, then get_mcp_tool_schema." ) session = await entry.ensure_connected() + context = ctx.context if isinstance(ctx.context, dict) else {} return await session.dispatch( tool, arguments or {}, label=namespaced_tool_name(connection, tool), result_transform=entry.result_transform, + supports_images=context.get("supports_images", True), ) diff --git a/strix/tools/mcp/client.py b/strix/tools/mcp/client.py index c0de7fab..4982ffcc 100644 --- a/strix/tools/mcp/client.py +++ b/strix/tools/mcp/client.py @@ -183,7 +183,9 @@ def _build_server(config: McpConnectionConfig) -> BuiltMcpServer: ) -def _mcp_result_to_tool_output(server: MCPServer, result: Any) -> Any: +def _mcp_result_to_tool_output( + server: MCPServer, result: Any, *, supports_images: bool = True +) -> Any: """Serialize a ``CallToolResult`` to a tool output, mirroring the agents SDK. This reproduces the serialization in ``agents.mcp.util.MCPUtil.invoke_mcp_tool`` @@ -199,6 +201,13 @@ def _mcp_result_to_tool_output(server: MCPServer, result: Any) -> Any: for item in result.content: if item.type == "text": outputs.append({"type": "text", "text": item.text}) + elif item.type == "image" and not supports_images: + outputs.append( + { + "type": "text", + "text": f"[{item.mimeType} image omitted: this model cannot view images]", + } + ) elif item.type == "image": outputs.append( {"type": "image", "image_url": f"data:{item.mimeType};base64,{item.data}"} @@ -217,6 +226,7 @@ async def dispatch_mcp_call( *, label: str, result_transform: ResultTransform | None = None, + supports_images: bool = True, ) -> Any: """Run one MCP tool call and convert its result to a tool output. @@ -235,7 +245,7 @@ async def dispatch_mcp_call( result = await server.call_tool(tool_name, arguments) if result_transform is not None: return result_transform(label, result.model_dump(mode="json")) - tool_output = _mcp_result_to_tool_output(server, result) + tool_output = _mcp_result_to_tool_output(server, result, supports_images=supports_images) if getattr(result, "isError", False): return _errored_tool_output(tool_output) return tool_output diff --git a/strix/tools/mcp/session.py b/strix/tools/mcp/session.py index f90a4f87..9cf85408 100644 --- a/strix/tools/mcp/session.py +++ b/strix/tools/mcp/session.py @@ -365,6 +365,7 @@ class SupervisedMcpSession: *, label: str, result_transform: ResultTransform | None = None, + supports_images: bool = True, ) -> Any: """Run one tool call with bounded retries for transient session failures. @@ -382,6 +383,7 @@ class SupervisedMcpSession: arguments, label=label, result_transform=result_transform, + supports_images=supports_images, ) outcome = await self._run_job(job, phase="call") diff --git a/tests/test_agent_factory_shell.py b/tests/test_agent_factory_shell.py index 6bef1211..0109bf52 100644 --- a/tests/test_agent_factory_shell.py +++ b/tests/test_agent_factory_shell.py @@ -115,3 +115,15 @@ def test_function_tools_are_result_bounded() -> None: by_name = {t.name: t for t in agent.tools} assert getattr(by_name["think"], "_strix_bounded", False) is True + + +@pytest.mark.parametrize(("supports_images", "has_view_image"), [(True, True), (False, False)]) +def test_view_image_is_left_out_for_text_only_models( + supports_images: bool, has_view_image: bool +) -> None: + agent = factory.build_strix_agent(is_root=True, supports_images=supports_images) + filesystem = agent.capabilities[0] + filesystem.bind(cast("Any", object())) + + view_image = next(tool for tool in filesystem.tools() if tool.name == "view_image") + assert cast("FunctionTool", view_image).is_enabled is has_view_image diff --git a/tests/test_mcp_client.py b/tests/test_mcp_client.py index 54f49f54..97d78893 100644 --- a/tests/test_mcp_client.py +++ b/tests/test_mcp_client.py @@ -18,7 +18,7 @@ from typing import TYPE_CHECKING, Any import pytest from agents.mcp import MCPServer, MCPServerStdio, MCPServerStreamableHttp from agents.tool_context import ToolContext -from mcp.types import CallToolResult, TextContent +from mcp.types import CallToolResult, ImageContent, TextContent from mcp.types import Tool as MCPTool from pydantic import ValidationError @@ -884,6 +884,40 @@ async def test_call_mcp_flags_an_errored_result_failed_for_the_tui() -> None: assert out == {"type": "text", "text": "boom:read_file", "success": False} +@pytest.mark.asyncio +async def test_call_mcp_replaces_images_for_a_text_only_model() -> None: + class _ImageServer(FakeMCPServer): + async def call_tool( + self, + tool_name: str, + arguments: dict[str, Any] | None, + meta: dict[str, Any] | None = None, + ) -> CallToolResult: + return CallToolResult( + content=[ + TextContent(type="text", text="shot"), + ImageContent(type="image", data="aGk=", mimeType="image/png"), + ] + ) + + registry = McpRegistry() + registry.add( + name="fs", server=_ImageServer("fs", [_mcp_tool("shot")]), purpose=None, tool_count=1 + ) + ctx = _ctx(registry) + args = json.dumps({"connection": "fs", "tool": "shot"}) + + out = await call_mcp.on_invoke_tool(ctx, args) + assert out[1] == {"type": "image", "image_url": "data:image/png;base64,aGk="} + + ctx.context["supports_images"] = False + out = await call_mcp.on_invoke_tool(ctx, args) + assert out == [ + {"type": "text", "text": "shot"}, + {"type": "text", "text": "[image/png image omitted: this model cannot view images]"}, + ] + + # --- generic MCP tools are the only MCP surface every agent gets ------------- diff --git a/tests/test_runner_root_prompt.py b/tests/test_runner_root_prompt.py index 0e24908d..631c9271 100644 --- a/tests/test_runner_root_prompt.py +++ b/tests/test_runner_root_prompt.py @@ -287,6 +287,15 @@ def test_requested_skills_follow_the_shared_prefix() -> None: assert "" in xss.split("")[1] +def test_text_only_prompt_drops_screenshot_guidance() -> None: + assert "view_image" in render_system_prompt(include_scope=False) + + prompt = render_system_prompt(include_scope=False, supports_images=False) + assert "view_image" not in prompt + assert "text-only model and cannot view images" in prompt + assert "