johannSoCodeyPublic

Phase 2: edit_file, bash, write_file

edit_file is the centerpiece and gets the failure modes spelled out:

- Not read this session, or changed on disk since, is a refusal. This is
  the rule that kills edits made against remembered rather than observed
  content.
- Zero matches never returns a bare "not found". It retries with
  whitespace collapsed and, when that hits, names the line range and
  prints what the file actually contains; otherwise it scores every line
  against the target and shows twenty lines around the best candidate. A
  miss carrying context is fixed in one turn.
- More than one match is refused with the line number of every occurrence.
  Silently taking the first is how a file gets quietly corrupted.
- Success returns a unified diff from a line-level LCS, not "OK", so the
  model can check the change against what it intended.
- Line endings and file mode are preserved, and the read record is
  refreshed so consecutive edits are not blocked by the mtime the previous
  edit caused.

Existence is checked before the read-before-edit rule: a wrong path should
say "no such file", not "you have not read this file".

bash runs in its own process group and kills the group on timeout — killing
only the leader leaves orphans holding the pipes and the loop hangs. Output
keeps the first and last hundred lines; head-only truncation would cut off
the end of a test run, which is where the failure is. A non-zero exit comes
back as an error with the output attached.

write_file stays separate from edit_file so a whole-file rewrite succeeding
cannot flatter the edit success rate, and it applies the same read-before-
write rule when replacing an existing file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Browse Files

Jjonicommitted 4 weeks ago1 parent db315e4commitdbdaa91

Showing 9 changed files with 1126 additions and 9 deletions.

+3−0cmd/harness/main.goView File

@@ -76,6 +76,9 @@ func run() error {
76	76	tools.NewRead(root, state),
77	77	tools.NewGlob(root),
78	78	tools.NewGrep(root),
	79	+ tools.NewEdit(root, state),
	80	+ tools.NewWrite(root, state),
	81	+ tools.NewBash(root),
79	82	)
80	83
81	84	render := &ui.Renderer{W: os.Stdout, Verbose: *verbose}

+4−7internal/agent/state.goView File

@@ -9,15 +9,12 @@ import (
9	9	"time"
10	10
11	11	"harness/internal/llm"
	12	+ "harness/internal/tools"
12	13	)
13	14
14		- // ReadRecord is when a file was last read and what its mtime was at the time.
15		- // Phase 2's edit_file uses this to refuse edits to files the model has not read
16		- // and to force a re-read when the file has changed underneath it.
17		- type ReadRecord struct {
18		- At time.Time
19		- Mod time.Time
20		- }
	15	+ // ReadRecord lives in tools so the edit tool can query the tracker without
	16	+ // importing this package.
	17	+ type ReadRecord = tools.ReadRecord
