Files
zero/internal/tools/write_tools_test.go
Kevin CodexandClaude Fable 5 2749447cdc fix(tools): make apply_patch tolerant and re-expose edit_file (#956)
* fix(tools): make apply_patch tolerant and re-expose edit_file

Head-to-head benchmarking against Grok Build on grok-4.6 (3 tasks × 3 runs)
showed Zero solving every task but spending 2.7× the tokens and 2.3× the
time on the multi-file feature task. Every one of the 20 apply_patch calls
in those runs failed, after which the model degraded to whole-file
write_file rewrites and read→write verification loops.

Root causes and fixes:

- The sandbox and the tool both rejected an absolute path that lies inside
  the workspace ("must stay inside the workspace"). Models routinely echo
  the absolute path read_file showed them. Absolute paths now flow through
  the normal workspace-scope validation (inside → allowed, outside → still
  denied); unified-diff headers are rewritten to a/ b/ relative form before
  git apply, and platform symlink prefixes (/var → /private/var) are
  normalised the same way the other file tools do.
- The structured parser demanded a byte-exact "*** Begin Patch" /
  "*** End Patch"; grok-4.6 writes "*** Begin Patch ***". Decorated marker
  spellings are accepted, and a unified range after "@@" ("-12,4 +12,6",
  optionally with a heading) no longer becomes a literal context search.
- Parse errors now carry a one-line format reminder so a failed call is
  recoverable in one retry instead of a format-guessing loop.
- edit_file (exact search/replace with uniqueness guard, CRLF and fuzzy
  fallbacks) is advertised to the model again alongside apply_patch, and
  the apply_patch description shows the structured format.
- Writers (edit_file, write_file, apply_patch) recorded a file's line total
  with lineCount, which is one higher than read_file's count for a file
  ending in a newline. RecordSeenRange resets every observation when the
  total changes, so a partial read_file after a successful edit silently
  discarded whole-file knowledge and the next edit was refused with
  "content that has not been read exactly in this session". All writers
  now record the same total read_file reports (trackedLineTotal).
- read_file's padded "  N | " line prefix was ~19% of every read (about
  1.5k tokens per 800-line file) and was re-sent on every later call. It is
  now the compact "N→" prefix models already know; nothing parsed the old
  form.
- The editing-discipline prompt steers: edit_file for a targeted change,
  apply_patch for multi-hunk/multi-file changes, write_file only for new
  files or full rewrites; a successful edit result is confirmation, so the
  file is not re-read to verify it.

Tests: regression tests for decorated markers, writer/reader line-total
agreement (edit → partial read → edit far away still succeeds), range hunk headers,
absolute in-workspace paths (structured + unified), outside-workspace
rejection, header-line-only relativisation, and tool visibility; the
prompt and tool-budget tests updated to the new contract.

Benchmark (grok-4.6, 3 tasks x 3 runs, harness in demo/bench-zero-vs-grok):
Zero 0.8.0 617 s / 1,527k tokens -> with this change 346 s / 493k tokens;
Grok Build 407 s / 713k. Every run of every configuration solved its task.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(tools): keep CRLF line endings when relativizing patch headers

relativizeUnifiedPatchPaths split on a normalized "\n" and rejoined with
"\n", so any CRLF patch that needed a header rewrite lost its line endings
everywhere; git apply could then fail to match a CRLF file. Split on "\n"
only and carry each line's trailing "\r" through, including on rewritten
headers.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(sandbox): share the structured-patch classifier with apply_patch

Review follow-ups for #956:

- The tool accepted decorated structured markers ("***Begin Patch") that
  the sandbox's applyPatchPaths did not recognise, so for that spelling the
  boundary scanned the patch as a unified diff, extracted no targets and
  validated nothing. The marker classifier now lives in the sandbox package
  (StructuredPatchMarker / IsStructuredPatch) and the tool delegates to it,
  so both sides accept exactly the same set; tests cover inside, traversal
  and outside targets for the no-space spelling at the boundary.
- Document the containment model at the apply site: structured patches are
  applied through an opened os.Root (descriptor-relative, no-follow);
  unified diffs keep the pre-existing git-apply pathname flow, which the
  absolute-path rewrite does not widen (they become the same root-relative
  form relative paths always used).
- OpenAI addendum: the one-file-per-call rule names read_file, edit_file and
  write_file explicitly and allows a single multi-file apply_patch.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* test(sandbox): compare slash-normalised patch paths so the check passes on Windows

structuredPatchHeaderPaths normalises separators to "/", so the absolute
outside-workspace path in the boundary test must be compared in its slash
form; on Windows the backslash original never matched and the Smoke job
failed.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* test(sandbox): validate the parsed patch path at the scope boundary

For each accepted marker spelling, pass the exact path applyPatchRequestPaths
produced to scope.validate: an outside path must be denied and an inside
path accepted, so a separator-handling regression between the parser and the
scope cannot slip past the test.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(tools): apply unified diffs in-process through the workspace root

Review follow-up for #956. Unified diffs were validated by pathname and then
handed to git apply, which reopened the targets by name — a check-to-use
window a symlink swap could exploit — and the absolute-path rewrite added in
this PR normalised `diff --git` headers with strings.Fields, which splits a
git C-quoted path containing whitespace.

Both go away by not shelling out at all: parseUnifiedPatch translates a
unified diff into the same operations a structured patch produces, and both
formats are applied by applyPatchOperations through os.OpenRoot — every
stat, read, create and write is descriptor-relative and refuses to follow a
link out of the root. git is no longer required by apply_patch.

Unified-diff support: modifications, creations (--- /dev/null), deletions
(+++ /dev/null), multiple hunks per file, git "rename from/to" (a move,
hunks optional) and "copy from/to" (a new copy operation whose destination
inherits only the tracker credit the source had), "\ No newline at end of
file" on either side, a/ b/ prefixes, C-quoted paths, CRLF patches, and
git's diff/index/mode lines. Hunks are placed at their "@@ -a,b" range when
the expected lines match there and located by context otherwise (so a
stale range still applies and duplicate blocks are disambiguated by the
range). Binary patches are rejected.

Removed: the git apply flow, relativizeUnifiedPatchPaths,
recheckPatchWriteTargets, missingPatchTargets, completeCreatedPatchTargets
and recordCreatedPatchTargets. validatePatchPaths/changedFilesFromPatch stay
for mutation-target listing.

