fix: 22 validated bug fixes from adversarial bug hunt

cmd/odek/bg_tools.go

@@ -360,7 +360,10 @@ func (t *bgStartTool) Call(args string) (string, error) {
 		Command        string `json:"command"`
 		TimeoutSeconds int    `json:"timeout_seconds"`
 	}
-	if err := json.Unmarshal([]byte(args), &p); err != nil || strings.TrimSpace(p.Command) == "" {
+	if err := json.Unmarshal([]byte(args), &p); err != nil {
+		return "", fmt.Errorf("bg_start: invalid arguments (malformed JSON): %w", err)
+	}
+	if strings.TrimSpace(p.Command) == "" {
 		return "", fmt.Errorf("bg_start requires a non-empty \"command\"")
 	}
 	// Spawn-time approval, shell parity: the loop's batch gate only covers

cmd/odek/bg_tools_error_test.go

@@ -0,0 +1,25 @@
+package main
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// Malformed JSON arguments must be reported as a decode failure, not as
+// "requires a non-empty command" — otherwise the model retries with the
+// same payload shape instead of fixing the JSON.
+func TestBgStartCall_MalformedJSONErrorMessage(t *testing.T) {
+	tool := &bgStartTool{}
+	_, err := tool.Call(`{bad json`)
+	if err == nil {
+		t.Fatal("expected error for malformed JSON args")
+	}
+	lower := strings.ToLower(err.Error())
+	if !strings.Contains(lower, "json") && !strings.Contains(lower, "invalid argument") {
+		t.Fatalf("error %q does not mention invalid arguments/JSON", err)
+	}
+	if json.Valid([]byte(`{bad json`)) {
+		t.Fatal("sanity: input unexpectedly valid JSON")
+	}
+}

cmd/odek/browser_race_test.go

@@ -0,0 +1,24 @@
+package main
+
+import (
+	"sync"
+	"testing"
+)
+
+// Zero-value browserTool lazy-initializes state and client inside Call;
+// concurrent first calls race and can build duplicate states.
+func TestBrowserTool_LazyInitRace(t *testing.T) {
+	tool := &browserTool{}
+	var wg sync.WaitGroup
+	for i := 0; i < 8; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			tool.Call(`{"action":"navigate","url":"http://127.0.0.1:1/"}`)
+		}()
+	}
+	wg.Wait()
+	if tool.state == nil || tool.client == nil {
+		t.Fatal("expected state and client to be initialized")
+	}
+}

cmd/odek/browser_tool.go

@@ -60,6 +60,14 @@ const maxBrowserElements = 500
 // history limit cannot be bypassed by a small number of huge pages.
 const maxBrowserSnapshotBytes = 1 * 1024 * 1024
 
+// truncatePageContent caps page text at maxBrowserSnapshotBytes, backing up
+// to a UTF-8 rune boundary so a multibyte character split by the cap never
+// ships U+FFFD mojibake, and appends a truncation marker.
+func truncatePageContent(content string) string {
+	return truncateUTF8Safe(content, maxBrowserSnapshotBytes) +
+		"\n[content truncated: exceeds per-snapshot byte cap]"
+}
+
 // browserState holds the shared state for one browser session.
 type browserState struct {
 	mu      sync.Mutex
@@ -74,6 +82,7 @@ type browserTool struct {
 	ctxTool
 	state           *browserState
 	client          *http.Client
+	initMu          sync.Mutex
 	dangerousConfig danger.DangerousConfig
 	trustedClasses  map[danger.RiskClass]bool
 }
@@ -172,7 +181,9 @@ func (t *browserTool) Call(argsJSON string) (string, error) {
 		return jsonError("action is required (navigate, snapshot, click, back)")
 	}
 
-	// Ensure state and client exist
+	// Ensure state and client exist exactly once under lock — parallel
+	// first calls would otherwise race and build duplicate states.
+	t.initMu.Lock()
 	if t.state == nil {
 		t.state = &browserState{nextRef: 1}
 	}
@@ -183,6 +194,7 @@ func (t *browserTool) Call(argsJSON string) (string, error) {
 			Transport:     ssrfGuardedTransport(),
 		}
 	}
