feat(loop): let a tool hand the model an image, not a description of one

CHANGELOG.md

@@ -7,6 +7,91 @@ the GitHub Release body, so a release with no entry here fails.
 
 Versioning follows [docs/versioning.md](docs/versioning.md).
 
+## [0.10.0] - 2026-09-08
+
+### Added
+
+- A tool can return images that the model itself looks at, instead of prose from
+  a second model describing them. Return
+  `tool_content(text, images=[image_attachment(png_bytes, "image/png", label=path)])`
+  from `agent_core.tool_content` — a plain JSON dict, not a dataclass, so it
+  survives the `json.dumps` boundary a sandbox-native tool's return value
+  crosses. `ToolResult` grows an `images` field; `result` remains the full text,
+  so observers, spill/recovery, repeat detection and the trajectory see exactly
+  what they saw before.
+
+  Whether the pixels reach the provider is decided by the loop, from
+  `ModelProfile.supports_images` (default `False`, so nothing changes for a
+  product that does not opt in) and the wire protocol. Only
+  `chat_completions` is implemented: the attachment becomes an OpenAI
+  `image_url` content part carrying a data URI. The Anthropic block spelling is
+  **not** interchangeable — a served OpenAI-compatible endpoint rejects it with
+  HTTP 400 and a pydantic union error, taking the whole turn down, so an
+  unimplemented protocol withholds rather than guessing.
+
+  `HistoryPolicy.max_images_in_history` (default 5) now does something: after
+  each tool batch the loop keeps that many of the newest images and replaces
+  the rest with text. Both flags existed as unread placeholders before this
+  release.
+
+  **An image that leaves the context always leaves a sentence behind** — when
+  it is withheld for capability reasons, and when it is evicted for room. This
+  is not cosmetic. In the calibration run, deleting the image block from an
+  otherwise working request did not make the model report a missing image: it
+  reported a four-digit code and three shapes, all invented. A tool result that
+  reads as though an image were delivered will be answered as though one were.
+  Products adding their own image paths should preserve this property.
+
+### Fixed
+
+- Structured image results now reject Base64-valid data that is not a
+  recognizable PNG, JPEG, WEBP or GIF, and reject a declared MIME type that
+  disagrees with the payload. Lightweight container checks also reject common
+  truncation and corruption before delivery (including PNG chunk/CRC damage).
+  The Apodex endpoint otherwise fails the entire completion while decoding the
+  bad image. Token-accounting dimensions are always read from the payload, so
+  stale producer metadata cannot price a 4K image as one token and bypass the
+  context guard.
+- `ExtensionsConfig.has_changed` compares a digest of the file's bytes instead
+  of `st_mtime > loaded_mtime`, so a skill toggled on disk is actually picked up
+  by `get_enabled_skills`. Timestamps are much coarser than the edits they were
+  being asked to order: the filesystem clock advances in 1 ms steps and two
+  consecutive writes collide on a single mtime about 92% of the time, so a
+  change landing in the same millisecond as the load was invisible. The strict
+  `>` also could not see a timestamp moving BACKWARD -- restoring a backup, a
+  `git checkout`, an `rsync --times` of an older revision -- and it reported a
+  change for an identical rewrite, forcing a reload with nothing to reload.
+  This surfaced as an intermittent failure in `test_skills_loader_reload.py`
+  whose rate tracked machine speed; the regression tests now pin both
+  timestamps to one value and fail deterministically without the fix.
+
+### Changed
+
+- `messages.text_of` renders an `image_url` content block as
+  `[image — not visible in this text-only rendering]` instead of the empty
+  string. Every caller is either sizing a message or building a text-only
+  rendering — the Anthropic message translation, a compaction summary, a
+  trajectory line — and in the rendering case the image is being dropped at
+  that call. **If a product already puts `image_url` blocks in `content`, its
+  flattened text changes**; nothing else in this package produced such blocks
+  before 0.10.0.
+- `tokens.estimate_message_tokens` charges for inline images, read off the
+  `image_meta` message key written at attach time (~1 token per 1024 pixels,
+  fitted against measured `prompt_tokens`; a 1080p screenshot is ~2.4K tokens,
+  a 4K one ~8.5K). It previously returned the length of the caption alone, so a
+  history of screenshots measured as nearly empty to the context guard and to
+  every compaction trigger. Estimates for image-free histories are unchanged.
+- `TrajectoryFileObserver` writes `[N KB of image data elided from trace]` in
+  place of an inline image's base64, via the new
+  `tool_content.redacted_for_trace`. Live tool-result JSON and JSONL entries
+  record the same redacted shape from `ToolResult.images`; previously that path
+  kept only the text and omitted that the tool returned an image. The block
+  keeps its `image_url` type and states its size, without copying the Base64.
+- `Message` gains the in-process key `image_meta`, positionally aligned with the
+  `image_url` blocks in `content`. It is outside `WIRE_MESSAGE_KEYS`, so
+  `for_wire` strips it. It is message-level rather than per-block because a
+  content part carrying an unknown key is rejected by the served endpoint.
+
 ## [0.9.1] - 2026-09-08
 
 ### Fixed

