mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
* Improve harness context efficiency
* Address harness efficiency review findings
* Make recaps idle and cancellable
Delay recap generation until the session has been inactive, orient it around the active goal and next task, cancel stale work on activity, and render the result ephemerally above the composer.
Also strengthen the formatter safety regression to exercise formatter-modified edit output.
* Track every replace-all edit span
Translate each replacement into its post-edit line range, including cumulative shifts from earlier replacements, so follow-up edits remain authorized for every model-supplied change.
* Make review regressions cross-platform
Verify replace-all tracking through the canonical edit tool path instead of a platform-sensitive temp path, and cover cancellation when recaps are disabled during generation.
* fix(tools): derive SeenWhole from the recorded ranges
SeenWhole was a flag set only by a SINGLE read covering the file, while
SeenRange judged coverage from the accumulated ranges. The two then
disagreed about identical reads: after reading lines 1-50 and 51-100 of a
100-line file, SeenRange(1,100) was true and SeenWhole was false.
That matters because write_file gates overwrites on SeenWhole. A file past
the read byte budget cannot be read whole in one call — the read is
truncated and RecordSeenRange is correctly skipped, since the model did not
see those lines — so no sequence of reads could ever set the flag, and the
overwrite was refused permanently with:
the intended change depends on content that has not been read exactly
in this session. Read the affected lines with read_file, then retry.
Advice that could not change the answer. edit_file was unaffected because it
gates on SeenRange, so the escape was to edit rather than rewrite, but a
model following the error text would loop.
SeenWhole is now computed from the ranges against the file's recorded line
count, so scoped reads that together cover a file make it writable and the
error text becomes actionable. The observation carries that line count, and
a read reporting a different one clears the accumulated ranges: they
describe a shape the file no longer has.
Coverage is judged by merging sorted ranges rather than walking every line
against every range, which the previous SeenRange did — same answer, and it
no longer scans line-by-line over a large file.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(tools): key the large-file test on the resolved path
read_file resolves the workspace root through EvalSymlinks before recording
anything, so the tracker is keyed on the resolved path. The test asserted
against the raw t.TempDir() path, which is the same string on Windows and a
different one on macOS, where the temp dir sits under /var — a symlink to
/private/var. The SeenWhole assertions would have looked up a key that never
existed: passing here, failing on macOS CI.
Also switches the line generator to fmt.Fprintf (staticcheck QF1012).
Both from CodeRabbit on #842.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(tools): actually switch the line generator to fmt.Fprintf
The previous commit's message claimed this was done. It was not: the edit
silently did not apply and I reported it fixed without re-reading the file,
so builder.WriteString(fmt.Sprintf(...)) survived at the head — caught by
@anandh8x, not by me.
Verified this time by running the linter rather than by reading the diff:
golangci-lint --enable-only unused,ineffassign,staticcheck ./internal/tools/
reports 0 issues.
* fix: keep exact-read workflows recoverable
* test: cover permission normalization wiring
* fix: commit file observations after output budgeting
* fix: defer file observations across retries
* test: normalize patch line endings
---------
Co-authored-by: Vasanthdev2004 <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>
243 lines
10 KiB
Go
243 lines
10 KiB
Go
package tools
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
"os"
|
|
"strings"
|
|
)
|
|
|
|
type editFileTool struct {
|
|
baseTool
|
|
workspaceRoot string
|
|
scope PathScope
|
|
}
|
|
|
|
func NewScopedEditFileTool(workspaceRoot string, scope PathScope) Tool {
|
|
return editFileTool{
|
|
baseTool: baseTool{
|
|
name: "edit_file",
|
|
description: "Replace an exact string in an existing file with uniqueness protection by default.",
|
|
parameters: Schema{
|
|
Type: "object",
|
|
Properties: map[string]PropertySchema{
|
|
"path": {Type: "string", Description: "Path of the file to edit."},
|
|
"old_string": {Type: "string", Description: "Exact string to replace. Must match byte-for-byte."},
|
|
"new_string": {Type: "string", Description: "Replacement string. May be empty."},
|
|
"replace_all": {Type: "boolean", Description: "Replace every occurrence instead of requiring uniqueness.", Default: false},
|
|
},
|
|
Required: []string{"path", "old_string", "new_string"},
|
|
AdditionalProperties: false,
|
|
},
|
|
safety: promptSafety(SideEffectWrite, "Edits files in place."),
|
|
capabilities: ToolCapabilities{Effect: EffectWorkspaceWrite, ThreadSafe: false, ResourceKeys: fileResourceKeys},
|
|
},
|
|
workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
|
|
scope: scope,
|
|
}
|
|
}
|
|
|
|
func (tool editFileTool) Run(ctx context.Context, args map[string]any) Result {
|
|
return tool.RunWithOptions(ctx, args, RunOptions{})
|
|
}
|
|
|
|
func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any, options RunOptions) Result {
|
|
requestedPath, err := aliasedStringArg(args, []string{"path", "file", "file_path", "filename"}, "", true, false)
|
|
if err != nil {
|
|
return errorResult("Error: Invalid arguments for edit_file: " + err.Error())
|
|
}
|
|
oldString, err := aliasedStringArg(args, []string{"old_string", "old", "search", "find", "old_str"}, "", true, false)
|
|
if err != nil {
|
|
return errorResult("Error: Invalid arguments for edit_file: " + err.Error())
|
|
}
|
|
newString, err := aliasedStringArg(args, []string{"new_string", "new", "replace", "replacement", "new_str"}, "", true, true)
|
|
if err != nil {
|
|
return errorResult("Error: Invalid arguments for edit_file: " + err.Error())
|
|
}
|
|
replaceAll, err := boolArg(args, "replace_all", false)
|
|
if err != nil {
|
|
return errorResult("Error: Invalid arguments for edit_file: " + err.Error())
|
|
}
|
|
|
|
absolutePath, relativePath, err := resolveScopedPath(tool.workspaceRoot, tool.scope, requestedPath)
|
|
if err != nil {
|
|
return errorResult("Error reading " + requestedPath + ": " + err.Error())
|
|
}
|
|
contentBytes, err := os.ReadFile(absolutePath)
|
|
if err != nil {
|
|
return errorResult("Error reading " + relativePath + ": " + err.Error())
|
|
}
|
|
// Refuse to edit a file that changed on disk outside Zero since it was last
|
|
// read: the model's old_string was formed against a stale view, so applying it
|
|
// now could silently corrupt the newer content.
|
|
if cerr := options.FileTracker.CheckConflict(absolutePath, contentBytes); cerr != nil {
|
|
return errorResult(fileConflictMessage(relativePath))
|
|
}
|
|
if options.FileTracker != nil {
|
|
if _, tracked := options.FileTracker.Version(absolutePath); !tracked {
|
|
return errorResult(fileUnseenMessage(relativePath))
|
|
}
|
|
}
|
|
content := string(contentBytes)
|
|
occurrences := strings.Count(content, oldString)
|
|
|
|
// CRLF fallback: read_file normalizes \r\n → \n before presenting content to
|
|
// the model, so the model's old_string will use \n line endings. When the raw
|
|
// file uses \r\n (common on Windows), the \n-based old_string won't match.
|
|
// Detect this and transparently normalize: if the direct match fails, translate
|
|
// old_string's line endings to \r\n and search again. If found, use the CRLF-translated
|
|
// old_string and new_string for the replacement to preserve the file's existing EOL style
|
|
// without rewriting EOLs in unrelated parts of the file.
|
|
if occurrences == 0 && strings.Contains(content, "\r\n") && !strings.Contains(oldString, "\r\n") {
|
|
crlfOldString := strings.ReplaceAll(oldString, "\n", "\r\n")
|
|
normalizedOccurrences := strings.Count(content, crlfOldString)
|
|
if normalizedOccurrences > 0 {
|
|
occurrences = normalizedOccurrences
|
|
oldString = crlfOldString
|
|
if !strings.Contains(newString, "\r\n") {
|
|
newString = strings.ReplaceAll(newString, "\n", "\r\n")
|
|
}
|
|
}
|
|
}
|
|
|
|
// Fuzzy fallback: when the exact string (and its CRLF translation) is absent,
|
|
// run a cascade of tolerant matchers (trimmed lines, block anchors, collapsed
|
|
// whitespace, indentation drift, escape normalization) to locate the span the
|
|
// model intended. Only a span that occurs literally in the file is accepted,
|
|
// so the replacement applied below is still exact.
|
|
if occurrences == 0 {
|
|
findOld, findNew := oldString, newString
|
|
if strings.Contains(content, "\r\n") && !strings.Contains(findOld, "\r\n") {
|
|
findOld = strings.ReplaceAll(findOld, "\n", "\r\n")
|
|
findNew = strings.ReplaceAll(findNew, "\n", "\r\n")
|
|
}
|
|
search, ferr := fuzzyEditMatch(content, findOld, replaceAll)
|
|
switch {
|
|
case ferr == nil:
|
|
oldString = search
|
|
// The model's new_string was written at old_string's (mismatched)
|
|
// indentation; re-shape it to the span actually being replaced so a
|
|
// tolerant match never strips indentation or a trailing CR.
|
|
newString = adaptReplacementToSpan(search, findOld, findNew)
|
|
occurrences = strings.Count(content, search)
|
|
case errors.Is(ferr, errEditFuzzyAmbiguous):
|
|
return errorResult("Error: old_string matches multiple locations in " + relativePath + " even after fuzzy matching. Provide more surrounding context to make the match unique, or pass replace_all: true.")
|
|
case errors.Is(ferr, errEditFuzzyNotFound):
|
|
// Fall through to the exact-match error below.
|
|
default:
|
|
return errorResult("Error editing " + relativePath + ": " + ferr.Error())
|
|
}
|
|
}
|
|
|
|
if occurrences == 0 {
|
|
return errorResult("Error: Could not find the exact string to replace in " + relativePath + ". The old_string must match the file byte-for-byte.")
|
|
}
|
|
if !replaceAll && occurrences > 1 {
|
|
return errorResult(fmt.Sprintf("Error: old_string matches %d locations in %s. Either make old_string more specific, or pass replace_all: true to replace every occurrence.", occurrences, relativePath))
|
|
}
|
|
if options.FileTracker != nil && !allOccurrencesSeen(options.FileTracker, absolutePath, content, oldString, replaceAll) {
|
|
return errorResult(fileUnseenMessage(relativePath))
|
|
}
|
|
|
|
previouslySeenWhole := options.FileTracker.SeenWhole(absolutePath)
|
|
updated := strings.Replace(content, oldString, newString, 1)
|
|
replacedCount := 1
|
|
if replaceAll {
|
|
updated = strings.ReplaceAll(content, oldString, newString)
|
|
replacedCount = occurrences
|
|
}
|
|
if updated == content {
|
|
return okResult("No changes: new_string is identical to old_string.")
|
|
}
|
|
editedSpans := replacementByteSpans(content, oldString, newString, replaceAll)
|
|
if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, requestedPath); err != nil {
|
|
return errorResult("Error writing " + relativePath + ": " + err.Error())
|
|
}
|
|
if err := os.WriteFile(absolutePath, []byte(updated), 0o644); err != nil {
|
|
return errorResult("Error writing " + relativePath + ": " + err.Error())
|
|
}
|
|
modelKnownContent := updated
|
|
// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
|
|
// FileTracker re-baseline: recording pre-format content would make the very
|
|
// next edit look like an external modification and trip the conflict guard.
|
|
updated = maybeFormatWrittenFile(ctx, absolutePath, updated)
|
|
// Re-baseline to the content we just wrote so subsequent edits in this session
|
|
// compare against the current on-disk state, not the pre-edit version.
|
|
newInfo, _ := os.Stat(absolutePath)
|
|
options.FileTracker.Record(absolutePath, []byte(updated), newInfo)
|
|
if updated == modelKnownContent {
|
|
if previouslySeenWhole {
|
|
options.FileTracker.RecordSeenRange(absolutePath, 1, lineCount(updated), lineCount(updated))
|
|
} else {
|
|
for _, span := range editedSpans {
|
|
options.FileTracker.RecordSeenBytes(absolutePath, span.start, span.end, len(updated))
|
|
}
|
|
}
|
|
}
|
|
|
|
suffix := ""
|
|
if replacedCount != 1 {
|
|
suffix = "s"
|
|
}
|
|
summary := fmt.Sprintf("Successfully edited %s (replaced %d occurrence%s).", relativePath, replacedCount, suffix)
|
|
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
|
|
result := okResult(summary)
|
|
result.ChangedFiles = []string{relativePath}
|
|
// Card-only preview (Display.Preview): the model's Output stays the one-line
|
|
// summary, so the red/green diff costs zero model tokens.
|
|
result.Display = Display{Summary: fmt.Sprintf("Edited %s", relativePath), Kind: "diff", Preview: boundedUnifiedDiff(relativePath, content, updated)}
|
|
return result
|
|
}
|
|
|
|
// replacementByteSpans returns every half-open post-edit byte range whose exact
|
|
// contents came from newString. Offsets are translated from the original with a
|
|
// cumulative delta so replace_all remains correct after earlier replacements.
|
|
func replacementByteSpans(content, oldString, newString string, replaceAll bool) []lineRange {
|
|
spans := make([]lineRange, 0, 1)
|
|
offset := 0
|
|
delta := 0
|
|
for {
|
|
index := strings.Index(content[offset:], oldString)
|
|
if index < 0 {
|
|
return spans
|
|
}
|
|
index += offset
|
|
start := index + delta
|
|
spans = append(spans, lineRange{start: start, end: start + len(newString)})
|
|
if !replaceAll {
|
|
return spans
|
|
}
|
|
offset = index + len(oldString)
|
|
delta += len(newString) - len(oldString)
|
|
}
|
|
}
|
|
|
|
func allOccurrencesSeen(tracker *FileTracker, path, content, oldString string, replaceAll bool) bool {
|
|
offset := 0
|
|
for {
|
|
index := strings.Index(content[offset:], oldString)
|
|
if index < 0 {
|
|
return true
|
|
}
|
|
index += offset
|
|
start, end := lineSpanForOffset(content, index, len(oldString))
|
|
if !tracker.SeenBytes(path, index, index+len(oldString)) && !tracker.SeenRange(path, start, end) {
|
|
return false
|
|
}
|
|
if !replaceAll {
|
|
return true
|
|
}
|
|
offset = index + len(oldString)
|
|
}
|
|
}
|
|
|
|
func lineSpanForOffset(content string, offset, length int) (int, int) {
|
|
if offset < 0 {
|
|
return 1, 1
|
|
}
|
|
start := strings.Count(content[:offset], "\n") + 1
|
|
end := start + strings.Count(content[offset:min(len(content), offset+length)], "\n")
|
|
return start, end
|
|
}
|