From 9162802c8ee4f66f0661c4f0a3f5a695a36426c6 Mon Sep 17 00:00:00 2001
From: Andy Bonventre <andybons@meetneptune.com>
Date: Wed, 12 Aug 2026 16:17:32 -0400
Subject: [PATCH 1/4] feat(engine): read_file returns images as real Blob
 content

engine/filetools.go's read_file tool now sniffs a file's magic bytes
(never its extension) and returns a Text+Blob ToolResult for a
recognized image (png/jpeg/gif/webp), matching the shape MCP already
produces. Every transcoder's existing Blob handling and imageclamp
pass apply with no new wiring. A 20MB pre-clamp cap on the file size
read_file will read in full stops it from loading an unbounded file
into memory; an over-cap image returns a clear text error, no Blob.

A transcode-time degrade of an image Blob to text for a model with no
vision capability is deliberately not implemented: no per-model vision
signal exists anywhere in the codebase to gate it on. Filed as
majorcontext/harness#129 instead of forcing an ugly seam.

AGENTS.md documents both the shipped behavior and the deferred gap.
---
 AGENTS.md                            |  46 ++++++++++
 engine/filetools.go                  |  73 +++++++++++++++
 engine/filetools_test.go             | 131 +++++++++++++++++++++++++++
 provider/anthropic/transcode_test.go |  58 ++++++++++++
 4 files changed, 308 insertions(+)

diff --git a/AGENTS.md b/AGENTS.md
index 9905d19..da36f0c 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -134,6 +134,52 @@ never written to the session log — a resumed session rediscovers them. Config
 `skills_dirs` (array; a non-empty project value overrides the user value
 entirely) and the repeatable `-skills-dir` run/serve flag drive it.
 
+### read_file image support
+
+The built-in `read_file` tool (`engine/filetools.go`) returns an image file
+as real visual content, not mangled text. `sniffImageMediaType` classifies
+the file by its magic bytes (`http.DetectContentType` over at most the
+first 512 bytes) — never by its extension: a `.txt` file that is actually a
+PNG is still recognized as an image, and a `.png` file that is actually
+text stays a text read. This closes the gap where an agent reading a
+screenshot got raw bytes fed to the model as if they were source text.
+
+For a recognized image (`image/png`, `image/jpeg`, `image/gif`,
+`image/webp`), `read_file` returns a `message.ToolResult` whose Content is
+`[Text, Blob]`: a one-line Text summary (format, byte size, and pixel
+dimensions when `image.DecodeConfig` can decode the header cheaply) followed
+by a `message.Blob` carrying the real file bytes. This is the same
+`Text`+`Blob` shape MCP's `mcpContentToParts` already produces
+(`engine/mcp.go`) — read_file is simply a second producer of it — so every
+transcoder's existing Blob handling and the imageclamp dimension/byte-size
+pass (`imageclamp.Clamp`, called from every transcoder's `transcodeRequest`)
+apply to a read_file image exactly as they do to an MCP one, with no new
+wiring. `read_file` never bypasses that clamp: it does not resize, re-encode,
+or otherwise touch pixels itself.
+
+`readFileMaxImageBytes` (20MB) caps the file size `read_file` will read in
+full once an image is detected — separate from and smaller than any
+provider's wire limit, which `imageclamp.Clamp` enforces at transcode time.
+This cap exists only so `read_file` itself never loads an unbounded file
+into memory; an over-cap image returns a plain text error and no Blob. A
+non-image binary file (sniffed as `application/octet-stream` or similar)
+is unaffected by this change and keeps its existing (unbounded) text-read
+behavior.
+
+**Known gap, filed as issue #129, not fixed here**: a transcode-time
+degrade of an image Blob to a text placeholder for a model with no vision
+capability is NOT implemented. No per-model vision-capability signal exists
+anywhere in the codebase to gate it on — the "embedded models.dev catalog"
+this file's own "Architecture" section describes as a design goal is not
+yet built (no such package or embedded data exists today), and
+`provider.Request` carries no capability flag comparable to `Effort` or
+`SessionKey` that a caller could set from one. Forcing this now would mean
+inventing an ad hoc, likely-wrong static model list inside this PR, so it
+is deferred to issue #129 rather than an ugly seam. Until it lands, a
+model with no vision support receives the image Blob exactly as any other
+vision-capable model does, and how it handles that block is between the
+model and its provider.
+
 ### Base loop retry
 
 The base interactive `Prompt` loop retries a transient provider error itself,
diff --git a/engine/filetools.go b/engine/filetools.go
index 28ddf85..da09be7 100644
--- a/engine/filetools.go
+++ b/engine/filetools.go
@@ -1,13 +1,22 @@
 package engine
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
+	"image"
+	_ "image/gif"  // register GIF decoder for image.DecodeConfig
+	_ "image/jpeg" // register JPEG decoder for image.DecodeConfig
+	_ "image/png"  // register PNG decoder for image.DecodeConfig
+	"io"
+	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
 
+	_ "golang.org/x/image/webp" // register WebP decoder for image.DecodeConfig
+
 	"github.com/majorcontext/harness/message"
 	"github.com/majorcontext/harness/provider"
 )
@@ -21,6 +30,51 @@ const (
 	readFileMaxLineLen = 2000
 )
 