Tests: the existing unified-diff suite now runs through the in-process
applier unchanged; new coverage for quoted whitespace paths (inside applies,
outside is refused untouched), a workspace path swapped for a symlink that
escapes the root (both formats refused, outside file unmodified), create +
delete + no-newline markers, range hint vs context fallback, rename with a
hunk, and binary/empty rejection.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(tools): harden unified-diff parsing and cover the copy operation

Review follow-ups for #956 (in-process unified applier):

- A hunk that ends before its declared line counts is reported as
  malformed instead of silently absorbing the next file's ---/+++ header
  pair (which then failed later against the wrong file). Both shapes are
  caught: leftover counts at a header or at end of input, and a --- line
  directly followed by +++ inside a hunk. The element after a patch's final
  newline is no longer counted as an empty context line, so a truncated
  hunk cannot look complete.
- "\ No newline at end of file" may only follow a file's final hunk; a
  marker before another hunk is rejected rather than leaking into that
  hunk's result.
- Zero-context insertions (git diff -U0) are documented as
  position-trusting by design, matching git apply without fuzz; hunks with
  context are verified at the range before it is used.

Tests: copy from/to (source kept, destination created with the hunk
applied and tracked with the source's whole-file state; existing
destination refused untouched; copy onto itself refused), over-declared and
truncated hunk counts, and the no-newline marker before vs after the final
hunk.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(tools): require an @@ line before treating ---/+++ inside a hunk as a file boundary

A removed "-- x" line directly followed by an added "++ y" line is valid
hunk content that also matches the ---/+++ header shape; the boundary check
now additionally requires the following line to be a hunk header, and a
regression test keeps such a pair intact. End-of-input strictness for a hunk
cut short is documented as deliberate: applying a truncated hunk would make
a partial change (removals without their replacements).

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* refactor(tools): share one line-count implementation between reader and writer

sourceLineCount and trackedLineTotal computed the same count; keeping two
copies invites exactly the reader/writer drift this PR fixed. The reader now
delegates to trackedLineTotal, which carries the tracker contract.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(tools): verify a unified deletion's expected content before removing the file

A unified diff's "+++ /dev/null" hunks state the content being removed, but
the in-process applier dropped them and deleted unconditionally, so a stale
deletion removed a file whose current content the patch never described.
Unified deletions now keep their hunks, require at least one, and run them
through the same verifier as updates: the hunks must match the current file
and leave nothing behind, otherwise the deletion is refused and the file is
untouched. A structured "*** Delete File" keeps its unconditional meaning.

Hardening from the same review: every non-create change re-reads its source
through the opened root immediately before commit and refuses to proceed if
the bytes differ from what was planned.

Tests: matching deletion, stale deletion (file byte-for-byte unchanged),
partial deletion, header-only deletion, multi-hunk deletion with one stale
hunk (nothing removed) and all-good hunks, structured delete unchanged, and
a source changed between planning and commit for update and delete.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* test(tools): a move with a missing source is refused before publishing

The pre-commit recheck now refuses a move whose source vanished after
planning instead of publishing the destination and then failing; update the
test to the safer contract (error names the recheck, nothing is created).

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(tools): byte-exact unified deletions and git's header-only empty-file forms

Review follow-ups for #956:

- A unified deletion is verified byte for byte: the removed lines (with the
  removed side's "\ No newline at end of file" honoured) must equal the
  current file exactly, with only the file's own line-ending style
  normalised. None of the whitespace tolerance used to locate update hunks
  applies, and a whitespace-only remainder no longer passes; a deletion is
  not recoverable, so it fails closed.
- git describes an empty file's deletion or creation with header lines only
  ("diff --git", "deleted file mode"/"new file mode", "index") and no
  hunks. Both forms are recognised: the deletion is applied only when the
  current file is empty, the creation writes an empty file, and they compose
  with hunk-carrying files in the same patch.
- The pre-commit recheck test also covers the copy operation and asserts no
  destination is created on refusal.

Tests: whitespace/indentation/blank-line/space-only/missing-newline
mismatches all refused with the file unchanged, matching no-newline marker
and CRLF file accepted, header-only deletion of a non-empty file refused,
multi-file patch with header-only delete + create + a normal hunk.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* test(tools): remove the move source after planning via a deterministic hook

The refused-move test now creates source.txt, plans the move, and removes
the file through a test-only pre-commit hook (structuredPatchBeforeCommit)
so it exercises the post-planning disappearance path rather than a source
that was never there. Assertions unchanged: refused before publishing, no
destination created.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* test(tui): read_file fixtures use the compact N→ prefix

Three TUI fixtures still built fake read_file output in the legacy
"  N | text" shape; they pass either way but pinned an outdated contract.
They now mirror what appendReadFileRange emits, and the shape comment and
the expanded-card assertion are updated.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

* fix(tools): name the committed files when a patch is partially applied

Review follow-up for #956. When a later change in a multi-file patch fails
(for example the pre-commit recheck refuses a file that changed after
planning), the error now lists exactly which files were already committed
and states that the rest are unchanged, instead of a generic "partially
applied" warning. The model can then re-read only what actually changed.

Test: a three-file structured patch whose second file is altered after
planning fails with "already committed: first.txt", first.txt holds the
patched content, and the other two files are untouched.

Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012JqvAf7zsP1qYFVZayKXHq

---------

Co-authored-by: Claude Fable 5 <[email protected]>
2026-08-25 20:23:48 +08:00

1299 lines
41 KiB
Go

package tools
import (
"context"
"errors"
"io"
"os"
"path/filepath"
"strings"
"testing"
)
func mustReadTestFile(t *testing.T, path string) string {
t.Helper()
content, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
return string(content)
}
func TestCoreToolsExposeWriteAndPlanTools(t *testing.T) {
toolset := CoreToolsScoped(t.TempDir(), nil)
byName := make(map[string]Tool, len(toolset))
for _, tool := range toolset {
byName[tool.Name()] = tool
}
for _, name := range []string{"write_file", "edit_file", "apply_patch"} {
tool, ok := byName[name]
if !ok {
t.Fatalf("expected core tools to include %s", name)
}
if tool.Safety().SideEffect != SideEffectWrite {
t.Fatalf("%s side effect = %s, want write", name, tool.Safety().SideEffect)
}
if tool.Safety().Permission != PermissionPrompt {
t.Fatalf("%s permission = %s, want prompt", name, tool.Safety().Permission)
}
}
planTool, ok := byName["update_plan"]
if !ok {
t.Fatalf("expected core tools to include update_plan")
}
if planTool.Safety().Permission != PermissionAllow {
t.Fatalf("update_plan permission = %s, want allow", planTool.Safety().Permission)
}
}
func TestRegistryBlocksPromptToolsWithoutGrant(t *testing.T) {
root := t.TempDir()
target := filepath.Join(root, "blocked.txt")
registry := NewRegistry()
registry.Register(NewScopedWriteFileTool(root, nil))
result := registry.Run(context.Background(), "write_file", map[string]any{
"path": "blocked.txt",
"content": "nope",
})
if result.Status != StatusError {
t.Fatalf("expected error status, got %s", result.Status)
}
if !strings.Contains(result.Output, "Permission required for write_file") {
t.Fatalf("expected permission error, got %q", result.Output)
}
if _, err := os.Stat(target); !os.IsNotExist(err) {
t.Fatalf("expected file to remain absent, stat err=%v", err)
}
}
func TestRegistryRunsPromptToolsWithGrant(t *testing.T) {
root := t.TempDir()
registry := NewRegistry()
registry.Register(NewScopedWriteFileTool(root, nil))
result := registry.RunWithOptions(context.Background(), "write_file", map[string]any{
"path": "allowed.txt",
"content": "hello",
}, RunOptions{PermissionGranted: true})
if result.Status != StatusOK {
t.Fatalf("expected ok status, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(filepath.Join(root, "allowed.txt"))
if err != nil {
t.Fatal(err)
}
if string(content) != "hello" {
t.Fatalf("expected written content, got %q", string(content))
}
}
func TestWriteFileToolCreatesAndProtectsExistingFiles(t *testing.T) {
root := t.TempDir()
tool := NewScopedWriteFileTool(root, nil)
created := tool.Run(context.Background(), map[string]any{
"path": "nested/file.txt",
"content": "first",
})
if created.Status != StatusOK {
t.Fatalf("expected create ok, got %s: %s", created.Status, created.Output)
}
if !strings.Contains(created.Output, "Created nested/file.txt") {
t.Fatalf("unexpected create output: %q", created.Output)
}
refused := tool.Run(context.Background(), map[string]any{
"path": "nested/file.txt",
"content": "second",
})
if refused.Status != StatusError {
t.Fatalf("expected overwrite refusal, got %s", refused.Status)
}
content, err := os.ReadFile(filepath.Join(root, "nested", "file.txt"))
if err != nil {
t.Fatal(err)
}
if string(content) != "first" {
t.Fatalf("expected original content, got %q", string(content))
}
overwrote := tool.Run(context.Background(), map[string]any{
"path": "nested/file.txt",
"content": "second",
"overwrite": true,
})
if overwrote.Status != StatusOK {
t.Fatalf("expected overwrite ok, got %s: %s", overwrote.Status, overwrote.Output)
}
content, err = os.ReadFile(filepath.Join(root, "nested", "file.txt"))
if err != nil {
t.Fatal(err)
}
if string(content) != "second" {
t.Fatalf("expected overwritten content, got %q", string(content))
}
}
func TestWriteFileToolRecordsCreatedFileButNotOverwrite(t *testing.T) {
root := t.TempDir()
registry := NewRegistry()
registry.Register(NewScopedWriteFileTool(root, nil))
tracker := NewFileTracker()
created := registry.RunWithOptions(context.Background(), "write_file", map[string]any{
"path": "scratch.txt",
"content": "first",
}, RunOptions{PermissionGranted: true, FileTracker: tracker})
if created.Status != StatusOK {
t.Fatalf("expected create ok, got %s: %s", created.Status, created.Output)
}
absPath := filepath.Join(root, "scratch.txt")
absPath, err := filepath.EvalSymlinks(absPath)
if err != nil {
t.Fatalf("EvalSymlinks(%q): %v", absPath, err)
}
if got := tracker.CreatedFiles(); len(got) != 1 || got[0] != absPath {
t.Fatalf("CreatedFiles() = %v, want [%s]", got, absPath)
}
overwrote := registry.RunWithOptions(context.Background(), "write_file", map[string]any{
"path": "scratch.txt",
"content": "second",
"overwrite": true,
}, RunOptions{PermissionGranted: true, FileTracker: tracker})
if overwrote.Status != StatusOK {
t.Fatalf("expected overwrite ok, got %s: %s", overwrote.Status, overwrote.Output)
}
// Overwriting an existing file is not a "new" creation, so CreatedFiles
// must not gain a second (duplicate) entry for it.
if got := tracker.CreatedFiles(); len(got) != 1 || got[0] != absPath {
t.Fatalf("CreatedFiles() after overwrite = %v, want unchanged [%s]", got, absPath)
}
}
func TestApplyPatchRecordsCreatedFileButNotExistingEdits(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "existing.txt"), []byte("old\n"), 0o644); err != nil {
t.Fatal(err)
}
registry := NewRegistry()
registry.Register(NewScopedApplyPatchTool(root, nil))
tracker := NewFileTracker()
patch := strings.Join([]string{
"diff --git a/scratch.txt b/scratch.txt",
"new file mode 100644",
"index 0000000..e965047",
"--- /dev/null",
"+++ b/scratch.txt",
"@@ -0,0 +1 @@",
"+hello",
"diff --git a/existing.txt b/existing.txt",
"--- a/existing.txt",
"+++ b/existing.txt",
"@@ -1 +1 @@",
"-old",
"+new",
"",
}, "\n")
result := registry.RunWithOptions(context.Background(), "apply_patch", map[string]any{
"patch": patch,
}, RunOptions{PermissionGranted: true, FileTracker: tracker})
if result.Status != StatusOK {
if gitApplyUnavailable(result.Output) {
t.Skipf("git binary unavailable: %s", result.Output)
}
t.Fatalf("expected apply_patch ok, got %s: %s", result.Status, result.Output)
}
absPath := filepath.Join(root, "scratch.txt")
absPath, err := filepath.EvalSymlinks(absPath)
if err != nil {
t.Fatalf("EvalSymlinks(%q): %v", absPath, err)
}
if got := tracker.CreatedFiles(); len(got) != 1 || got[0] != absPath {
t.Fatalf("CreatedFiles() = %v, want [%s]", got, absPath)
}
}
func TestWriteFileSummaryReportsLineCount(t *testing.T) {
root := t.TempDir()
tool := NewScopedWriteFileTool(root, nil)
// Three lines, no trailing newline -> "3 lines" (not a byte count).
result := tool.Run(context.Background(), map[string]any{
"path": "multi.txt",
"content": "one\ntwo\nthree",
})
if result.Status != StatusOK {
t.Fatalf("expected ok, got %s: %s", result.Status, result.Output)
}
if !strings.Contains(result.Output, "(3 lines)") {
t.Fatalf("summary should report a line count, got %q", result.Output)
}
if strings.Contains(result.Output, "bytes") {
t.Errorf("summary should no longer report bytes: %q", result.Output)
}
}
func TestWriteFileToolAllowsEmptyContent(t *testing.T) {
root := t.TempDir()
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "empty.txt",
"content": "",
})
if result.Status != StatusOK {
t.Fatalf("expected ok status, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(filepath.Join(root, "empty.txt"))
if err != nil {
t.Fatal(err)
}
if string(content) != "" {
t.Fatalf("expected empty file, got %q", string(content))
}
}
func TestWriteFileToolReportsTypeErrorsForEmptyAllowedStrings(t *testing.T) {
result := NewScopedWriteFileTool(t.TempDir(), nil).Run(context.Background(), map[string]any{
"path": "bad.txt",
"content": 42,
})
if result.Status != StatusError {
t.Fatalf("expected error status, got %s", result.Status)
}
if !strings.Contains(result.Output, "content must be a string") {
t.Fatalf("expected string type error, got %q", result.Output)
}
}
func TestWriteFileToolRejectsOutsideWorkspace(t *testing.T) {
root := t.TempDir()
outside := filepath.Join(t.TempDir(), "outside.txt")
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": outside,
"content": "secret",
})
if result.Status != StatusError {
t.Fatalf("expected error status, got %s", result.Status)
}
if !strings.Contains(result.Output, "must stay inside the workspace") {
t.Fatalf("expected workspace error, got %q", result.Output)
}
if _, err := os.Stat(outside); !os.IsNotExist(err) {
t.Fatalf("expected outside file to remain absent, stat err=%v", err)
}
}
func TestWriteFileToolRejectsSymlinkParent(t *testing.T) {
root := t.TempDir()
realDirectory := filepath.Join(root, "real")
if err := os.MkdirAll(realDirectory, 0o755); err != nil {
t.Fatal(err)
}
if err := os.Symlink(realDirectory, filepath.Join(root, "link")); err != nil {
t.Skipf("symlink unavailable: %v", err)
}
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "link/escape.txt",
"content": "secret",
})
if result.Status != StatusError {
t.Fatalf("expected error status, got %s", result.Status)
}
if !strings.Contains(result.Output, "must not traverse symlink") {
t.Fatalf("expected symlink error, got %q", result.Output)
}
if _, err := os.Stat(filepath.Join(realDirectory, "escape.txt")); !os.IsNotExist(err) {
t.Fatalf("expected symlink target file to remain absent, stat err=%v", err)
}
}
func TestEditFileToolReplacesExactStrings(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "code.go")
writeTestFile(t, path, "const a = 1\nconst b = 2\n")
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "code.go",
"old_string": "const a = 1",
"new_string": "const a = 42",
})
if result.Status != StatusOK {
t.Fatalf("expected edit ok, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if string(content) != "const a = 42\nconst b = 2\n" {
t.Fatalf("unexpected edited content: %q", string(content))
}
}
func TestEditFileToolReplacesCRLF(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "code.go")
writeTestFile(t, path, "const a = 1\r\nconst b = 2\r\n")
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "code.go",
"old_string": "const a = 1\nconst b = 2",
"new_string": "const a = 42\nconst b = 24",
})
if result.Status != StatusOK {
t.Fatalf("expected edit ok, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if string(content) != "const a = 42\r\nconst b = 24\r\n" {
t.Fatalf("unexpected edited content: %q", string(content))
}
}
func TestEditFileToolEmitsUnifiedDiff(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "code.go"), "const a = 1\nconst b = 2\n")
res := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "code.go", "old_string": "const a = 1", "new_string": "const a = 42",
})
if res.Status != StatusOK {
t.Fatalf("edit failed: %s", res.Output)
}
// The model-facing Output stays the one-line summary; the red/green diff lives
// on the card-only Display.Preview, so it costs the model zero tokens.
if !strings.HasPrefix(res.Output, "Successfully edited") {
t.Fatalf("summary must be the Output: %q", res.Output)
}
if strings.Contains(res.Output, "@@") {
t.Fatalf("Output must NOT carry the diff (card-only preview): %q", res.Output)
}
for _, want := range []string{"@@", "-const a = 1", "+const a = 42"} {
if !strings.Contains(res.Display.Preview, want) {
t.Fatalf("edit preview missing diff marker %q: %q", want, res.Display.Preview)
}
}
}
func TestWriteFileToolEmitsAdditionsDiff(t *testing.T) {
root := t.TempDir()
res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "new.txt", "content": "line one\nline two\n",
})
if res.Status != StatusOK {
t.Fatalf("write failed: %s", res.Output)
}
if strings.Contains(res.Output, "@@") {
t.Fatalf("Output must stay summary-only (the diff is card-only): %q", res.Output)
}
for _, want := range []string{"@@", "+line one", "+line two"} {
if !strings.Contains(res.Display.Preview, want) {
t.Fatalf("new-file preview missing additions diff %q: %q", want, res.Display.Preview)
}
}
if strings.Contains(res.Display.Preview, "\n-line") {
t.Fatalf("a fresh-create diff must have no removed lines: %q", res.Display.Preview)
}
}
func TestWriteFileToolOverwriteEmitsRedGreenDiff(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "f.txt"), "old line\nkeep\n")
res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "f.txt", "content": "new line\nkeep\n", "overwrite": true,
})
if res.Status != StatusOK {
t.Fatalf("overwrite failed: %s", res.Output)
}
if strings.Contains(res.Output, "@@") {
t.Fatalf("Output must stay summary-only (the diff is card-only): %q", res.Output)
}
for _, want := range []string{"-old line", "+new line"} {
if !strings.Contains(res.Display.Preview, want) {
t.Fatalf("overwrite preview missing %q: %q", want, res.Display.Preview)
}
}
}
func TestEditFileToolAllowsDeletingRegions(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "notes.txt")
writeTestFile(t, path, "keep\nremove\nkeep\n")
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "notes.txt",
"old_string": "remove\n",
"new_string": "",
})
if result.Status != StatusOK {
t.Fatalf("expected edit ok, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if string(content) != "keep\nkeep\n" {
t.Fatalf("unexpected edited content: %q", string(content))
}
}
func TestEditFileToolRejectsMissingAndAmbiguousMatches(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "dup.txt")
writeTestFile(t, path, "x\nx\n")
tool := NewScopedEditFileTool(root, nil)
missing := tool.Run(context.Background(), map[string]any{
"path": "dup.txt",
"old_string": "missing",
"new_string": "y",
})
if missing.Status != StatusError || !strings.Contains(missing.Output, "Could not find") {
t.Fatalf("expected missing error, got %s: %s", missing.Status, missing.Output)
}
ambiguous := tool.Run(context.Background(), map[string]any{
"path": "dup.txt",
"old_string": "x",
"new_string": "y",
})
if ambiguous.Status != StatusError || !strings.Contains(ambiguous.Output, "matches 2 locations") {
t.Fatalf("expected ambiguity error, got %s: %s", ambiguous.Status, ambiguous.Output)
}
all := tool.Run(context.Background(), map[string]any{
"path": "dup.txt",
"old_string": "x",
"new_string": "y",
"replace_all": true,
})
if all.Status != StatusOK {
t.Fatalf("expected replace_all ok, got %s: %s", all.Status, all.Output)
}
content, err := os.ReadFile(path)
if err != nil {
t.Fatal(err)
}
if string(content) != "y\ny\n" {
t.Fatalf("expected all replacements, got %q", string(content))
}
}
func TestApplyPatchToolAppliesUnifiedDiff(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "hello.txt"), "hello\nold\n")
patch := strings.Join([]string{
"diff --git a/hello.txt b/hello.txt",
"--- a/hello.txt",
"+++ b/hello.txt",
"@@ -1,2 +1,2 @@",
" hello",
"-old",
"+new",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{
"patch": patch,
})
if result.Status != StatusOK {
t.Fatalf("expected patch ok, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(filepath.Join(root, "hello.txt"))
if err != nil {
t.Fatal(err)
}
if strings.ReplaceAll(string(content), "\r\n", "\n") != "hello\nnew\n" {
t.Fatalf("unexpected patched content: %q", string(content))
}
}
func TestApplyPatchToolAppliesStructuredPatch(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "hello.txt"), "hello\nold\n")
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: hello.txt",
"@@",
" hello",
"-old",
"+new",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch should apply, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(filepath.Join(root, "hello.txt"))
if err != nil {
t.Fatal(err)
}
if got := strings.ReplaceAll(string(content), "\r\n", "\n"); got != "hello\nnew\n" {
t.Fatalf("structured patch content = %q", got)
}
}
func TestApplyPatchToolAppliesStructuredAddAndMove(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "old.txt"), "old\n")
patch := strings.Join([]string{
"*** Begin Patch",
"*** Add File: nested/new.txt",
"+created",
"*** Update File: old.txt",
"*** Move to: moved.txt",
"@@",
"-old",
"+moved",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch should apply, got %s: %s", result.Status, result.Output)
}
if got, err := os.ReadFile(filepath.Join(root, "nested", "new.txt")); err != nil || string(got) != "created\n" {
t.Fatalf("added file = %q, err = %v", got, err)
}
if got, err := os.ReadFile(filepath.Join(root, "moved.txt")); err != nil || string(got) != "moved\n" {
t.Fatalf("moved file = %q, err = %v", got, err)
}
if _, err := os.Stat(filepath.Join(root, "old.txt")); !os.IsNotExist(err) {
t.Fatalf("old source should be removed, stat err = %v", err)
}
if got := result.ChangedFiles; strings.Join(got, ",") != "nested/new.txt,old.txt,moved.txt" {
t.Fatalf("ChangedFiles = %v", got)
}
}
func TestApplyPatchToolStructuredPatchMatchesWhitespaceTolerantly(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "hello.txt"), " hello \n")
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: hello.txt",
"@@",
"-hello",
"+goodbye",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch should tolerate surrounding whitespace, got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(filepath.Join(root, "hello.txt"))
if err != nil {
t.Fatal(err)
}
if got := string(content); got != "goodbye\n" {
t.Fatalf("structured patch content = %q", got)
}
}
func TestApplyPatchToolStructuredPatchPreservesWhitespaceTolerantContext(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "example.go")
original := "func f() {\n\tif cond {\n\t\tdoWork()\n\t\tlogIt()\n\t}\n}\n"
writeTestFile(t, path, original)
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: example.go",
"@@",
" if cond {",
" doWork()",
"-\t\tlogIt()",
"+\t\tlogIt(ctx)",
" }",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch failed: %s", result.Output)
}
want := "func f() {\n\tif cond {\n\t\tdoWork()\n\t\tlogIt(ctx)\n\t}\n}\n"
if got := mustReadTestFile(t, path); got != want {
t.Fatalf("structured patch rewrote unchanged context:\n got %q\nwant %q", got, want)
}
}
func TestApplyPatchToolStructuredPatchPreservesMixedContextLineEndings(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "example.txt")
original := "alpha\r\n\tcontext\r\n\told\nomega\r\n"
writeTestFile(t, path, original)
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: example.txt",
"@@",
" alpha",
" context",
"-\told",
"+\tnew",
" omega",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch failed: %s", result.Output)
}
want := "alpha\r\n\tcontext\r\n\tnew\nomega\r\n"
if got := mustReadTestFile(t, path); got != want {
t.Fatalf("structured patch rewrote context line endings:\n got %q\nwant %q", got, want)
}
}
func TestApplyPatchToolStructuredPatchInsertsAtContext(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "example.txt"), "anchor\nmiddle\nremove\n")
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: example.txt",
"@@ anchor",
"+inserted",
"@@ middle",
"-remove",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured insertion failed: %s", result.Output)
}
content, err := os.ReadFile(filepath.Join(root, "example.txt"))
if err != nil {
t.Fatal(err)
}
if got, want := string(content), "anchor\ninserted\nmiddle\n"; got != want {
t.Fatalf("structured insertion content = %q, want %q", got, want)
}
}
func TestApplyPatchToolStructuredPatchRejectsAmbiguousMatch(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "example.go")
original := "func a() {\n\treturn nil\n}\n\nfunc b() {\n\treturn nil\n}\n"
writeTestFile(t, path, original)
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: example.go",
"@@",
"-\treturn nil",
"+\treturn errNotFound",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusError || !strings.Contains(strings.ToLower(result.Output), "ambiguous") {
t.Fatalf("ambiguous structured patch should be refused, got %s: %s", result.Status, result.Output)
}
if got := mustReadTestFile(t, path); got != original {
t.Fatalf("ambiguous structured patch changed the file: %q", got)
}
}
func TestApplyPatchToolStructuredPatchPreservesCRLF(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "example.txt")
writeTestFile(t, path, "alpha\r\nbravo\r\ncharlie\r\n")
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: example.txt",
"@@",
"-bravo",
"+BRAVO",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch failed: %s", result.Output)
}
if got, want := mustReadTestFile(t, path), "alpha\r\nBRAVO\r\ncharlie\r\n"; got != want {
t.Fatalf("structured patch content = %q, want %q", got, want)
}
}
func TestApplyPatchToolStructuredPatchPreservesMissingFinalNewline(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "example.txt")
writeTestFile(t, path, "alpha\nbravo")
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: example.txt",
"@@",
"-alpha",
"+ALPHA",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch failed: %s", result.Output)
}
if got, want := mustReadTestFile(t, path), "ALPHA\nbravo"; got != want {
t.Fatalf("structured patch content = %q, want %q", got, want)
}
}
func TestApplyPatchToolStructuredPatchKeepsInsertedEmptyFileWithoutFinalNewline(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "empty.txt")
writeTestFile(t, path, "")
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: empty.txt",
"@@",
"+content",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("structured patch failed: %s", result.Output)
}
if got, want := mustReadTestFile(t, path), "content"; got != want {
t.Fatalf("structured patch content = %q, want %q", got, want)
}
}
func TestApplyPatchToolStructuredPatchRejectsOutOfOrderEndHunk(t *testing.T) {
root := t.TempDir()
path := filepath.Join(root, "example.txt")
original := "alpha\nbravo\ncharlie\ndelta\necho\n"
writeTestFile(t, path, original)
patch := strings.Join([]string{
"*** Begin Patch",
"*** Update File: example.txt",
"@@",
"-delta",
"+DELTA",
"@@",
"-delta",
"-echo",
"+tail",
"*** End of File",
"*** End Patch",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusError {
t.Fatalf("out-of-order structured patch should be refused, got %s: %s", result.Status, result.Output)
}
if got := mustReadTestFile(t, path); got != original {
t.Fatalf("out-of-order structured patch changed the file: %q", got)
}
}
func TestStructuredPatchAddDoesNotOverwriteRacedDestination(t *testing.T) {
root := t.TempDir()
targetPath := filepath.Join(root, "created.txt")
writeTestFile(t, targetPath, "other writer\n")
workspace, err := os.OpenRoot(root)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
change := structuredPatchChange{
kind: structuredPatchAdd,
to: structuredPatchTarget{absolute: targetPath, relative: "created.txt"},
after: "patch content\n", mode: 0o644,
}
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
if err == nil || !errors.Is(err, os.ErrExist) {
t.Fatalf("raced add destination = %v, want os.ErrExist", err)
}
if strings.Contains(err.Error(), "exclusive-copy fallback") {
t.Fatalf("existing destination should be reported directly: %v", err)
}
if got := mustReadTestFile(t, targetPath); got != "other writer\n" {
t.Fatalf("raced add overwrote another writer's file: %q", got)
}
}
func TestStructuredPatchFailedDeleteDoesNotRecreateMissingFile(t *testing.T) {
root := t.TempDir()
workspace, err := os.OpenRoot(root)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
missingPath := filepath.Join(root, "missing.txt")
change := structuredPatchChange{
kind: structuredPatchDelete,
from: structuredPatchTarget{absolute: missingPath, relative: "missing.txt"},
before: "removed by another writer\n", mode: 0o644,
}
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
if err == nil {
t.Fatal("delete of an already removed file should fail")
}
if _, statErr := os.Stat(missingPath); !os.IsNotExist(statErr) {
t.Fatalf("failed delete recreated the missing file: %v", statErr)
}
}
// A move whose source vanishes after planning is refused by the pre-commit
// recheck before the destination is published, so nothing is left half-done.
func TestStructuredPatchMoveWithMissingSourceIsRefusedBeforePublishing(t *testing.T) {
root := t.TempDir()
workspace, err := os.OpenRoot(root)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
sourcePath := filepath.Join(root, "source.txt")
destinationPath := filepath.Join(root, "destination.txt")
writeTestFile(t, sourcePath, "removed source\n")
change := structuredPatchChange{
kind: structuredPatchUpdate,
from: structuredPatchTarget{absolute: sourcePath, relative: "source.txt"},
to: structuredPatchTarget{absolute: destinationPath, relative: "destination.txt"},
before: "removed source\n", after: "moved content\n", mode: 0o644,
}
// The source exists at planning time and disappears just before commit.
removed := false
structuredPatchBeforeCommit = func(structuredPatchChange) {
if err := os.Remove(sourcePath); err != nil {
t.Fatal(err)
}
removed = true
}
defer func() { structuredPatchBeforeCommit = nil }()
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
if !removed {
t.Fatal("pre-commit hook did not run")
}
if err == nil || !strings.Contains(err.Error(), "before commit") || strings.Contains(err.Error(), "partially applied") {
t.Fatalf("move with a missing source = %v, want a pre-commit refusal with nothing applied", err)
}
if _, statErr := os.Stat(sourcePath); !os.IsNotExist(statErr) {
t.Fatalf("failed move recreated a missing source: %v", statErr)
}
if _, statErr := os.Stat(destinationPath); !os.IsNotExist(statErr) {
t.Fatalf("refused move must not publish the destination: %v", statErr)
}
}
func TestStructuredPatchCreateOnlyFallsBackWhenHardLinksUnsupported(t *testing.T) {
root := t.TempDir()
workspace, err := os.OpenRoot(root)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
sourcePath := filepath.Join(root, "source.tmp")
writeTestFile(t, sourcePath, "complete content\n")
if err := workspace.Chmod("source.tmp", 0o444); err != nil {
t.Fatal(err)
}
committed, err := publishStructuredPatchNoReplaceWith(
workspace,
"source.tmp",
"target.txt",
0o444,
func(string, string) error { return errors.New("hard links unsupported") },
)
if err != nil || !committed {
t.Fatalf("fallback publish = committed %v, error %v", committed, err)
}
if got := mustReadTestFile(t, filepath.Join(root, "target.txt")); got != "complete content\n" {
t.Fatalf("fallback target content = %q", got)
}
if _, statErr := os.Stat(sourcePath); !os.IsNotExist(statErr) {
t.Fatalf("read-only source was not cleaned up: %v", statErr)
}
}
func TestStructuredPatchCopyFailureReportsSurvivingPartialTarget(t *testing.T) {
root := t.TempDir()
workspace, err := os.OpenRoot(root)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
sourcePath := filepath.Join(root, "source.tmp")
writeTestFile(t, sourcePath, "complete content\n")
copyErr := errors.New("injected copy failure")
cleanupErr := errors.New("injected target cleanup failure")
committed, err := copyStructuredPatchNoReplaceWith(
workspace,
"source.tmp",
"target.txt",
0o644,
func(destination io.Writer, _ io.Reader) (int64, error) {
written, writeErr := io.WriteString(destination, "partial")
if writeErr != nil {
return int64(written), writeErr
}
return int64(written), copyErr
},
func(root *os.Root, name string) error {
if name == "target.txt" {
return cleanupErr
}
return removeStructuredPatchTemp(root, name)
},
)
if !committed || !errors.Is(err, copyErr) || !errors.Is(err, cleanupErr) {
t.Fatalf("copy failure = committed %v, error %v", committed, err)
}
targetPath := filepath.Join(root, "target.txt")
if got := mustReadTestFile(t, targetPath); got != "partial" {
t.Fatalf("partial target content = %q", got)
}
}
func TestStructuredPatchPartialFailureLeavesCompletedChangeAndClearsTrackedState(t *testing.T) {
root := t.TempDir()
trackedPath := filepath.Join(root, "tracked.txt")
writeTestFile(t, trackedPath, "original\n")
workspace, err := os.OpenRoot(root)
if err != nil {
t.Fatal(err)
}
defer workspace.Close()
trackedPath, err = filepath.EvalSymlinks(trackedPath)
if err != nil {
t.Fatal(err)
}
info, err := os.Stat(trackedPath)
if err != nil {
t.Fatal(err)
}
tracker := NewFileTracker()
tracker.Record(trackedPath, []byte("original\n"), info)
tracker.RecordSeenRange(trackedPath, 1, 1, 1)
trackedTarget := structuredPatchTarget{absolute: trackedPath, relative: "tracked.txt"}
blockedPath := filepath.Join(root, "blocked")
if err := os.Mkdir(blockedPath, 0o755); err != nil {
t.Fatal(err)
}
writeTestFile(t, filepath.Join(blockedPath, "sentinel"), "keep\n")
changes := []structuredPatchChange{
{
kind: structuredPatchUpdate, from: trackedTarget, to: trackedTarget,
before: "original\n", after: "updated\n", mode: 0o644,
},
{
kind: structuredPatchAdd,
to: structuredPatchTarget{absolute: blockedPath, relative: "blocked"},
after: "cannot replace a non-empty directory\n", mode: 0o644,
},
}
err = applyStructuredPatchChanges(workspace, changes, tracker)
if err == nil || !strings.Contains(err.Error(), "partially applied") {
t.Fatalf("second change = %v, want partial-application error", err)
}
if got := mustReadTestFile(t, trackedPath); got != "updated\n" {
t.Fatalf("completed change was unexpectedly altered: %q", got)
}
if _, tracked := tracker.Version(trackedPath); tracked || tracker.SeenWhole(trackedPath) {
t.Fatal("partial failure retained stale file-tracker state")
}
}
func TestApplyPatchToolRejectsMalformedOrEscapingStructuredPatchBeforeWriting(t *testing.T) {
root := t.TempDir()
tool := NewScopedApplyPatchTool(root, nil)
for name, patch := range map[string]string{
"empty update": strings.Join([]string{
"*** Begin Patch",
"*** Update File: target.txt",
"*** End Patch",
}, "\n"),
"escape": strings.Join([]string{
"*** Begin Patch",
"*** Add File: ../outside.txt",
"+nope",
"*** End Patch",
}, "\n"),
} {
t.Run(name, func(t *testing.T) {
result := tool.Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusError {
t.Fatalf("expected error, got %s: %s", result.Status, result.Output)
}
if strings.Contains(result.Output, "No valid patches") {
t.Fatalf("structured patch should fail before git apply, got %q", result.Output)
}
if name == "empty update" {
if _, err := os.Stat(filepath.Join(root, "target.txt")); !os.IsNotExist(err) {
t.Fatalf("empty update must not write target file, stat err = %v", err)
}
}
})
}
if _, err := os.Stat(filepath.Join(filepath.Dir(root), "outside.txt")); !os.IsNotExist(err) {
t.Fatalf("escaping patch must not write outside workspace, stat err = %v", err)
}
}
// A hunk-body line that removes content beginning with "-- " appears in the diff
// as "--- ..."; it must NOT be mistaken for a file header (which previously made
// apply_patch reject a valid patch as targeting an outside path).
func TestApplyPatchToolHandlesHunkBodyLookingLikeHeader(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "notes.md"), "keep\n-- /etc/old\n")
patch := strings.Join([]string{
"diff --git a/notes.md b/notes.md",
"--- a/notes.md",
"+++ b/notes.md",
"@@ -1,2 +1,1 @@",
" keep",
"--- /etc/old",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusOK {
t.Fatalf("expected patch ok (hunk body must not be parsed as a header), got %s: %s", result.Status, result.Output)
}
content, err := os.ReadFile(filepath.Join(root, "notes.md"))
if err != nil {
t.Fatal(err)
}
if strings.ReplaceAll(string(content), "\r\n", "\n") != "keep\n" {
t.Fatalf("unexpected patched content: %q", string(content))
}
}
func TestApplyPatchToolRejectsHunkCountInflationHidingEscapePath(t *testing.T) {
root := t.TempDir()
writeTestFile(t, filepath.Join(root, "safe.txt"), "old\n")
// A crafted section heading after the closing "@@" injects a "+9,999999"
// token. If parseHunkCounts scanned the whole line it would treat 999999 as
// the new-line count, stay stuck in hunk mode, and swallow the second file
// header below — hiding the ../escape.txt write from path validation.
patch := strings.Join([]string{
"diff --git a/safe.txt b/safe.txt",
"--- a/safe.txt",
"+++ b/safe.txt",
"@@ -1,1 +1,1 @@ +9,999999",
"-old",
"+new",
"--- a/../escape.txt",
"+++ b/../escape.txt",
"@@ -1,1 +1,1 @@",
"-secret",
"+pwned",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if result.Status != StatusError {
t.Fatalf("crafted hunk header must not hide the out-of-workspace path, got %s: %s", result.Status, result.Output)
}
if !strings.Contains(result.Output, "must stay inside the workspace") {
t.Fatalf("expected workspace-confinement rejection, got %q", result.Output)
}
}
func TestApplyPatchToolRejectsSymlinkPath(t *testing.T) {
root := t.TempDir()
realDirectory := filepath.Join(root, "real")
if err := os.MkdirAll(realDirectory, 0o755); err != nil {
t.Fatal(err)
}
if err := os.Symlink(realDirectory, filepath.Join(root, "link")); err != nil {
t.Skipf("symlink unavailable: %v", err)
}
patch := strings.Join([]string{
"diff --git a/link/new.txt b/link/new.txt",
"new file mode 100644",
"index 0000000..e965047",
"--- /dev/null",
"+++ b/link/new.txt",
"@@ -0,0 +1 @@",
"+hello",
"",
}, "\n")
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{
"patch": patch,
})
if result.Status != StatusError {
t.Fatalf("expected error status, got %s", result.Status)
}
if !strings.Contains(result.Output, "must not traverse symlink") {
t.Fatalf("expected symlink error, got %q", result.Output)
}
if _, err := os.Stat(filepath.Join(realDirectory, "new.txt")); !os.IsNotExist(err) {
t.Fatalf("expected symlink target file to remain absent, stat err=%v", err)
}
}
func TestApplyPatchToolRejectsOutsideWorkspace(t *testing.T) {
root := t.TempDir()
outside := t.TempDir()
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{
"cwd": outside,
"patch": strings.Join([]string{
"diff --git a/nope.txt b/nope.txt",
"--- a/nope.txt",
"+++ b/nope.txt",
"@@ -0,0 +1 @@",
"+nope",
"",
}, "\n"),
})
if result.Status != StatusError {
t.Fatalf("expected error status, got %s", result.Status)
}
if !strings.Contains(result.Output, "must stay inside the workspace") {
t.Fatalf("expected workspace error, got %q", result.Output)
}
}
// Finding 3: apply_patch with cwd != "." must report WORKSPACE-relative
// ChangedFiles (cwd-prefixed), not cwd-relative paths. Otherwise the session's
// rewind/diff layer keys off the wrong path.
func TestApplyPatchReportsWorkspaceRelativeChangedFilesUnderCwd(t *testing.T) {
root := t.TempDir()
subdir := filepath.Join(root, "sub", "dir")
if err := os.MkdirAll(subdir, 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(subdir, "a.txt"), []byte("one\n"), 0o644); err != nil {
t.Fatal(err)
}
patch := "--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-one\n+two\n"
res := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch, "cwd": "sub/dir"})
if res.Status != StatusOK {
if gitApplyUnavailable(res.Output) {
t.Skipf("git binary unavailable: %s", res.Output)
}
t.Fatalf("apply_patch with cwd failed (possible regression): %s", res.Output)
}
if len(res.ChangedFiles) != 1 || res.ChangedFiles[0] != "sub/dir/a.txt" {
t.Fatalf("ChangedFiles = %v, want [sub/dir/a.txt]", res.ChangedFiles)
}
}
func TestWriteFileReportsChangedFileAndDisplay(t *testing.T) {
root := t.TempDir()
res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{"path": "notes.txt", "content": "hello"})
if res.Status != StatusOK {
t.Fatalf("status=%s output=%s", res.Status, res.Output)
}
if len(res.ChangedFiles) != 1 || res.ChangedFiles[0] != "notes.txt" {
t.Fatalf("ChangedFiles = %v, want [notes.txt]", res.ChangedFiles)
}
if res.Display.Kind != "file" {
t.Errorf("Display.Kind = %q, want file", res.Display.Kind)
}
if res.Display.Summary == "" {
t.Error("expected a non-empty Display.Summary")
}
}
func TestEditFileReportsChangedFileAndDisplay(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "f.txt"), []byte("alpha beta"), 0o644); err != nil {
t.Fatal(err)
}
res := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{"path": "f.txt", "old_string": "alpha", "new_string": "gamma"})
if res.Status != StatusOK {
t.Fatalf("status=%s output=%s", res.Status, res.Output)
}
if len(res.ChangedFiles) != 1 || res.ChangedFiles[0] != "f.txt" {
t.Fatalf("ChangedFiles = %v, want [f.txt]", res.ChangedFiles)
}
if res.Display.Kind != "diff" {
t.Errorf("Display.Kind = %q, want diff", res.Display.Kind)
}
}
func TestApplyPatchReportsChangedFiles(t *testing.T) {
root := t.TempDir()
if err := os.WriteFile(filepath.Join(root, "a.txt"), []byte("one\n"), 0o644); err != nil {
t.Fatal(err)
}
patch := "--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-one\n+two\n"
res := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
if res.Status != StatusOK {
if gitApplyUnavailable(res.Output) {
t.Skipf("git binary unavailable: %s", res.Output)
}
t.Fatalf("apply_patch failed (possible regression): %s", res.Output)
}
found := false
for _, f := range res.ChangedFiles {
if f == "a.txt" {
found = true
}
}
if !found {
t.Fatalf("expected a.txt in ChangedFiles, got %v", res.ChangedFiles)
}
if res.Display.Kind != "diff" {
t.Errorf("Display.Kind = %q, want diff", res.Display.Kind)
}
}
func TestWriteFileAcceptsContentAlias(t *testing.T) {
root := t.TempDir()
// minimax-style: content under an alias key instead of "content".
res := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
"path": "shop.html",
"contents": "<html>hi</html>",
})
if res.Status != StatusOK {
t.Fatalf("alias content should write, got %s: %s", res.Status, res.Output)
}
got, _ := os.ReadFile(filepath.Join(root, "shop.html"))
if string(got) != "<html>hi</html>" {
t.Fatalf("file content = %q", got)
}
}
// gitApplyUnavailable reports whether an apply_patch failure is due to the git
// binary being absent (an environment condition worth skipping) rather than a
// real regression (which must fail the test). apply_patch shells out to
// `git apply`; a missing binary surfaces as exec's "executable file not found".
func gitApplyUnavailable(output string) bool {
return strings.Contains(output, "executable file not found")
}