agent_core/components/observers/trajectory.py

@@ -24,6 +24,7 @@
     ToolResult,
     TurnContext,
 )
+from agent_core.tool_content import redacted_for_trace, redacted_tool_result_content
 
 _FORMATS: tuple[str, ...] = ("json", "jsonl")
 _DEFAULT_FORMATS: tuple[str, ...] = _FORMATS
@@ -399,7 +400,10 @@ def _message_to_dict(self, m: Any) -> dict | None:
         is a copy. Anything that isn't a role-bearing dict is dropped.
         """
         if isinstance(m, dict) and m.get("role"):
-            return dict(m)
+            # ``redacted_for_trace`` swaps an inline image's base64 for its
+            # size. Verbatim, one 1080p screenshot writes ~137 KB here for
+            # every turn it stays in history.
+            return dict(redacted_for_trace(m))
         return None
 
     # ── Lifecycle hooks ─────────────────────────────────────────────────
@@ -563,15 +567,20 @@ async def on_tool_result(
         # — that fallback advances ``_tool_results_seen``, so sharing it would
         # double-count, and a synthesised id matches nothing outside the
         # snapshot anyway. Empty here means the runtime itself had no id.
-        self._write_jsonl({
+        jsonl_record: dict[str, Any] = {
             "t": "result",
             "turn": ctx.turn,
             "name": result.name,
             "tool_call_id": getattr(result, "tool_call_id", "") or "",
             "result": result.result,
             "error": result.is_error,
             "ms": result.duration_ms,
-        })
+        }
+        if result.images:
+            # The result event predates message attachment/capability gating, so
+            # this states what the TOOL returned, not that every profile saw it.
+            jsonl_record["images"] = redacted_tool_result_content("", result.images)
+        self._write_jsonl(jsonl_record)
 
         if "json" in self._formats:
             cid = getattr(result, "tool_call_id", "") or ""
@@ -584,10 +593,14 @@ async def on_tool_result(
                 )
                 self._tool_results_seen[ctx.turn] = seen + 1
             body = _clip(self._stringify(result.result), _BODY_MAX_CHARS)
+            rendered_body = f"[error] {body}" if result.is_error else body
             self._append_message({
                 "role": "tool",
                 "tool_call_id": cid,
-                "content": f"[error] {body}" if result.is_error else body,
+                "content": redacted_tool_result_content(
+                    rendered_body,
+                    result.images,
+                ),
             })
             self._flush_json()
         return None

agent_core/components/skills/extensions_config.py

@@ -7,10 +7,10 @@
 
 from __future__ import annotations
 
+import hashlib
 import json
 import logging
 import os
-from contextlib import suppress
 from pathlib import Path
 from typing import Any
 
@@ -56,7 +56,8 @@ class ExtensionsConfig(BaseModel):
 
     skills: dict[str, SkillStateConfig] = Field(default_factory=dict)
     _file_path: Path | None = PrivateAttr(default=None)
-    _file_mtime: float = PrivateAttr(default=0.0)
+    # Digest of the bytes this config was parsed from -- see ``has_changed``.
+    _file_digest: str = PrivateAttr(default="")
 
     model_config = {"populate_by_name": True}
 
@@ -79,14 +80,15 @@ def from_file(cls, config_path: str | Path | None = None) -> ExtensionsConfig:
             return cls()
 
         try:
-            with open(resolved, encoding="utf-8") as f:
-                data = json.load(f)
+            raw = resolved.read_bytes()
+            data = json.loads(raw.decode("utf-8"))
             _resolve_env_variables(data)
             logger.info("Loaded extensions config from %s", resolved)
             instance = cls.model_validate(data)
             instance._file_path = resolved
-            with suppress(OSError):
-                instance._file_mtime = resolved.stat().st_mtime
+            # Digest the exact bytes that were parsed, so the stored fingerprint
+            # and the loaded state can never describe different file contents.
+            instance._file_digest = _digest(raw)
             return instance
         except Exception as e:
             logger.warning("Failed to load extensions config %s: %s", resolved, e)
@@ -103,11 +105,33 @@ def source_path(self) -> Path | None:
         return self._file_path
 
     def has_changed(self) -> bool:
-        """Return True if the backing file has been modified since load."""
+        """Return True if the backing file's contents differ from what we hold.
+
+        Compares a digest of the bytes, not the modification time. Two reasons,
+        both of which bit this code:
+
+        Timestamps are far coarser than the edits they are meant to order. The
+        filesystem clock here advances in 1 ms steps, and two consecutive writes
+        land on an identical mtime about 92% of the time -- so a change made
+        within a millisecond of the load was simply invisible, and an operator
+        toggling a skill got the old state until something else touched the
+        file. That was reaching the test suite as an intermittent failure whose
+        rate tracked how fast the machine happened to be running.
+
+        A strict ``>`` also cannot see a file whose timestamp moves BACKWARD,
+        which is the normal outcome of restoring a backup, a ``git checkout``,
+        or an ``rsync --times`` of an older revision. The content changed; the
+        config went on reporting that it had not.
+
+        The file is a small JSON document and this is called from
+        ``get_enabled_skills``, which its callers cache -- reading it is cheaper
+        than being wrong about it. An identical rewrite correctly reports no
+        change, since nothing needs reloading.
+        """
         if self._file_path is None or not self._file_path.is_file():
             return False
         try:
-            return self._file_path.stat().st_mtime > self._file_mtime
+            return _digest(self._file_path.read_bytes()) != self._file_digest
         except OSError:
             return False
 
@@ -117,6 +141,11 @@ def is_skill_enabled(self, skill_name: str) -> bool:
         return state.enabled if state else True
 
 
+def _digest(raw: bytes) -> str:
+    """Content fingerprint. Not a security boundary -- just change detection."""
+    return hashlib.blake2b(raw, digest_size=16).hexdigest()
+
+
 def _resolve_env_variables(obj: Any) -> Any:
     """Recursively replace $VAR_NAME with environment variable values."""
     if isinstance(obj, str) and obj.startswith("$"):

agent_core/loop_types.py

@@ -312,6 +312,13 @@ class ToolResult:
     error_kind: str = ""
     # Opaque host-owned handle for a result body shed from model context.
     result_id: str = ""
+    # Image attachments the tool returned alongside ``result``, validated by
+    # ``agent_core.tool_content.parse_tool_content``. ``result`` stays the
+    # complete TEXT of the result, so every consumer that reads a string --
+    # observers, spill/recovery, repeat detection, the trajectory -- is
+    # unaffected by a tool that attaches images. Only the message built for the
+    # provider looks at this, and only when the model can actually see it.
+    images: list[dict[str, Any]] = field(default_factory=list[dict[str, Any]])
     # Host-provided repeated-invocation metadata. Execution is never skipped.
     repeat_count: int = 1
     repeat_recovery_id: str = ""

agent_core/messages.py

@@ -75,6 +75,15 @@ class Message(TypedDict, total=False):
     # the text back, which is what makes an index distinguishable from a summary
     # that happens to quote one. Filtered out by ``for_wire``.
     spill_refs: list[str]
+    # Per-image bookkeeping for the ``image_url`` blocks in ``content``, in the
+    # same order: ``{"label": str, "tokens": int}`` each. Written by
+    # ``runtime.loop.image_attach.attach_images`` and read back by eviction (to
+    # name an image it is removing) and by the token estimate (so a history of
+    # images is not costed at zero). It cannot live inside the content blocks:
+    # a content part carrying an unknown key is rejected outright by the served
+    # endpoint's pydantic union, whereas a message-level key outside
+    # ``WIRE_MESSAGE_KEYS`` is dropped by ``for_wire``.
+    image_meta: list[dict[str, Any]]
 
 
 # ── Wire boundary ────────────────────────────────────────────────────────
@@ -222,6 +231,10 @@ def assistant_msg_with_reasoning(
 # ── Helpers ──────────────────────────────────────────────────────────────
 
 
+# What an inline image renders as once its blocks are flattened away.
+_IMAGE_PLACEHOLDER = "[image — not visible in this text-only rendering]"
+
+
 def text_of(content: Any) -> str:
     """Flatten an OpenAI/Anthropic message content to plain text."""
     if content is None:
@@ -235,6 +248,18 @@ def text_of(content: Any) -> str:
                 parts.append(block)
             elif isinstance(block, dict):
                 content_block = cast(dict[str, object], block)
+                # An image flattened to text must leave a mark. Every caller is
+                # either estimating size or building a text-only rendering --
+                # the Anthropic translation, a compaction summary, a trajectory
+                # line -- and in the rendering case the image is being dropped
+                # right here. Returning nothing for it yields a tool result that
+                # reads as though it were pure text and never mentioned an
+                # image, which is the exact input that made the calibration
+                # model invent a reading of a picture it could not see (see
+                # ``runtime.loop.image_attach``).
+                if content_block.get("type") == "image_url":
+                    parts.append(_IMAGE_PLACEHOLDER)
+                    continue
                 val = content_block.get("text") or content_block.get("content") or ""
                 if isinstance(val, str):
                     parts.append(val)

agent_core/runtime/loop/agent_loop.py

@@ -49,6 +49,7 @@
     DefaultMessageCompactor,
     estimate_tokens,
 )
+from agent_core.runtime.loop.image_attach import attach_images, evict_old_images
 from agent_core.runtime.loop.llm_client import (
     RUNAWAY_STATE_KEY,
     TRUNCATION_CONTINUATION_GUIDANCE,
@@ -527,6 +528,8 @@ async def _run_loop_inner(
                 runtime.body_has_spill_reference,
                 runtime.render_tool_result,
                 result_max_chars=tool_result_cap,
+                profile=profile,
+                max_images_in_history=policy.max_images_in_history,
             )
             total_tool_calls += tool_calls_executed
             if stop_reason:
@@ -1259,6 +1262,10 @@ async def _execute_tool_calls(
     ],
     *,
     result_max_chars: int | None = None,
+    profile: ModelProfile | None = None,
+    # -1 disables eviction. Not 0: a defaulted caller must not silently mean
+    # "throw every image away", which is what a 0 default would spell.
+    max_images_in_history: int = -1,
 ) -> tuple[str, int]:
     executable: list[tuple[int, dict]] = []
     synthetic: list[tuple[int, ToolResult]] = []
@@ -1356,8 +1363,26 @@ async def _execute_tool_calls(
             tr_result.tool_call_id,
         )
         cast("dict[str, Any]", history_message).update(message_metadata)
+        # After the recovery handle and the host metadata, so the text the model
+        # reads is final before it becomes the text block of a multimodal
+        # message. Called unconditionally: when the model cannot see images
+        # ``attach_images`` writes the note saying so, and a caller that skipped
+        # it on capability grounds would produce the one shape that is actually
+        # dangerous -- a result reading as though an image had been delivered,
+        # with no image in it.
+        attach_images(
+            history_message,
+            tr_result.images,
+            profile=profile or ModelProfile(model_id="default", provider="openai"),
+        )
         messages.append(history_message)
 
+    # Bound the history's image count once the batch is in, not before: the
+    # newest results are the ones worth keeping, and evicting first would let a
+    # turn that returned several images push out its own.
+    if max_images_in_history >= 0:
+        evict_old_images(messages, max_images_in_history)
+
     if any(result.interrupted for result in results):
         wait_interventions = await notify_observers(obs, "on_tool_wait_interrupted", ctx)
         merged_wait = merge_interventions(wait_interventions)