+// readFileMaxImageBytes bounds the file size read_file will read in full
+// when it detects an image (see sniffImageMediaType). The transcode-time
+// imageclamp pass (imageclamp.Clamp) enforces each provider's own wire
+// size limit; this cap exists only to stop read_file itself from loading
+// an unbounded file into memory before that clamp ever runs. It is a var,
+// not a const, so a test can shrink it instead of writing a real
+// oversized fixture.
+var readFileMaxImageBytes = 20 * 1024 * 1024 // 20MB
+
+// readFileImageMediaTypes lists the image formats read_file recognizes and
+// returns as a message.Blob for the model to see directly. Every other
+// sniffed content type — including any other image/* MIME type
+// http.DetectContentType might report, such as image/bmp — keeps
+// read_file's existing text-reading behavior unchanged.
+var readFileImageMediaTypes = map[string]bool{
+	"image/png":  true,
+	"image/jpeg": true,
+	"image/gif":  true,
+	"image/webp": true,
+}
+
+// sniffImageMediaType classifies the file at path by its magic bytes —
+// never by its extension. It reads at most the first 512 bytes (all
+// http.DetectContentType considers) so a large file is never fully read
+// just to answer this question. A file named ".txt" that actually holds
+// PNG bytes is reported as image/png; a file named ".png" that actually
+// holds plain text is reported as a non-image content type. isImage is
+// true only for a format read_file knows how to hand to the model as a
+// Blob (see readFileImageMediaTypes).
+func sniffImageMediaType(path string) (mediaType string, isImage bool, err error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return "", false, err
+	}
+	defer f.Close()
+
+	buf := make([]byte, 512)
+	n, err := f.Read(buf)
+	if n == 0 && err != nil && err != io.EOF {
+		return "", false, err
+	}
+	ct := http.DetectContentType(buf[:n])
+	return ct, readFileImageMediaTypes[ct], nil
+}
+
 // resolvePath resolves a tool path argument against the session working
 // directory. Absolute paths pass through unchanged.
 func (s *Session) resolvePath(path string) string {
@@ -62,6 +116,25 @@ func readFileTool() Tool {
 			if info.IsDir() {
 				return nil, fmt.Errorf("read_file: %s is a directory", path)
 			}
+
+			if mediaType, isImage, sniffErr := sniffImageMediaType(path); sniffErr == nil && isImage {
+				if info.Size() > int64(readFileMaxImageBytes) {
+					return nil, fmt.Errorf("read_file: %s is a %d-byte %s image, over the %d-byte read_file image limit", path, info.Size(), mediaType, readFileMaxImageBytes)
+				}
+				imgData, err := os.ReadFile(path)
+				if err != nil {
+					return nil, fmt.Errorf("read_file: %w", err)
+				}
+				summary := fmt.Sprintf("%s image, %d bytes", mediaType, len(imgData))
+				if cfg, _, err := image.DecodeConfig(bytes.NewReader(imgData)); err == nil {
+					summary = fmt.Sprintf("%s image, %d bytes, %dx%d pixels", mediaType, len(imgData), cfg.Width, cfg.Height)
+				}
+				return message.Parts{
+					&message.Text{Text: summary},
+					&message.Blob{MediaType: mediaType, Data: imgData},
+				}, nil
+			}
+
 			data, err := os.ReadFile(path)
 			if err != nil {
 				return nil, fmt.Errorf("read_file: %w", err)
diff --git a/engine/filetools_test.go b/engine/filetools_test.go
index 1a91210..87d7afd 100644
--- a/engine/filetools_test.go
+++ b/engine/filetools_test.go
@@ -1,9 +1,12 @@
 package engine
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
+	"image"
+	"image/png"
 	"os"
 	"path/filepath"
 	"strings"
@@ -209,6 +212,134 @@ func TestReadFileAbsolutePath(t *testing.T) {
 	}
 }
 
+// runToolParts invokes a built-in tool on a throwaway session rooted at
+// workDir and returns the raw message.Parts, not just the Text()
+// concatenation runTool returns — needed to inspect a returned Blob part.
+func runToolParts(t *testing.T, tool Tool, workDir, args string) (message.Parts, error) {
+	t.Helper()
+	s := NewSession(Config{WorkDir: workDir})
+	return tool.Run(context.Background(), s, json.RawMessage(args))
+}
+
+// tinyPNG builds a real, compliant, tiny PNG — a 2x2 solid image — so image
+// tests exercise genuine image bytes without a committed binary fixture
+// (AGENTS.md's fixture-size lesson from #101: keep test images tiny).
+func tinyPNG(t *testing.T) []byte {
+	t.Helper()
+	img := image.NewRGBA(image.Rect(0, 0, 2, 2))
+	var buf bytes.Buffer
+	if err := png.Encode(&buf, img); err != nil {
+		t.Fatalf("png.Encode: %v", err)
+	}
+	return buf.Bytes()
+}
+
+func TestReadFileImagePNGReturnsTextAndBlob(t *testing.T) {
+	dir := t.TempDir()
+	data := tinyPNG(t)
+	if err := os.WriteFile(filepath.Join(dir, "shot.png"), data, 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	parts, err := runToolParts(t, readFileTool(), dir, `{"path":"shot.png"}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(parts) != 2 {
+		t.Fatalf("parts = %d, want 2 (Text, Blob): %+v", len(parts), parts)
+	}
+	text, ok := parts[0].(*message.Text)
+	if !ok {
+		t.Fatalf("parts[0] = %T, want *message.Text", parts[0])
+	}
+	for _, want := range []string{"image/png", fmt.Sprintf("%d bytes", len(data)), "2x2"} {
+		if !strings.Contains(text.Text, want) {
+			t.Errorf("summary %q missing %q", text.Text, want)
+		}
+	}
+	blob, ok := parts[1].(*message.Blob)
+	if !ok {
+		t.Fatalf("parts[1] = %T, want *message.Blob", parts[1])
+	}
+	if blob.MediaType != "image/png" {
+		t.Errorf("MediaType = %q, want image/png", blob.MediaType)
+	}
+	if !bytes.Equal(blob.Data, data) {
+		t.Errorf("Blob.Data does not round-trip the source file bytes")
+	}
+}
+
+// TestReadFileImageExtensionLieTextNamedPNGStaysText is the surplus-direction
+// half of the extension-lies pair: a file NAMED .png that actually holds
+// plain text bytes must NOT be sniffed as an image. Trusting the extension
+// alone would wrongly wrap plain source text in an image Blob.
+func TestReadFileImageExtensionLieTextNamedPNGStaysText(t *testing.T) {
+	dir := t.TempDir()
+	writeTestFile(t, filepath.Join(dir, "notes.png"), "just some text\nsecond line\n")
+
+	parts, err := runToolParts(t, readFileTool(), dir, `{"path":"notes.png"}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(parts) != 1 {
+		t.Fatalf("parts = %d, want 1 (Text only): %+v", len(parts), parts)
+	}
+	text, ok := parts[0].(*message.Text)
+	if !ok {
+		t.Fatalf("parts[0] = %T, want *message.Text", parts[0])
+	}
+	if !strings.Contains(text.Text, "1→just some text") {
+		t.Errorf("out = %q, want ordinary line-numbered text", text.Text)
+	}
+}
+
+// TestReadFileImageExtensionLiePNGNamedTxtIsSniffedAsImage is the missing-
+// direction half: a file named .txt that actually holds PNG magic bytes must
+// still be recognized as an image. Extension is a hint only; magic bytes are
+// authoritative (AGENTS.md: "never trust extension alone").
+func TestReadFileImageExtensionLiePNGNamedTxtIsSniffedAsImage(t *testing.T) {
+	dir := t.TempDir()
+	data := tinyPNG(t)
+	if err := os.WriteFile(filepath.Join(dir, "disguised.txt"), data, 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	parts, err := runToolParts(t, readFileTool(), dir, `{"path":"disguised.txt"}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(parts) != 2 {
+		t.Fatalf("parts = %d, want 2 (Text, Blob): %+v", len(parts), parts)
+	}
+	blob, ok := parts[1].(*message.Blob)
+	if !ok {
+		t.Fatalf("parts[1] = %T, want *message.Blob", parts[1])
+	}
+	if blob.MediaType != "image/png" {
+		t.Errorf("MediaType = %q, want image/png", blob.MediaType)
+	}
+}
+
+func TestReadFileImageOverCapReturnsTextErrorNoBlob(t *testing.T) {
+	dir := t.TempDir()
+	data := tinyPNG(t)
+	if err := os.WriteFile(filepath.Join(dir, "shot.png"), data, 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	orig := readFileMaxImageBytes
+	readFileMaxImageBytes = len(data) - 1 // force this exact file over cap
+	t.Cleanup(func() { readFileMaxImageBytes = orig })
+
+	_, err := runToolParts(t, readFileTool(), dir, `{"path":"shot.png"}`)
+	if err == nil {
+		t.Fatal("want error for over-cap image")
+	}
+	if !strings.Contains(err.Error(), "image") {
+		t.Errorf("error = %q, want it to name the image/limit", err)
+	}
+}
+
 func TestWriteFileCreatesNestedDirs(t *testing.T) {
 	dir := t.TempDir()
 	out, err := runTool(t, writeFileTool(), dir, `{"path":"a/b/c.txt","content":"hello"}`)
diff --git a/provider/anthropic/transcode_test.go b/provider/anthropic/transcode_test.go
index 00915be..a9bb197 100644
--- a/provider/anthropic/transcode_test.go
+++ b/provider/anthropic/transcode_test.go
@@ -3,6 +3,7 @@ package anthropic
 import (
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -245,6 +246,63 @@ func TestTranscodeToolCallAndResult(t *testing.T) {
 	}
 }
 
+// TestTranscodeReadFileImageArrivesAsRealWireImageBlock is the read_file
+// counterpart of TestTranscodeToolCallAndResult above: engine/filetools.go's
+// read_file tool now returns exactly this shape for an image file — a Text
+// summary part ("image/png image, N bytes, WxH pixels") followed by a Blob
+// part carrying the real file bytes (engine/filetools_test.go's
+// TestReadFileImagePNGReturnsTextAndBlob proves read_file itself builds this
+// shape from its own production entry point, Tool.Run). This test proves the
+// OTHER half: that shape, once it reaches a tool_result, transcodes to a
+// real wire "image" content block on the Anthropic route — the only route
+// that recurses into tool-result Blobs at all (Limits.RecurseToolResults;
+// see imageclamp.Limits' doc comment) — with its bytes intact, not a text
+// placeholder or an omission note. openai and openaicompat instead replace a
+// tool-result Blob with a "[N image attachment(s) omitted]" note
+// (toolResultOutput, provider/openai/transcode.go); that is pre-existing,
+// unrelated wire-format behavior this PR does not change.
+func TestTranscodeReadFileImageArrivesAsRealWireImageBlock(t *testing.T) {
+	png := tinyPNG(t)
+	summary := fmt.Sprintf("image/png image, %d bytes, 2x2 pixels", len(png))
+	out := mustTranscode(t, baseRequest(
+		message.Message{Role: message.RoleUser, Parts: message.Parts{&message.Text{Text: "read shot.png"}}},
+		message.Message{Role: message.RoleAssistant, Parts: message.Parts{
+			&message.ToolCall{CallID: "toolu_rf", Name: "read_file", Arguments: json.RawMessage(`{"path":"shot.png"}`)},
+		}},
+		message.Message{Role: message.RoleTool, Parts: message.Parts{
+			&message.ToolResult{CallID: "toolu_rf", Content: message.Parts{
+				&message.Text{Text: summary},
+				&message.Blob{MediaType: "image/png", Data: png},
+			}},
+		}},
+	))
+
+	res := out.Messages[2]
+	if res.Role != "user" {
+		t.Fatalf("tool result role = %s", res.Role)
+	}
+	tr := res.Content[0]
+	if tr.Type != "tool_result" || tr.ToolUseID != "toolu_rf" {
+		t.Fatalf("tool_result = %+v", tr)
+	}
+	if len(tr.Content) != 2 {
+		t.Fatalf("tool_result content = %d blocks, want 2 (text, image): %+v", len(tr.Content), tr.Content)
+	}
+	if tr.Content[0].Type != "text" || tr.Content[0].Text != summary {
+		t.Errorf("text block = %+v, want text %q", tr.Content[0], summary)
+	}
+	img := tr.Content[1]
+	if img.Type != "image" {
+		t.Fatalf("second block Type = %q, want %q (a real image block, not a placeholder)", img.Type, "image")
+	}
+	if img.Source == nil || img.Source.Type != "base64" || img.Source.MediaType != "image/png" {
+		t.Fatalf("image Source = %+v", img.Source)
+	}
+	if want := base64.StdEncoding.EncodeToString(png); img.Source.Data != want {
+		t.Errorf("image Source.Data does not round-trip read_file's original bytes")
+	}
+}
+
 // TestTranscodeEmptyToolResultContentNeverOmitsWireField is the red-first
 // regression test for NEP-5272's B1 finding: a ToolResult with empty
 // Content used to transcode to a tool_result block whose Content ended up

From 02647ff6ec772dc329d10ca062cf17c3253ff57e Mon Sep 17 00:00:00 2001
From: Andy Bonventre <andybons@meetneptune.com>
Date: Wed, 12 Aug 2026 16:33:07 -0400
Subject: [PATCH 2/4] fix(engine): harden read_file image detection against
 review findings

Address the automated and Opus review of the read_file image feature:

- Single open/read pass (readImageIfDetected replaces the old
  sniff-then-reread split): fixes a double-open and dropped bytes on a
  non-regular file.
- io.ReadFull for the magic-byte sniff instead of one Read, so a short
  read(2) on a pipe or FUSE mount can no longer misclassify a real
  image as text.
- The 20MB cap now binds on an io.LimitReader over the read itself,
  not a pre-read os.Stat size a concurrently growing file could
  outrun (TOCTOU).
- image.DecodeConfig must succeed before committing to the image path,
  so a corrupt or truncated file that only matches a magic-byte prefix
  falls back to a text read instead of shipping an unusable Blob.
- read_file's tool description now mentions image support, so a model
  has a reason to reach for it on a screenshot.
- Fixed "image/png image" double-word phrasing in the summary and
  error text.

Also, per AGENTS.md's "verification drives the production entry
point" rule, added a test that drives read_file through
Session.runToolCalls (the real per-turn dispatch path) rather than
only Tool.Run directly, and a test proving a truncated PNG falls back
to text. AGENTS.md's new section is corrected to state plainly that
only the Anthropic route puts a tool-result image on the wire today;
openai/openaicompat replace it with a text omission note.
---
 AGENTS.md                            | 102 +++++++++++++---------
 engine/filetools.go                  | 123 +++++++++++++++++++--------
 engine/filetools_test.go             |  85 ++++++++++++++++--
 provider/anthropic/transcode_test.go |   4 +-
 4 files changed, 231 insertions(+), 83 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index da36f0c..179f6f3 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -136,49 +136,71 @@ entirely) and the repeatable `-skills-dir` run/serve flag drive it.
 
 ### read_file image support
 
-The built-in `read_file` tool (`engine/filetools.go`) returns an image file
-as real visual content, not mangled text. `sniffImageMediaType` classifies
-the file by its magic bytes (`http.DetectContentType` over at most the
-first 512 bytes) — never by its extension: a `.txt` file that is actually a
-PNG is still recognized as an image, and a `.png` file that is actually
-text stays a text read. This closes the gap where an agent reading a
-screenshot got raw bytes fed to the model as if they were source text.
-
-For a recognized image (`image/png`, `image/jpeg`, `image/gif`,
-`image/webp`), `read_file` returns a `message.ToolResult` whose Content is
-`[Text, Blob]`: a one-line Text summary (format, byte size, and pixel
-dimensions when `image.DecodeConfig` can decode the header cheaply) followed
-by a `message.Blob` carrying the real file bytes. This is the same
-`Text`+`Blob` shape MCP's `mcpContentToParts` already produces
-(`engine/mcp.go`) — read_file is simply a second producer of it — so every
-transcoder's existing Blob handling and the imageclamp dimension/byte-size
-pass (`imageclamp.Clamp`, called from every transcoder's `transcodeRequest`)
-apply to a read_file image exactly as they do to an MCP one, with no new
-wiring. `read_file` never bypasses that clamp: it does not resize, re-encode,
-or otherwise touch pixels itself.
-
-`readFileMaxImageBytes` (20MB) caps the file size `read_file` will read in
-full once an image is detected — separate from and smaller than any
-provider's wire limit, which `imageclamp.Clamp` enforces at transcode time.
-This cap exists only so `read_file` itself never loads an unbounded file
-into memory; an over-cap image returns a plain text error and no Blob. A
-non-image binary file (sniffed as `application/octet-stream` or similar)
-is unaffected by this change and keeps its existing (unbounded) text-read
+The built-in `read_file` tool (`engine/filetools.go`) can return an image
+file as real visual content, not mangled text. `readImageIfDetected`
+classifies the file by its magic bytes (`http.DetectContentType` over at
+most the first 512 bytes) — never by its extension: a `.txt` file that is
+actually a PNG is still recognized as an image, and a `.png` file that is
+actually text stays a text read. On a recognized image (`image/png`,
+`image/jpeg`, `image/gif`, `image/webp`), `read_file` returns a
+`message.ToolResult` whose Content is `[Text, Blob]`: a one-line Text
+summary (format, byte size, and pixel dimensions) followed by a
+`message.Blob` carrying the real file bytes. This is the same `Text`+`Blob`
+shape MCP's `mcpContentToParts` already produces (`engine/mcp.go`) —
+`read_file` is a second producer of it — so every transcoder's existing
+Blob handling and the imageclamp dimension/byte-size pass
+(`imageclamp.Clamp`, called from every transcoder's `transcodeRequest`)
+apply with no new wiring. `read_file` never bypasses that clamp: it does
+not resize, re-encode, or otherwise touch pixels itself.
+
+**Only the Anthropic route puts a tool-result image on the wire.**
+`imageclamp.Limits.RecurseToolResults` is true for `provider/anthropic`
+only; `provider/openai` and `provider/openaicompat` set it false and
+instead replace a tool-result Blob with a text note,
+`"[N image attachment(s) omitted]"` (`toolResultOutput`,
+`provider/openai/transcode.go` and `provider/openaicompat/transcode.go`).
+This is pre-existing wire-format behavior `read_file` inherits, not
+something this feature introduces, but it means a `read_file` image reaches
+the model as pixels only on the Anthropic route; on the other two the model
+sees only the one-line Text summary.
+
+`readImageIfDetected` opens the file once and applies three guards, in
+order:
+
+1. The sniff read uses `io.ReadFull`, not a single `Read`, so a short
+   `read(2)` — realistic on a pipe or FUSE mount — never misclassifies a
+   real image as plain text.
+2. The read is bounded at `readFileMaxImageBytes` (20MB) bytes, checked
+   against an `io.LimitReader` over the same open handle, never against a
+   separately captured `os.Stat` size a concurrently growing file could
+   outrun. This cap is separate from and smaller than any provider's own
+   wire limit, which `imageclamp.Clamp` enforces at transcode time; it
+   exists only so `read_file` itself never loads an unbounded file into
+   memory. An over-cap image returns a plain text error and no Blob.
+3. The body must decode with `image.DecodeConfig` before `read_file`
+   commits to the image path. A corrupt or truncated file that merely opens
+   with a matching magic-byte prefix fails this check and falls back to an
+   ordinary text read instead of shipping a Blob the model cannot use. This
+   guard is not airtight for GIF: the `GIF87a`/`GIF89a` header carries no
+   checksum, so text that happens to start with those exact six bytes still
+   "decodes" with fabricated dimensions. A real file colliding with that
+   prefix is vanishingly unlikely; this is a documented, accepted residual.
+
+A non-image binary file (sniffed as `application/octet-stream` or similar)
+is unaffected by this feature and keeps its existing (unbounded) text-read
 behavior.
 
-**Known gap, filed as issue #129, not fixed here**: a transcode-time
-degrade of an image Blob to a text placeholder for a model with no vision
-capability is NOT implemented. No per-model vision-capability signal exists
-anywhere in the codebase to gate it on — the "embedded models.dev catalog"
-this file's own "Architecture" section describes as a design goal is not
-yet built (no such package or embedded data exists today), and
+**Known gap, filed as issue #129**: a transcode-time degrade of an image
+Blob to a text placeholder for a model with no vision capability is not
+implemented. No per-model vision-capability signal exists anywhere in the
+codebase to gate it on — the embedded models.dev catalog this file's own
+"Architecture" section describes as a design goal is not yet built, and
 `provider.Request` carries no capability flag comparable to `Effort` or
-`SessionKey` that a caller could set from one. Forcing this now would mean
-inventing an ad hoc, likely-wrong static model list inside this PR, so it
-is deferred to issue #129 rather than an ugly seam. Until it lands, a
-model with no vision support receives the image Blob exactly as any other
-vision-capable model does, and how it handles that block is between the
-model and its provider.
+`SessionKey` that a caller could set from one. Building this now would mean
+inventing an ad hoc, likely-wrong static model list, so it is deferred to
+issue #129. Until it lands, a model with no vision support receives the
+image Blob exactly as any vision-capable model does; how it handles that
+block is between the model and its provider.
 
 ### Base loop retry
 
diff --git a/engine/filetools.go b/engine/filetools.go
index da09be7..7c65623 100644
--- a/engine/filetools.go
+++ b/engine/filetools.go
@@ -30,15 +30,21 @@ const (
 	readFileMaxLineLen = 2000
 )
 
-// readFileMaxImageBytes bounds the file size read_file will read in full
-// when it detects an image (see sniffImageMediaType). The transcode-time
-// imageclamp pass (imageclamp.Clamp) enforces each provider's own wire
-// size limit; this cap exists only to stop read_file itself from loading
-// an unbounded file into memory before that clamp ever runs. It is a var,
-// not a const, so a test can shrink it instead of writing a real
-// oversized fixture.
+// readFileMaxImageBytes bounds the total bytes read_file will read from a
+// detected image. The transcode-time imageclamp pass (imageclamp.Clamp)
+// enforces each provider's own wire size limit; this cap exists only to
+// stop read_file itself from loading an unbounded file into memory before
+// that clamp ever runs. readImageIfDetected checks this bound against the
+// read itself (an io.LimitReader over the open file handle), never against
+// a separately captured os.Stat size, which a file that grows after the
+// stat could outrun. It is a var, not a const, so a test can shrink it
+// instead of writing a real oversized fixture.
 var readFileMaxImageBytes = 20 * 1024 * 1024 // 20MB
 
+// imageSniffLen is how many leading bytes classify a file by magic bytes —
+// the same bound http.DetectContentType itself considers.
+const imageSniffLen = 512
+
 // readFileImageMediaTypes lists the image formats read_file recognizes and
 // returns as a message.Blob for the model to see directly. Every other
 // sniffed content type — including any other image/* MIME type
@@ -51,28 +57,79 @@ var readFileImageMediaTypes = map[string]bool{
 	"image/webp": true,
 }
 
-// sniffImageMediaType classifies the file at path by its magic bytes —
-// never by its extension. It reads at most the first 512 bytes (all
-// http.DetectContentType considers) so a large file is never fully read
-// just to answer this question. A file named ".txt" that actually holds
-// PNG bytes is reported as image/png; a file named ".png" that actually
-// holds plain text is reported as a non-image content type. isImage is
-// true only for a format read_file knows how to hand to the model as a
-// Blob (see readFileImageMediaTypes).
-func sniffImageMediaType(path string) (mediaType string, isImage bool, err error) {
+// readImageIfDetected opens path once and, if it is a recognized image,
+// returns its full bytes, media type, and pixel dimensions. It reports
+// ok=false — never an error — for a file that is not a recognized image,
+// so the caller falls through to the ordinary text read unchanged.
+//
+// Classification is by magic bytes only (http.DetectContentType), never by
+// the file's extension: a ".txt" that is really a PNG is still recognized;
+// a ".png" that is really text is not. The sniff read uses io.ReadFull, not
+// a single Read, because one read(2) can return short on a pipe or FUSE
+// mount (this repo already treats such mounts as first-class, see config
+// session_sync: "volume") — a short read must never silently misclassify a
+// real image as plain text.
+//
+// The image body must also decode with image.DecodeConfig before this
+// function commits to the image path: a corrupt or truncated file that
+// merely opens with a matching magic-byte prefix (PNG's 8-byte signature,
+// JPEG's SOI marker, WebP's RIFF/WEBP container) fails PNG/JPEG/WebP's own
+// structural header checks and falls back to a plain text read instead of
+// shipping a Blob the model cannot use — cost-free, since the same decode
+// already runs to read the file's pixel dimensions for the Text summary.
+// This gate is NOT airtight for GIF: the GIF87a/GIF89a header has no
+// checksum, so any bytes following a literal "GIF87a"/"GIF89a" prefix
+// still "decode" with fabricated width/height. A real-world text file
+// beginning with those exact six bytes is vanishingly unlikely; this is a
+// documented, accepted residual, not a silent gap.
+//
+// The total read is bounded at readFileMaxImageBytes+1 bytes via
+// io.LimitReader over the SAME open handle the sniff used — one open, one
+// read pass, and a cap that binds on bytes actually read rather than a
+// pre-read os.Stat size a concurrently growing file could outrun. An
+// over-cap image returns a non-nil err (ok=false, no partial data) so the
+// caller reports a clear error instead of falling through to an unbounded
+// read of a file already known to be an oversized image.
+func readImageIfDetected(path string) (data []byte, mediaType string, width, height int, ok bool, err error) {
 	f, err := os.Open(path)
 	if err != nil {
-		return "", false, err
+		return nil, "", 0, 0, false, err
 	}
 	defer f.Close()
 
-	buf := make([]byte, 512)
-	n, err := f.Read(buf)
-	if n == 0 && err != nil && err != io.EOF {
-		return "", false, err
+	sniff := make([]byte, imageSniffLen)
+	n, rerr := io.ReadFull(f, sniff)
+	if rerr != nil && rerr != io.ErrUnexpectedEOF && rerr != io.EOF {
+		return nil, "", 0, 0, false, rerr
+	}
+	sniff = sniff[:n]
+	mediaType = http.DetectContentType(sniff)
+	if !readFileImageMediaTypes[mediaType] {
+		return nil, mediaType, 0, 0, false, nil
+	}
+
+	budget := int64(readFileMaxImageBytes) - int64(len(sniff))
+	if budget < 0 {
+		budget = 0
+	}
+	rest, rerr := io.ReadAll(io.LimitReader(f, budget+1))
+	if rerr != nil {
+		return nil, mediaType, 0, 0, false, rerr
+	}
+	full := append(sniff, rest...)
+	if len(full) > readFileMaxImageBytes {
+		return nil, mediaType, 0, 0, false, fmt.Errorf("image (%s) exceeds the %d-byte read_file image limit", mediaType, readFileMaxImageBytes)
+	}
+
+	cfg, _, derr := image.DecodeConfig(bytes.NewReader(full))
+	if derr != nil {
+		// Sniffed as an image by magic bytes, but the body does not decode
+		// as one (corrupt, truncated, or a false-positive magic-byte
+		// match). Fall through to the ordinary text read rather than
+		// shipping a Blob the model cannot use.
+		return nil, mediaType, 0, 0, false, nil
 	}
-	ct := http.DetectContentType(buf[:n])
-	return ct, readFileImageMediaTypes[ct], nil
+	return full, mediaType, cfg.Width, cfg.Height, true, nil
 }
 
 // resolvePath resolves a tool path argument against the session working
@@ -88,7 +145,7 @@ func readFileTool() Tool {
 	return Tool{
 		Def: provider.ToolDef{
 			Name:        "read_file",
-			Description: "Read a file and return its content with line numbers (N→ prefixes). Prefer this over shell commands like cat, head, or sed for reading files. Relative paths resolve against the session working directory.",
+			Description: "Read a file and return its content with line numbers (N→ prefixes). A recognized image file (PNG, JPEG, GIF, WebP) is returned as an actual image instead, so use this tool to view a screenshot or picture. Prefer this over shell commands like cat, head, or sed for reading files. Relative paths resolve against the session working directory.",
 			InputSchema: json.RawMessage(`{
 				"type": "object",
 				"properties": {
@@ -117,18 +174,12 @@ func readFileTool() Tool {
 				return nil, fmt.Errorf("read_file: %s is a directory", path)
 			}
 
-			if mediaType, isImage, sniffErr := sniffImageMediaType(path); sniffErr == nil && isImage {
-				if info.Size() > int64(readFileMaxImageBytes) {
-					return nil, fmt.Errorf("read_file: %s is a %d-byte %s image, over the %d-byte read_file image limit", path, info.Size(), mediaType, readFileMaxImageBytes)
-				}
-				imgData, err := os.ReadFile(path)
-				if err != nil {
-					return nil, fmt.Errorf("read_file: %w", err)
-				}
-				summary := fmt.Sprintf("%s image, %d bytes", mediaType, len(imgData))
-				if cfg, _, err := image.DecodeConfig(bytes.NewReader(imgData)); err == nil {
-					summary = fmt.Sprintf("%s image, %d bytes, %dx%d pixels", mediaType, len(imgData), cfg.Width, cfg.Height)
-				}
+			imgData, mediaType, width, height, isImage, imgErr := readImageIfDetected(path)
+			if imgErr != nil {
+				return nil, fmt.Errorf("read_file: %s: %w", path, imgErr)
+			}
+			if isImage {
+				summary := fmt.Sprintf("image (%s), %d bytes, %dx%d pixels", mediaType, len(imgData), width, height)
 				return message.Parts{
 					&message.Text{Text: summary},
 					&message.Blob{MediaType: mediaType, Data: imgData},
diff --git a/engine/filetools_test.go b/engine/filetools_test.go
index 87d7afd..5768fe7 100644
--- a/engine/filetools_test.go
+++ b/engine/filetools_test.go
@@ -296,7 +296,7 @@ func TestReadFileImageExtensionLieTextNamedPNGStaysText(t *testing.T) {
 // TestReadFileImageExtensionLiePNGNamedTxtIsSniffedAsImage is the missing-
 // direction half: a file named .txt that actually holds PNG magic bytes must
 // still be recognized as an image. Extension is a hint only; magic bytes are
-// authoritative (AGENTS.md: "never trust extension alone").
+// authoritative (AGENTS.md: read_file classifies "never by its extension").
 func TestReadFileImageExtensionLiePNGNamedTxtIsSniffedAsImage(t *testing.T) {
 	dir := t.TempDir()
 	data := tinyPNG(t)
@@ -328,15 +328,90 @@ func TestReadFileImageOverCapReturnsTextErrorNoBlob(t *testing.T) {
 	}
 
 	orig := readFileMaxImageBytes
-	readFileMaxImageBytes = len(data) - 1 // force this exact file over cap
+	wantCap := len(data) - 1
+	readFileMaxImageBytes = wantCap // force this exact file over cap
 	t.Cleanup(func() { readFileMaxImageBytes = orig })
 
-	_, err := runToolParts(t, readFileTool(), dir, `{"path":"shot.png"}`)
+	parts, err := runToolParts(t, readFileTool(), dir, `{"path":"shot.png"}`)
 	if err == nil {
 		t.Fatal("want error for over-cap image")
 	}
-	if !strings.Contains(err.Error(), "image") {
-		t.Errorf("error = %q, want it to name the image/limit", err)
+	if parts != nil {
+		t.Errorf("parts = %+v, want nil (no Blob) on an over-cap image error", parts)
+	}
+	wantSubstr := fmt.Sprintf("%d-byte read_file image limit", wantCap)
+	if !strings.Contains(err.Error(), wantSubstr) {
+		t.Errorf("error = %q, want it to contain %q", err, wantSubstr)
+	}
+}
+
+// TestReadFileImageTruncatedPNGFallsBackToText proves the DecodeConfig gate
+// in readImageIfDetected: a file whose first 8 bytes are a genuine PNG
+// signature, but whose body is not a real PNG (a truncated download, a
+// corrupt write), fails image.DecodeConfig and is read as ordinary text
+// instead of shipping a Blob the model cannot use.
+func TestReadFileImageTruncatedPNGFallsBackToText(t *testing.T) {
+	dir := t.TempDir()
+	pngSignature := []byte("\x89PNG\r\n\x1a\n")
+	body := append(pngSignature, []byte("not a real IHDR chunk")...)
+	if err := os.WriteFile(filepath.Join(dir, "broken.png"), body, 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	parts, err := runToolParts(t, readFileTool(), dir, `{"path":"broken.png"}`)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(parts) != 1 {
+		t.Fatalf("parts = %d, want 1 (Text only, no Blob for an undecodable image): %+v", len(parts), parts)
+	}
+	if _, ok := parts[0].(*message.Text); !ok {
+		t.Fatalf("parts[0] = %T, want *message.Text", parts[0])
+	}
+}
+
+// TestReadFileImageToolCallProducesBlobToolResult drives read_file through
+// the SAME production dispatch path a real turn uses — an assistant
+// ToolCall executed by Session.runToolCalls, not a direct Tool.Run call —
+// closing the gap between the engine-level Tool.Run tests above and the
+// transcode-level golden test in provider/anthropic/transcode_test.go,
+// which hand-builds a ToolResult shaped like read_file's output rather than
+// obtaining one from read_file itself (AGENTS.md's "verification drives
+// the production entry point" rule).
+func TestReadFileImageToolCallProducesBlobToolResult(t *testing.T) {
+	dir := t.TempDir()
+	data := tinyPNG(t)
+	if err := os.WriteFile(filepath.Join(dir, "shot.png"), data, 0o644); err != nil {
+		t.Fatal(err)
+	}
+	s := NewSession(Config{WorkDir: dir})
+	asst := &message.Message{
+		Role: message.RoleAssistant,
+		Parts: message.Parts{
+			&message.ToolCall{CallID: "call1", Name: "read_file", Arguments: json.RawMessage(`{"path":"shot.png"}`)},
+		},
+	}
+
+	results := s.runToolCalls(context.Background(), asst)
+	if len(results) != 1 {
+		t.Fatalf("results = %d, want 1", len(results))
+	}
+	tr, ok := results[0].(*message.ToolResult)
+	if !ok {
+		t.Fatalf("results[0] = %T, want *message.ToolResult", results[0])
+	}
+	if tr.IsError {
+		t.Fatalf("ToolResult.IsError = true, content: %+v", tr.Content)
+	}
+	if len(tr.Content) != 2 {
+		t.Fatalf("ToolResult.Content = %d parts, want 2 (Text, Blob): %+v", len(tr.Content), tr.Content)
+	}
+	blob, ok := tr.Content[1].(*message.Blob)
+	if !ok {
+		t.Fatalf("ToolResult.Content[1] = %T, want *message.Blob", tr.Content[1])
+	}
+	if blob.MediaType != "image/png" || !bytes.Equal(blob.Data, data) {
+		t.Errorf("Blob = %+v, want MediaType image/png and Data matching the source file", blob)
 	}
 }
 
diff --git a/provider/anthropic/transcode_test.go b/provider/anthropic/transcode_test.go
index a9bb197..3e5623e 100644
--- a/provider/anthropic/transcode_test.go
+++ b/provider/anthropic/transcode_test.go
@@ -249,7 +249,7 @@ func TestTranscodeToolCallAndResult(t *testing.T) {
 // TestTranscodeReadFileImageArrivesAsRealWireImageBlock is the read_file
 // counterpart of TestTranscodeToolCallAndResult above: engine/filetools.go's
 // read_file tool now returns exactly this shape for an image file — a Text
-// summary part ("image/png image, N bytes, WxH pixels") followed by a Blob
+// summary part ("image (image/png), N bytes, WxH pixels") followed by a Blob
 // part carrying the real file bytes (engine/filetools_test.go's
 // TestReadFileImagePNGReturnsTextAndBlob proves read_file itself builds this
 // shape from its own production entry point, Tool.Run). This test proves the
@@ -263,7 +263,7 @@ func TestTranscodeToolCallAndResult(t *testing.T) {
 // unrelated wire-format behavior this PR does not change.
 func TestTranscodeReadFileImageArrivesAsRealWireImageBlock(t *testing.T) {
 	png := tinyPNG(t)
-	summary := fmt.Sprintf("image/png image, %d bytes, 2x2 pixels", len(png))
+	summary := fmt.Sprintf("image (image/png), %d bytes, 2x2 pixels", len(png))
 	out := mustTranscode(t, baseRequest(
 		message.Message{Role: message.RoleUser, Parts: message.Parts{&message.Text{Text: "read shot.png"}}},
 		message.Message{Role: message.RoleAssistant, Parts: message.Parts{

From a670a200966656322414c1a0209ae7b4d5da8655 Mon Sep 17 00:00:00 2001
From: Andy Bonventre <andybons@meetneptune.com>
Date: Wed, 12 Aug 2026 16:44:42 -0400
Subject: [PATCH 3/4] fix(engine): route-neutral read_file image description,
 real short-read guard test

Address round-2 review of the read_file image feature:

- read_file's tool Description no longer implies every provider ships
  the image on the wire; it now says "where the current provider
  supports tool-result images" (only provider/anthropic does today).
- Extract sniffMediaType(io.Reader) out of readImageIfDetected so a
  test can drive it with iotest.OneByteReader. Added
  TestSniffMediaTypeSurvivesShortReads and red-verified it against the
  actual io.ReadFull mechanism (reverted to a plain Read, confirmed
  the test fails, restored). An earlier claim that this mechanism had
  been red-verified was made before the test existed; this closes that
  gap for real.
- Fixed a duplicated "bytes" in AGENTS.md's read_file section.
---
 AGENTS.md                |  2 +-
 engine/filetools.go      | 30 +++++++++++++++++++++++-------
 engine/filetools_test.go | 22 ++++++++++++++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 179f6f3..c49fad6 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -170,7 +170,7 @@ order:
 1. The sniff read uses `io.ReadFull`, not a single `Read`, so a short
    `read(2)` — realistic on a pipe or FUSE mount — never misclassifies a
    real image as plain text.
-2. The read is bounded at `readFileMaxImageBytes` (20MB) bytes, checked
+2. The read is bounded at `readFileMaxImageBytes` (20MB), checked
    against an `io.LimitReader` over the same open handle, never against a
    separately captured `os.Stat` size a concurrently growing file could
    outrun. This cap is separate from and smaller than any provider's own
diff --git a/engine/filetools.go b/engine/filetools.go
index 7c65623..d01fc8b 100644
--- a/engine/filetools.go
+++ b/engine/filetools.go
@@ -57,6 +57,25 @@ var readFileImageMediaTypes = map[string]bool{
 	"image/webp": true,
 }
 
+// sniffMediaType reads up to imageSniffLen bytes from r via io.ReadFull and
+// classifies them via http.DetectContentType, returning the classification
+// and the sniffed bytes themselves (so a caller reading the rest of the
+// stream does not re-read them). Taking an io.Reader, not a path, is what
+// lets TestSniffMediaTypeSurvivesShortReads drive it with
+// iotest.OneByteReader: a plain single Read against such a source returns
+// one byte per call, so a caller using Read directly would misclassify
+// almost every real image — io.ReadFull is what makes this deterministic
+// against a short-read source (a pipe, a FUSE/network mount, a signal).
+func sniffMediaType(r io.Reader) (mediaType string, sniffed []byte, err error) {
+	buf := make([]byte, imageSniffLen)
+	n, rerr := io.ReadFull(r, buf)
+	if rerr != nil && rerr != io.ErrUnexpectedEOF && rerr != io.EOF {
+		return "", nil, rerr
+	}
+	buf = buf[:n]
+	return http.DetectContentType(buf), buf, nil
+}
+
 // readImageIfDetected opens path once and, if it is a recognized image,
 // returns its full bytes, media type, and pixel dimensions. It reports
 // ok=false — never an error — for a file that is not a recognized image,
@@ -97,13 +116,10 @@ func readImageIfDetected(path string) (data []byte, mediaType string, width, hei
 	}
 	defer f.Close()
 
-	sniff := make([]byte, imageSniffLen)
-	n, rerr := io.ReadFull(f, sniff)
-	if rerr != nil && rerr != io.ErrUnexpectedEOF && rerr != io.EOF {
-		return nil, "", 0, 0, false, rerr
+	mediaType, sniff, err := sniffMediaType(f)
+	if err != nil {
+		return nil, "", 0, 0, false, err
 	}
-	sniff = sniff[:n]
-	mediaType = http.DetectContentType(sniff)
 	if !readFileImageMediaTypes[mediaType] {
 		return nil, mediaType, 0, 0, false, nil
 	}
@@ -145,7 +161,7 @@ func readFileTool() Tool {
 	return Tool{
 		Def: provider.ToolDef{
 			Name:        "read_file",
-			Description: "Read a file and return its content with line numbers (N→ prefixes). A recognized image file (PNG, JPEG, GIF, WebP) is returned as an actual image instead, so use this tool to view a screenshot or picture. Prefer this over shell commands like cat, head, or sed for reading files. Relative paths resolve against the session working directory.",
+			Description: "Read a file and return its content with line numbers (N→ prefixes). A recognized image file (PNG, JPEG, GIF, WebP) is returned as an image where the current provider supports tool-result images. Prefer this over shell commands like cat, head, or sed for reading files. Relative paths resolve against the session working directory.",
 			InputSchema: json.RawMessage(`{
 				"type": "object",
 				"properties": {
diff --git a/engine/filetools_test.go b/engine/filetools_test.go
index 5768fe7..ce2035d 100644
--- a/engine/filetools_test.go
+++ b/engine/filetools_test.go
@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"testing/iotest"
 
 	"github.com/majorcontext/harness/message"
 	"github.com/majorcontext/harness/plugin"
@@ -234,6 +235,27 @@ func tinyPNG(t *testing.T) []byte {
 	return buf.Bytes()
 }
 
+// TestSniffMediaTypeSurvivesShortReads red-verifies the io.ReadFull sniff
+// in sniffMediaType: iotest.OneByteReader wraps the source so every Read
+// call returns exactly one byte, the shape a pipe, a FUSE/network mount, or
+// a signal-interrupted read(2) can produce. A single plain Read against
+// such a source would see only the first byte and misclassify almost every
+// real image; io.ReadFull is what makes classification correct regardless
+// of how many underlying reads it takes.
+func TestSniffMediaTypeSurvivesShortReads(t *testing.T) {
+	data := tinyPNG(t)
+	mediaType, sniff, err := sniffMediaType(iotest.OneByteReader(bytes.NewReader(data)))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if mediaType != "image/png" {
+		t.Errorf("mediaType = %q, want image/png", mediaType)
+	}
+	if !bytes.Equal(sniff, data) {
+		t.Errorf("sniffed %d bytes, want all %d source bytes (file is under imageSniffLen)", len(sniff), len(data))
+	}
+}
+
 func TestReadFileImagePNGReturnsTextAndBlob(t *testing.T) {
 	dir := t.TempDir()
 	data := tinyPNG(t)

From 13bcc737cb206a777f48fbef33bcaa6ffb01607e Mon Sep 17 00:00:00 2001
From: Andy Bonventre <andybons@meetneptune.com>
Date: Wed, 12 Aug 2026 16:58:12 -0400
Subject: [PATCH 4/4] fix(engine): read_file's text path no longer double-opens
 the file

readPathContent replaces readImageIfDetected: it opens the target path
exactly once and reads it exactly once regardless of outcome. Before
this, a non-image read (the overwhelmingly common case) did an extra
open+512-byte-read+close for the image sniff and then a second,
separate os.ReadFile for the actual text content.

The text outcome now continues reading from the same handle right
after the sniff (io.ReadAll(f)), so no bytes are lost on a
non-regular file (a FIFO, a streaming mount) either. The image path's
DecodeConfig-failure fallback (a magic-byte match that turns out not
to be a real image) also now reads the true remainder through the
same handle instead of silently truncating the text fallback at the
image cap.

AGENTS.md's read_file section is updated for the rename and now notes
the summary-vs-clamped-image mismatch: imageclamp.Clamp can downscale
or re-encode an image later, at transcode time, so the dimensions
read_file reported when it read the file can drift from what the
model is eventually shown. This is accepted, not fixed here.
---
 AGENTS.md                |  59 ++++++++++---------
 engine/filetools.go      | 123 ++++++++++++++++++++++-----------------
 engine/filetools_test.go |   2 +-
 3 files changed, 104 insertions(+), 80 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index c49fad6..94dea69 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -137,21 +137,26 @@ entirely) and the repeatable `-skills-dir` run/serve flag drive it.
 ### read_file image support
 
 The built-in `read_file` tool (`engine/filetools.go`) can return an image
-file as real visual content, not mangled text. `readImageIfDetected`
-classifies the file by its magic bytes (`http.DetectContentType` over at
-most the first 512 bytes) — never by its extension: a `.txt` file that is
-actually a PNG is still recognized as an image, and a `.png` file that is
-actually text stays a text read. On a recognized image (`image/png`,
-`image/jpeg`, `image/gif`, `image/webp`), `read_file` returns a
-`message.ToolResult` whose Content is `[Text, Blob]`: a one-line Text
-summary (format, byte size, and pixel dimensions) followed by a
-`message.Blob` carrying the real file bytes. This is the same `Text`+`Blob`
-shape MCP's `mcpContentToParts` already produces (`engine/mcp.go`) —
-`read_file` is a second producer of it — so every transcoder's existing
-Blob handling and the imageclamp dimension/byte-size pass
-(`imageclamp.Clamp`, called from every transcoder's `transcodeRequest`)
-apply with no new wiring. `read_file` never bypasses that clamp: it does
-not resize, re-encode, or otherwise touch pixels itself.
+file as real visual content, not mangled text. `readPathContent` opens the
+target path exactly once and classifies it by its magic bytes
+(`http.DetectContentType` over at most the first 512 bytes) — never by its
+extension: a `.txt` file that is actually a PNG is still recognized as an
+image, and a `.png` file that is actually text stays a text read. On a
+recognized image (`image/png`, `image/jpeg`, `image/gif`, `image/webp`),
+`read_file` returns a `message.ToolResult` whose Content is `[Text, Blob]`:
+a one-line Text summary (format, byte size, and pixel dimensions) followed
+by a `message.Blob` carrying the real file bytes. This is the same
+`Text`+`Blob` shape MCP's `mcpContentToParts` already produces
+(`engine/mcp.go`) — `read_file` is a second producer of it — so every
+transcoder's existing Blob handling and the imageclamp dimension/byte-size
+pass (`imageclamp.Clamp`, called from every transcoder's
+`transcodeRequest`) apply with no new wiring. `read_file` never bypasses
+that clamp: it does not resize, re-encode, or otherwise touch pixels
+itself. Because `imageclamp.Clamp` runs later, at transcode time, an image
+it downscales or re-encodes can end up described by dimensions or a byte
+size that no longer match the summary `read_file` reported when it read
+the file; this is a known, accepted mismatch, not a defect to fix in
+`read_file` itself.
 
 **Only the Anthropic route puts a tool-result image on the wire.**
 `imageclamp.Limits.RecurseToolResults` is true for `provider/anthropic`
@@ -164,8 +169,7 @@ something this feature introduces, but it means a `read_file` image reaches
 the model as pixels only on the Anthropic route; on the other two the model
 sees only the one-line Text summary.
 
-`readImageIfDetected` opens the file once and applies three guards, in
-order:
+`readPathContent` applies three guards on the image path, in order:
 
 1. The sniff read uses `io.ReadFull`, not a single `Read`, so a short
    `read(2)` — realistic on a pipe or FUSE mount — never misclassifies a
@@ -178,17 +182,20 @@ order:
    exists only so `read_file` itself never loads an unbounded file into
    memory. An over-cap image returns a plain text error and no Blob.
 3. The body must decode with `image.DecodeConfig` before `read_file`
-   commits to the image path. A corrupt or truncated file that merely opens
-   with a matching magic-byte prefix fails this check and falls back to an
-   ordinary text read instead of shipping a Blob the model cannot use. This
-   guard is not airtight for GIF: the `GIF87a`/`GIF89a` header carries no
-   checksum, so text that happens to start with those exact six bytes still
-   "decodes" with fabricated dimensions. A real file colliding with that
-   prefix is vanishingly unlikely; this is a documented, accepted residual.
+   commits to the image outcome. A corrupt or truncated file that merely
+   opens with a matching magic-byte prefix fails this check; `read_file`
+   then reads the true remainder of the file (unbounded, same handle) and
+   returns it as ordinary text instead of shipping a Blob the model cannot
+   use. This guard is not airtight for GIF: the `GIF87a`/`GIF89a` header
+   carries no checksum, so text that happens to start with those exact six
+   bytes still "decodes" with fabricated dimensions. A real file colliding
+   with that prefix is vanishingly unlikely; this is a documented, accepted
+   residual.
 
 A non-image binary file (sniffed as `application/octet-stream` or similar)
-is unaffected by this feature and keeps its existing (unbounded) text-read
-behavior.
+keeps `read_file`'s existing (unbounded) text-read behavior; `readPathContent`
+still reads it exactly once, through the same handle its sniff already
+opened.
 
 **Known gap, filed as issue #129**: a transcode-time degrade of an image
 Blob to a text placeholder for a model with no vision capability is not
diff --git a/engine/filetools.go b/engine/filetools.go
index d01fc8b..55b0b25 100644
--- a/engine/filetools.go
+++ b/engine/filetools.go
@@ -34,7 +34,7 @@ const (
 // detected image. The transcode-time imageclamp pass (imageclamp.Clamp)
 // enforces each provider's own wire size limit; this cap exists only to
 // stop read_file itself from loading an unbounded file into memory before
-// that clamp ever runs. readImageIfDetected checks this bound against the
+// that clamp ever runs. readPathContent checks this bound against the
 // read itself (an io.LimitReader over the open file handle), never against
 // a separately captured os.Stat size, which a file that grows after the
 // stat could outrun. It is a var, not a const, so a test can shrink it
@@ -76,76 +76,97 @@ func sniffMediaType(r io.Reader) (mediaType string, sniffed []byte, err error) {
 	return http.DetectContentType(buf), buf, nil
 }
 
-// readImageIfDetected opens path once and, if it is a recognized image,
-// returns its full bytes, media type, and pixel dimensions. It reports
-// ok=false — never an error — for a file that is not a recognized image,
-// so the caller falls through to the ordinary text read unchanged.
+// fileContent is the outcome of readPathContent: either a detected image's
+// Blob-ready bytes, or a non-image file's raw bytes for the ordinary text
+// read. Exactly one of ImageData or TextData is populated.
+type fileContent struct {
+	IsImage       bool
+	ImageData     []byte
+	MediaType     string
+	Width, Height int
+	TextData      []byte
+}
+
+// readPathContent opens path exactly once and, on every outcome, reads it
+// exactly once — no second os.Open, no re-read of bytes already
+// classified. Earlier revisions of read_file's image detection opened the
+// file twice (once to sniff, once via os.ReadFile) even on a plain text
+// read, the far more common case; this unifies both outcomes behind one
+// handle.
+//
+// Classification is by magic bytes only (http.DetectContentType via
+// sniffMediaType), never by the file's extension: a ".txt" that is really
+// a PNG is still recognized; a ".png" that is really text is not.
 //
-// Classification is by magic bytes only (http.DetectContentType), never by
-// the file's extension: a ".txt" that is really a PNG is still recognized;
-// a ".png" that is really text is not. The sniff read uses io.ReadFull, not
-// a single Read, because one read(2) can return short on a pipe or FUSE
-// mount (this repo already treats such mounts as first-class, see config
-// session_sync: "volume") — a short read must never silently misclassify a
-// real image as plain text.
+// A recognized image type is read under a bound: readFileMaxImageBytes+1
+// bytes via io.LimitReader over the SAME open handle the sniff used — a
+// cap that binds on bytes actually read, never a pre-read os.Stat size a
+// concurrently growing file could outrun. An over-cap image returns a
+// non-nil error (no partial TextData) so the caller reports a clear error
+// instead of falling through to an unbounded read of a file already known
+// to be an oversized image.
 //
 // The image body must also decode with image.DecodeConfig before this
-// function commits to the image path: a corrupt or truncated file that
+// function commits to the image outcome: a corrupt or truncated file that
 // merely opens with a matching magic-byte prefix (PNG's 8-byte signature,
 // JPEG's SOI marker, WebP's RIFF/WEBP container) fails PNG/JPEG/WebP's own
-// structural header checks and falls back to a plain text read instead of
-// shipping a Blob the model cannot use — cost-free, since the same decode
-// already runs to read the file's pixel dimensions for the Text summary.
-// This gate is NOT airtight for GIF: the GIF87a/GIF89a header has no
-// checksum, so any bytes following a literal "GIF87a"/"GIF89a" prefix
-// still "decode" with fabricated width/height. A real-world text file
-// beginning with those exact six bytes is vanishingly unlikely; this is a
-// documented, accepted residual, not a silent gap.
-//
-// The total read is bounded at readFileMaxImageBytes+1 bytes via
-// io.LimitReader over the SAME open handle the sniff used — one open, one
-// read pass, and a cap that binds on bytes actually read rather than a
-// pre-read os.Stat size a concurrently growing file could outrun. An
-// over-cap image returns a non-nil err (ok=false, no partial data) so the
-// caller reports a clear error instead of falling through to an unbounded
-// read of a file already known to be an oversized image.
-func readImageIfDetected(path string) (data []byte, mediaType string, width, height int, ok bool, err error) {
+// structural header checks. On that failure this reads the true remainder
+// of the file (unbounded, via the same handle) so the text fallback is
+// complete rather than silently cut off at the image cap — reachable only
+// when magic bytes matched an image signature yet the body failed
+// structural validation, so an oversized file in this branch is already
+// known not to be a real image. This gate is NOT airtight for GIF: the
+// GIF87a/GIF89a header has no checksum, so any bytes following a literal
+// "GIF87a"/"GIF89a" prefix still "decode" with fabricated width/height. A
+// real-world text file beginning with those exact six bytes is
+// vanishingly unlikely; this is a documented, accepted residual, not a
+// silent gap.
+func readPathContent(path string) (fileContent, error) {
 	f, err := os.Open(path)
 	if err != nil {
-		return nil, "", 0, 0, false, err
+		return fileContent{}, err
 	}
 	defer f.Close()
 
 	mediaType, sniff, err := sniffMediaType(f)
 	if err != nil {
-		return nil, "", 0, 0, false, err
+		return fileContent{}, err
 	}
 	if !readFileImageMediaTypes[mediaType] {
-		return nil, mediaType, 0, 0, false, nil
+		rest, err := io.ReadAll(f)
+		if err != nil {
+			return fileContent{}, err
+		}
+		return fileContent{TextData: append(sniff, rest...)}, nil
 	}
 
 	budget := int64(readFileMaxImageBytes) - int64(len(sniff))
 	if budget < 0 {
 		budget = 0
 	}
-	rest, rerr := io.ReadAll(io.LimitReader(f, budget+1))
-	if rerr != nil {
-		return nil, mediaType, 0, 0, false, rerr
+	capped, err := io.ReadAll(io.LimitReader(f, budget+1))
+	if err != nil {
+		return fileContent{}, err
 	}
-	full := append(sniff, rest...)
+	full := append(sniff, capped...)
 	if len(full) > readFileMaxImageBytes {
-		return nil, mediaType, 0, 0, false, fmt.Errorf("image (%s) exceeds the %d-byte read_file image limit", mediaType, readFileMaxImageBytes)
+		return fileContent{}, fmt.Errorf("image (%s) exceeds the %d-byte read_file image limit", mediaType, readFileMaxImageBytes)
 	}
 
 	cfg, _, derr := image.DecodeConfig(bytes.NewReader(full))
 	if derr != nil {
 		// Sniffed as an image by magic bytes, but the body does not decode
 		// as one (corrupt, truncated, or a false-positive magic-byte
-		// match). Fall through to the ordinary text read rather than
-		// shipping a Blob the model cannot use.
-		return nil, mediaType, 0, 0, false, nil
+		// match). Read the true remainder so the text fallback is
+		// complete, not silently cut off at the image cap; see the doc
+		// comment above for why this is safe.
+		rest, err := io.ReadAll(f)
+		if err != nil {
+			return fileContent{}, err
+		}
+		return fileContent{TextData: append(full, rest...)}, nil
 	}
-	return full, mediaType, cfg.Width, cfg.Height, true, nil
+	return fileContent{IsImage: true, ImageData: full, MediaType: mediaType, Width: cfg.Width, Height: cfg.Height}, nil
 }
 
 // resolvePath resolves a tool path argument against the session working
@@ -190,22 +211,18 @@ func readFileTool() Tool {
 				return nil, fmt.Errorf("read_file: %s is a directory", path)
 			}
 
-			imgData, mediaType, width, height, isImage, imgErr := readImageIfDetected(path)
-			if imgErr != nil {
-				return nil, fmt.Errorf("read_file: %s: %w", path, imgErr)
+			content, err := readPathContent(path)
+			if err != nil {
+				return nil, fmt.Errorf("read_file: %s: %w", path, err)
 			}
-			if isImage {
-				summary := fmt.Sprintf("image (%s), %d bytes, %dx%d pixels", mediaType, len(imgData), width, height)
+			if content.IsImage {
+				summary := fmt.Sprintf("image (%s), %d bytes, %dx%d pixels", content.MediaType, len(content.ImageData), content.Width, content.Height)
 				return message.Parts{
 					&message.Text{Text: summary},
-					&message.Blob{MediaType: mediaType, Data: imgData},
+					&message.Blob{MediaType: content.MediaType, Data: content.ImageData},
 				}, nil
 			}
-
-			data, err := os.ReadFile(path)
-			if err != nil {
-				return nil, fmt.Errorf("read_file: %w", err)
-			}
+			data := content.TextData
 
 			lines := strings.Split(string(data), "\n")
 			// A trailing newline produces one empty trailing element; drop it.
diff --git a/engine/filetools_test.go b/engine/filetools_test.go
index ce2035d..278f165 100644
--- a/engine/filetools_test.go
+++ b/engine/filetools_test.go
@@ -368,7 +368,7 @@ func TestReadFileImageOverCapReturnsTextErrorNoBlob(t *testing.T) {
 }
 
 // TestReadFileImageTruncatedPNGFallsBackToText proves the DecodeConfig gate
-// in readImageIfDetected: a file whose first 8 bytes are a genuine PNG
+// in readPathContent: a file whose first 8 bytes are a genuine PNG
 // signature, but whose body is not a real PNG (a truncated download, a
 // corrupt write), fails image.DecodeConfig and is read as ordinary text
 // instead of shipping a Blob the model cannot use.