+	t.initMu.Unlock()
 
 	switch args.Action {
 	case "navigate":
@@ -472,8 +484,9 @@ func parseHTML(ctx context.Context, html, pageURL string, status int) browserSna
 
 	snap.Content = strings.Join(contentParts, "\n")
 	if len(snap.Content) > maxBrowserSnapshotBytes {
-		snap.Content = snap.Content[:maxBrowserSnapshotBytes] +
-			"\n[content truncated: exceeds per-snapshot byte cap]"
+		// Back up to a UTF-8 rune boundary so a multibyte character split
+		// by the cap never ships U+FFFD mojibake.
+		snap.Content = truncatePageContent(snap.Content)
 	}
 	snap.Elements = elements
 

cmd/odek/browser_truncation_test.go

@@ -0,0 +1,30 @@
+package main
+
+import (
+	"strings"
+	"testing"
+	"unicode/utf8"
+)
+
+// Snapshot truncation must back up to a UTF-8 rune boundary so multibyte
+// characters cut by the cap never ship U+FFFD mojibake.
+func TestTruncatePageContent_RuneBoundary(t *testing.T) {
+	// Build content where the cap lands mid-rune: 'é' is 2 bytes.
+	unit := strings.Repeat("a", 9) + "é"
+	var b strings.Builder
+	for b.Len() < maxBrowserSnapshotBytes {
+		b.WriteString(unit)
+	}
+	content := b.String()
+	got := truncatePageContent(content)
+	trimmed := strings.TrimSuffix(got, "\n[content truncated: exceeds per-snapshot byte cap]")
+	if !utf8.ValidString(trimmed) {
+		t.Fatal("truncated content is not valid UTF-8")
+	}
+	if strings.ContainsRune(trimmed, utf8.RuneError) {
+		t.Fatal("truncated content contains U+FFFD replacement rune")
+	}
+	if len(trimmed) > maxBrowserSnapshotBytes {
+		t.Fatalf("truncated content %d bytes exceeds cap %d", len(trimmed), maxBrowserSnapshotBytes)
+	}
+}

cmd/odek/perf_tools.go

@@ -269,8 +269,7 @@ func (t *batchPatchTool) Call(argsJSON string) (result string, err error) {
 			continue
 		}
 
-		diff := fmt.Sprintf("--- a/%s\n+++ b/%s\n@@ -1 +1 @@\n-%s\n+%s\n",
-			p.Path, p.Path, truncatePreviewLine(original, 100), truncatePreviewLine(modified, 100))
+		diff := patchPreviewDiff(p.Path, original, modified, p.OldString, p.NewString)
 
 		// Preserve the original file's mode.
 		origMode := os.FileMode(0644)
@@ -344,6 +343,59 @@ func (t *batchPatchTool) Call(argsJSON string) (result string, err error) {
 // truncatePreviewLine shortens one side of a batch_patch preview line to max
 // bytes, backing off to a UTF-8 rune boundary so multibyte content never
 // renders as U+FFFD mojibake in the diff.
+// patchPreviewDiff renders a truthful unified-diff preview of a batch_patch
+// edit: the hunk covers the region around the actual old_string match, so the
+// -/+ lines show the real change even when the match sits far past the file
+// head (a fixed first-N-bytes window renders identical lines for both sides).
+func patchPreviewDiff(path, original, modified, oldString, newString string) string {
+	header := fmt.Sprintf("--- a/%s\n+++ b/%s\n", path, path)
+	const ctxBytes = 30 // context bytes kept on each side of the match
+	offset := strings.Index(original, oldString)
+	if offset < 0 {
+		// Match not found (e.g. preview computed before the check): fall
+		// back to a head preview of both versions.
+		return header + fmt.Sprintf("@@ -1 +1 @@\n-%s\n+%s\n",
+			truncatePreviewLine(original, 100), truncatePreviewLine(modified, 100))
+	}
+	newOffset := strings.Index(modified, newString)
+	if newOffset < 0 {
+		newOffset = offset
+	}
+	start := offset - ctxBytes
+	if start < 0 {
+		start = 0
+	}
+	end := offset + len(oldString) + ctxBytes
+	if end > len(original) {
+		end = len(original)
+	}
+	newStart := newOffset - ctxBytes
+	if newStart < 0 {
+		newStart = 0
+	}
+	newEnd := newOffset + len(newString) + ctxBytes
+	if newEnd > len(modified) {
+		newEnd = len(modified)
+	}
+	startLine := 1 + strings.Count(original[:start], "\n")
+	newStartLine := 1 + strings.Count(modified[:newStart], "\n")
+	var b strings.Builder
+	fmt.Fprintf(&b, "@@ -%d,%d +%d,%d @@\n", startLine, end-start, newStartLine, newEnd-newStart)
+	for _, ln := range strings.SplitAfter(original[start:end], "\n") {
+		if ln == "" {
+			continue
+		}
+		fmt.Fprintf(&b, "-%s\n", strings.TrimSuffix(ln, "\n"))
+	}
+	for _, ln := range strings.SplitAfter(modified[newStart:newEnd], "\n") {
+		if ln == "" {
+			continue
+		}
+		fmt.Fprintf(&b, "+%s\n", strings.TrimSuffix(ln, "\n"))
+	}
+	return header + b.String()
+}
+
 func truncatePreviewLine(s string, max int) string {
 	if len(s) <= max {
 		return s
@@ -1311,8 +1363,15 @@ func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int)
 	resultBytes := 0
 
 	var skipped []string
+	var rootErr error
 	filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
 		if err != nil || info == nil {
+			// Surface a missing/unreadable root instead of returning a
+			// silent count:0 result for a path that was never scanned.
+			if path == root {
+				rootErr = err
+				return err
+			}
 			return nil
 		}
 		if info.IsDir() {
@@ -1398,6 +1457,13 @@ func (t *multiGrepTool) searchPattern(pattern, root, fileGlob string, limit int)
 		return nil
 	})
 
+	rootErrOut := rootErr
+	if rootErrOut != nil {
+		return grepPatternResult{
+			Pattern: pattern,
+			Error:   fmt.Sprintf("cannot walk root %q: %v", root, rootErrOut),
+		}
+	}
 	return grepPatternResult{
 		Pattern: pattern,
 		Matches: matches,

cmd/odek/perf_tools_diff_test.go

@@ -0,0 +1,40 @@
+package main
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+)
+
+// The batch_patch preview hunk must be truthful: it shows the region around
+// the actual match, not a hardcoded first-100-bytes diff that renders
+// identical -/+ lines when old_string sits past byte 100.
+func TestPatchPreviewDiff_OffsetMatch(t *testing.T) {
+	padding := strings.Repeat("x", 300)
+	original := padding + "\nold line\n" + strings.Repeat("y", 100)
+	modified := padding + "\nnew line\n" + strings.Repeat("y", 100)
+	diff := patchPreviewDiff("f.txt", original, modified, "old line", "new line")
+	if strings.Contains(diff, "@@ -1 +1 @@") {
+		t.Fatalf("preview still uses hardcoded hunk header: %s", diff)
+	}
+	if !strings.Contains(diff, "-old line") || !strings.Contains(diff, "+new line") {
+		t.Fatalf("preview hunk does not show the changed lines:\n%s", diff)
+	}
+}
+
+func TestPatchPreviewDiff_HeadMatch(t *testing.T) {
+	original := "alpha\nbeta\ngamma"
+	modified := "alpha\nBETA\ngamma"
+	diff := patchPreviewDiff("f.txt", original, modified, "beta", "BETA")
+	if !strings.Contains(diff, "-beta") || !strings.Contains(diff, "+BETA") {
+		t.Fatalf("head-match preview wrong:\n%s", diff)
+	}
+}
+
+func TestPatchPreviewDiff_OldNotFound(t *testing.T) {
+	diff := patchPreviewDiff("f.txt", "abc", "abd", "zzz", "q")
+	if diff == "" || !strings.Contains(diff, "f.txt") {
+		t.Fatalf("expected fallback diff naming the file, got %q", diff)
+	}
+	fmt.Print()
+}

cmd/odek/perf_tools_multigrep_root_test.go

@@ -0,0 +1,26 @@
+package main
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
+
+// A nonexistent walk root must surface an error, not a silent count:0
+// result for a path that was never scanned.
+func TestMultiGrep_NonexistentRootIsError(t *testing.T) {
+	tool := &multiGrepTool{}
+	out, _ := tool.Call(`{"patterns":["x"],"path":"/nonexistent-dir-xyz-123456"}`)
+	var res struct {
+		Results []struct {
+			Error string `json:"error"`
+		} `json:"results"`
+	}
+	if err := json.Unmarshal([]byte(out), &res); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if len(res.Results) != 1 || res.Results[0].Error == "" ||
+		!strings.Contains(res.Results[0].Error, "/nonexistent-dir-xyz-123456") {
+		t.Fatalf("expected root error surfaced, got: %s", out)
+	}
+}