21	18
22	19	// State is the conversation history plus the per-session file-read tracking.
23	20	type State struct {

+103−0internal/tools/bash.goaddedView File

++ package tools
++ import (
++ 	"bytes"
++ 	"context"
++ 	"fmt"
++ 	"os/exec"
++ 	"strings"
++ 	"syscall"
++ 	"time"
++ )
++ const (
++ 	defaultBashTimeout = 60 * time.Second
++ 	maxBashTimeout     = 10 * time.Minute
++ 	// Output is capped head and tail. Head-only truncation would cut off the
++ 	// end of a test run, which is exactly where the failure is.
++ 	bashHeadLines = 100
++ 	bashTailLines = 100
++ )
++ // BashArgs is the bash input.
++ type BashArgs struct {
++ 	Command   string `json:"command" jsonschema:"description=Shell command to run, from the working root"`
++ 	TimeoutMS int    `json:"timeout_ms,omitempty" jsonschema:"description=Timeout in milliseconds; defaults to 60000, maximum 600000"`
++ }
++ // NewBash builds the bash tool.
++ func NewBash(root string) Tool {
++ 	return New("bash", strings.TrimSpace(`
++ Run a shell command from the working root and return its exit code, stdout and
++ stderr. Use it to run the project's tests and build. Output is capped, keeping
++ the first and last hundred lines. Prefer glob and grep over find and cat: they
++ are faster and their output is bounded.`),
++ 		func(ctx context.Context, in BashArgs) (string, error) {
++ 			if strings.TrimSpace(in.Command) == "" {
++ 				return "", fmt.Errorf("command is required")
++ 			}
++ 			timeout := defaultBashTimeout
++ 			if in.TimeoutMS > 0 {
++ 				timeout = time.Duration(in.TimeoutMS) * time.Millisecond
++ 			}
++ 			if timeout > maxBashTimeout {
++ 				timeout = maxBashTimeout
++ 			}
++ 			ctx, cancel := context.WithTimeout(ctx, timeout)
++ 			defer cancel()
++ 			cmd := exec.CommandContext(ctx, "bash", "-c", in.Command)
++ 			cmd.Dir = resolveBase(root, "")
++ 			// A new process group, so a command that spawns children can be
++ 			// killed whole. Killing only the leader leaves orphans holding the
++ 			// pipes open and the loop hangs waiting on them.
++ 			cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
++ 			cmd.Cancel = func() error {
++ 				if cmd.Process == nil {
++ 					return nil
++ 				}
++ 				return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
++ 			}
++ 			cmd.WaitDelay = 2 * time.Second
++ 			var stdout, stderr bytes.Buffer
++ 			cmd.Stdout = &stdout
++ 			cmd.Stderr = &stderr
++ 			start := time.Now()
++ 			runErr := cmd.Run()
++ 			elapsed := time.Since(start)
++ 			exitCode := cmd.ProcessState.ExitCode()
++ 			timedOut := ctx.Err() == context.DeadlineExceeded
++ 			var b strings.Builder
++ 			switch {
++ 			case timedOut:
++ 				fmt.Fprintf(&b, "timed out after %s (killed)\n", timeout)
++ 			case runErr != nil && exitCode < 0:
++ 				fmt.Fprintf(&b, "failed to run: %v\n", runErr)
++ 			default:
++ 				fmt.Fprintf(&b, "exit code: %d (%s)\n", exitCode, elapsed.Round(time.Millisecond))
++ 			}
++ 			writeStream(&b, "stdout", stdout.String())
++ 			writeStream(&b, "stderr", stderr.String())
++ 			out := strings.TrimRight(b.String(), "\n")
++ 			// A non-zero exit is reported as an error so the model sees it as a
++ 			// failure it has to respond to, with the output still attached.
++ 			if timedOut || exitCode != 0 {
++ 				return "", fmt.Errorf("%s", out)
++ 			}
++ 			return out, nil
++ 		})
++ }
++ func writeStream(b *strings.Builder, label, s string) {
++ 	s = strings.TrimRight(s, "\n")
++ 	if s == "" {
++ 		return
++ 	}
++ 	fmt.Fprintf(b, "\n--- %s ---\n%s\n", label, headTail(s, bashHeadLines, bashTailLines))
++ }

+113−0internal/tools/diff.goaddedView File

++ package tools
++ import (
++ 	"fmt"
++ 	"strings"
++ )
++ // diffContext is how many unchanged lines to show either side of a change.
++ const diffContext = 3
++ type diffOp struct {
++ 	kind byte // ' ' keep, '-' remove, '+' add
++ 	text string
++ }
++ // diffLines is a line-level LCS diff. The regions being diffed are edit-sized,
++ // so the quadratic table is not a problem; the guard below is for the
++ // pathological case of a whole-file replacement.
++ func diffLines(a, b []string) []diffOp {
++ 	if len(a)*len(b) > 4_000_000 {
++ 		ops := make([]diffOp, 0, len(a)+len(b))
++ 		for _, l := range a {
++ 			ops = append(ops, diffOp{'-', l})
++ 		}
++ 		for _, l := range b {
++ 			ops = append(ops, diffOp{'+', l})
++ 		}
++ 		return ops
++ 	}
++ 	// lcs[i][j] is the length of the longest common subsequence of a[i:] and b[j:].
++ 	lcs := make([][]int, len(a)+1)
++ 	for i := range lcs {
++ 		lcs[i] = make([]int, len(b)+1)
++ 	}
++ 	for i := len(a) - 1; i >= 0; i-- {
++ 		for j := len(b) - 1; j >= 0; j-- {
++ 			if a[i] == b[j] {
++ 				lcs[i][j] = lcs[i+1][j+1] + 1
++ 			} else if lcs[i+1][j] >= lcs[i][j+1] {
++ 				lcs[i][j] = lcs[i+1][j]
++ 			} else {
++ 				lcs[i][j] = lcs[i][j+1]
++ 			}
++ 		}
++ 	}
++ 	var ops []diffOp
++ 	i, j := 0, 0
++ 	for i < len(a) && j < len(b) {
++ 		switch {
++ 		case a[i] == b[j]:
++ 			ops = append(ops, diffOp{' ', a[i]})
++ 			i++
++ 			j++
++ 		case lcs[i+1][j] >= lcs[i][j+1]:
++ 			ops = append(ops, diffOp{'-', a[i]})
++ 			i++
++ 		default:
++ 			ops = append(ops, diffOp{'+', b[j]})
++ 			j++
++ 		}
++ 	}
++ 	for ; i < len(a); i++ {
++ 		ops = append(ops, diffOp{'-', a[i]})
++ 	}
++ 	for ; j < len(b); j++ {
++ 		ops = append(ops, diffOp{'+', b[j]})
++ 	}
++ 	return ops
++ }
++ // unifiedDiff renders the change to a region as a unified diff hunk. startLine
++ // is the 1-indexed line in the original file where oldRegion begins, and before
++ // and after are the surrounding context lines.
++ func unifiedDiff(path string, before, oldRegion, newRegion, after []string, startLine int) string {
++ 	ops := diffLines(oldRegion, newRegion)
++ 	oldStart := startLine - len(before)
++ 	if oldStart < 1 {
++ 		oldStart = 1
++ 	}
++ 	oldCount := len(before) + len(oldRegion) + len(after)
++ 	newCount := len(before) + len(newRegion) + len(after)
++ 	var b strings.Builder
++ 	fmt.Fprintf(&b, "--- %s\n+++ %s\n", path, path)
++ 	fmt.Fprintf(&b, "@@ -%d,%d +%d,%d @@\n", oldStart, oldCount, oldStart, newCount)
++ 	for _, l := range before {
++ 		fmt.Fprintf(&b, " %s\n", l)
++ 	}
++ 	for _, op := range ops {
++ 		fmt.Fprintf(&b, "%c%s\n", op.kind, op.text)
++ 	}
++ 	for _, l := range after {
++ 		fmt.Fprintf(&b, " %s\n", l)
++ 	}
++ 	return strings.TrimRight(b.String(), "\n")
++ }
++ // contextAround returns up to diffContext lines either side of [start, end) in
++ // lines, where start and end are 0-indexed.
++ func contextAround(lines []string, start, end int) (before, after []string) {
++ 	lo := start - diffContext
++ 	if lo < 0 {
++ 		lo = 0
++ 	}
++ 	hi := end + diffContext
++ 	if hi > len(lines) {
++ 		hi = len(lines)
++ 	}
++ 	return lines[lo:start], lines[end:hi]
++ }

+288−0internal/tools/edit.goaddedView File

++ package tools
++ import (
++ 	"context"
++ 	"fmt"
++ 	"os"
++ 	"strings"
++ )
++ // EditArgs is the edit_file input.
++ type EditArgs struct {
++ 	Path       string `json:"path" jsonschema:"description=Path to the file, absolute or relative to the working root"`
++ 	OldStr     string `json:"old_str" jsonschema:"description=Exact text to replace, including indentation. Must appear exactly once unless replace_all is set"`
++ 	NewStr     string `json:"new_str" jsonschema:"description=Text to replace it with. Use an empty string to delete"`
++ 	ReplaceAll bool   `json:"replace_all,omitempty" jsonschema:"description=Replace every occurrence instead of requiring a unique match"`
++ }
++ // NewEdit builds the edit_file tool. track must be the same tracker read_file
++ // writes to.
++ func NewEdit(root string, track FileTracker) Tool {
++ 	return New("edit_file", strings.TrimSpace(`
++ Replace exact text in a file you have already read. old_str must match the file
++ byte for byte, including indentation, and must be unique — include surrounding
++ lines to disambiguate rather than hoping. Returns a unified diff of what
++ changed, which you should check against what you meant to do.`),
++ 		func(_ context.Context, in EditArgs) (string, error) {
++ 			path, err := resolve(root, in.Path)
++ 			if err != nil {
++ 				return "", err
++ 			}
++ 			if in.OldStr == "" {
++ 				return "", fmt.Errorf("old_str is required; use write_file to create a new file")
++ 			}
++ 			if in.OldStr == in.NewStr {
++ 				return "", fmt.Errorf("old_str and new_str are identical, so this edit would do nothing")
++ 			}
++ 			// Existence is checked before the read-before-edit rule, so a
++ 			// wrong path reports the wrong path rather than "you have not read
++ 			// this file", which would send the model looking for the wrong bug.
++ 			raw, err := os.ReadFile(path)
++ 			if err != nil {
++ 				if os.IsNotExist(err) {
++ 					return "", fmt.Errorf("no such file: %s", in.Path)
++ 				}
++ 				return "", err
++ 			}
++ 			if err := checkRead(track, path, in.Path); err != nil {
++ 				return "", err
++ 			}
++ 			content, crlf := normalizeEndings(string(raw))
++ 			oldText, newText := normalizeInput(in.OldStr), normalizeInput(in.NewStr)
++ 			switch n := strings.Count(content, oldText); {
++ 			case n == 0:
++ 				return "", missReport(in.Path, content, oldText)
++ 			case n > 1 && !in.ReplaceAll:
++ 				return "", ambiguityReport(in.Path, content, oldText, n)
++ 			}
++ 			updated := content
++ 			count := 1
++ 			if in.ReplaceAll {
++ 				count = strings.Count(content, oldText)
++ 				updated = strings.ReplaceAll(content, oldText, newText)
++ 			} else {
++ 				updated = strings.Replace(content, oldText, newText, 1)
++ 			}
++ 			info, err := os.Stat(path)
++ 			perm := os.FileMode(0o644)
++ 			if err == nil {
++ 				perm = info.Mode().Perm()
++ 			}
++ 			if err := os.WriteFile(path, []byte(restoreEndings(updated, crlf)), perm); err != nil {
++ 				return "", err
++ 			}
++ 			// The file just changed, so the recorded read is stale by
++ 			// definition. Re-point it at what is now on disk.
++ 			if info, err := os.Stat(path); err == nil {
++ 				track.MarkRead(path, info.ModTime())
++ 			}
++ 			return editSummary(in.Path, content, oldText, newText, count), nil
++ 		})
++ }
++ // checkRead enforces the read-before-edit rule, and forces a re-read when the
++ // file has changed since. This single rule kills a large class of edits against
++ // remembered rather than observed content.
++ func checkRead(track FileTracker, path, display string) error {
++ 	if track == nil {
++ 		return nil
++ 	}
++ 	rec, ok := track.LastRead(path)
++ 	if !ok {
++ 		return fmt.Errorf("%s has not been read this session; read_file it first so you are editing what is actually there", display)
++ 	}
++ 	info, err := os.Stat(path)
++ 	if err != nil {
++ 		return nil // the read below will produce the real error
++ 	}
++ 	if info.ModTime().After(rec.Mod) {
++ 		return fmt.Errorf("%s changed on disk since you read it; read_file it again before editing", display)
++ 	}
++ 	return nil
++ }
++ // missReport turns "not found" into something recoverable. A bare miss costs
++ // several turns of blind retrying; a miss carrying the actual text costs one.
++ func missReport(display, content, oldText string) error {
++ 	oldLines := strings.Split(oldText, "\n")
++ 	lines := strings.Split(content, "\n")
++ 	// Whitespace is the usual culprit, so try again ignoring it.
++ 	if start, end, ok := findNormalized(lines, oldLines); ok {
++ 		var b strings.Builder
++ 		fmt.Fprintf(&b, "old_str was not found in %s, but the same text is there with different whitespace at lines %d-%d. The file actually contains:\n\n",
++ 			display, start+1, end)
++ 		for i := start; i < end; i++ {
++ 			fmt.Fprintf(&b, "%6d\t%s\n", i+1, capLine(lines[i]))
++ 		}
++ 		b.WriteString("\nCopy that text exactly, including its indentation.")
++ 		return fmt.Errorf("%s", b.String())
++ 	}
++ 	// Otherwise show the neighbourhood of the closest line we can find.
++ 	best := closestLine(lines, oldLines[0])
++ 	if best < 0 {
++ 		return fmt.Errorf("old_str was not found in %s, and nothing in the file resembles it. Read the file again — you may have the wrong path", display)
++ 	}
++ 	lo, hi := best-10, best+10
++ 	if lo < 0 {
++ 		lo = 0
++ 	}
++ 	if hi > len(lines) {
++ 		hi = len(lines)
++ 	}
++ 	var b strings.Builder
++ 	fmt.Fprintf(&b, "old_str was not found in %s. The closest thing in the file is around line %d:\n\n", display, best+1)
++ 	for i := lo; i < hi; i++ {
++ 		fmt.Fprintf(&b, "%6d\t%s\n", i+1, capLine(lines[i]))
++ 	}
++ 	b.WriteString("\nUse text copied from there, or read_file a wider range if the target is elsewhere.")
++ 	return fmt.Errorf("%s", b.String())
++ }
++ // ambiguityReport refuses a non-unique match and says where every occurrence
++ // is, so the next attempt can be made unique. Silently taking the first match
++ // would corrupt the file in a way nobody notices for a week.
++ func ambiguityReport(display, content, oldText string, n int) error {
++ 	var b strings.Builder
++ 	fmt.Fprintf(&b, "old_str appears %d times in %s, at lines: %s.\n\n", n, display, joinInts(occurrenceLines(content, oldText)))
++ 	b.WriteString("Add surrounding lines to old_str so it matches exactly one place, or set replace_all if every occurrence should change.")
++ 	return fmt.Errorf("%s", b.String())
++ }
++ // editSummary renders the applied change as a unified diff.
++ func editSummary(display, content, oldText, newText string, count int) string {
++ 	idx := strings.Index(content, oldText)
++ 	startLine := strings.Count(content[:idx], "\n")
++ 	lines := strings.Split(content, "\n")
++ 	oldRegion := strings.Split(oldText, "\n")
++ 	newRegion := strings.Split(newText, "\n")
++ 	end := startLine + len(oldRegion)
++ 	if end > len(lines) {
++ 		end = len(lines)
++ 	}
++ 	before, after := contextAround(lines, startLine, end)
++ 	out := unifiedDiff(display, before, oldRegion, newRegion, after, startLine+1)
++ 	if count > 1 {
++ 		return fmt.Sprintf("Replaced %d occurrences. First one:\n\n%s", count, out)
++ 	}
++ 	return out
++ }
++ // findNormalized looks for the target lines ignoring leading, trailing and
++ // repeated internal whitespace. Returns a half-open [start, end) line range.
++ func findNormalized(lines, target []string) (int, int, bool) {
++ 	if len(target) == 0 || len(target) > len(lines) {
++ 		return 0, 0, false
++ 	}
++ 	norm := make([]string, len(lines))
++ 	for i, l := range lines {
++ 		norm[i] = collapse(l)
++ 	}
++ 	want := make([]string, len(target))
++ 	for i, l := range target {
++ 		want[i] = collapse(l)
++ 	}
++ 	for i := 0; i+len(want) <= len(norm); i++ {
++ 		match := true
++ 		for j := range want {
++ 			if norm[i+j] != want[j] {
++ 				match = false
++ 				break
++ 			}
++ 		}
++ 		if match {
++ 			return i, i + len(want), true
++ 		}
++ 	}
++ 	return 0, 0, false
++ }
++ // closestLine scores every line against want and returns the best index, or -1
++ // when nothing is even loosely similar.
++ func closestLine(lines []string, want string) int {
++ 	want = collapse(want)
++ 	if want == "" {
++ 		return -1
++ 	}
++ 	wantTokens := strings.Fields(want)
++ 	best, bestScore := -1, 0.0
++ 	for i, l := range lines {
++ 		score := similarity(strings.Fields(collapse(l)), wantTokens)
++ 		if score > bestScore {
++ 			best, bestScore = i, score
++ 		}
++ 	}
++ 	if bestScore < 0.34 {
++ 		return -1
++ 	}
++ 	return best
++ }
++ // similarity is the fraction of want's tokens present in have.
++ func similarity(have, want []string) float64 {
++ 	if len(want) == 0 {
++ 		return 0
++ 	}
++ 	seen := make(map[string]int, len(have))
++ 	for _, t := range have {
++ 		seen[t]++
++ 	}
++ 	var hits int
++ 	for _, t := range want {
++ 		if seen[t] > 0 {
++ 			seen[t]--
++ 			hits++
++ 		}
++ 	}
++ 	return float64(hits) / float64(len(want))
++ }
++ func collapse(s string) string { return strings.Join(strings.Fields(s), " ") }
++ // occurrenceLines returns the 1-indexed line of each occurrence of sub.
++ func occurrenceLines(content, sub string) []int {
++ 	var out []int
++ 	for off := 0; ; {
++ 		i := strings.Index(content[off:], sub)
++ 		if i < 0 {
++ 			return out
++ 		}
++ 		out = append(out, strings.Count(content[:off+i], "\n")+1)
++ 		off += i + len(sub)
++ 	}
++ }
++ func joinInts(ns []int) string {
++ 	parts := make([]string, len(ns))
++ 	for i, n := range ns {
++ 		parts[i] = fmt.Sprint(n)
++ 	}
++ 	return strings.Join(parts, ", ")
++ }
++ // normalizeEndings converts CRLF to LF and reports whether the file used CRLF,
++ // so the original ending can be restored on write.
++ func normalizeEndings(s string) (string, bool) {
++ 	if strings.Contains(s, "\r\n") {
++ 		return strings.ReplaceAll(s, "\r\n", "\n"), true
++ 	}
++ 	return s, false
++ }
++ func restoreEndings(s string, crlf bool) string {
++ 	if !crlf {
++ 		return s
++ 	}
++ 	return strings.ReplaceAll(s, "\n", "\r\n")
++ }
++ // normalizeInput strips CR from model-supplied text, which otherwise fails to
++ // match a file we have already normalized.
++ func normalizeInput(s string) string { return strings.ReplaceAll(s, "\r\n", "\n") }

+324−0internal/tools/edit_test.goaddedView File

++ package tools
++ import (
++ 	"context"
++ 	"encoding/json"
++ 	"os"
++ 	"path/filepath"
++ 	"strings"
++ 	"sync"
++ 	"testing"
++ 	"time"
++ )
++ // memTracker is a FileTracker backed by a map.
++ type memTracker struct {
++ 	mu   sync.Mutex
++ 	recs map[string]ReadRecord
++ }
++ func newTracker() *memTracker { return &memTracker{recs: map[string]ReadRecord{}} }
++ func (m *memTracker) MarkRead(path string, mod time.Time) {
++ 	m.mu.Lock()
++ 	defer m.mu.Unlock()
++ 	m.recs[path] = ReadRecord{At: time.Now(), Mod: mod}
++ }
++ func (m *memTracker) LastRead(path string) (ReadRecord, bool) {
++ 	m.mu.Lock()
++ 	defer m.mu.Unlock()
++ 	r, ok := m.recs[path]
++ 	return r, ok
++ }
++ // editFixture writes a file and marks it read, which is the normal precondition.
++ func editFixture(t *testing.T, content string) (root string, track *memTracker, tool Tool) {
++ 	t.Helper()
++ 	root = t.TempDir()
++ 	path := writeFile(t, root, "sample.go", content)
++ 	track = newTracker()
++ 	info, err := os.Stat(path)
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	track.MarkRead(path, info.ModTime())
++ 	return root, track, NewEdit(root, track)
++ }
++ func edit(t *testing.T, tool Tool, args EditArgs) (string, error) {
++ 	t.Helper()
++ 	raw, err := json.Marshal(args)
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	return tool.Execute(context.Background(), raw)
++ }
++ const sample = `package main
++ import "fmt"
++ func greet(name string) {
++ 	fmt.Println("hello", name)
++ }
++ func main() {
++ 	greet("world")
++ }
++ `
++ func TestEditExactlyOneMatchAppliesAndReturnsADiff(t *testing.T) {
++ 	root, _, tool := editFixture(t, sample)
++ 	out, err := edit(t, tool, EditArgs{
++ 		Path:   "sample.go",
++ 		OldStr: `	fmt.Println("hello", name)`,
++ 		NewStr: `	fmt.Printf("hello %s\n", name)`,
++ 	})
++ 	if err != nil {
++ 		t.Fatalf("edit: %v", err)
++ 	}
++ 	got, err := os.ReadFile(filepath.Join(root, "sample.go"))
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	if !strings.Contains(string(got), `fmt.Printf("hello %s\n", name)`) {
++ 		t.Errorf("file was not changed:\n%s", got)
++ 	}
++ 	if strings.Contains(string(got), `fmt.Println("hello", name)`) {
++ 		t.Errorf("old text still present:\n%s", got)
++ 	}
++ 	// The diff is what the model uses to confirm it did what it meant.
++ 	for _, want := range []string{"--- sample.go", "@@ -", `-	fmt.Println("hello", name)`, `+	fmt.Printf`, " func greet(name string) {"} {
++ 		if !strings.Contains(out, want) {
++ 			t.Errorf("diff missing %q:\n%s", want, out)
++ 		}
++ 	}
++ }
++ func TestEditZeroMatchesCarriesRecoveryContext(t *testing.T) {
++ 	tests := []struct {
++ 		name    string
++ 		content string
++ 		oldStr  string
++ 		want    []string
++ 	}{
++ 		{
++ 			name:    "indentation differs: say so and show the real line",
++ 			content: sample,
++ 			oldStr:  `    fmt.Println("hello", name)`, // spaces, but the file uses a tab
++ 			want:    []string{"different whitespace at lines 6-6", "Copy that text exactly", `	fmt.Println("hello", name)`},
++ 		},
++ 		{
++ 			name:    "internal whitespace differs beyond recognition",
++ 			content: sample,
++ 			oldStr:  `fmt.Println( "hello",  name )`,
++ 			want:    []string{"nothing in the file resembles it"},
++ 		},
++ 		{
++ 			name:    "near miss shows the neighbourhood with line numbers",
++ 			content: sample,
++ 			oldStr:  `	fmt.Println("goodbye", name)`,
++ 			want:    []string{"closest thing in the file is around line", "     6\t", "read_file a wider range"},
++ 		},
++ 		{
++ 			name:    "nothing similar at all",
++ 			content: sample,
++ 			oldStr:  "zzz qqq vvv nothing like this",
++ 			want:    []string{"nothing in the file resembles it"},
++ 		},
++ 	}
++ 	for _, tc := range tests {
++ 		t.Run(tc.name, func(t *testing.T) {
++ 			root, _, tool := editFixture(t, tc.content)
++ 			_, err := edit(t, tool, EditArgs{Path: "sample.go", OldStr: tc.oldStr, NewStr: "replacement"})
++ 			if err == nil {
++ 				t.Fatal("want an error")
++ 			}
++ 			for _, w := range tc.want {
++ 				if !strings.Contains(err.Error(), w) {
++ 					t.Errorf("error missing %q:\n%s", w, err)
++ 				}
++ 			}
++ 			after, _ := os.ReadFile(filepath.Join(root, "sample.go"))
++ 			if string(after) != tc.content {
++ 				t.Error("a failed edit must leave the file untouched")
++ 			}
++ 		})
++ 	}
++ }
++ func TestEditMultipleMatchesRefusesAndListsEveryLine(t *testing.T) {
++ 	const dup = "a := 1\nb := 2\na := 1\nc := 3\na := 1\n"
++ 	root, _, tool := editFixture(t, dup)
++ 	_, err := edit(t, tool, EditArgs{Path: "sample.go", OldStr: "a := 1", NewStr: "a := 9"})
++ 	if err == nil {
++ 		t.Fatal("want a refusal: silently taking the first match corrupts files")
++ 	}
++ 	for _, want := range []string{"appears 3 times", "lines: 1, 3, 5", "replace_all"} {
++ 		if !strings.Contains(err.Error(), want) {
++ 			t.Errorf("error missing %q:\n%s", want, err)
++ 		}
++ 	}
++ 	after, _ := os.ReadFile(filepath.Join(root, "sample.go"))
++ 	if string(after) != dup {
++ 		t.Error("an ambiguous edit must change nothing")
++ 	}
++ }
++ func TestEditReplaceAll(t *testing.T) {
++ 	root, _, tool := editFixture(t, "a := 1\nb := 2\na := 1\n")
++ 	out, err := edit(t, tool, EditArgs{Path: "sample.go", OldStr: "a := 1", NewStr: "a := 9", ReplaceAll: true})
++ 	if err != nil {
++ 		t.Fatalf("edit: %v", err)
++ 	}
++ 	got, _ := os.ReadFile(filepath.Join(root, "sample.go"))
++ 	if string(got) != "a := 9\nb := 2\na := 9\n" {
++ 		t.Errorf("got:\n%q", got)
++ 	}
++ 	if !strings.Contains(out, "Replaced 2 occurrences") {
++ 		t.Errorf("summary should say how many changed:\n%s", out)
++ 	}
++ }
++ func TestEditRequiresAReadFirst(t *testing.T) {
++ 	root := t.TempDir()
++ 	writeFile(t, root, "unread.go", sample)
++ 	track := newTracker()
++ 	_, err := edit(t, NewEdit(root, track), EditArgs{Path: "unread.go", OldStr: "package main", NewStr: "package x"})
++ 	if err == nil || !strings.Contains(err.Error(), "has not been read this session") {
++ 		t.Fatalf("error = %v, want a read-before-edit refusal", err)
++ 	}
++ 	if got, _ := os.ReadFile(filepath.Join(root, "unread.go")); string(got) != sample {
++ 		t.Error("file must be untouched")
++ 	}
++ }
++ func TestEditForcesReReadWhenTheFileChangedUnderneath(t *testing.T) {
++ 	root, track, tool := editFixture(t, sample)
++ 	path := filepath.Join(root, "sample.go")
++ 	// Something else wrote the file after the model read it.
++ 	track.MarkRead(path, time.Now().Add(-time.Hour))
++ 	os.WriteFile(path, []byte(sample+"\n// appended\n"), 0o644)
++ 	_, err := edit(t, tool, EditArgs{Path: "sample.go", OldStr: "package main", NewStr: "package x"})
++ 	if err == nil || !strings.Contains(err.Error(), "changed on disk since you read it") {
++ 		t.Fatalf("error = %v, want a stale-read refusal", err)
++ 	}
++ }
++ func TestEditPreservesCRLF(t *testing.T) {
++ 	root, track, _ := editFixture(t, "")
++ 	path := writeFile(t, root, "crlf.go", "package main\r\n\r\nfunc main() {}\r\n")
++ 	info, _ := os.Stat(path)
++ 	track.MarkRead(path, info.ModTime())
++ 	out, err := edit(t, NewEdit(root, track), EditArgs{
++ 		Path:   "crlf.go",
++ 		OldStr: "func main() {}",
++ 		NewStr: "func main() {\n\tprintln(\"hi\")\n}",
++ 	})
++ 	if err != nil {
++ 		t.Fatalf("edit: %v", err)
++ 	}
++ 	got, _ := os.ReadFile(path)
++ 	if strings.Contains(out, "\r") {
++ 		t.Error("the diff shown to the model should not carry carriage returns")
++ 	}
++ 	if want := "package main\r\n\r\nfunc main() {\r\n\tprintln(\"hi\")\r\n}\r\n"; string(got) != want {
++ 		t.Errorf("CRLF not preserved:\ngot  %q\nwant %q", got, want)
++ 	}
++ 	if strings.Contains(strings.ReplaceAll(string(got), "\r\n", ""), "\n") {
++ 		t.Error("mixed line endings were introduced")
++ 	}
++ }
++ func TestEditPreservesFileMode(t *testing.T) {
++ 	root, track, _ := editFixture(t, "")
++ 	path := writeFile(t, root, "run.sh", "#!/bin/sh\necho hi\n")
++ 	os.Chmod(path, 0o755)
++ 	info, _ := os.Stat(path)
++ 	track.MarkRead(path, info.ModTime())
++ 	if _, err := edit(t, NewEdit(root, track), EditArgs{Path: "run.sh", OldStr: "echo hi", NewStr: "echo bye"}); err != nil {
++ 		t.Fatalf("edit: %v", err)
++ 	}
++ 	after, _ := os.Stat(path)
++ 	if after.Mode().Perm() != 0o755 {
++ 		t.Errorf("mode = %v, want 0755 preserved", after.Mode().Perm())
++ 	}
++ }
++ func TestEditRefreshesTheReadRecord(t *testing.T) {
++ 	root, track, tool := editFixture(t, sample)
++ 	path := filepath.Join(root, "sample.go")
++ 	if _, err := edit(t, tool, EditArgs{Path: "sample.go", OldStr: "package main", NewStr: "package other"}); err != nil {
++ 		t.Fatalf("first edit: %v", err)
++ 	}
++ 	// A second edit must not be blocked by the mtime change the first one caused.
++ 	if _, err := edit(t, tool, EditArgs{Path: "sample.go", OldStr: `import "fmt"`, NewStr: `import "os"`}); err != nil {
++ 		t.Fatalf("second edit should be allowed: %v", err)
++ 	}
++ 	rec, ok := track.LastRead(path)
++ 	if !ok {
++ 		t.Fatal("read record disappeared")
++ 	}
++ 	info, _ := os.Stat(path)
++ 	if !rec.Mod.Equal(info.ModTime()) {
++ 		t.Error("read record should track the file as the edit left it")
++ 	}
++ }
++ func TestEditArgumentValidation(t *testing.T) {
++ 	_, _, tool := editFixture(t, sample)
++ 	tests := []struct {
++ 		name, want string
++ 		args       EditArgs
++ 	}{
++ 		{name: "missing path", want: "path is required", args: EditArgs{OldStr: "a", NewStr: "b"}},
++ 		{name: "missing old_str", want: "old_str is required", args: EditArgs{Path: "sample.go", NewStr: "b"}},
++ 		{name: "no-op edit", want: "identical", args: EditArgs{Path: "sample.go", OldStr: "a", NewStr: "a"}},
++ 		{name: "missing file", want: "no such file", args: EditArgs{Path: "ghost.go", OldStr: "a", NewStr: "b"}},
++ 	}
++ 	for _, tc := range tests {
++ 		t.Run(tc.name, func(t *testing.T) {
++ 			if _, err := edit(t, tool, tc.args); err == nil || !strings.Contains(err.Error(), tc.want) {
++ 				t.Errorf("error = %v, want %q", err, tc.want)
++ 			}
++ 		})
++ 	}
++ }
++ func TestEditDeletion(t *testing.T) {
++ 	root, _, tool := editFixture(t, "keep\ndrop me\nkeep\n")
++ 	if _, err := edit(t, tool, EditArgs{Path: "sample.go", OldStr: "drop me\n", NewStr: ""}); err != nil {
++ 		t.Fatalf("edit: %v", err)
++ 	}
++ 	got, _ := os.ReadFile(filepath.Join(root, "sample.go"))
++ 	if string(got) != "keep\nkeep\n" {
++ 		t.Errorf("got %q", got)
++ 	}
++ }
++ func TestUnifiedDiffShape(t *testing.T) {
++ 	got := unifiedDiff("f.go", []string{"ctx"}, []string{"a", "b"}, []string{"a", "c"}, []string{"tail"}, 5)
++ 	want := strings.Join([]string{
++ 		"--- f.go",
++ 		"+++ f.go",
++ 		"@@ -4,4 +4,4 @@",
++ 		" ctx",
++ 		" a",
++ 		"-b",
++ 		"+c",
++ 		" tail",
++ 	}, "\n")
++ 	if got != want {
++ 		t.Errorf("got:\n%s\nwant:\n%s", got, want)
++ 	}
++ }

+197−0internal/tools/exec_test.goaddedView File

++ package tools
++ import (
++ 	"context"
++ 	"encoding/json"
++ 	"fmt"
++ 	"os"
++ 	"path/filepath"
++ 	"strings"
++ 	"testing"
++ 	"time"
++ )
++ func run(t *testing.T, tool Tool, args any) (string, error) {
++ 	t.Helper()
++ 	raw, err := json.Marshal(args)
++ 	if err != nil {
++ 		t.Fatal(err)
++ 	}
++ 	return tool.Execute(context.Background(), raw)
++ }
++ func TestBashSuccess(t *testing.T) {
++ 	dir := t.TempDir()
++ 	writeFile(t, dir, "hello.txt", "hi\n")
++ 	out, err := run(t, NewBash(dir), BashArgs{Command: "cat hello.txt"})
++ 	if err != nil {
++ 		t.Fatalf("bash: %v", err)
++ 	}
++ 	if !strings.Contains(out, "exit code: 0") || !strings.Contains(out, "--- stdout ---\nhi") {
++ 		t.Errorf("got:\n%s", out)
++ 	}
++ 	if strings.Contains(out, "stderr") {
++ 		t.Errorf("empty streams should be omitted:\n%s", out)
++ 	}
++ }
++ func TestBashNonZeroExitIsAnErrorCarryingOutput(t *testing.T) {
++ 	_, err := run(t, NewBash(t.TempDir()), BashArgs{Command: "echo out; echo problem >&2; exit 3"})
++ 	if err == nil {
++ 		t.Fatal("a failing command must come back as an error the model reacts to")
++ 	}
++ 	for _, want := range []string{"exit code: 3", "--- stdout ---\nout", "--- stderr ---\nproblem"} {
++ 		if !strings.Contains(err.Error(), want) {
++ 			t.Errorf("missing %q in:\n%s", want, err)
++ 		}
++ 	}
++ }
++ func TestBashCapsOutputKeepingBothEnds(t *testing.T) {
++ 	out, err := run(t, NewBash(t.TempDir()), BashArgs{Command: "seq 1 5000"})
++ 	if err != nil {
++ 		t.Fatalf("bash: %v", err)
++ 	}
++ 	if !strings.Contains(out, "\n1\n") {
++ 		t.Error("head was dropped")
++ 	}
++ 	if !strings.Contains(out, "5000") {
++ 		t.Error("tail was dropped — that is where test failures live")
++ 	}
++ 	if !strings.Contains(out, "lines elided") {
++ 		t.Error("truncation should be announced")
++ 	}
++ 	if n := strings.Count(out, "\n"); n > bashHeadLines+bashTailLines+10 {
++ 		t.Errorf("output not capped: %d lines", n)
++ 	}
++ }
++ func TestBashTimeoutKillsTheProcessGroup(t *testing.T) {
++ 	if testing.Short() {
++ 		t.Skip("waits on a real child process")
++ 	}
++ 	dir := t.TempDir()
++ 	marker := filepath.Join(dir, "child-survived")
++ 	// The child outlives its parent shell unless the whole group is killed.
++ 	cmd := fmt.Sprintf(`(sleep 2; touch %q) & sleep 5`, marker)
++ 	start := time.Now()
++ 	_, err := run(t, NewBash(dir), BashArgs{Command: cmd, TimeoutMS: 300})
++ 	if err == nil || !strings.Contains(err.Error(), "timed out") {
++ 		t.Fatalf("error = %v, want a timeout", err)
++ 	}
++ 	if elapsed := time.Since(start); elapsed > 3*time.Second {
++ 		t.Errorf("took %s: the timeout did not take effect promptly", elapsed)
++ 	}
++ 	time.Sleep(3 * time.Second)
++ 	if _, err := os.Stat(marker); err == nil {
++ 		t.Error("the orphaned child kept running: the process group was not killed")
++ 	}
++ }
++ func TestBashRunsInTheWorkingRoot(t *testing.T) {
++ 	dir := t.TempDir()
++ 	out, err := run(t, NewBash(dir), BashArgs{Command: "pwd"})
++ 	if err != nil {
++ 		t.Fatalf("bash: %v", err)
++ 	}
++ 	resolved, _ := filepath.EvalSymlinks(dir)
++ 	if !strings.Contains(out, dir) && !strings.Contains(out, resolved) {
++ 		t.Errorf("pwd = %s, want the working root %s", out, dir)
++ 	}
++ }
++ func TestBashRespectsCancellation(t *testing.T) {
++ 	raw, _ := json.Marshal(BashArgs{Command: "sleep 10"})
++ 	ctx, cancel := context.WithCancel(context.Background())
++ 	go func() { time.Sleep(100 * time.Millisecond); cancel() }()
++ 	start := time.Now()
++ 	if _, err := NewBash(t.TempDir()).Execute(ctx, raw); err == nil {
++ 		t.Fatal("want an error")
++ 	}
++ 	if elapsed := time.Since(start); elapsed > 3*time.Second {
++ 		t.Errorf("took %s: cancellation did not propagate", elapsed)
++ 	}
++ }
++ func TestBashEmptyCommand(t *testing.T) {
++ 	if _, err := run(t, NewBash(t.TempDir()), BashArgs{Command: "   "}); err == nil ||
++ 		!strings.Contains(err.Error(), "command is required") {
++ 		t.Errorf("error = %v", err)
++ 	}
++ }
++ func TestWriteFileCreatesIncludingParents(t *testing.T) {
++ 	dir := t.TempDir()
++ 	out, err := run(t, NewWrite(dir, newTracker()), WriteArgs{
++ 		Path: "internal/new/thing.go", Content: "package thing\n",
++ 	})
++ 	if err != nil {
++ 		t.Fatalf("write: %v", err)
++ 	}
++ 	got, err := os.ReadFile(filepath.Join(dir, "internal/new/thing.go"))
++ 	if err != nil {
++ 		t.Fatalf("file not created: %v", err)
++ 	}
++ 	if string(got) != "package thing\n" {
++ 		t.Errorf("content = %q", got)
++ 	}
++ 	if !strings.Contains(out, "Created") || !strings.Contains(out, "14 bytes") {
++ 		t.Errorf("summary = %q", out)
++ 	}
++ }
++ func TestWriteFileRefusesToClobberAnUnreadFile(t *testing.T) {
++ 	dir := t.TempDir()
++ 	writeFile(t, dir, "existing.go", "package existing\n")
++ 	_, err := run(t, NewWrite(dir, newTracker()), WriteArgs{Path: "existing.go", Content: "package gone\n"})
++ 	if err == nil || !strings.Contains(err.Error(), "has not been read this session") {
++ 		t.Fatalf("error = %v, want a read-before-write refusal", err)
++ 	}
++ 	if !strings.Contains(err.Error(), "replaces the whole file") {
++ 		t.Errorf("error should say why that matters here: %v", err)
++ 	}
++ 	got, _ := os.ReadFile(filepath.Join(dir, "existing.go"))
++ 	if string(got) != "package existing\n" {
++ 		t.Error("file must be untouched")
++ 	}
++ }
++ func TestWriteFileReplacesAReadFile(t *testing.T) {
++ 	dir := t.TempDir()
++ 	path := writeFile(t, dir, "existing.go", "package existing\n")
++ 	track := newTracker()
++ 	info, _ := os.Stat(path)
++ 	track.MarkRead(path, info.ModTime())
++ 	out, err := run(t, NewWrite(dir, track), WriteArgs{Path: "existing.go", Content: "package replaced\n"})
++ 	if err != nil {
++ 		t.Fatalf("write: %v", err)
++ 	}
++ 	if !strings.Contains(out, "Replaced") {
++ 		t.Errorf("summary = %q, want it to distinguish a rewrite from a create", out)
++ 	}
++ 	got, _ := os.ReadFile(path)
++ 	if string(got) != "package replaced\n" {
++ 		t.Errorf("content = %q", got)
++ 	}
++ }
++ func TestWriteFilePreservesCRLFOnRewrite(t *testing.T) {
++ 	dir := t.TempDir()
++ 	path := writeFile(t, dir, "win.txt", "one\r\ntwo\r\n")
++ 	track := newTracker()
++ 	info, _ := os.Stat(path)
++ 	track.MarkRead(path, info.ModTime())
++ 	if _, err := run(t, NewWrite(dir, track), WriteArgs{Path: "win.txt", Content: "three\nfour\n"}); err != nil {
++ 		t.Fatalf("write: %v", err)
++ 	}
++ 	got, _ := os.ReadFile(path)
++ 	if string(got) != "three\r\nfour\r\n" {
++ 		t.Errorf("got %q, want CRLF preserved", got)
++ 	}
++ }

+14−2internal/tools/read.goView File

@@ -13,12 +13,24 @@ import (
13	13	// ask for a window.
14	14	const defaultReadLimit = 2000
15	15
16		- // ReadTracker records that a file was read, so edit_file (Phase 2) can refuse
17		- // to edit a file the model has not looked at. agent.State implements it.
	16	+ // ReadRecord is when a file was last read and what its mtime was at the time.
	17	+ type ReadRecord struct {
	18	+ At time.Time
	19	+ Mod time.Time
	20	+ }
	21	+
	22	+ // ReadTracker records that a file was read.
18	23	type ReadTracker interface {
19	24	MarkRead(path string, mod time.Time)
20	25	}
21	26
	27	+ // FileTracker additionally answers what was read, which is what lets edit_file
	28	+ // refuse to edit a file the model has not looked at. agent.State implements it.
	29	+ type FileTracker interface {
	30	+ ReadTracker
	31	+ LastRead(path string) (ReadRecord, bool)
	32	+ }
	33	+
22	34	// ReadArgs is the read_file input.
23	35	type ReadArgs struct {
24	36	Path string `json:"path" jsonschema:"description=Path to the file, absolute or relative to the working root"`

+80−0internal/tools/write.goaddedView File

++ package tools
++ import (
++ 	"context"
++ 	"fmt"
++ 	"os"
++ 	"path/filepath"
++ 	"strings"
++ )
++ // WriteArgs is the write_file input.
++ type WriteArgs struct {
++ 	Path    string `json:"path" jsonschema:"description=Path to write, absolute or relative to the working root"`
++ 	Content string `json:"content" jsonschema:"description=Full contents of the file"`
++ }
++ // NewWrite builds the write_file tool. It is kept separate from edit_file so
++ // the two do not blur together in the metrics: a whole-file rewrite succeeding
++ // says nothing about whether targeted edits work.
++ func NewWrite(root string, track FileTracker) Tool {
++ 	return New("write_file", strings.TrimSpace(`
++ Write a complete file, creating it or replacing it entirely. Use this for new
++ files. To change part of an existing file use edit_file instead — a full rewrite
++ discards anything you did not think to include.`),
++ 		func(_ context.Context, in WriteArgs) (string, error) {
++ 			path, err := resolve(root, in.Path)
++ 			if err != nil {
++ 				return "", err
++ 			}
++ 			info, statErr := os.Stat(path)
++ 			existed := statErr == nil
++ 			if existed && info.IsDir() {
++ 				return "", fmt.Errorf("%s is a directory", in.Path)
++ 			}
++ 			content := in.Content
++ 			perm := os.FileMode(0o644)
++ 			if existed {
++ 				// Overwriting is a destructive act on content the model may
++ 				// only be guessing at, so the same read-before-write rule as
++ 				// edit_file applies.
++ 				if err := checkRead(track, path, in.Path); err != nil {
++ 					return "", fmt.Errorf("%s (write_file replaces the whole file)", err)
++ 				}
++ 				perm = info.Mode().Perm()
++ 				if prev, readErr := os.ReadFile(path); readErr == nil {
++ 					if _, crlf := normalizeEndings(string(prev)); crlf {
++ 						content = restoreEndings(normalizeInput(content), true)
++ 					}
++ 				}
++ 			} else if dir := filepath.Dir(path); dir != "." {
++ 				if err := os.MkdirAll(dir, 0o755); err != nil {
++ 					return "", err
++ 				}
++ 			}
++ 			if err := os.WriteFile(path, []byte(content), perm); err != nil {
++ 				return "", err
++ 			}
++ 			if track != nil {
++ 				if info, err := os.Stat(path); err == nil {
++ 					track.MarkRead(path, info.ModTime())
++ 				}
++ 			}
++ 			verb := "Created"
++ 			if existed {
++ 				verb = "Replaced"
++ 			}
++ 			return fmt.Sprintf("%s %s (%d bytes, %d lines).", verb, in.Path, len(content), countLines(content)), nil
++ 		})
++ }
++ func countLines(s string) int {
++ 	if s == "" {
++ 		return 0
++ 	}
++ 	return len(splitLines(s))
++ }