runner: keep readCapped's truncation on a rune boundary

cmd/entire/cli/runner_gather.go

@@ -11,6 +11,7 @@ import (
 	"strconv"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"github.com/entireio/cli/cmd/entire/cli/api"
 	"github.com/entireio/cli/cmd/entire/cli/osroot"
@@ -193,7 +194,37 @@ func readCapped(repoRoot, name string, maxLen int) (string, bool) {
 	}
 	s := string(data)
 	if len(s) > maxLen {
-		s = s[:maxLen] + "\n…(truncated)…"
+		// maxLen is a byte budget, and s[:maxLen] can land inside a multi-byte
+		// rune, which would put an invalid UTF-8 sequence in the prompt this
+		// feeds. A continuation byte at the cut is exactly what "we cut
+		// mid-rune" means, so back off the continuation bytes — at most
+		// UTFMax-1 of them, which is the furthest a rune's start can be.
+		//
+		// RuneStart at the cut rather than "is s[:cut] valid UTF-8?": the
+		// question is whether OUR cut split a rune, and validating the prefix
+		// answers a different one. It walks the whole 6KB, and it answers "no"
+		// for a doc that is not UTF-8 at all (a latin-1 README) — which, chased
+		// far enough, drops the file's content in favour of a bare truncation
+		// marker. Invalidity our cut did not cause is the file's own, and the
+		// under-cap path above passes those bytes through too.
+		//
+		// The floor is explicit rather than implied by an iteration count: a
+		// count alone underflows at maxLen=1 over a file of continuation bytes,
+		// where the third pass indexes s[-1].
+		cut := maxLen
+		for lo := max(0, maxLen-(utf8.UTFMax-1)); cut > lo; cut-- {
+			if utf8.RuneStart(s[cut]) {
+				break
+			}
+		}
+		if !utf8.RuneStart(s[cut]) {
+			// No rune start in the window, so the invalidity is the file's own
+			// and not something our cut introduced. Keep the bytes: dropping
+			// them is how an earlier revision handed the caller a truncation
+			// marker and no content.
+			cut = maxLen
+		}
+		s = s[:cut] + "\n…(truncated)…"
 	}
 	return s, true
 }

cmd/entire/cli/runner_gather_test.go

@@ -8,6 +8,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"unicode/utf8"
 
 	"github.com/entireio/cli/cmd/entire/cli/api"
 	"github.com/entireio/cli/cmd/entire/cli/testutil"
@@ -54,3 +55,144 @@ func TestGatherTrailsUsesNativeRepoBaseForFindings(t *testing.T) {
 		t.Fatalf("request paths = %v, want native list and findings routes", paths)
 	}
 }
+
+// TestReadCapped_TruncatesOnARuneBoundary pins that the byte budget does not cut
+// a multi-byte rune in half. The output is embedded in the prompt the runner
+// sends to a model, so an invalid UTF-8 sequence there is a defect the caller
+// cannot see; the callers' constants are labelled "max chars" while the cap is
+// applied in bytes, which is what made this easy to miss.
+func TestReadCapped_TruncatesOnARuneBoundary(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	// "é" is two bytes, so an odd cap always lands mid-rune.
+	body := strings.Repeat("é", 100)
+	testutil.WriteFile(t, dir, "CLAUDE.md", body)
+
+	for _, cap := range []int{1, 7, 21, 99} {
+		got, ok := readCapped(dir, "CLAUDE.md", cap)
+		if !ok {
+			t.Fatalf("readCapped(cap=%d) not ok", cap)
+		}
+		if !utf8.ValidString(got) {
+			t.Errorf("readCapped(cap=%d) produced invalid UTF-8: %q", cap, got)
+		}
+		if !strings.Contains(got, "truncated") {
+			t.Errorf("readCapped(cap=%d) should mark the truncation, got %q", cap, got)
+		}
+	}
+}
+
+// A file inside the cap comes back whole, with no truncation marker.
+func TestReadCapped_ReturnsShortFilesWhole(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	testutil.WriteFile(t, dir, "README.md", "héllo")
+
+	got, ok := readCapped(dir, "README.md", 100)
+	if !ok {
+		t.Fatal("readCapped() not ok")
+	}
+	if got != "héllo" {
+		t.Errorf("readCapped() = %q, want héllo", got)
+	}
+}
+
+// A missing file is not an error, just absent.
+func TestReadCapped_MissingFile(t *testing.T) {
+	t.Parallel()
+
+	if _, ok := readCapped(t.TempDir(), "nope.md", 10); ok {
+		t.Error("readCapped() on a missing file should report not-ok")
+	}
+}
+
+// TestReadCapped_KeepsContentOfANonUTF8File is the other half of the boundary
+// fix. Scanning back for a valid prefix unconditionally walks to 0 on a doc that
+// is not UTF-8 at all — every prefix is invalid and "" is not — so the caller
+// got the truncation marker and none of the content. Invalidity our cut did not
+// cause is the file's own, and the under-cap path passes those bytes through
+// too.
+func TestReadCapped_KeepsContentOfANonUTF8File(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	// 0xE9 is "é" in latin-1 and invalid on its own in UTF-8. Go strings hold
+	// arbitrary bytes, so testutil.WriteFile carries it fine.
+	body := string(append([]byte{0xE9}, []byte(strings.Repeat("resume of the project. ", 20))...))
+	testutil.WriteFile(t, dir, "CLAUDE.md", body)
+
+	got, ok := readCapped(dir, "CLAUDE.md", 40)
+	if !ok {
+		t.Fatal("readCapped() not ok")
+	}
+	if !strings.Contains(got, "resume of the project") {
+		t.Errorf("readCapped() dropped the file's content: %q", got)
+	}
+}
+
+// TestReadCapped_FileStartingMidRune is the floor on the boundary backup. The
+// loop walks back at most UTFMax-1 continuation bytes, so a file that opens
+// with them cannot drive the cut to zero and lose its content.
+func TestReadCapped_FileStartingMidRune(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	// Nothing but continuation bytes: every position looks mid-rune.
+	body := strings.Repeat("\x80", 200)
+	testutil.WriteFile(t, dir, "CLAUDE.md", body)
+
+	got, ok := readCapped(dir, "CLAUDE.md", 40)
+	if !ok {
+		t.Fatal("readCapped() not ok")
+	}
+	content := strings.TrimSuffix(got, "\n…(truncated)…")
+	if len(content) < 40-(utf8.UTFMax-1) {
+		t.Errorf("readCapped() kept only %d bytes; the backup must be bounded", len(content))
+	}
+}
+
+// TestReadCapped_SmallCapsOverContinuationBytes is the regression for an
+// index-out-of-range panic: the boundary backoff ran a fixed UTFMax-1 passes
+// with no floor, so at maxLen=1 over a file of continuation bytes the third
+// pass indexed s[-1]. No caller passes a cap this small (the smallest is 400),
+// which is exactly why it needs pinning here.
+//
+// Every cap from 0 up past the backoff window is exercised, over bodies chosen
+// so the cut can never find a rune start: a count-based bound is wrong for all
+// of maxLen < UTFMax-1, not just for one value.
+func TestReadCapped_SmallCapsOverContinuationBytes(t *testing.T) {
+	t.Parallel()
+
+	for _, body := range []string{
+		strings.Repeat("\x80", 50), // nothing but continuation bytes
+		"\x80\x80\x80" + strings.Repeat("a", 50),
+		strings.Repeat("é", 50), // valid, but every odd cut is mid-rune
+	} {
+		for cap := range 8 {
+			dir := t.TempDir()
+			testutil.WriteFile(t, dir, "go.mod", body)
+
+			got, ok := readCapped(dir, "go.mod", cap)
+			if !ok {
+				t.Fatalf("readCapped(cap=%d) not ok", cap)
+			}
+			content := strings.TrimSuffix(got, "\n…(truncated)…")
+			if len(content) > cap {
+				t.Errorf("readCapped(cap=%d) returned %d bytes, over the cap", cap, len(content))
+			}
+			// The contract, stated as the two ways out: either the cut landed on
+			// a rune boundary, or it kept the file's own bytes because there was
+			// no boundary to land on. Both are fine; silently substituting
+			// something else is not.
+			//
+			// Note an empty result is legitimate at a cap below one rune's width
+			// — no non-empty valid prefix exists — which is why this asserts the
+			// disjunction rather than "not empty".
+			if !utf8.ValidString(content) && content != body[:cap] {
+				t.Errorf("readCapped(cap=%d) = %q: neither valid UTF-8 nor the file's own bytes", cap, content)
+			}
+		}
+	}
+}