mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 09:49:34 -10:00
Compare commits
15
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
6ca569d09c | ||
|
|
9d324c487e | ||
|
|
abbed6f791 | ||
|
|
fa68f834d5 | ||
|
|
e54e5af6d7 | ||
|
|
2b08339aee | ||
|
|
64df522343 | ||
|
|
7cb13ccc22 | ||
|
|
73c2d59afb | ||
|
|
9e2dad38e2 | ||
|
|
e894db9ab4 | ||
|
|
45a3b59a8d | ||
|
|
fd88bde58d | ||
|
|
a9897f45e8 | ||
|
|
20a22cadac |
No files matched your search
@@ -3,6 +3,7 @@ package acp
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/url"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"unicode"
|
||||
"unicode/utf8"
|
||||
@@ -238,22 +239,59 @@ func toolCallResult(result agent.ToolResult) ToolCallUpdate {
|
||||
}
|
||||
|
||||
func toolResultContent(result agent.ToolResult) []ToolCallContent {
|
||||
content := make([]ToolCallContent, 0, 1+len(result.FileDiffs))
|
||||
text := strings.TrimRight(result.Output, "\n")
|
||||
if text == "" {
|
||||
text = result.Display.Summary
|
||||
}
|
||||
if text == "" {
|
||||
return nil
|
||||
return appendToolResultDiffs(content, result.FileDiffs)
|
||||
}
|
||||
return []ToolCallContent{ToolContent(TextBlock(text))}
|
||||
content = append(content, ToolContent(TextBlock(text)))
|
||||
return appendToolResultDiffs(content, result.FileDiffs)
|
||||
}
|
||||
|
||||
func appendToolResultDiffs(content []ToolCallContent, diffs []tools.FileDiff) []ToolCallContent {
|
||||
for _, diff := range diffs {
|
||||
if !filepath.IsAbs(diff.Path) || (!diff.OldExists && !diff.NewExists) {
|
||||
continue
|
||||
}
|
||||
var oldText *string
|
||||
if diff.OldExists {
|
||||
old := diff.OldText
|
||||
oldText = &old
|
||||
}
|
||||
var newText *string
|
||||
if diff.NewExists {
|
||||
updated := diff.NewText
|
||||
newText = &updated
|
||||
}
|
||||
content = append(content, ToolCallContent{Type: "diff", Path: diff.Path, OldText: oldText, NewText: newText})
|
||||
}
|
||||
return content
|
||||
}
|
||||
|
||||
func toolResultLocations(result agent.ToolResult) []ToolCallLocation {
|
||||
locs := make([]ToolCallLocation, 0, len(result.ChangedFiles))
|
||||
for _, f := range result.ChangedFiles {
|
||||
if strings.TrimSpace(f) == "" {
|
||||
locs := make([]ToolCallLocation, 0, len(result.FileDiffs)+len(result.ChangedFiles))
|
||||
seen := make(map[string]bool, len(result.FileDiffs)+len(result.ChangedFiles))
|
||||
for _, diff := range result.FileDiffs {
|
||||
path := diff.Path
|
||||
if path == "" || seen[path] {
|
||||
continue
|
||||
}
|
||||
seen[path] = true
|
||||
locs = append(locs, ToolCallLocation{Path: path})
|
||||
}
|
||||
// FileDiff.Path is canonical absolute path data while ChangedFiles is
|
||||
// normally workspace-relative. Without the trusted workspace root these
|
||||
// coordinate systems cannot be correlated safely: a suffix match would let
|
||||
// /workspace/sub/a.go consume the fallback for a distinct root a.go. The
|
||||
// shared seen set deduplicates only identities already exactly comparable.
|
||||
for _, f := range result.ChangedFiles {
|
||||
if f == "" || seen[f] {
|
||||
continue
|
||||
}
|
||||
seen[f] = true
|
||||
locs = append(locs, ToolCallLocation{Path: f})
|
||||
}
|
||||
return locs
|
||||
|
||||
@@ -2,6 +2,7 @@ package acp
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"unicode"
|
||||
@@ -300,21 +301,26 @@ func TestToolCallStart(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestToolCallResult(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "a.go")
|
||||
ok := toolCallResult(agent.ToolResult{
|
||||
ToolCallID: "tc1",
|
||||
Name: "edit_file",
|
||||
Status: tools.StatusOK,
|
||||
Output: "applied\n",
|
||||
ChangedFiles: []string{"a.go", ""},
|
||||
FileDiffs: []tools.FileDiff{{Path: path, OldExists: true, NewExists: true, OldText: "before\n", NewText: "after\n"}},
|
||||
})
|
||||
if ok.SessionUpdate != UpdateToolCallUpdate || ok.Status != ToolStatusCompleted {
|
||||
t.Fatalf("unexpected ok result: %+v", ok)
|
||||
}
|
||||
if len(ok.Content) != 1 || ok.Content[0].Type != "content" || ok.Content[0].Content.Text != "applied" {
|
||||
if len(ok.Content) != 2 || ok.Content[0].Type != "content" || ok.Content[0].Content.Text != "applied" {
|
||||
t.Fatalf("unexpected content: %+v", ok.Content)
|
||||
}
|
||||
if len(ok.Locations) != 1 || ok.Locations[0].Path != "a.go" {
|
||||
t.Fatalf("blank changed files should be dropped, got %+v", ok.Locations)
|
||||
if diff := ok.Content[1]; diff.Type != "diff" || diff.Path != path || diff.OldText == nil || *diff.OldText != "before\n" || diff.NewText == nil || *diff.NewText != "after\n" {
|
||||
t.Fatalf("unexpected diff content: %+v", diff)
|
||||
}
|
||||
if len(ok.Locations) != 2 || ok.Locations[0].Path != path || ok.Locations[1].Path != "a.go" {
|
||||
t.Fatalf("unproven absolute/relative aliases must both remain visible, got %+v", ok.Locations)
|
||||
}
|
||||
|
||||
failed := toolCallResult(agent.ToolResult{ToolCallID: "tc2", Status: tools.StatusError, Output: "boom"})
|
||||
@@ -323,6 +329,190 @@ func TestToolCallResult(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolCallDiffJSONDistinguishesEmptyFilesAndDeletion(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "empty.txt")
|
||||
content := appendToolResultDiffs(nil, []tools.FileDiff{
|
||||
{Path: path, OldExists: false, NewExists: true, NewText: ""},
|
||||
{Path: path, OldExists: true, NewExists: true, OldText: "before", NewText: ""},
|
||||
{Path: path, OldExists: true, NewExists: false, OldText: "before"},
|
||||
})
|
||||
if len(content) != 3 {
|
||||
t.Fatalf("diff content = %#v", content)
|
||||
}
|
||||
for index, diff := range content {
|
||||
encoded, err := json.Marshal(diff)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var wire map[string]any
|
||||
if err := json.Unmarshal(encoded, &wire); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if wire["path"] != path {
|
||||
t.Fatalf("wire diff %d = %s", index, encoded)
|
||||
}
|
||||
switch index {
|
||||
case 0:
|
||||
oldText, present := wire["oldText"]
|
||||
if !present || oldText != nil || wire["newText"] != "" {
|
||||
t.Fatalf("create diff = %s, want null oldText and empty newText", encoded)
|
||||
}
|
||||
case 1:
|
||||
if wire["oldText"] != "before" || wire["newText"] != "" {
|
||||
t.Fatalf("empty replacement diff = %s", encoded)
|
||||
}
|
||||
case 2:
|
||||
newText, present := wire["newText"]
|
||||
if wire["oldText"] != "before" || !present || newText != nil {
|
||||
t.Fatalf("deletion diff = %s, want oldText and null newText", encoded)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolResultLocationsPreserveDistinctPathIdentities(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
rootPath := filepath.Join(root, "a.go")
|
||||
nestedPath := filepath.Join(root, "sub", "a.go")
|
||||
diff := func(path string) tools.FileDiff {
|
||||
return tools.FileDiff{Path: path, OldExists: true, NewExists: true, OldText: "before", NewText: "after"}
|
||||
}
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
diffs []tools.FileDiff
|
||||
want []string
|
||||
}{
|
||||
{name: "both rich", diffs: []tools.FileDiff{diff(rootPath), diff(nestedPath)}, want: []string{rootPath, nestedPath, "a.go", filepath.Join("sub", "a.go")}},
|
||||
{name: "root rich", diffs: []tools.FileDiff{diff(rootPath)}, want: []string{rootPath, "a.go", filepath.Join("sub", "a.go")}},
|
||||
{name: "nested rich", diffs: []tools.FileDiff{diff(nestedPath)}, want: []string{nestedPath, "a.go", filepath.Join("sub", "a.go")}},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
locations := toolResultLocations(agent.ToolResult{
|
||||
ChangedFiles: []string{"a.go", filepath.Join("sub", "a.go")},
|
||||
FileDiffs: tc.diffs,
|
||||
})
|
||||
if len(locations) != len(tc.want) {
|
||||
t.Fatalf("locations = %#v, want %#v", locations, tc.want)
|
||||
}
|
||||
for index := range tc.want {
|
||||
if locations[index].Path != tc.want[index] {
|
||||
t.Fatalf("locations = %#v, want %#v", locations, tc.want)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolCallResultPreservesWhitespaceInFilePaths(t *testing.T) {
|
||||
relativePath := " report.txt "
|
||||
absolutePath := filepath.Join(t.TempDir(), relativePath)
|
||||
update := toolCallResult(agent.ToolResult{
|
||||
ChangedFiles: []string{relativePath},
|
||||
FileDiffs: []tools.FileDiff{{
|
||||
Path: absolutePath, OldExists: true, NewExists: true, OldText: "before", NewText: "after",
|
||||
}},
|
||||
})
|
||||
if len(update.Content) != 1 || update.Content[0].Path != absolutePath {
|
||||
t.Fatalf("diff content path = %#v, want %q", update.Content, absolutePath)
|
||||
}
|
||||
if len(update.Locations) != 2 || update.Locations[0].Path != absolutePath || update.Locations[1].Path != relativePath {
|
||||
t.Fatalf("locations = %#v, want exact paths %q and %q", update.Locations, absolutePath, relativePath)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolResultLocationsDeduplicateOnlyExactPaths(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "a.go")
|
||||
locations := toolResultLocations(agent.ToolResult{
|
||||
ChangedFiles: []string{path, path},
|
||||
FileDiffs: []tools.FileDiff{{Path: path, OldExists: true, NewExists: true, OldText: "before", NewText: "after"}},
|
||||
})
|
||||
if len(locations) != 1 || locations[0].Path != path {
|
||||
t.Fatalf("exact duplicate locations = %#v", locations)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeletedFileEmitsDiffAndKeepsLocations(t *testing.T) {
|
||||
relativePath := "deleted.go"
|
||||
absolutePath := filepath.Join(t.TempDir(), relativePath)
|
||||
update := toolCallResult(agent.ToolResult{
|
||||
ChangedFiles: []string{relativePath},
|
||||
FileDiffs: []tools.FileDiff{{
|
||||
Path: absolutePath, OldExists: true, NewExists: false, OldText: "before",
|
||||
}},
|
||||
})
|
||||
if len(update.Content) != 1 || update.Content[0].OldText == nil || *update.Content[0].OldText != "before" || update.Content[0].NewText != nil {
|
||||
t.Fatalf("deleted file diff = %#v, want oldText with null newText", update.Content)
|
||||
}
|
||||
if len(update.Locations) != 2 || update.Locations[0].Path != absolutePath || update.Locations[1].Path != relativePath {
|
||||
t.Fatalf("deleted file locations = %#v", update.Locations)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolCallResultEmitsOnlyRedactedFileDiffs(t *testing.T) {
|
||||
secret := "sk-proj-abcdefghijklmnopqrstuvwxyz"
|
||||
path := filepath.Join(t.TempDir(), "secret.txt")
|
||||
scrubbed := tools.ScrubResultSecrets(tools.Result{FileDiffs: []tools.FileDiff{{
|
||||
Path: path, OldExists: true, NewExists: true, OldText: "token=" + secret, NewText: "safe",
|
||||
}}})
|
||||
update := toolCallResult(agent.ToolResult{ToolCallID: "call", Status: tools.StatusError, FileDiffs: scrubbed.FileDiffs})
|
||||
if len(update.Content) != 1 || update.Content[0].OldText == nil || strings.Contains(*update.Content[0].OldText, secret) {
|
||||
t.Fatalf("ACP content leaked unredacted diff: %#v", update.Content)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolCallResultOmitsSemanticallyUnchangedRedactedDiff(t *testing.T) {
|
||||
oldSecret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
||||
newSecret := "ghp_9876543210ZYXWVUTSRQPONMLKJIHGFEDCBA"
|
||||
scrubbed := tools.ScrubResultSecrets(tools.Result{
|
||||
ChangedFiles: []string{"credentials.txt"},
|
||||
FileDiffs: []tools.FileDiff{{
|
||||
Path: filepath.Join(t.TempDir(), "credentials.txt"), OldExists: true, NewExists: true,
|
||||
OldText: "token=" + oldSecret, NewText: "token=" + newSecret,
|
||||
}},
|
||||
})
|
||||
update := toolCallResult(agent.ToolResult{
|
||||
ToolCallID: "call", Status: tools.StatusOK,
|
||||
ChangedFiles: scrubbed.ChangedFiles, FileDiffs: scrubbed.FileDiffs,
|
||||
})
|
||||
if len(update.Content) != 0 {
|
||||
t.Fatalf("ACP emitted semantically unchanged redacted diff: %#v", update.Content)
|
||||
}
|
||||
if len(update.Locations) != 1 || update.Locations[0].Path != "credentials.txt" {
|
||||
t.Fatalf("ACP path fallback = %#v", update.Locations)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolCallResultOmitsDefaultIgnorableSplitSecretsOnEitherSide(t *testing.T) {
|
||||
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
|
||||
for name, separator := range map[string]string{
|
||||
"combining grapheme joiner": "\u034f",
|
||||
"variation selector": "\ufe0f",
|
||||
} {
|
||||
for _, side := range []string{"old", "new"} {
|
||||
t.Run(name+" "+side, func(t *testing.T) {
|
||||
obfuscated := secret[:20] + separator + secret[20:]
|
||||
diff := tools.FileDiff{
|
||||
Path: filepath.Join(t.TempDir(), "secret.txt"), OldExists: true, NewExists: true,
|
||||
OldText: "safe old", NewText: "safe new",
|
||||
}
|
||||
if side == "old" {
|
||||
diff.OldText = obfuscated
|
||||
} else {
|
||||
diff.NewText = obfuscated
|
||||
}
|
||||
scrubbed := tools.ScrubResultSecrets(tools.Result{FileDiffs: []tools.FileDiff{diff}})
|
||||
if !scrubbed.Redacted || len(scrubbed.FileDiffs) != 0 {
|
||||
t.Fatalf("registry boundary retained an obfuscated secret: %#v", scrubbed)
|
||||
}
|
||||
update := toolCallResult(agent.ToolResult{ToolCallID: "call", Status: tools.StatusOK, FileDiffs: scrubbed.FileDiffs})
|
||||
if len(update.Content) != 0 {
|
||||
t.Fatalf("ACP content retained an obfuscated secret: %#v", update.Content)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlanUpdateAndStatus(t *testing.T) {
|
||||
upd := planUpdate([]tools.PlanItem{
|
||||
{Content: "step a", Status: "completed"},
|
||||
|
||||
+24
-3
@@ -235,9 +235,30 @@ type ToolCallContent struct {
|
||||
// type == "content"
|
||||
Content *ContentBlock `json:"content,omitempty"`
|
||||
// type == "diff"
|
||||
Path string `json:"path,omitempty"`
|
||||
OldText string `json:"oldText,omitempty"`
|
||||
NewText string `json:"newText,omitempty"`
|
||||
Path string `json:"path,omitempty"`
|
||||
OldText *string `json:"oldText,omitempty"`
|
||||
NewText *string `json:"newText,omitempty"`
|
||||
}
|
||||
|
||||
// MarshalJSON preserves ACP's discriminated content union. A diff always has
|
||||
// path, while nullable oldText/newText distinguish creation, deletion, and an
|
||||
// existing file replaced with empty content. Other content variants omit all
|
||||
// diff fields rather than serializing irrelevant nulls.
|
||||
func (content ToolCallContent) MarshalJSON() ([]byte, error) {
|
||||
if content.Type == "diff" {
|
||||
return json.Marshal(struct {
|
||||
Type string `json:"type"`
|
||||
Path string `json:"path"`
|
||||
OldText *string `json:"oldText"`
|
||||
NewText *string `json:"newText"`
|
||||
}{
|
||||
Type: content.Type, Path: content.Path, OldText: content.OldText, NewText: content.NewText,
|
||||
})
|
||||
}
|
||||
return json.Marshal(struct {
|
||||
Type string `json:"type"`
|
||||
Content *ContentBlock `json:"content,omitempty"`
|
||||
}{Type: content.Type, Content: content.Content})
|
||||
}
|
||||
|
||||
func ToolContent(block ContentBlock) ToolCallContent {
|
||||
|
||||
@@ -1527,6 +1527,7 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
|
||||
Images: result.Images,
|
||||
Redacted: result.Redacted,
|
||||
ChangedFiles: result.ChangedFiles,
|
||||
FileDiffs: result.FileDiffs,
|
||||
ChangeSummaries: result.ChangeSummaries,
|
||||
Display: result.HumanDisplay(),
|
||||
Outcome: result.Outcome,
|
||||
@@ -1833,6 +1834,10 @@ func runToolForUnsandboxedRetry(ctx context.Context, registry *tools.Registry, n
|
||||
}
|
||||
|
||||
func toolResultFromPrePermissionReject(call ToolCall, result tools.Result) ToolResult {
|
||||
// PrePermissionRejecter runs before Registry.RunWithOptions, so it must
|
||||
// explicitly cross the same transcript/redaction boundary before its result
|
||||
// can be forwarded through ACP.
|
||||
result = tools.ScrubResultSecrets(result)
|
||||
output, outputRedacted := scrubInterceptedOutput(result.Output)
|
||||
display := result.Display
|
||||
summary, summaryRedacted := scrubInterceptedOutput(display.Summary)
|
||||
@@ -1860,6 +1865,7 @@ func toolResultFromPrePermissionReject(call ToolCall, result tools.Result) ToolR
|
||||
Meta: meta,
|
||||
Redacted: result.Redacted || outputRedacted || summaryRedacted || metaRedacted,
|
||||
ChangedFiles: result.ChangedFiles,
|
||||
FileDiffs: result.FileDiffs,
|
||||
ChangeSummaries: result.ChangeSummaries,
|
||||
Display: display,
|
||||
LoadedTools: loadedToolsFromResult(meta),
|
||||
@@ -2153,6 +2159,7 @@ func askUserFallbackResult(ctx context.Context, registry *tools.Registry, call T
|
||||
Meta: result.Meta,
|
||||
Redacted: result.Redacted,
|
||||
ChangedFiles: result.ChangedFiles,
|
||||
FileDiffs: result.FileDiffs,
|
||||
ChangeSummaries: result.ChangeSummaries,
|
||||
Display: result.HumanDisplay(),
|
||||
Outcome: result.Outcome,
|
||||
|
||||
@@ -27,6 +27,23 @@ type mockProvider struct {
|
||||
requests []zeroruntime.CompletionRequest
|
||||
}
|
||||
|
||||
func TestPrePermissionRejectScrubsFileDiffs(t *testing.T) {
|
||||
secret := "sk-proj-abcdefghijklmnopqrstuvwxyz"
|
||||
result := toolResultFromPrePermissionReject(ToolCall{ID: "call", Name: "test"}, tools.Result{
|
||||
Status: tools.StatusError,
|
||||
FileDiffs: []tools.FileDiff{{
|
||||
Path: filepath.Join(t.TempDir(), "secret.txt"),
|
||||
OldExists: true,
|
||||
NewExists: true,
|
||||
OldText: "token=" + secret,
|
||||
NewText: "safe",
|
||||
}},
|
||||
})
|
||||
if len(result.FileDiffs) != 1 || strings.Contains(result.FileDiffs[0].OldText, secret) || !result.Redacted {
|
||||
t.Fatalf("pre-permission FileDiff = %#v, redacted = %t", result.FileDiffs, result.Redacted)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTypedExecutionOutcomeOverridesLegacySandboxHeuristics(t *testing.T) {
|
||||
engine := sandbox.NewEngine(sandbox.EngineOptions{WorkspaceRoot: t.TempDir(), Policy: sandbox.DefaultPolicy()})
|
||||
call := ToolCall{Name: tools.ExecCommandToolName}
|
||||
|
||||
@@ -84,6 +84,7 @@ type ToolResult struct {
|
||||
Images []zeroruntime.ImageBlock
|
||||
Redacted bool
|
||||
ChangedFiles []string
|
||||
FileDiffs []tools.FileDiff
|
||||
// ChangeSummaries are non-selectable generated-tree summaries emitted by
|
||||
// command execution; callers must not schedule per-file work from them.
|
||||
ChangeSummaries []execution.Change
|
||||
|
||||
@@ -2,8 +2,10 @@ package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -363,7 +365,7 @@ func TestUnifiedPatchCopyOperation(t *testing.T) {
|
||||
if content, _ := os.ReadFile(filepath.Join(root, "dst.txt")); string(content) != "hello\nnew\n" {
|
||||
t.Fatalf("copy destination = %q", string(content))
|
||||
}
|
||||
if len(result.ChangedFiles) != 2 || result.ChangedFiles[0] != "src.txt" || result.ChangedFiles[1] != "dst.txt" {
|
||||
if len(result.ChangedFiles) != 1 || result.ChangedFiles[0] != "dst.txt" {
|
||||
t.Fatalf("changed files = %v", result.ChangedFiles)
|
||||
}
|
||||
destination, err := filepath.EvalSymlinks(filepath.Join(root, "dst.txt"))
|
||||
@@ -571,9 +573,9 @@ func TestApplyStructuredPatchChangeRefusesSourceChangedAfterPlanning(t *testing.
|
||||
"copy": {kind: structuredPatchCopy, from: target, to: destination, before: "planned\n", after: "planned\n", mode: 0o644},
|
||||
} {
|
||||
writeTestFile(t, path, "changed after planning\n")
|
||||
committed, err := applyStructuredPatchChange(workspace, change)
|
||||
if err == nil || committed || !strings.Contains(err.Error(), "changed on disk between planning and commit") {
|
||||
t.Fatalf("%s: expected a refusal, got committed=%v err=%v", name, committed, err)
|
||||
outcome, err := applyStructuredPatchChange(workspace, change)
|
||||
if err == nil || len(outcome.completed) != 0 || len(outcome.incompletePaths) != 0 || !strings.Contains(err.Error(), "changed on disk between planning and commit") {
|
||||
t.Fatalf("%s: expected a refusal, got outcome=%#v err=%v", name, outcome, err)
|
||||
}
|
||||
if content, _ := os.ReadFile(path); string(content) != "changed after planning\n" {
|
||||
t.Fatalf("%s: file must be untouched, got %q", name, string(content))
|
||||
@@ -709,4 +711,158 @@ func TestApplyPatchOperationsReportsCommittedPrefixOnFailure(t *testing.T) {
|
||||
if content, _ := os.ReadFile(filepath.Join(root, "third.txt")); string(content) != "three\n" {
|
||||
t.Fatalf("third.txt must be untouched, got %q", string(content))
|
||||
}
|
||||
if got := result.ChangedFiles; len(got) != 1 || got[0] != "first.txt" {
|
||||
t.Fatalf("partial patch ChangedFiles = %#v, want committed prefix only", got)
|
||||
}
|
||||
resolvedFirst, err := filepath.EvalSymlinks(filepath.Join(root, "first.txt"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := result.FileDiffs; len(got) != 1 || got[0] != (FileDiff{Path: resolvedFirst, OldExists: true, NewExists: true, OldText: "one\n", NewText: "ONE\n"}) {
|
||||
t.Fatalf("partial patch FileDiffs = %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyPatchOperationsReportsWorkspaceRelativeCommittedPrefixUnderCwd(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
applyRoot := filepath.Join(root, "sub", "dir")
|
||||
if err := os.MkdirAll(applyRoot, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
writeTestFile(t, filepath.Join(applyRoot, "first.txt"), "one\n")
|
||||
writeTestFile(t, filepath.Join(applyRoot, "second.txt"), "two\n")
|
||||
structuredPatchBeforeCommit = func(change structuredPatchChange) {
|
||||
if change.to.relative == "second.txt" {
|
||||
writeTestFile(t, filepath.Join(applyRoot, "second.txt"), "changed\n")
|
||||
}
|
||||
}
|
||||
t.Cleanup(func() { structuredPatchBeforeCommit = nil })
|
||||
patch := strings.Join([]string{
|
||||
"*** Begin Patch",
|
||||
"*** Update File: first.txt", "@@", "-one", "+ONE",
|
||||
"*** Update File: second.txt", "@@", "-two", "+TWO",
|
||||
"*** End Patch", "",
|
||||
}, "\n")
|
||||
|
||||
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"cwd": "sub/dir", "patch": patch,
|
||||
})
|
||||
if result.Status != StatusError || !strings.Contains(result.Output, "already committed: sub/dir/first.txt") {
|
||||
t.Fatalf("nested partial failure = status=%s output=%q", result.Status, result.Output)
|
||||
}
|
||||
if got := result.ChangedFiles; len(got) != 1 || got[0] != "sub/dir/first.txt" {
|
||||
t.Fatalf("nested partial ChangedFiles = %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyPatchSeparatesVerifiedAndUnverifiedPartialPublications(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
priorPublish := structuredPatchPublishNoReplace
|
||||
structuredPatchPublishNoReplace = func(workspace *os.Root, source, target string, mode os.FileMode) (bool, error) {
|
||||
if target != "uncertain.txt" {
|
||||
return priorPublish(workspace, source, target, mode)
|
||||
}
|
||||
file, err := workspace.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode.Perm())
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if _, err := file.WriteString("partial"); err != nil {
|
||||
_ = file.Close()
|
||||
return true, err
|
||||
}
|
||||
if err := file.Close(); err != nil {
|
||||
return true, err
|
||||
}
|
||||
return true, errors.New("injected failure after partial publication")
|
||||
}
|
||||
t.Cleanup(func() { structuredPatchPublishNoReplace = priorPublish })
|
||||
|
||||
patch := strings.Join([]string{
|
||||
"*** Begin Patch",
|
||||
"*** Add File: verified.txt", "+verified",
|
||||
"*** Add File: uncertain.txt", "+intended complete content",
|
||||
"*** Add File: untouched.txt", "+untouched",
|
||||
"*** End Patch", "",
|
||||
}, "\n")
|
||||
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
|
||||
|
||||
if result.Status != StatusError {
|
||||
t.Fatalf("partial publication status = %s, want error", result.Status)
|
||||
}
|
||||
for _, want := range []string{"already committed: verified.txt", "published but unverified: uncertain.txt"} {
|
||||
if !strings.Contains(result.Output, want) {
|
||||
t.Fatalf("partial publication error must contain %q: %s", want, result.Output)
|
||||
}
|
||||
}
|
||||
committedFragment := strings.SplitN(result.Output, "; published but unverified:", 2)[0]
|
||||
if strings.Contains(committedFragment, "uncertain.txt") {
|
||||
t.Fatalf("unverified path appears in committed fragment: %s", result.Output)
|
||||
}
|
||||
if got, want := result.ChangedFiles, []string{"verified.txt"}; !slices.Equal(got, want) {
|
||||
t.Fatalf("ChangedFiles = %#v, want verified publications only %#v", got, want)
|
||||
}
|
||||
resolvedVerified, err := filepath.EvalSymlinks(filepath.Join(root, "verified.txt"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := result.FileDiffs; len(got) != 1 || got[0].Path != resolvedVerified {
|
||||
t.Fatalf("FileDiffs = %#v, want verified.txt only", got)
|
||||
}
|
||||
if got := mustReadTestFile(t, filepath.Join(root, "verified.txt")); got != "verified\n" {
|
||||
t.Fatalf("verified publication = %q", got)
|
||||
}
|
||||
if got := mustReadTestFile(t, filepath.Join(root, "uncertain.txt")); got != "partial" {
|
||||
t.Fatalf("unverified publication = %q", got)
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(root, "untouched.txt")); !os.IsNotExist(err) {
|
||||
t.Fatalf("later publication must remain untouched: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyPatchMoveReportsPublishedDestinationWhenSourceRemovalFails(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeTestFile(t, filepath.Join(root, "source.txt"), "before\n")
|
||||
priorRemove := structuredPatchRemove
|
||||
structuredPatchRemove = func(workspace *os.Root, name string) error {
|
||||
if name == "source.txt" {
|
||||
return os.ErrPermission
|
||||
}
|
||||
return priorRemove(workspace, name)
|
||||
}
|
||||
t.Cleanup(func() { structuredPatchRemove = priorRemove })
|
||||
|
||||
patch := strings.Join([]string{
|
||||
"*** Begin Patch",
|
||||
"*** Update File: source.txt",
|
||||
"*** Move to: destination.txt",
|
||||
"@@",
|
||||
"-before",
|
||||
"+after",
|
||||
"*** End Patch",
|
||||
"",
|
||||
}, "\n")
|
||||
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
|
||||
if result.Status != StatusError {
|
||||
t.Fatalf("move status = %s, want error", result.Status)
|
||||
}
|
||||
if got := mustReadTestFile(t, filepath.Join(root, "source.txt")); got != "before\n" {
|
||||
t.Fatalf("source content = %q", got)
|
||||
}
|
||||
if got := mustReadTestFile(t, filepath.Join(root, "destination.txt")); got != "after\n" {
|
||||
t.Fatalf("destination content = %q", got)
|
||||
}
|
||||
if got, want := result.ChangedFiles, []string{"destination.txt"}; !slices.Equal(got, want) {
|
||||
t.Fatalf("ChangedFiles = %#v, want %#v", got, want)
|
||||
}
|
||||
resolvedDestination, err := filepath.EvalSymlinks(filepath.Join(root, "destination.txt"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
wantDiff := FileDiff{Path: resolvedDestination, OldExists: false, NewExists: true, NewText: "after\n"}
|
||||
if got := result.FileDiffs; len(got) != 1 || got[0] != wantDiff {
|
||||
t.Fatalf("FileDiffs = %#v, want %#v", got, []FileDiff{wantDiff})
|
||||
}
|
||||
if !strings.Contains(result.Display.Preview, "destination.txt") || strings.Contains(result.Display.Preview, "source.txt") {
|
||||
t.Fatalf("preview must show only destination creation: %q", result.Display.Preview)
|
||||
}
|
||||
}
|
||||
@@ -1,19 +1,122 @@
|
||||
package tools
|
||||
|
||||
import udiff "github.com/aymanbagabas/go-udiff"
|
||||
import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"unicode"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/redaction"
|
||||
udiff "github.com/aymanbagabas/go-udiff"
|
||||
)
|
||||
|
||||
// maxToolPreviewBytes caps the inline diff a write tool appends to its result, so
|
||||
// a large generated file can't flood the transcript or balloon the persisted
|
||||
// session events. Past this the tool falls back to its summary line alone.
|
||||
const maxToolPreviewBytes = 48 * 1024
|
||||
|
||||
// A structured replacement contains two complete file sides, so its transport
|
||||
// budget is intentionally separate from the single rendered-preview budget.
|
||||
// Each side may be as large as a normal preview; aggregate producers apply the
|
||||
// two-sided result cap in file order.
|
||||
const maxToolResultFileDiffBytes = 2 * maxToolPreviewBytes
|
||||
|
||||
// FileDiff is a human-facing before/after file change. Registry-boundary
|
||||
// redaction applies to both sides before any caller receives it.
|
||||
type FileDiff struct {
|
||||
// Path is the canonical absolute path required by ACP diff content. The
|
||||
// separate ChangedFiles result remains workspace-relative for local UI use.
|
||||
Path string
|
||||
// OldExists and NewExists distinguish an empty file from a missing side of a
|
||||
// create/delete/move. Empty strings alone cannot encode that difference.
|
||||
OldExists bool
|
||||
NewExists bool
|
||||
OldText string
|
||||
NewText string
|
||||
}
|
||||
|
||||
// boundedFileDiff declines rather than truncating: a truncated side would look
|
||||
// like an exact file replacement. Callers keep ChangedFiles as the safe
|
||||
// fallback for large, unsafe, or unchanged content. Each complete side gets the
|
||||
// same bound as a rendered preview; aggregate result producers apply their own
|
||||
// cap. Control bytes are rejected. Ordinary Unicode format/space characters are
|
||||
// retained unless removing them reveals a credential shape that the normal
|
||||
// redactor could not see in the original text.
|
||||
func boundedFileDiff(path, oldText, newText string, oldExists, newExists bool) (FileDiff, bool) {
|
||||
if !filepath.IsAbs(path) || (!oldExists && !newExists) ||
|
||||
(oldExists == newExists && oldText == newText) ||
|
||||
!utf8.ValidString(oldText) || !utf8.ValidString(newText) ||
|
||||
unsafeDiffText(oldText) || unsafeDiffText(newText) ||
|
||||
len(oldText) > maxToolPreviewBytes || len(newText) > maxToolPreviewBytes {
|
||||
return FileDiff{}, false
|
||||
}
|
||||
return FileDiff{Path: path, OldExists: oldExists, NewExists: newExists, OldText: oldText, NewText: newText}, true
|
||||
}
|
||||
|
||||
func unsafeDiffText(text string) bool {
|
||||
if !utf8.ValidString(text) {
|
||||
return true
|
||||
}
|
||||
hasCanonicalizableSeparator := false
|
||||
for _, r := range text {
|
||||
switch r {
|
||||
case '\n', '\r', '\t', ' ':
|
||||
continue
|
||||
}
|
||||
if unicode.IsControl(r) {
|
||||
return true
|
||||
}
|
||||
if isDiffCredentialIgnorable(r) {
|
||||
hasCanonicalizableSeparator = true
|
||||
}
|
||||
}
|
||||
if !hasCanonicalizableSeparator {
|
||||
return false
|
||||
}
|
||||
return diffTextRevealsObfuscatedSecret(text)
|
||||
}
|
||||
|
||||
func diffTextRevealsObfuscatedSecret(text string) bool {
|
||||
if !utf8.ValidString(text) {
|
||||
return false
|
||||
}
|
||||
canonical := strings.Map(func(r rune) rune {
|
||||
switch r {
|
||||
case '\n', '\r', '\t', ' ':
|
||||
return r
|
||||
}
|
||||
if isDiffCredentialIgnorable(r) {
|
||||
return -1
|
||||
}
|
||||
return r
|
||||
}, text)
|
||||
return canonical != text && redaction.RedactString(canonical, redaction.Options{}) != canonical
|
||||
}
|
||||
|
||||
// isDiffCredentialIgnorable mirrors Unicode's Default_Ignorable_Code_Point
|
||||
// derived property using the tables exposed by the Go standard library. Cf
|
||||
// catches format controls, while the other two tables cover non-Cf default
|
||||
// ignorables such as U+034F COMBINING GRAPHEME JOINER and variation selectors.
|
||||
// Unicode whitespace is also removable because it can split a credential that
|
||||
// the boundary redactor would otherwise recognize. The caller preserves normal
|
||||
// ASCII layout whitespace before consulting this helper.
|
||||
func isDiffCredentialIgnorable(r rune) bool {
|
||||
return unicode.Is(unicode.Cf, r) ||
|
||||
unicode.Is(unicode.Other_Default_Ignorable_Code_Point, r) ||
|
||||
unicode.Is(unicode.Variation_Selector, r) ||
|
||||
unicode.IsSpace(r)
|
||||
}
|
||||
|
||||
// boundedUnifiedDiff returns a unified diff of oldContent -> newContent labelled
|
||||
// with path, suitable for the TUI's diff card renderer. A create (oldContent "")
|
||||
// yields an all-additions (green) preview; an overwrite/edit yields red/green.
|
||||
// Returns "" when there is no change or the diff exceeds maxToolPreviewBytes.
|
||||
// Returns "" when there is no change, the rendered diff is unsafe text, or the
|
||||
// diff exceeds maxToolPreviewBytes. This applies the same unsafe-text gate as
|
||||
// FileDiff to what reaches Display.Preview, without discarding a safe hunk only
|
||||
// because an unrelated part of the source file contains an unsafe byte.
|
||||
func boundedUnifiedDiff(path, oldContent, newContent string) string {
|
||||
diff := udiff.Unified(path, path, oldContent, newContent)
|
||||
if diff == "" || len(diff) > maxToolPreviewBytes {
|
||||
if diff == "" || !utf8.ValidString(diff) || unsafeDiffText(diff) || len(diff) > maxToolPreviewBytes {
|
||||
return ""
|
||||
}
|
||||
return diff
|
||||
|
||||
@@ -0,0 +1,186 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestBoundedFileDiffRefusesPartialOrBinaryContent(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "a.txt")
|
||||
if diff, ok := boundedFileDiff(path, "old", "new", true, true); !ok || diff.Path != path || !diff.OldExists || !diff.NewExists || diff.OldText != "old" || diff.NewText != "new" {
|
||||
t.Fatalf("small text diff = %#v, %t", diff, ok)
|
||||
}
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
old string
|
||||
new string
|
||||
}{
|
||||
{"unchanged", "same", "same"},
|
||||
{"binary old", string([]byte{0xff}), "text"},
|
||||
{"binary new", "text", string([]byte{0xff})},
|
||||
{"nul old", "token=sk-proj-abc\x00def", "text"},
|
||||
{"escape new", "text", "token=sk-proj-abc\x1bdef"},
|
||||
{"c1 old", "token=sk-proj-abc\u0085def", "text"},
|
||||
{"zero width space", "token=sk-proj-abc\u200bdef", "text"},
|
||||
{"zero width joiner", "token=sk-proj-abc\u200ddef", "text"},
|
||||
{"byte order mark", "token=sk-proj-abc\ufeffdef", "text"},
|
||||
{"soft hyphen", "token=sk-proj-abc\u00addef", "text"},
|
||||
{"non breaking space", "token=sk-proj-abc\u00a0def", "text"},
|
||||
{"too large", strings.Repeat("a", maxToolPreviewBytes+1), "b"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if diff, ok := boundedFileDiff(path, tc.old, tc.new, true, true); ok || diff != (FileDiff{}) {
|
||||
t.Fatalf("unexpected diff = %#v, %t", diff, ok)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBoundedFileDiffPreservesEmptyFileOperations(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "empty.txt")
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
oldExists, newExists bool
|
||||
}{
|
||||
{name: "create", newExists: true},
|
||||
{name: "delete", oldExists: true},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
diff, ok := boundedFileDiff(path, "", "", tc.oldExists, tc.newExists)
|
||||
if !ok || diff.OldExists != tc.oldExists || diff.NewExists != tc.newExists {
|
||||
t.Fatalf("empty %s = %#v, %t", tc.name, diff, ok)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBoundedUnifiedDiffRejectsUnsafeRichText(t *testing.T) {
|
||||
if got := boundedUnifiedDiff("safe.txt", "before\n", "after\n"); got == "" {
|
||||
t.Fatal("safe unified diff was unexpectedly omitted")
|
||||
}
|
||||
for _, content := range []string{
|
||||
"token=sk-proj-abc\x00def",
|
||||
"token=sk-proj-abc\x1bdef",
|
||||
"token=sk-proj-abc\u200bdef",
|
||||
string([]byte{0xff}),
|
||||
} {
|
||||
if got := boundedUnifiedDiff("secret.txt", content, "safe\n"); got != "" {
|
||||
t.Fatalf("unsafe rich diff = %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
old := strings.Repeat("unchanged\n", 12) + "form\ffeed\n" + strings.Repeat("unchanged\n", 12)
|
||||
updated := strings.Replace(old, "unchanged\n", "changed\n", 1)
|
||||
if got := boundedUnifiedDiff("safe-hunk.txt", old, updated); got == "" || strings.Contains(got, "\f") {
|
||||
t.Fatalf("safe hunk near unrelated unsafe text = %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStructuredPatchFileDiffsPreserveEmptyOperationsAndResultBudget(t *testing.T) {
|
||||
root, err := filepath.EvalSymlinks(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
changes := []structuredPatchChange{
|
||||
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "created"), relative: "created"}},
|
||||
{kind: structuredPatchDelete, from: structuredPatchTarget{absolute: filepath.Join(root, "deleted"), relative: "deleted"}},
|
||||
{kind: structuredPatchUpdate, from: structuredPatchTarget{absolute: filepath.Join(root, "from"), relative: "from"}, to: structuredPatchTarget{absolute: filepath.Join(root, "to"), relative: "to"}},
|
||||
}
|
||||
diffs := fileDiffsFromStructuredPatch(".", changes)
|
||||
if len(diffs) != 4 {
|
||||
t.Fatalf("empty create/delete/move diffs = %#v", diffs)
|
||||
}
|
||||
for _, diff := range diffs {
|
||||
if diff.Path == "" || (!diff.OldExists && !diff.NewExists) {
|
||||
t.Fatalf("invalid diff = %#v", diff)
|
||||
}
|
||||
}
|
||||
|
||||
large := strings.Repeat("x", maxToolResultFileDiffBytes/3+1)
|
||||
budgeted := fileDiffsFromStructuredPatch(".", []structuredPatchChange{
|
||||
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "one")}, after: large},
|
||||
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "two")}, after: "tiny"},
|
||||
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "three")}, after: large},
|
||||
{kind: structuredPatchAdd, to: structuredPatchTarget{absolute: filepath.Join(root, "four")}, after: large},
|
||||
})
|
||||
if len(budgeted) != 3 || filepath.Base(budgeted[0].Path) != "one" || filepath.Base(budgeted[1].Path) != "two" || filepath.Base(budgeted[2].Path) != "three" {
|
||||
t.Fatalf("ordered aggregate file-diff budget = %#v", budgeted)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStructuredPatchFileDiffsKeepEligibleSameBasenameSibling(t *testing.T) {
|
||||
root, err := filepath.EvalSymlinks(t.TempDir())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
diffs := fileDiffsFromStructuredPatch(".", []structuredPatchChange{
|
||||
{
|
||||
kind: structuredPatchAdd,
|
||||
to: structuredPatchTarget{absolute: filepath.Join(root, "a.go"), relative: "a.go"},
|
||||
after: strings.Repeat("x", maxToolPreviewBytes+1),
|
||||
},
|
||||
{
|
||||
kind: structuredPatchAdd,
|
||||
to: structuredPatchTarget{absolute: filepath.Join(root, "sub", "a.go"), relative: filepath.Join("sub", "a.go")},
|
||||
after: "package sub\n",
|
||||
},
|
||||
})
|
||||
if len(diffs) != 1 || diffs[0].Path != filepath.Join(root, "sub", "a.go") {
|
||||
t.Fatalf("same-basename rich diffs = %#v", diffs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBoundedDiffPreservesOrdinaryUnicodeButRejectsObfuscatedSecrets(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "unicode.txt")
|
||||
for name, content := range map[string]string{
|
||||
"family emoji": "family: 👨👩👧👦\n",
|
||||
"emoji variation selector": "heart: ❤️\n",
|
||||
"combining grapheme joiner": "ordinary\u034fprose\n",
|
||||
"nonbreaking space": "ordinary\u00a0prose\n",
|
||||
"byte order mark": "\ufeffdocument\n",
|
||||
"soft hyphen": "co\u00adoperate\n",
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
if _, ok := boundedFileDiff(path, "before\n", content, true, true); !ok {
|
||||
t.Fatalf("ordinary Unicode content was rejected: %q", content)
|
||||
}
|
||||
if preview := boundedUnifiedDiff("unicode.txt", "before\n", content); preview == "" {
|
||||
t.Fatal("ordinary Unicode preview was omitted")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
|
||||
for name, separator := range map[string]string{
|
||||
"zero width space": "\u200b",
|
||||
"zero width joiner": "\u200d",
|
||||
"byte order mark": "\ufeff",
|
||||
"soft hyphen": "\u00ad",
|
||||
"nonbreaking space": "\u00a0",
|
||||
"combining grapheme joiner": "\u034f",
|
||||
"variation selector": "\ufe0f",
|
||||
} {
|
||||
t.Run("split "+name, func(t *testing.T) {
|
||||
obfuscated := secret[:20] + separator + secret[20:]
|
||||
if _, ok := boundedFileDiff(path, "before\n", obfuscated, true, true); ok {
|
||||
t.Fatalf("obfuscated credential produced a rich diff: %q", obfuscated)
|
||||
}
|
||||
if preview := boundedUnifiedDiff("secret.txt", "before\n", obfuscated); preview != "" {
|
||||
t.Fatalf("obfuscated credential produced preview: %q", preview)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileMarksSuppressedObfuscatedSecretAsRedacted(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
|
||||
obfuscated := secret[:20] + "\u200b" + secret[20:]
|
||||
result := NewScopedWriteFileTool(root, nil).Run(t.Context(), map[string]any{
|
||||
"path": "secret.txt", "content": obfuscated,
|
||||
})
|
||||
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 || !result.Redacted {
|
||||
t.Fatalf("obfuscated-secret result = status=%s changed=%#v diffs=%#v redacted=%t", result.Status, result.ChangedFiles, result.FileDiffs, result.Redacted)
|
||||
}
|
||||
}
|
||||
@@ -80,6 +80,10 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
|
||||
}
|
||||
}
|
||||
content := string(contentBytes)
|
||||
priorInfo, err := os.Stat(absolutePath)
|
||||
if err != nil {
|
||||
return errorResult("Error reading " + relativePath + ": " + err.Error())
|
||||
}
|
||||
occurrences := strings.Count(content, oldString)
|
||||
|
||||
// CRLF fallback: read_file normalizes \r\n → \n before presenting content to
|
||||
@@ -153,19 +157,25 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
|
||||
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 {
|
||||
if err := commitFileContents(absolutePath, priorInfo, &content, updated); 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.
|
||||
formatting := maybeFormatWrittenFile(ctx, absolutePath, updated)
|
||||
formatting := maybeFormatWrittenFileScoped(ctx, tool.workspaceRoot, tool.scope, absolutePath, updated, priorInfo.Mode().Perm())
|
||||
updated = formatting.Content
|
||||
finalContentKnown := formatting.ContentKnown
|
||||
// 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)
|
||||
if updated == modelKnownContent {
|
||||
newInfo := formatting.Info
|
||||
if newInfo == nil {
|
||||
newInfo, _ = os.Stat(absolutePath)
|
||||
}
|
||||
if !finalContentKnown {
|
||||
options.FileTracker.Forget(absolutePath)
|
||||
} else if updated == modelKnownContent {
|
||||
// OUR edit, so we know precisely which lines moved: RecordEdit carries
|
||||
// across the reads this edit did not disturb instead of dropping them.
|
||||
//
|
||||
@@ -195,12 +205,25 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
|
||||
}
|
||||
summary := fmt.Sprintf("Successfully edited %s (replaced %d occurrence%s).", relativePath, replacedCount, suffix)
|
||||
summary += formatting.notice(relativePath)
|
||||
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
|
||||
if finalContentKnown {
|
||||
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
|
||||
}
|
||||
result := okResult(summary)
|
||||
result.ChangedFiles = []string{relativePath}
|
||||
if finalContentKnown {
|
||||
if diff, ok := boundedFileDiff(absolutePath, content, updated, true, true); ok {
|
||||
result.FileDiffs = []FileDiff{diff}
|
||||
} else if diffTextRevealsObfuscatedSecret(content) || diffTextRevealsObfuscatedSecret(updated) {
|
||||
result.Redacted = true
|
||||
}
|
||||
}
|
||||
// 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)}
|
||||
preview := ""
|
||||
if finalContentKnown {
|
||||
preview = boundedUnifiedDiff(relativePath, content, updated)
|
||||
}
|
||||
result.Display = Display{Summary: fmt.Sprintf("Edited %s", relativePath), Kind: "diff", Preview: preview}
|
||||
return result
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
)
|
||||
|
||||
var errFileChangedDuringWrite = errors.New("file changed on disk before the write committed")
|
||||
|
||||
// fileWriteBeforeCommit is a deterministic test hook. Production leaves it
|
||||
// nil; tests use it to replace a path after observation but before opening the
|
||||
// object that will actually be mutated.
|
||||
var fileWriteBeforeCommit func(path string)
|
||||
|
||||
// fileWriteStat is a deterministic test seam for proving that the opened-file
|
||||
// identity is captured before the final preimage comparison. Production uses
|
||||
// the file descriptor directly.
|
||||
var fileWriteStat = func(file *os.File) (os.FileInfo, error) { return file.Stat() }
|
||||
|
||||
// commitFileContents binds an overwrite to the file identity and bytes that
|
||||
// the caller observed. A create uses exclusive creation. An overwrite opens the
|
||||
// observed object without truncation, verifies identity/content through that
|
||||
// handle, then truncates and writes the same handle. A path replacement before
|
||||
// or during commit therefore fails instead of publishing stale rich evidence.
|
||||
//
|
||||
// expectedInfo nil means the caller observed a missing path. expectedContent
|
||||
// may be nil for an existing but unreadable file; that path may still be
|
||||
// overwritten, but callers must omit rich before/after evidence.
|
||||
func commitFileContents(path string, expectedInfo os.FileInfo, expectedContent *string, content string) error {
|
||||
if fileWriteBeforeCommit != nil {
|
||||
fileWriteBeforeCommit(path)
|
||||
}
|
||||
|
||||
if expectedInfo == nil {
|
||||
file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
openedInfo, err := fileWriteStat(file)
|
||||
if err != nil {
|
||||
_ = file.Close()
|
||||
return err
|
||||
}
|
||||
return writeAndVerifyFileIdentity(path, file, openedInfo, content, false)
|
||||
}
|
||||
|
||||
flags := os.O_WRONLY
|
||||
if expectedContent != nil {
|
||||
flags = os.O_RDWR
|
||||
}
|
||||
file, err := os.OpenFile(path, flags, 0)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
openedInfo, err := fileWriteStat(file)
|
||||
if err != nil {
|
||||
_ = file.Close()
|
||||
return err
|
||||
}
|
||||
if !os.SameFile(expectedInfo, openedInfo) {
|
||||
_ = file.Close()
|
||||
return errFileChangedDuringWrite
|
||||
}
|
||||
pathInfo, err := os.Stat(path)
|
||||
if err != nil || !os.SameFile(openedInfo, pathInfo) {
|
||||
_ = file.Close()
|
||||
return errFileChangedDuringWrite
|
||||
}
|
||||
if expectedContent != nil {
|
||||
current, readErr := io.ReadAll(file)
|
||||
if readErr != nil {
|
||||
_ = file.Close()
|
||||
return readErr
|
||||
}
|
||||
if string(current) != *expectedContent {
|
||||
_ = file.Close()
|
||||
return errFileChangedDuringWrite
|
||||
}
|
||||
}
|
||||
return writeAndVerifyFileIdentity(path, file, openedInfo, content, true)
|
||||
}
|
||||
|
||||
func writeAndVerifyFileIdentity(path string, file *os.File, openedInfo os.FileInfo, content string, truncate bool) error {
|
||||
if truncate {
|
||||
if err := file.Truncate(0); err != nil {
|
||||
_ = file.Close()
|
||||
return err
|
||||
}
|
||||
if _, err := file.Seek(0, io.SeekStart); err != nil {
|
||||
_ = file.Close()
|
||||
return err
|
||||
}
|
||||
}
|
||||
if _, err := io.WriteString(file, content); err != nil {
|
||||
_ = file.Close()
|
||||
return err
|
||||
}
|
||||
if err := file.Close(); err != nil {
|
||||
return err
|
||||
}
|
||||
pathInfo, err := os.Stat(path)
|
||||
if err != nil || !os.SameFile(openedInfo, pathInfo) {
|
||||
return fmt.Errorf("%w: path identity changed", errFileChangedDuringWrite)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,131 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func installFileWriteRace(t *testing.T, mutate func(string)) {
|
||||
t.Helper()
|
||||
prior := fileWriteBeforeCommit
|
||||
fileWriteBeforeCommit = mutate
|
||||
t.Cleanup(func() { fileWriteBeforeCommit = prior })
|
||||
}
|
||||
|
||||
func installFileWriteStat(t *testing.T, stat func(*os.File) (os.FileInfo, error)) {
|
||||
t.Helper()
|
||||
prior := fileWriteStat
|
||||
fileWriteStat = stat
|
||||
t.Cleanup(func() { fileWriteStat = prior })
|
||||
}
|
||||
|
||||
func TestWriteFileRefusesCreateAndOverwriteRaces(t *testing.T) {
|
||||
t.Run("create", func(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
target := filepath.Join(root, "created.txt")
|
||||
installFileWriteRace(t, func(path string) {
|
||||
if err := os.WriteFile(path, []byte("other writer\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
})
|
||||
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "created.txt", "content": "zero\n",
|
||||
})
|
||||
if result.Status != StatusError {
|
||||
t.Fatalf("raced create status = %s, want error", result.Status)
|
||||
}
|
||||
if got, err := os.ReadFile(target); err != nil || string(got) != "other writer\n" {
|
||||
t.Fatalf("raced create content = %q, err=%v", got, err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("overwrite", func(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
target := filepath.Join(root, "existing.txt")
|
||||
if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
installFileWriteRace(t, func(path string) {
|
||||
if err := os.WriteFile(path, []byte("other writer\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
})
|
||||
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "existing.txt", "content": "zero\n", "overwrite": true,
|
||||
})
|
||||
if result.Status != StatusError || !strings.Contains(result.Output, errFileChangedDuringWrite.Error()) {
|
||||
t.Fatalf("raced overwrite = %s: %s", result.Status, result.Output)
|
||||
}
|
||||
if got, err := os.ReadFile(target); err != nil || string(got) != "other writer\n" {
|
||||
t.Fatalf("raced overwrite content = %q, err=%v", got, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestEditFileRefusesPreimageRace(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
target := filepath.Join(root, "existing.txt")
|
||||
if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
installFileWriteRace(t, func(path string) {
|
||||
if err := os.WriteFile(path, []byte("other writer\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
})
|
||||
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "existing.txt", "old_string": "observed", "new_string": "zero",
|
||||
})
|
||||
if result.Status != StatusError || !strings.Contains(result.Output, errFileChangedDuringWrite.Error()) {
|
||||
t.Fatalf("raced edit = %s: %s", result.Status, result.Output)
|
||||
}
|
||||
if got, err := os.ReadFile(target); err != nil || string(got) != "other writer\n" {
|
||||
t.Fatalf("raced edit content = %q, err=%v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestOverwriteDoesNotStatOpenedFileAfterFinalPreimageComparison(t *testing.T) {
|
||||
for name, run := range map[string]func(string) Result{
|
||||
"write overwrite": func(root string) Result {
|
||||
return NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "existing.txt", "content": "zero\n", "overwrite": true,
|
||||
})
|
||||
},
|
||||
"edit": func(root string) Result {
|
||||
return NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "existing.txt", "old_string": "observed", "new_string": "zero",
|
||||
})
|
||||
},
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
target := filepath.Join(root, "existing.txt")
|
||||
if err := os.WriteFile(target, []byte("observed\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
statCalls := 0
|
||||
installFileWriteStat(t, func(file *os.File) (os.FileInfo, error) {
|
||||
statCalls++
|
||||
if statCalls == 2 {
|
||||
// This preserves the inode, so identity-only checks cannot detect it.
|
||||
if err := os.WriteFile(target, []byte("other writer\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
return file.Stat()
|
||||
})
|
||||
|
||||
result := run(root)
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("overwrite status = %s: %s", result.Status, result.Output)
|
||||
}
|
||||
if statCalls != 1 {
|
||||
t.Fatalf("opened file was statted %d times; the final byte comparison must be followed directly by mutation", statCalls)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -3,11 +3,15 @@ package tools
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/sandbox"
|
||||
)
|
||||
|
||||
// Format-on-write for the mutating file tools. When enabled, a successful
|
||||
@@ -44,7 +48,9 @@ var formatOnWriteTimeout = 10 * time.Second
|
||||
// and finds out from a CI format check it cannot see, which is the thing this
|
||||
// feature exists to prevent.
|
||||
type formatOnWriteResult struct {
|
||||
Content string
|
||||
Content string
|
||||
ContentKnown bool
|
||||
Info os.FileInfo
|
||||
// Formatter is the binary that was run, named in the notice so the user can
|
||||
// tell a slow gofmt from a slow prettier.
|
||||
Formatter string
|
||||
@@ -117,14 +123,32 @@ func formatOnWriteEnabled() bool {
|
||||
return value != "" && value != "0" && !strings.EqualFold(value, "false")
|
||||
}
|
||||
|
||||
// maybeFormatWrittenFile runs the configured formatter for absolutePath (when
|
||||
// enabled and on PATH) and returns the file's content afterwards. Best-effort
|
||||
// throughout: any failure — no formatter, formatter error, timeout, unreadable
|
||||
// result — returns writtenContent so the caller's state matches the last write
|
||||
// it performed itself. Only the timeout is reported back, for the reason on
|
||||
// formatOnWriteResult.
|
||||
var runFormatOnWriteCommand = func(ctx context.Context, binaryPath string, arguments []string, directory string) error {
|
||||
formatter := exec.CommandContext(ctx, binaryPath, arguments...)
|
||||
formatter.Dir = directory
|
||||
formatter.Stdin = strings.NewReader("")
|
||||
return formatter.Run()
|
||||
}
|
||||
|
||||
var readFormattedFile = readRootedFile
|
||||
|
||||
// maybeFormatWrittenFile is the unscoped test-facing wrapper. Production
|
||||
// callers use maybeFormatWrittenFileScoped so a formatter cannot redirect the
|
||||
// final read or recovery write outside the configured write roots.
|
||||
func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenContent string) formatOnWriteResult {
|
||||
unformatted := formatOnWriteResult{Content: writtenContent}
|
||||
restoreMode := os.FileMode(0o644)
|
||||
if info, err := os.Stat(absolutePath); err == nil {
|
||||
restoreMode = info.Mode().Perm()
|
||||
}
|
||||
return maybeFormatWrittenFileScoped(ctx, filepath.Dir(absolutePath), nil, absolutePath, writtenContent, restoreMode)
|
||||
}
|
||||
|
||||
// maybeFormatWrittenFileScoped runs the configured formatter and returns only
|
||||
// content verified through a descriptor-bound root. Formatter failures restore
|
||||
// writtenContent through that same root; if restoration or the final read
|
||||
// fails, ContentKnown is false and callers omit exact diff evidence.
|
||||
func maybeFormatWrittenFileScoped(ctx context.Context, workspaceRoot string, scope PathScope, absolutePath string, writtenContent string, restoreMode os.FileMode) formatOnWriteResult {
|
||||
unformatted := formatOnWriteResult{Content: writtenContent, ContentKnown: true}
|
||||
if !formatOnWriteEnabled() {
|
||||
return unformatted
|
||||
}
|
||||
@@ -136,42 +160,90 @@ func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenCon
|
||||
if err != nil {
|
||||
return unformatted
|
||||
}
|
||||
root, relativePath, err := openFormattedFileRoot(workspaceRoot, scope, absolutePath)
|
||||
if err != nil {
|
||||
unformatted.ContentKnown = false
|
||||
return unformatted
|
||||
}
|
||||
defer root.Close()
|
||||
|
||||
formatCtx, cancel := context.WithTimeout(ctx, formatOnWriteTimeout)
|
||||
defer cancel()
|
||||
arguments := append(append([]string(nil), command[1:]...), absolutePath)
|
||||
formatter := exec.CommandContext(formatCtx, binaryPath, arguments...)
|
||||
formatter.Dir = filepath.Dir(absolutePath)
|
||||
formatter.Stdin = strings.NewReader("")
|
||||
if err := formatter.Run(); err != nil {
|
||||
if err := runFormatOnWriteCommand(formatCtx, binaryPath, arguments, filepath.Dir(absolutePath)); err != nil {
|
||||
unformatted.Formatter = command[0]
|
||||
// THE FORMATTER EDITS IN PLACE, SO A FAILED RUN CAN LEAVE THE FILE
|
||||
// NEITHER FORMATTED NOR AS WRITTEN. Killed by the deadline or by the
|
||||
// caller, or exiting partway through its own rewrite, the target can hold
|
||||
// a truncation. Returning the written bytes on top of that would leave the
|
||||
// tracker baseline and the diff preview describing a file that is not on
|
||||
// disk, which is a worse failure than the missing formatting: the next
|
||||
// edit compares against content the file does not have.
|
||||
//
|
||||
// Written back unconditionally on this path rather than only when the
|
||||
// bytes differ. Comparing first means reading the file to find out, and a
|
||||
// read that fails leaves the same ambiguity this exists to remove.
|
||||
if restoreErr := os.WriteFile(absolutePath, []byte(writtenContent), 0o644); restoreErr != nil {
|
||||
if restoreErr := restoreFormattedFile(root, relativePath, writtenContent, restoreMode); restoreErr != nil {
|
||||
unformatted.RestoreFailed = true
|
||||
unformatted.ContentKnown = false
|
||||
} else if restored, info, readErr := readFormattedFile(root, relativePath); readErr != nil {
|
||||
unformatted.ContentKnown = false
|
||||
} else {
|
||||
unformatted.Content = string(restored)
|
||||
unformatted.Info = info
|
||||
}
|
||||
// OUR deadline, not the caller's cancellation and not the formatter's own
|
||||
// exit status. A cancelled tool call is already being reported as
|
||||
// cancelled, and a formatter that ran and refused the file usually means
|
||||
// content it could not parse, which the write itself does not promise to
|
||||
// fix. Neither is this notice's business; the restore above is, for all
|
||||
// three.
|
||||
if errors.Is(formatCtx.Err(), context.DeadlineExceeded) && ctx.Err() == nil {
|
||||
unformatted.TimedOut = true
|
||||
}
|
||||
return unformatted
|
||||
}
|
||||
formatted, err := os.ReadFile(absolutePath)
|
||||
|
||||
formatted, info, err := readFormattedFile(root, relativePath)
|
||||
if err != nil {
|
||||
unformatted.ContentKnown = false
|
||||
return unformatted
|
||||
}
|
||||
return formatOnWriteResult{Content: string(formatted), Formatter: command[0]}
|
||||
return formatOnWriteResult{Content: string(formatted), ContentKnown: true, Info: info, Formatter: command[0]}
|
||||
}
|
||||
|
||||
func restoreFormattedFile(root *os.Root, relativePath string, content string, mode os.FileMode) error {
|
||||
file, err := root.OpenFile(relativePath, os.O_WRONLY|os.O_TRUNC|os.O_CREATE, mode)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if _, err := io.WriteString(file, content); err != nil {
|
||||
_ = file.Close()
|
||||
return err
|
||||
}
|
||||
return file.Close()
|
||||
}
|
||||
|
||||
// openFormattedFileRoot opens the write root before the formatter runs and
|
||||
// computes the target relative to that descriptor-bound root. Atomic in-root
|
||||
// replacement remains valid; a formatter that swaps the target to an escaping
|
||||
// symlink is rejected when readFormattedFile opens it through the root.
|
||||
func openFormattedFileRoot(workspaceRoot string, scope PathScope, absolutePath string) (*os.Root, string, error) {
|
||||
roots, err := scopedRoots(workspaceRoot, scope)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
var firstErr error
|
||||
for _, configuredRoot := range roots {
|
||||
resolvedRoot, err := filepath.Abs(configuredRoot)
|
||||
if err == nil {
|
||||
resolvedRoot, err = filepath.EvalSymlinks(resolvedRoot)
|
||||
}
|
||||
if err != nil {
|
||||
if firstErr == nil {
|
||||
firstErr = err
|
||||
}
|
||||
continue
|
||||
}
|
||||
candidate := sandbox.NormalizePrefixForRoot(absolutePath, resolvedRoot)
|
||||
relativePath, err := filepath.Rel(resolvedRoot, candidate)
|
||||
if err != nil || relativePath == ".." || strings.HasPrefix(relativePath, ".."+string(filepath.Separator)) || filepath.IsAbs(relativePath) {
|
||||
continue
|
||||
}
|
||||
root, err := os.OpenRoot(resolvedRoot)
|
||||
if err != nil {
|
||||
if firstErr == nil {
|
||||
firstErr = err
|
||||
}
|
||||
continue
|
||||
}
|
||||
return root, relativePath, nil
|
||||
}
|
||||
if firstErr != nil {
|
||||
return nil, "", firstErr
|
||||
}
|
||||
return nil, "", fmt.Errorf("%s must stay inside the configured write roots", absolutePath)
|
||||
}
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
@@ -98,7 +99,7 @@ func TestFormatOnWriteFormatsAndKeepsTrackerConsistent(t *testing.T) {
|
||||
func TestFormatOnWriteSkipsUnknownExtensions(t *testing.T) {
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
formatting := maybeFormatWrittenFile(context.Background(), filepath.Join(t.TempDir(), "notes.xyz"), "raw text")
|
||||
if formatting.Content != "raw text" {
|
||||
if formatting.Content != "raw text" || !formatting.ContentKnown {
|
||||
t.Fatalf("unknown extension must pass through: %q", formatting.Content)
|
||||
}
|
||||
if notice := formatting.notice("notes.xyz"); notice != "" {
|
||||
@@ -115,10 +116,418 @@ func TestFormatOnWriteFormatterLookupFailure(t *testing.T) {
|
||||
t.Fatal(err)
|
||||
}
|
||||
formatting := maybeFormatWrittenFile(context.Background(), targetPath, uglyContent)
|
||||
if formatting.Content != uglyContent {
|
||||
if formatting.Content != uglyContent || !formatting.ContentKnown {
|
||||
t.Fatalf("missing formatter must return written content, got %q", formatting.Content)
|
||||
}
|
||||
if notice := formatting.notice("a.go"); notice != "" {
|
||||
t.Fatalf("an uninstalled formatter is a standing fact, not a miss worth reporting, got %q", notice)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatOnWriteReadsMutatedFileAfterFormatterFailure(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
targetPath := filepath.Join(t.TempDir(), "a.go")
|
||||
if err := os.WriteFile(targetPath, []byte("requested"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
|
||||
if err := os.WriteFile(targetPath, []byte("formatter-mutated"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return exec.ErrNotFound
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), targetPath, "requested")
|
||||
if !formatting.ContentKnown || formatting.Content != "requested" {
|
||||
t.Fatalf("formatter failure content = %q, known=%t", formatting.Content, formatting.ContentKnown)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatOnWriteReportsWhenFormatterFailureCannotBeRestored(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
targetPath := filepath.Join(t.TempDir(), "a.go")
|
||||
if err := os.WriteFile(targetPath, []byte("requested"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
|
||||
if err := os.Remove(targetPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Mkdir(targetPath, 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return exec.ErrNotFound
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), targetPath, "requested")
|
||||
if !formatting.RestoreFailed {
|
||||
t.Fatal("failed recovery was not reported")
|
||||
}
|
||||
if formatting.ContentKnown {
|
||||
t.Fatalf("failed recovery reported known content %q", formatting.Content)
|
||||
}
|
||||
if notice := formatting.notice("a.go"); !strings.Contains(notice, "WARNING: a.go may not hold what was written") {
|
||||
t.Fatalf("failed recovery notice = %q", notice)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileRecreatesFormatterDeletedTargetWithOriginalMode(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
existing bool
|
||||
wantMode os.FileMode
|
||||
overwrite bool
|
||||
}{
|
||||
{name: "create", wantMode: 0o644},
|
||||
{name: "overwrite", existing: true, wantMode: 0o751, overwrite: true},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
if tc.existing {
|
||||
if err := os.WriteFile(targetPath, []byte("before\n"), tc.wantMode); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
|
||||
if err := os.Remove(targetPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return exec.ErrNotFound
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
|
||||
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "a.go", "content": "requested\n", "overwrite": tc.overwrite,
|
||||
})
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("write status = %s: %s", result.Status, result.Output)
|
||||
}
|
||||
content, err := os.ReadFile(targetPath)
|
||||
if err != nil || string(content) != "requested\n" {
|
||||
t.Fatalf("recreated content = %q, err=%v", content, err)
|
||||
}
|
||||
if runtime.GOOS != "windows" {
|
||||
info, err := os.Stat(targetPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := info.Mode().Perm(); got != tc.wantMode {
|
||||
t.Fatalf("recreated mode = %o, want %o", got, tc.wantMode)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestFormatOnWriteMarksUnreadableFinalStateUnknown(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
targetPath := filepath.Join(t.TempDir(), "a.go")
|
||||
if err := os.WriteFile(targetPath, []byte("requested"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
priorReader := readFormattedFile
|
||||
runFormatOnWriteCommand = func(context.Context, string, []string, string) error { return nil }
|
||||
readFormattedFile = func(*os.Root, string) ([]byte, os.FileInfo, error) { return nil, nil, os.ErrPermission }
|
||||
t.Cleanup(func() {
|
||||
runFormatOnWriteCommand = priorRunner
|
||||
readFormattedFile = priorReader
|
||||
})
|
||||
|
||||
formatting := maybeFormatWrittenFile(context.Background(), targetPath, "requested")
|
||||
if formatting.ContentKnown || formatting.Content != "requested" {
|
||||
t.Fatalf("unreadable formatter result = %q, known=%t", formatting.Content, formatting.ContentKnown)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileUsesVerifiedBytesAfterFormatterFailure(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
|
||||
if err := os.WriteFile(targetPath, []byte("formatter-mutated\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return exec.ErrNotFound
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
|
||||
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "a.go", "content": "requested\n",
|
||||
})
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("write status = %s: %s", result.Status, result.Output)
|
||||
}
|
||||
if got := result.FileDiffs; len(got) != 1 || got[0].NewText != "requested\n" {
|
||||
t.Fatalf("formatter-failure FileDiff = %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEditFileUsesVerifiedBytesAfterFormatterFailure(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
if err := os.WriteFile(targetPath, []byte("before\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(_ context.Context, _ string, _ []string, _ string) error {
|
||||
if err := os.WriteFile(targetPath, []byte("formatter-mutated\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return exec.ErrNotFound
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
|
||||
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "a.go", "old_string": "before", "new_string": "requested",
|
||||
})
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("edit status = %s: %s", result.Status, result.Output)
|
||||
}
|
||||
if got := result.FileDiffs; len(got) != 1 || got[0].OldText != "before\n" || got[0].NewText != "requested\n" {
|
||||
t.Fatalf("formatter-failure edit FileDiff = %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileOmitsRichDiffWhenFormatterFinalReadFails(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
priorReader := readFormattedFile
|
||||
runFormatOnWriteCommand = func(context.Context, string, []string, string) error { return nil }
|
||||
readFormattedFile = func(*os.Root, string) ([]byte, os.FileInfo, error) { return nil, nil, os.ErrPermission }
|
||||
t.Cleanup(func() {
|
||||
runFormatOnWriteCommand = priorRunner
|
||||
readFormattedFile = priorReader
|
||||
})
|
||||
|
||||
result := NewScopedWriteFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "a.go", "content": "requested\n",
|
||||
})
|
||||
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 {
|
||||
t.Fatalf("unverified formatter result = status=%s changed=%#v diffs=%#v", result.Status, result.ChangedFiles, result.FileDiffs)
|
||||
}
|
||||
if result.Display.Preview != "" {
|
||||
t.Fatalf("unverified formatter result exposed stale preview: %q", result.Display.Preview)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEditFileOmitsPreviewWhenFormatterFinalReadFails(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
if err := os.WriteFile(targetPath, []byte("before\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
priorReader := readFormattedFile
|
||||
runFormatOnWriteCommand = func(context.Context, string, []string, string) error { return nil }
|
||||
readFormattedFile = func(*os.Root, string) ([]byte, os.FileInfo, error) { return nil, nil, os.ErrPermission }
|
||||
t.Cleanup(func() {
|
||||
runFormatOnWriteCommand = priorRunner
|
||||
readFormattedFile = priorReader
|
||||
})
|
||||
|
||||
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "a.go", "old_string": "before", "new_string": "requested",
|
||||
})
|
||||
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 {
|
||||
t.Fatalf("unverified formatter result = status=%s changed=%#v diffs=%#v", result.Status, result.ChangedFiles, result.FileDiffs)
|
||||
}
|
||||
if result.Display.Preview != "" {
|
||||
t.Fatalf("unverified formatter result exposed stale preview: %q", result.Display.Preview)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileOmitsRichEvidenceWhenFormatterReplacesTargetWithOutOfRootSymlink(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
resolvedRoot, err := filepath.EvalSymlinks(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
trackedPath := filepath.Join(resolvedRoot, "a.go")
|
||||
outsidePath := filepath.Join(t.TempDir(), "outside.go")
|
||||
outsideContent := "package external\n\nconst Secret = \"outside\"\n"
|
||||
if err := os.WriteFile(outsidePath, []byte(outsideContent), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
|
||||
if err := os.Remove(targetPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(outsidePath, targetPath); err != nil {
|
||||
t.Skipf("symlinks unavailable: %v", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
tracker := NewFileTracker()
|
||||
|
||||
diagnosticsCalled := false
|
||||
result := NewScopedWriteFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
|
||||
"path": "a.go", "content": "package requested\n",
|
||||
}, RunOptions{FileTracker: tracker, Diagnostics: func(context.Context, string) string {
|
||||
diagnosticsCalled = true
|
||||
return "must not run"
|
||||
}})
|
||||
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 || result.Display.Preview != "" {
|
||||
t.Fatalf("out-of-root formatter result = status=%s changed=%#v diffs=%#v preview=%q output=%q", result.Status, result.ChangedFiles, result.FileDiffs, result.Display.Preview, result.Output)
|
||||
}
|
||||
if diagnosticsCalled {
|
||||
t.Fatal("diagnostics must not inspect an unverified formatter target")
|
||||
}
|
||||
if _, tracked := tracker.Version(trackedPath); tracked {
|
||||
t.Fatal("out-of-root formatter target must not be recorded in the tracker")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEditFileOmitsRichEvidenceWhenFormatterReplacesTargetWithOutOfRootSymlink(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
resolvedRoot, err := filepath.EvalSymlinks(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
trackedPath := filepath.Join(resolvedRoot, "a.go")
|
||||
if err := os.WriteFile(targetPath, []byte("package before\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
outsidePath := filepath.Join(t.TempDir(), "outside.go")
|
||||
outsideContent := "package external\n\nconst Secret = \"outside\"\n"
|
||||
if err := os.WriteFile(outsidePath, []byte(outsideContent), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
|
||||
if err := os.Remove(targetPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(outsidePath, targetPath); err != nil {
|
||||
t.Skipf("symlinks unavailable: %v", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
tracker := NewFileTracker()
|
||||
read := NewScopedReadFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
|
||||
"path": "a.go",
|
||||
}, RunOptions{FileTracker: tracker})
|
||||
if read.Status != StatusOK {
|
||||
t.Fatalf("read before edit failed: %s", read.Output)
|
||||
}
|
||||
|
||||
diagnosticsCalled := false
|
||||
result := NewScopedEditFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
|
||||
"path": "a.go", "old_string": "before", "new_string": "requested",
|
||||
}, RunOptions{FileTracker: tracker, Diagnostics: func(context.Context, string) string {
|
||||
diagnosticsCalled = true
|
||||
return "must not run"
|
||||
}})
|
||||
if result.Status != StatusOK || len(result.ChangedFiles) != 1 || len(result.FileDiffs) != 0 || result.Display.Preview != "" {
|
||||
t.Fatalf("out-of-root formatter result = status=%s changed=%#v diffs=%#v preview=%q output=%q", result.Status, result.ChangedFiles, result.FileDiffs, result.Display.Preview, result.Output)
|
||||
}
|
||||
if diagnosticsCalled {
|
||||
t.Fatal("diagnostics must not inspect an unverified formatter target")
|
||||
}
|
||||
if _, tracked := tracker.Version(trackedPath); tracked {
|
||||
t.Fatal("out-of-root formatter target must not be recorded in the tracker")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileAcceptsInRootAtomicFormatterReplacement(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
formatted := "package formatted\n"
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
|
||||
tempPath := filepath.Join(root, "formatter.tmp")
|
||||
if err := os.WriteFile(tempPath, []byte(formatted), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return os.Rename(tempPath, targetPath)
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
tracker := NewFileTracker()
|
||||
|
||||
result := NewScopedWriteFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
|
||||
"path": "a.go", "content": "package requested\n",
|
||||
}, RunOptions{FileTracker: tracker})
|
||||
if result.Status != StatusOK || len(result.FileDiffs) != 1 || result.FileDiffs[0].NewText != formatted {
|
||||
t.Fatalf("atomic formatter write = status=%s diffs=%#v output=%q", result.Status, result.FileDiffs, result.Output)
|
||||
}
|
||||
resolvedRoot, err := filepath.EvalSymlinks(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
version, tracked := tracker.Version(filepath.Join(resolvedRoot, "a.go"))
|
||||
if !tracked || version.Hash != HashContent([]byte(formatted)) {
|
||||
t.Fatalf("atomic formatter tracker = %#v, tracked=%t", version, tracked)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEditFileAcceptsInRootAtomicFormatterReplacement(t *testing.T) {
|
||||
requireGofmt(t)
|
||||
t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "a.go")
|
||||
if err := os.WriteFile(targetPath, []byte("package before\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
formatted := "package formatted\n"
|
||||
priorRunner := runFormatOnWriteCommand
|
||||
runFormatOnWriteCommand = func(context.Context, string, []string, string) error {
|
||||
tempPath := filepath.Join(root, "formatter.tmp")
|
||||
if err := os.WriteFile(tempPath, []byte(formatted), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return os.Rename(tempPath, targetPath)
|
||||
}
|
||||
t.Cleanup(func() { runFormatOnWriteCommand = priorRunner })
|
||||
tracker := NewFileTracker()
|
||||
read := NewScopedReadFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
|
||||
"path": "a.go",
|
||||
}, RunOptions{FileTracker: tracker})
|
||||
if read.Status != StatusOK {
|
||||
t.Fatalf("read before edit failed: %s", read.Output)
|
||||
}
|
||||
|
||||
result := NewScopedEditFileTool(root, nil).(optionsAwareTool).RunWithOptions(context.Background(), map[string]any{
|
||||
"path": "a.go", "old_string": "before", "new_string": "requested",
|
||||
}, RunOptions{FileTracker: tracker})
|
||||
if result.Status != StatusOK || len(result.FileDiffs) != 1 || result.FileDiffs[0].NewText != formatted {
|
||||
t.Fatalf("atomic formatter edit = status=%s diffs=%#v output=%q", result.Status, result.FileDiffs, result.Output)
|
||||
}
|
||||
resolvedRoot, err := filepath.EvalSymlinks(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
version, tracked := tracker.Version(filepath.Join(resolvedRoot, "a.go"))
|
||||
if !tracked || version.Hash != HashContent([]byte(formatted)) {
|
||||
t.Fatalf("atomic formatter tracker = %#v, tracked=%t", version, tracked)
|
||||
}
|
||||
}
|
||||
@@ -343,6 +343,33 @@ func scrubResultSecrets(res Result) Result {
|
||||
res.Display.Preview = scrubbed
|
||||
res.Redacted = true
|
||||
}
|
||||
fileDiffs := make([]FileDiff, 0, len(res.FileDiffs))
|
||||
for _, diff := range res.FileDiffs {
|
||||
// Never normalize control bytes in a diff: normalizing after redaction can
|
||||
// reassemble a split credential. Decline unsafe rich content entirely and
|
||||
// leave ChangedFiles as the safe fallback.
|
||||
if unsafeDiffText(diff.OldText) || unsafeDiffText(diff.NewText) {
|
||||
res.Redacted = true
|
||||
continue
|
||||
}
|
||||
if scrubbed := redaction.RedactString(diff.OldText, redaction.Options{}); scrubbed != diff.OldText {
|
||||
diff.OldText = scrubbed
|
||||
res.Redacted = true
|
||||
}
|
||||
if scrubbed := redaction.RedactString(diff.NewText, redaction.Options{}); scrubbed != diff.NewText {
|
||||
diff.NewText = scrubbed
|
||||
res.Redacted = true
|
||||
}
|
||||
// Redaction can collapse two distinct credentials to the same token. At
|
||||
// this final outbound boundary, retain rich evidence only when the
|
||||
// transformed sides still describe a real transition; ChangedFiles
|
||||
// remains the safe path-only fallback.
|
||||
if diff.OldExists == diff.NewExists && diff.OldText == diff.NewText {
|
||||
continue
|
||||
}
|
||||
fileDiffs = append(fileDiffs, diff)
|
||||
}
|
||||
res.FileDiffs = fileDiffs
|
||||
// Meta values carry model-controlled strings (e.g. glob pattern, bash cwd) and
|
||||
// are forwarded into the transcript, so they are part of the boundary too.
|
||||
for key, value := range res.Meta {
|
||||
@@ -354,6 +381,13 @@ func scrubResultSecrets(res Result) Result {
|
||||
return res
|
||||
}
|
||||
|
||||
// ScrubResultSecrets applies the registry's transcript boundary to a result
|
||||
// that was produced before Registry.RunWithOptions could own it, such as a
|
||||
// local pre-permission rejection in the agent loop.
|
||||
func ScrubResultSecrets(res Result) Result {
|
||||
return scrubResultSecrets(res)
|
||||
}
|
||||
|
||||
func CoreReadOnlyToolsScoped(workspaceRoot string, scope PathScope) []Tool {
|
||||
return []Tool{
|
||||
NewScopedReadMinifiedFileTool(workspaceRoot, scope),
|
||||
|
||||
@@ -445,13 +445,120 @@ func (t denyTool) Run(context.Context, map[string]any) Result { return Result{St
|
||||
// must be scrubbed too, not just the tool-execution paths.
|
||||
func TestScrubResultSecretsRedactsPreview(t *testing.T) {
|
||||
secret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
||||
res := scrubResultSecrets(Result{Display: Display{Preview: "+++ b/x\n+token := \"" + secret + "\""}})
|
||||
res := scrubResultSecrets(Result{
|
||||
Display: Display{Preview: "+++ b/x\n+token := \"" + secret + "\""},
|
||||
FileDiffs: []FileDiff{{Path: filepath.Join(t.TempDir(), "x"), OldExists: true, NewExists: true, OldText: "before " + secret, NewText: "after " + secret}},
|
||||
})
|
||||
if strings.Contains(res.Display.Preview, secret) {
|
||||
t.Errorf("Display.Preview (the card-only code preview) must be redacted, leaked: %q", res.Display.Preview)
|
||||
}
|
||||
if !res.Redacted {
|
||||
t.Error("scrubbing a secret from the preview should set Redacted")
|
||||
}
|
||||
if len(res.FileDiffs) != 1 || strings.Contains(res.FileDiffs[0].OldText, secret) || strings.Contains(res.FileDiffs[0].NewText, secret) {
|
||||
t.Errorf("FileDiff must be redacted: %#v", res.FileDiffs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScrubResultSecretsDropsControlSplitFileDiff(t *testing.T) {
|
||||
secret := "sk-proj-abcdefghijklmnopqrstuvwxyz"
|
||||
res := scrubResultSecrets(Result{FileDiffs: []FileDiff{{
|
||||
Path: filepath.Join(t.TempDir(), "x"),
|
||||
OldExists: true,
|
||||
NewExists: true,
|
||||
OldText: "token=" + secret[:12] + "\x00" + secret[12:],
|
||||
NewText: "safe",
|
||||
}}})
|
||||
if len(res.FileDiffs) != 0 || !res.Redacted {
|
||||
t.Fatalf("unsafe FileDiff = %#v, redacted = %t", res.FileDiffs, res.Redacted)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScrubResultSecretsDropsDefaultIgnorableSplitFileDiffOnEitherSide(t *testing.T) {
|
||||
secret := "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGG"
|
||||
for name, separator := range map[string]string{
|
||||
"combining grapheme joiner": "\u034f",
|
||||
"variation selector": "\ufe0f",
|
||||
} {
|
||||
for _, side := range []string{"old", "new"} {
|
||||
t.Run(name+" "+side, func(t *testing.T) {
|
||||
obfuscated := secret[:20] + separator + secret[20:]
|
||||
diff := FileDiff{
|
||||
Path: filepath.Join(t.TempDir(), "x"), OldExists: true, NewExists: true,
|
||||
OldText: "safe old", NewText: "safe new",
|
||||
}
|
||||
if side == "old" {
|
||||
diff.OldText = obfuscated
|
||||
} else {
|
||||
diff.NewText = obfuscated
|
||||
}
|
||||
res := scrubResultSecrets(Result{FileDiffs: []FileDiff{diff}})
|
||||
if len(res.FileDiffs) != 0 || !res.Redacted {
|
||||
t.Fatalf("unsafe FileDiff = %#v, redacted = %t", res.FileDiffs, res.Redacted)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestScrubResultSecretsDoesNotMutateCallerFileDiffSlice(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "x")
|
||||
secret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
||||
retainedOld := "before token=" + secret
|
||||
retainedNew := "after token=" + secret
|
||||
original := []FileDiff{
|
||||
{Path: path, OldExists: true, NewExists: true, OldText: "token=sk-proj-abc\x00def", NewText: "unsafe"},
|
||||
{Path: path, OldExists: true, NewExists: true, OldText: retainedOld, NewText: retainedNew},
|
||||
}
|
||||
result := scrubResultSecrets(Result{FileDiffs: original})
|
||||
if len(result.FileDiffs) != 1 || strings.Contains(result.FileDiffs[0].OldText, secret) || strings.Contains(result.FileDiffs[0].NewText, secret) {
|
||||
t.Fatalf("filtered FileDiffs = %#v", result.FileDiffs)
|
||||
}
|
||||
if original[0].NewText != "unsafe" || original[1].OldText != retainedOld || original[1].NewText != retainedNew {
|
||||
t.Fatalf("caller slice was mutated: %#v", original)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScrubResultSecretsDropsSemanticallyUnchangedRedactedDiff(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "credentials.txt")
|
||||
oldSecret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
||||
newSecret := "ghp_9876543210ZYXWVUTSRQPONMLKJIHGFEDCBA"
|
||||
result := scrubResultSecrets(Result{
|
||||
ChangedFiles: []string{"credentials.txt"},
|
||||
FileDiffs: []FileDiff{{
|
||||
Path: path, OldExists: true, NewExists: true,
|
||||
OldText: "token=" + oldSecret, NewText: "token=" + newSecret,
|
||||
}},
|
||||
})
|
||||
if len(result.FileDiffs) != 0 {
|
||||
t.Fatalf("semantically unchanged redacted diff = %#v", result.FileDiffs)
|
||||
}
|
||||
if got, want := result.ChangedFiles, []string{"credentials.txt"}; !slices.Equal(got, want) {
|
||||
t.Fatalf("ChangedFiles = %#v, want %#v", got, want)
|
||||
}
|
||||
if !result.Redacted {
|
||||
t.Fatal("credential rotation must set Redacted")
|
||||
}
|
||||
}
|
||||
|
||||
func TestScrubResultSecretsKeepsRealTransitionAroundRedactedSecrets(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "credentials.txt")
|
||||
oldSecret := "ghp_ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
||||
newSecret := "ghp_9876543210ZYXWVUTSRQPONMLKJIHGFEDCBA"
|
||||
result := scrubResultSecrets(Result{FileDiffs: []FileDiff{{
|
||||
Path: path, OldExists: true, NewExists: true,
|
||||
OldText: "environment=staging token=" + oldSecret,
|
||||
NewText: "environment=production token=" + newSecret,
|
||||
}}})
|
||||
if len(result.FileDiffs) != 1 {
|
||||
t.Fatalf("redacted real transition = %#v", result.FileDiffs)
|
||||
}
|
||||
if result.FileDiffs[0].OldText == result.FileDiffs[0].NewText {
|
||||
t.Fatalf("redacted transition became unchanged: %#v", result.FileDiffs[0])
|
||||
}
|
||||
if strings.Contains(result.FileDiffs[0].OldText, oldSecret) || strings.Contains(result.FileDiffs[0].NewText, newSecret) {
|
||||
t.Fatalf("redacted transition leaked a secret: %#v", result.FileDiffs[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunWithOptionsScrubsSecretsOnDenialPaths(t *testing.T) {
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
)
|
||||
|
||||
// readRootedFile returns bytes and identity from the same opened object, and
|
||||
// verifies that the rooted path still names that object after the read. Root.Open
|
||||
// binds symlink containment to use rather than relying on a pathname pre-check.
|
||||
func readRootedFile(root *os.Root, relativePath string) ([]byte, os.FileInfo, error) {
|
||||
file, err := root.Open(relativePath)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
closed := false
|
||||
defer func() {
|
||||
if !closed {
|
||||
_ = file.Close()
|
||||
}
|
||||
}()
|
||||
openedInfo, err := file.Stat()
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
if !openedInfo.Mode().IsRegular() {
|
||||
return nil, nil, fmt.Errorf("%s is not a regular file", relativePath)
|
||||
}
|
||||
content, err := io.ReadAll(file)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
pathInfo, err := root.Stat(relativePath)
|
||||
if err != nil || !os.SameFile(openedInfo, pathInfo) {
|
||||
return nil, nil, fmt.Errorf("%w: path identity changed", errFileChangedDuringWrite)
|
||||
}
|
||||
if err := file.Close(); err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
closed = true
|
||||
return content, openedInfo, nil
|
||||
}
|
||||
@@ -86,12 +86,13 @@ type structuredPatchTarget struct {
|
||||
}
|
||||
|
||||
type structuredPatchChange struct {
|
||||
kind structuredPatchKind
|
||||
from structuredPatchTarget
|
||||
to structuredPatchTarget
|
||||
before string
|
||||
after string
|
||||
mode os.FileMode
|
||||
kind structuredPatchKind
|
||||
from structuredPatchTarget
|
||||
to structuredPatchTarget
|
||||
before string
|
||||
after string
|
||||
mode os.FileMode
|
||||
beforeInfo os.FileInfo
|
||||
}
|
||||
|
||||
// unifiedHunkRangePattern recognises a unified-diff range header ("-12,4 +12,6",
|
||||
@@ -156,8 +157,14 @@ func applyPatchOperations(applyRoot, relativeRoot string, operations []structure
|
||||
}
|
||||
}
|
||||
}
|
||||
if err := applyStructuredPatchChanges(workspace, changes, options.FileTracker); err != nil {
|
||||
return errorResult("Error applying patch: " + err.Error())
|
||||
applyOutcome, err := applyStructuredPatchChanges(workspace, relativeRoot, changes, options.FileTracker)
|
||||
if err != nil {
|
||||
result := errorResult("Error applying patch: " + err.Error())
|
||||
result.ChangedFiles = changedFilesFromStructuredPatch(relativeRoot, applyOutcome.committed)
|
||||
result.FileDiffs = fileDiffsFromStructuredPatch(relativeRoot, applyOutcome.committed)
|
||||
result.Redacted = structuredPatchContainsObfuscatedSecret(applyOutcome.committed)
|
||||
result.Display = Display{Summary: result.Output, Kind: "diff", Preview: structuredPatchPreview(applyOutcome.committed)}
|
||||
return result
|
||||
}
|
||||
|
||||
for _, change := range changes {
|
||||
@@ -183,10 +190,78 @@ func applyPatchOperations(applyRoot, relativeRoot string, operations []structure
|
||||
}
|
||||
result := okResult(summary)
|
||||
result.ChangedFiles = changedFilesFromStructuredPatch(relativeRoot, changes)
|
||||
result.FileDiffs = fileDiffsFromStructuredPatch(relativeRoot, changes)
|
||||
result.Redacted = structuredPatchContainsObfuscatedSecret(changes)
|
||||
result.Display = Display{Summary: summary, Kind: "diff", Preview: structuredPatchPreview(changes)}
|
||||
return result
|
||||
}
|
||||
|
||||
func structuredPatchContainsObfuscatedSecret(changes []structuredPatchChange) bool {
|
||||
for _, change := range changes {
|
||||
if diffTextRevealsObfuscatedSecret(change.before) || diffTextRevealsObfuscatedSecret(change.after) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func fileDiffsFromStructuredPatch(_ string, changes []structuredPatchChange) []FileDiff {
|
||||
const maxToolResultFileDiffs = 64
|
||||
diffs := make([]FileDiff, 0, len(changes)*2)
|
||||
usedBytes := 0
|
||||
appendGroup := func(group ...FileDiff) bool {
|
||||
if len(group) == 0 || len(diffs)+len(group) > maxToolResultFileDiffs {
|
||||
return false
|
||||
}
|
||||
groupBytes := 0
|
||||
for _, diff := range group {
|
||||
groupBytes += len(diff.OldText) + len(diff.NewText)
|
||||
}
|
||||
if usedBytes+groupBytes > maxToolResultFileDiffBytes {
|
||||
return false
|
||||
}
|
||||
diffs = append(diffs, group...)
|
||||
usedBytes += groupBytes
|
||||
return true
|
||||
}
|
||||
makeDiff := func(path, before, after string, oldExists, newExists bool) (FileDiff, bool) {
|
||||
return boundedFileDiff(path, before, after, oldExists, newExists)
|
||||
}
|
||||
for _, change := range changes {
|
||||
var group []FileDiff
|
||||
switch {
|
||||
case change.kind == structuredPatchDelete:
|
||||
if diff, ok := makeDiff(change.from.absolute, change.before, "", true, false); ok {
|
||||
group = append(group, diff)
|
||||
}
|
||||
case change.kind == structuredPatchAdd:
|
||||
if diff, ok := makeDiff(change.to.absolute, "", change.after, false, true); ok {
|
||||
group = append(group, diff)
|
||||
}
|
||||
case change.kind == structuredPatchCopy && change.from.absolute != change.to.absolute:
|
||||
// A copy leaves its source unchanged; the destination is a create.
|
||||
if diff, ok := makeDiff(change.to.absolute, "", change.after, false, true); ok {
|
||||
group = append(group, diff)
|
||||
}
|
||||
case change.kind == structuredPatchUpdate && change.from.absolute != change.to.absolute:
|
||||
// A move is two filesystem changes, not a destination overwrite.
|
||||
from, fromOK := makeDiff(change.from.absolute, change.before, "", true, false)
|
||||
to, toOK := makeDiff(change.to.absolute, "", change.after, false, true)
|
||||
if fromOK && toOK {
|
||||
group = append(group, from, to)
|
||||
}
|
||||
default:
|
||||
if diff, ok := makeDiff(change.to.absolute, change.before, change.after, true, true); ok {
|
||||
group = append(group, diff)
|
||||
}
|
||||
}
|
||||
if len(group) > 0 && !appendGroup(group...) {
|
||||
break
|
||||
}
|
||||
}
|
||||
return diffs
|
||||
}
|
||||
|
||||
func parseStructuredPatch(patch string) ([]structuredPatchOperation, error) {
|
||||
normalized := strings.TrimSpace(strings.TrimPrefix(strings.ReplaceAll(patch, "\r\n", "\n"), "\ufeff"))
|
||||
if normalized == "" {
|
||||
@@ -394,15 +469,12 @@ func planStructuredPatch(root *os.Root, operations []structuredPatchOperation, t
|
||||
}
|
||||
change.after = operation.contents
|
||||
case structuredPatchDelete, structuredPatchUpdate, structuredPatchCopy:
|
||||
info, err := root.Stat(from.relative)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("stating %s: %w", from.relative, err)
|
||||
}
|
||||
change.mode = info.Mode()
|
||||
content, err := root.ReadFile(from.relative)
|
||||
content, info, err := readRootedFile(root, from.relative)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("reading %s: %w", from.relative, err)
|
||||
}
|
||||
change.mode = info.Mode()
|
||||
change.beforeInfo = info
|
||||
if err := tracker.CheckConflict(from.absolute, content); err != nil {
|
||||
return nil, fmt.Errorf("%s", fileConflictMessage(from.relative))
|
||||
}
|
||||
@@ -672,38 +744,112 @@ func findStructuredPatchSequence(lines, wanted []string, start int, endOfFile bo
|
||||
return -1, false
|
||||
}
|
||||
|
||||
func applyStructuredPatchChanges(root *os.Root, changes []structuredPatchChange, tracker *FileTracker) error {
|
||||
type structuredPatchApplyOutcome struct {
|
||||
committed []structuredPatchChange
|
||||
incompletePaths []string
|
||||
}
|
||||
|
||||
type structuredPatchChangeOutcome struct {
|
||||
completed []structuredPatchChange
|
||||
incompletePaths []string
|
||||
}
|
||||
|
||||
func applyStructuredPatchChanges(root *os.Root, relativeRoot string, changes []structuredPatchChange, tracker *FileTracker) (structuredPatchApplyOutcome, error) {
|
||||
// committed lists, in order, the paths whose change reached disk before a
|
||||
// later change failed, so the caller (and the model) knows exactly which
|
||||
// files now hold the patched content and which were never touched.
|
||||
var committed []string
|
||||
var outcome structuredPatchApplyOutcome
|
||||
for _, change := range changes {
|
||||
done, err := applyStructuredPatchChange(root, change)
|
||||
if done {
|
||||
committed = append(committed, structuredPatchChangePaths(change)...)
|
||||
}
|
||||
changeOutcome, err := applyStructuredPatchChange(root, change)
|
||||
outcome.committed = append(outcome.committed, changeOutcome.completed...)
|
||||
outcome.incompletePaths = append(outcome.incompletePaths, changeOutcome.incompletePaths...)
|
||||
if err != nil {
|
||||
forgetStructuredPatchFiles(tracker, changes)
|
||||
if len(committed) > 0 {
|
||||
return fmt.Errorf("%w; patch was partially applied — already committed: %s; the remaining files are unchanged; re-read the committed files before retrying", err, strings.Join(committed, ", "))
|
||||
committedPaths := changedFilesFromStructuredPatch(relativeRoot, outcome.committed)
|
||||
unverifiedPaths := appendUniqueStructuredPatchPaths(nil, relativeRoot, outcome.incompletePaths)
|
||||
if len(committedPaths) > 0 || len(unverifiedPaths) > 0 {
|
||||
parts := []string{"patch was partially applied"}
|
||||
if len(committedPaths) > 0 {
|
||||
parts = append(parts, "already committed: "+strings.Join(committedPaths, ", "))
|
||||
}
|
||||
if len(unverifiedPaths) > 0 {
|
||||
parts = append(parts, "published but unverified: "+strings.Join(unverifiedPaths, ", "))
|
||||
}
|
||||
parts = append(parts, "the remaining files are unchanged")
|
||||
if len(unverifiedPaths) > 0 {
|
||||
parts = append(parts, "re-read the listed files before retrying")
|
||||
} else {
|
||||
parts = append(parts, "re-read the committed files before retrying")
|
||||
}
|
||||
return outcome, fmt.Errorf("%w; %s", err, strings.Join(parts, "; "))
|
||||
}
|
||||
return err
|
||||
return outcome, err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
return outcome, nil
|
||||
}
|
||||
|
||||
// structuredPatchChangePaths names the workspace-relative paths a committed
|
||||
// change touched: the destination, plus the source of a move or copy.
|
||||
func structuredPatchChangePaths(change structuredPatchChange) []string {
|
||||
if change.kind == structuredPatchDelete {
|
||||
return []string{change.from.relative}
|
||||
// completedStructuredPatchEffect turns a partially completed compound change
|
||||
// into the exact filesystem sub-effect that is still verifiable after the
|
||||
// error. In particular, a failed move may have published its destination while
|
||||
// leaving the source in place; that is a destination creation, not a move.
|
||||
func completedStructuredPatchEffect(root *os.Root, change structuredPatchChange) (structuredPatchChange, bool) {
|
||||
if change.to.relative == "" {
|
||||
return structuredPatchChange{}, false
|
||||
}
|
||||
paths := []string{change.to.relative}
|
||||
if change.from.absolute != change.to.absolute && change.from.relative != "" {
|
||||
paths = append([]string{change.from.relative}, paths...)
|
||||
content, _, err := readRootedFile(root, change.to.relative)
|
||||
if err != nil || string(content) != change.after {
|
||||
return structuredPatchChange{}, false
|
||||
}
|
||||
return paths
|
||||
switch change.kind {
|
||||
case structuredPatchAdd, structuredPatchCopy:
|
||||
return structuredPatchChange{
|
||||
kind: structuredPatchAdd,
|
||||
to: change.to,
|
||||
after: change.after,
|
||||
mode: change.mode,
|
||||
}, true
|
||||
case structuredPatchUpdate:
|
||||
if change.from.absolute != change.to.absolute {
|
||||
return structuredPatchChange{
|
||||
kind: structuredPatchAdd,
|
||||
to: change.to,
|
||||
after: change.after,
|
||||
mode: change.mode,
|
||||
}, true
|
||||
}
|
||||
}
|
||||
return structuredPatchChange{}, false
|
||||
}
|
||||
|
||||
func incompleteStructuredPatchWrite(root *os.Root, change structuredPatchChange, published bool) structuredPatchChangeOutcome {
|
||||
if !published {
|
||||
return structuredPatchChangeOutcome{}
|
||||
}
|
||||
if completed, ok := completedStructuredPatchEffect(root, change); ok {
|
||||
return structuredPatchChangeOutcome{completed: []structuredPatchChange{completed}}
|
||||
}
|
||||
if change.to.relative != "" {
|
||||
return structuredPatchChangeOutcome{incompletePaths: []string{change.to.relative}}
|
||||
}
|
||||
return structuredPatchChangeOutcome{}
|
||||
}
|
||||
|
||||
func appendUniqueStructuredPatchPaths(existing []string, relativeRoot string, paths []string) []string {
|
||||
seen := make(map[string]bool, len(existing)+len(paths))
|
||||
for _, path := range existing {
|
||||
seen[path] = true
|
||||
}
|
||||
for _, path := range paths {
|
||||
if relativeRoot != "" && relativeRoot != "." {
|
||||
path = filepath.ToSlash(filepath.Join(relativeRoot, path))
|
||||
}
|
||||
if path != "" && !seen[path] {
|
||||
seen[path] = true
|
||||
existing = append(existing, path)
|
||||
}
|
||||
}
|
||||
return existing
|
||||
}
|
||||
|
||||
func forgetStructuredPatchFiles(tracker *FileTracker, changes []structuredPatchChange) {
|
||||
@@ -723,7 +869,17 @@ func forgetStructuredPatchFiles(tracker *FileTracker, changes []structuredPatchC
|
||||
// deterministically; it is nil in production.
|
||||
var structuredPatchBeforeCommit func(change structuredPatchChange)
|
||||
|
||||
func applyStructuredPatchChange(root *os.Root, change structuredPatchChange) (bool, error) {
|
||||
// structuredPatchBeforeRename runs after a same-path update has staged and
|
||||
// closed its replacement but before the final preimage recheck and rename.
|
||||
// Tests use it to reproduce a competing writer deterministically.
|
||||
var structuredPatchBeforeRename func(change structuredPatchChange)
|
||||
|
||||
// structuredPatchRemove is a deterministic test seam for failures after a
|
||||
// move destination has been published. Production removes through the opened
|
||||
// workspace root.
|
||||
var structuredPatchRemove = func(root *os.Root, name string) error { return root.Remove(name) }
|
||||
|
||||
func applyStructuredPatchChange(root *os.Root, change structuredPatchChange) (structuredPatchChangeOutcome, error) {
|
||||
if structuredPatchBeforeCommit != nil {
|
||||
structuredPatchBeforeCommit(change)
|
||||
}
|
||||
@@ -732,39 +888,63 @@ func applyStructuredPatchChange(root *os.Root, change structuredPatchChange) (bo
|
||||
// by another process in between is refused rather than overwritten or
|
||||
// removed.
|
||||
if change.kind != structuredPatchAdd {
|
||||
current, err := root.ReadFile(change.from.relative)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("re-reading %s before commit: %w", change.from.relative, err)
|
||||
}
|
||||
if string(current) != change.before {
|
||||
return false, fmt.Errorf("%s changed on disk between planning and commit; re-read it and retry", change.from.relative)
|
||||
if err := recheckStructuredPatchPreimage(root, change); err != nil {
|
||||
return structuredPatchChangeOutcome{}, err
|
||||
}
|
||||
}
|
||||
switch change.kind {
|
||||
case structuredPatchDelete:
|
||||
if err := root.Remove(change.from.relative); err != nil {
|
||||
return false, fmt.Errorf("deleting %s: %w", change.from.relative, err)
|
||||
if err := structuredPatchRemove(root, change.from.relative); err != nil {
|
||||
return structuredPatchChangeOutcome{}, fmt.Errorf("deleting %s: %w", change.from.relative, err)
|
||||
}
|
||||
return true, nil
|
||||
return structuredPatchChangeOutcome{completed: []structuredPatchChange{change}}, nil
|
||||
case structuredPatchAdd, structuredPatchCopy:
|
||||
return writeStructuredPatchFile(root, change.to, change.after, change.mode, true)
|
||||
var beforePublish func() error
|
||||
if change.kind == structuredPatchCopy {
|
||||
beforePublish = structuredPatchPrePublishGuard(root, change)
|
||||
}
|
||||
published, err := writeStructuredPatchFile(root, change.to, change.after, change.mode, true, beforePublish)
|
||||
if err != nil {
|
||||
return incompleteStructuredPatchWrite(root, change, published), err
|
||||
}
|
||||
return structuredPatchChangeOutcome{completed: []structuredPatchChange{change}}, nil
|
||||
case structuredPatchUpdate:
|
||||
moving := change.from.absolute != change.to.absolute
|
||||
committed, err := writeStructuredPatchFile(root, change.to, change.after, change.mode, moving)
|
||||
published, err := writeStructuredPatchFile(root, change.to, change.after, change.mode, moving, structuredPatchPrePublishGuard(root, change))
|
||||
if err != nil {
|
||||
return committed, err
|
||||
return incompleteStructuredPatchWrite(root, change, published), err
|
||||
}
|
||||
if moving {
|
||||
if err := root.Remove(change.from.relative); err != nil {
|
||||
return true, fmt.Errorf("removing moved source %s: %w", change.from.relative, err)
|
||||
if err := structuredPatchRemove(root, change.from.relative); err != nil {
|
||||
return incompleteStructuredPatchWrite(root, change, true), fmt.Errorf("removing moved source %s: %w", change.from.relative, err)
|
||||
}
|
||||
}
|
||||
return true, nil
|
||||
return structuredPatchChangeOutcome{completed: []structuredPatchChange{change}}, nil
|
||||
}
|
||||
return false, fmt.Errorf("unsupported structured patch operation")
|
||||
return structuredPatchChangeOutcome{}, fmt.Errorf("unsupported structured patch operation")
|
||||
}
|
||||
|
||||
func writeStructuredPatchFile(root *os.Root, target structuredPatchTarget, content string, mode os.FileMode, createOnly bool) (bool, error) {
|
||||
func recheckStructuredPatchPreimage(root *os.Root, change structuredPatchChange) error {
|
||||
current, info, err := readRootedFile(root, change.from.relative)
|
||||
if err != nil {
|
||||
return fmt.Errorf("re-reading %s before commit: %w", change.from.relative, err)
|
||||
}
|
||||
if (change.beforeInfo != nil && !os.SameFile(change.beforeInfo, info)) || string(current) != change.before {
|
||||
return fmt.Errorf("%s changed on disk between planning and commit; re-read it and retry", change.from.relative)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func structuredPatchPrePublishGuard(root *os.Root, change structuredPatchChange) func() error {
|
||||
return func() error {
|
||||
if change.kind == structuredPatchUpdate && change.from.absolute == change.to.absolute && structuredPatchBeforeRename != nil {
|
||||
structuredPatchBeforeRename(change)
|
||||
}
|
||||
return recheckStructuredPatchPreimage(root, change)
|
||||
}
|
||||
}
|
||||
|
||||
func writeStructuredPatchFile(root *os.Root, target structuredPatchTarget, content string, mode os.FileMode, createOnly bool, beforePublish func() error) (bool, error) {
|
||||
parent := filepath.Dir(target.relative)
|
||||
if err := root.MkdirAll(parent, 0o755); err != nil {
|
||||
return false, fmt.Errorf("creating parent directory for %s: %w", target.relative, err)
|
||||
@@ -785,8 +965,13 @@ func writeStructuredPatchFile(root *os.Root, target structuredPatchTarget, conte
|
||||
if err := temp.Close(); err != nil {
|
||||
return false, fmt.Errorf("writing %s: %w", target.relative, err)
|
||||
}
|
||||
if beforePublish != nil {
|
||||
if err := beforePublish(); err != nil {
|
||||
return false, err
|
||||
}
|
||||
}
|
||||
if createOnly {
|
||||
return publishStructuredPatchNoReplace(root, tempName, target.relative, mode)
|
||||
return structuredPatchPublishNoReplace(root, tempName, target.relative, mode)
|
||||
}
|
||||
if err := root.Rename(tempName, target.relative); err != nil {
|
||||
return false, fmt.Errorf("writing %s: %w", target.relative, err)
|
||||
@@ -798,6 +983,11 @@ func publishStructuredPatchNoReplace(root *os.Root, source, target string, mode
|
||||
return publishStructuredPatchNoReplaceWith(root, source, target, mode, root.Link)
|
||||
}
|
||||
|
||||
// structuredPatchPublishNoReplace is a deterministic test seam for failures
|
||||
// after a create-only target has become visible. Production uses the atomic
|
||||
// no-replace publisher above.
|
||||
var structuredPatchPublishNoReplace = publishStructuredPatchNoReplace
|
||||
|
||||
func publishStructuredPatchNoReplaceWith(root *os.Root, source, target string, mode os.FileMode, link func(string, string) error) (bool, error) {
|
||||
if linkErr := link(source, target); linkErr == nil {
|
||||
cleanupErr := removeStructuredPatchTemp(root, source)
|
||||
@@ -914,9 +1104,19 @@ func changedFilesFromStructuredPatch(relativeRoot string, changes []structuredPa
|
||||
seen := make(map[string]bool)
|
||||
var paths []string
|
||||
for _, change := range changes {
|
||||
targets := []structuredPatchTarget{change.to}
|
||||
if change.from.absolute != change.to.absolute {
|
||||
targets = append([]structuredPatchTarget{change.from}, targets...)
|
||||
var targets []structuredPatchTarget
|
||||
switch change.kind {
|
||||
case structuredPatchDelete:
|
||||
targets = []structuredPatchTarget{change.from}
|
||||
case structuredPatchUpdate:
|
||||
targets = []structuredPatchTarget{change.to}
|
||||
if change.from.absolute != change.to.absolute {
|
||||
targets = append([]structuredPatchTarget{change.from}, targets...)
|
||||
}
|
||||
default:
|
||||
// Adds and copies mutate only their destination; the copy source is
|
||||
// evidence for the operation, not a changed file.
|
||||
targets = []structuredPatchTarget{change.to}
|
||||
}
|
||||
for _, target := range targets {
|
||||
path := target.relative
|
||||
|
||||
@@ -120,6 +120,10 @@ type Result struct {
|
||||
// entries under a granted extra write root are absolute, since
|
||||
// workspace-relative would be ambiguous there.
|
||||
ChangedFiles []string
|
||||
// FileDiffs carries exact, bounded before/after text for built-in file
|
||||
// mutations. A missing entry means the client must fall back to ChangedFiles
|
||||
// rather than inventing a partial diff.
|
||||
FileDiffs []FileDiff
|
||||
// ChangeSummaries contains bounded generated-tree changes. These are shown
|
||||
// in session evidence and the Files panel but are never treated as files to
|
||||
// open or diagnose individually.
|
||||
|
||||
@@ -12,6 +12,7 @@ type writeFileTool struct {
|
||||
baseTool
|
||||
workspaceRoot string
|
||||
scope PathScope
|
||||
readFile func(string) ([]byte, error)
|
||||
}
|
||||
|
||||
func NewScopedWriteFileTool(workspaceRoot string, scope PathScope) Tool {
|
||||
@@ -34,6 +35,7 @@ func NewScopedWriteFileTool(workspaceRoot string, scope PathScope) Tool {
|
||||
},
|
||||
workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
|
||||
scope: scope,
|
||||
readFile: os.ReadFile,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -61,8 +63,10 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
|
||||
}
|
||||
|
||||
existed := false
|
||||
if _, err := os.Stat(absolutePath); err == nil {
|
||||
var priorInfo os.FileInfo
|
||||
if info, err := os.Stat(absolutePath); err == nil {
|
||||
existed = true
|
||||
priorInfo = info
|
||||
if !overwrite {
|
||||
return errorResult("Error: " + relativePath + " already exists. Pass overwrite: true to replace it.")
|
||||
}
|
||||
@@ -82,7 +86,7 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
|
||||
// Fail CLOSED: if the tracked file can't be re-read to verify it, refuse
|
||||
// the overwrite rather than clobbering a file whose current state is
|
||||
// unknown (it may have been replaced or removed out from under us).
|
||||
current, rerr := os.ReadFile(absolutePath)
|
||||
current, rerr := tool.readFile(absolutePath)
|
||||
if rerr != nil {
|
||||
return errorResult(fileConflictMessage(relativePath))
|
||||
}
|
||||
@@ -95,9 +99,11 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
|
||||
// Capture the prior content (before we replace it) so an overwrite can show a
|
||||
// real diff; a fresh create stays "" and previews as all-additions.
|
||||
priorContent := ""
|
||||
priorContentKnown := !existed
|
||||
if existed {
|
||||
if prev, rerr := os.ReadFile(absolutePath); rerr == nil {
|
||||
if prev, rerr := tool.readFile(absolutePath); rerr == nil {
|
||||
priorContent = string(prev)
|
||||
priorContentKnown = true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -107,20 +113,36 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
|
||||
if err := recheckScopedWriteTarget(tool.workspaceRoot, tool.scope, requestedPath); err != nil {
|
||||
return errorResult("Error writing file " + relativePath + ": " + err.Error())
|
||||
}
|
||||
if err := os.WriteFile(absolutePath, []byte(content), 0o644); err != nil {
|
||||
var expectedContent *string
|
||||
if priorContentKnown {
|
||||
expectedContent = &priorContent
|
||||
}
|
||||
if err := commitFileContents(absolutePath, priorInfo, expectedContent, content); err != nil {
|
||||
return errorResult("Error writing file " + relativePath + ": " + err.Error())
|
||||
}
|
||||
modelKnownContent := content
|
||||
// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
|
||||
// FileTracker baseline: recording pre-format content would make the very
|
||||
// next edit look like an external modification and trip the conflict guard.
|
||||
formatting := maybeFormatWrittenFile(ctx, absolutePath, content)
|
||||
restoreMode := os.FileMode(0o644)
|
||||
if priorInfo != nil {
|
||||
restoreMode = priorInfo.Mode().Perm()
|
||||
}
|
||||
formatting := maybeFormatWrittenFileScoped(ctx, tool.workspaceRoot, tool.scope, absolutePath, content, restoreMode)
|
||||
content = formatting.Content
|
||||
finalContentKnown := formatting.ContentKnown
|
||||
// Baseline the freshly written content so a later edit/overwrite in this
|
||||
// session compares against what is now on disk.
|
||||
newInfo, _ := os.Stat(absolutePath)
|
||||
options.FileTracker.Record(absolutePath, []byte(content), newInfo)
|
||||
if content == modelKnownContent {
|
||||
newInfo := formatting.Info
|
||||
if newInfo == nil {
|
||||
newInfo, _ = os.Stat(absolutePath)
|
||||
}
|
||||
if finalContentKnown {
|
||||
options.FileTracker.Record(absolutePath, []byte(content), newInfo)
|
||||
} else {
|
||||
options.FileTracker.Forget(absolutePath)
|
||||
}
|
||||
if finalContentKnown && content == modelKnownContent {
|
||||
options.FileTracker.RecordSeenRange(absolutePath, 1, trackedLineTotal(content), trackedLineTotal(content))
|
||||
}
|
||||
if !existed {
|
||||
@@ -139,13 +161,28 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
|
||||
}
|
||||
summary := fmt.Sprintf("%s %s (%d lines).", verb, relativePath, lines)
|
||||
summary += formatting.notice(relativePath)
|
||||
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
|
||||
if finalContentKnown {
|
||||
summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
|
||||
}
|
||||
result := okResult(summary)
|
||||
result.ChangedFiles = []string{relativePath}
|
||||
// Do not pretend an unreadable overwrite was a creation. The write may be
|
||||
// valid, but ACP only receives an exact before/after pair we actually saw.
|
||||
if priorContentKnown && finalContentKnown {
|
||||
if diff, ok := boundedFileDiff(absolutePath, priorContent, content, existed, true); ok {
|
||||
result.FileDiffs = []FileDiff{diff}
|
||||
} else if diffTextRevealsObfuscatedSecret(priorContent) || diffTextRevealsObfuscatedSecret(content) {
|
||||
result.Redacted = true
|
||||
}
|
||||
}
|
||||
// Card-only preview: a real unified diff (all-green for a create, red/green for
|
||||
// an overwrite) on Display.Preview. Output stays the summary, so the model never
|
||||
// re-reads the file — the rich preview costs zero model tokens.
|
||||
result.Display = Display{Summary: summary, Kind: "file", Preview: boundedUnifiedDiff(relativePath, priorContent, content)}
|
||||
preview := ""
|
||||
if priorContentKnown && finalContentKnown {
|
||||
preview = boundedUnifiedDiff(relativePath, priorContent, content)
|
||||
}
|
||||
result.Display = Display{Summary: summary, Kind: "file", Preview: preview}
|
||||
return result
|
||||
}
|
||||
|
||||
|
||||
@@ -6,6 +6,8 @@ import (
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"slices"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
@@ -323,6 +325,10 @@ func TestEditFileToolReplacesExactStrings(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
path := filepath.Join(root, "code.go")
|
||||
writeTestFile(t, path, "const a = 1\nconst b = 2\n")
|
||||
path, err := filepath.EvalSymlinks(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
result := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "code.go",
|
||||
@@ -367,7 +373,12 @@ func TestEditFileToolReplacesCRLF(t *testing.T) {
|
||||
|
||||
func TestEditFileToolEmitsUnifiedDiff(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeTestFile(t, filepath.Join(root, "code.go"), "const a = 1\nconst b = 2\n")
|
||||
path := filepath.Join(root, "code.go")
|
||||
writeTestFile(t, path, "const a = 1\nconst b = 2\n")
|
||||
path, err := filepath.EvalSymlinks(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
res := NewScopedEditFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "code.go", "old_string": "const a = 1", "new_string": "const a = 42",
|
||||
})
|
||||
@@ -387,6 +398,9 @@ func TestEditFileToolEmitsUnifiedDiff(t *testing.T) {
|
||||
t.Fatalf("edit preview missing diff marker %q: %q", want, res.Display.Preview)
|
||||
}
|
||||
}
|
||||
if got := res.FileDiffs; len(got) != 1 || got[0].Path != path || !got[0].OldExists || !got[0].NewExists || got[0].OldText != "const a = 1\nconst b = 2\n" || got[0].NewText != "const a = 42\nconst b = 2\n" {
|
||||
t.Fatalf("file diffs = %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileToolEmitsAdditionsDiff(t *testing.T) {
|
||||
@@ -408,6 +422,13 @@ func TestWriteFileToolEmitsAdditionsDiff(t *testing.T) {
|
||||
if strings.Contains(res.Display.Preview, "\n-line") {
|
||||
t.Fatalf("a fresh-create diff must have no removed lines: %q", res.Display.Preview)
|
||||
}
|
||||
path, err := filepath.EvalSymlinks(filepath.Join(root, "new.txt"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := res.FileDiffs; len(got) != 1 || got[0].Path != path || got[0].OldExists || !got[0].NewExists || got[0].OldText != "" || got[0].NewText != "line one\nline two\n" {
|
||||
t.Fatalf("file diffs = %#v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileToolOverwriteEmitsRedGreenDiff(t *testing.T) {
|
||||
@@ -429,6 +450,28 @@ func TestWriteFileToolOverwriteEmitsRedGreenDiff(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteFileToolOmitsDiffWhenOverwritePreimageCannotBeRead(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
path := filepath.Join(root, "private.txt")
|
||||
writeTestFile(t, path, "before\n")
|
||||
tool := NewScopedWriteFileTool(root, nil).(writeFileTool)
|
||||
tool.readFile = func(string) ([]byte, error) { return nil, os.ErrPermission }
|
||||
registry := NewRegistry()
|
||||
registry.Register(tool)
|
||||
result := registry.RunWithOptions(context.Background(), tool.Name(), map[string]any{
|
||||
"path": "private.txt", "content": "after\n", "overwrite": true,
|
||||
}, RunOptions{PermissionGranted: true})
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("write = %s", result.Output)
|
||||
}
|
||||
if len(result.FileDiffs) != 0 {
|
||||
t.Fatalf("unreadable preimage must not produce a create-like diff: %#v", result.FileDiffs)
|
||||
}
|
||||
if got, err := os.ReadFile(path); err != nil || string(got) != "after\n" {
|
||||
t.Fatalf("written content = %q, err = %v", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEditFileToolAllowsDeletingRegions(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
path := filepath.Join(root, "notes.txt")
|
||||
@@ -584,6 +627,17 @@ func TestApplyPatchToolAppliesStructuredAddAndMove(t *testing.T) {
|
||||
if got := result.ChangedFiles; strings.Join(got, ",") != "nested/new.txt,old.txt,moved.txt" {
|
||||
t.Fatalf("ChangedFiles = %v", got)
|
||||
}
|
||||
resolvedRoot, err := filepath.EvalSymlinks(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got, want := result.FileDiffs, []FileDiff{
|
||||
{Path: filepath.Join(resolvedRoot, "nested", "new.txt"), OldExists: false, NewExists: true, OldText: "", NewText: "created\n"},
|
||||
{Path: filepath.Join(resolvedRoot, "old.txt"), OldExists: true, NewExists: false, OldText: "old\n", NewText: ""},
|
||||
{Path: filepath.Join(resolvedRoot, "moved.txt"), OldExists: false, NewExists: true, OldText: "", NewText: "moved\n"},
|
||||
}; !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("FileDiffs = %#v, want %#v", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyPatchToolStructuredPatchMatchesWhitespaceTolerantly(t *testing.T) {
|
||||
@@ -831,7 +885,7 @@ func TestStructuredPatchAddDoesNotOverwriteRacedDestination(t *testing.T) {
|
||||
after: "patch content\n", mode: 0o644,
|
||||
}
|
||||
|
||||
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
|
||||
_, err = applyStructuredPatchChanges(workspace, ".", []structuredPatchChange{change}, nil)
|
||||
if err == nil || !errors.Is(err, os.ErrExist) {
|
||||
t.Fatalf("raced add destination = %v, want os.ErrExist", err)
|
||||
}
|
||||
@@ -857,7 +911,7 @@ func TestStructuredPatchFailedDeleteDoesNotRecreateMissingFile(t *testing.T) {
|
||||
before: "removed by another writer\n", mode: 0o644,
|
||||
}
|
||||
|
||||
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
|
||||
_, err = applyStructuredPatchChanges(workspace, ".", []structuredPatchChange{change}, nil)
|
||||
if err == nil {
|
||||
t.Fatal("delete of an already removed file should fail")
|
||||
}
|
||||
@@ -894,7 +948,7 @@ func TestStructuredPatchMoveWithMissingSourceIsRefusedBeforePublishing(t *testin
|
||||
}
|
||||
defer func() { structuredPatchBeforeCommit = nil }()
|
||||
|
||||
err = applyStructuredPatchChanges(workspace, []structuredPatchChange{change}, nil)
|
||||
_, err = applyStructuredPatchChanges(workspace, ".", []structuredPatchChange{change}, nil)
|
||||
if !removed {
|
||||
t.Fatal("pre-commit hook did not run")
|
||||
}
|
||||
@@ -909,6 +963,59 @@ func TestStructuredPatchMoveWithMissingSourceIsRefusedBeforePublishing(t *testin
|
||||
}
|
||||
}
|
||||
|
||||
func TestStructuredPatchUpdatePreservesCompetingWriteBeforeRename(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
competing string
|
||||
replaceIdentity bool
|
||||
}{
|
||||
{name: "content changed", competing: "competing writer\n"},
|
||||
{name: "identity changed with same content", competing: "planned\n", replaceIdentity: true},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
targetPath := filepath.Join(root, "file.txt")
|
||||
writeTestFile(t, targetPath, "planned\n")
|
||||
patch := "*** Begin Patch\n*** Update File: file.txt\n@@\n-planned\n+patched\n*** End Patch\n"
|
||||
hookRan := false
|
||||
var competingInfo os.FileInfo
|
||||
structuredPatchBeforeRename = func(change structuredPatchChange) {
|
||||
if tc.replaceIdentity {
|
||||
tempPath := filepath.Join(root, "competing.tmp")
|
||||
writeTestFile(t, tempPath, tc.competing)
|
||||
if err := os.Rename(tempPath, targetPath); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
} else if err := os.WriteFile(targetPath, []byte(tc.competing), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var err error
|
||||
competingInfo, err = os.Stat(targetPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
hookRan = true
|
||||
}
|
||||
t.Cleanup(func() { structuredPatchBeforeRename = nil })
|
||||
|
||||
result := NewScopedApplyPatchTool(root, nil).Run(context.Background(), map[string]any{"patch": patch})
|
||||
if !hookRan {
|
||||
t.Fatal("pre-rename hook did not run")
|
||||
}
|
||||
if result.Status != StatusError || len(result.ChangedFiles) != 0 || len(result.FileDiffs) != 0 {
|
||||
t.Fatalf("raced update = status=%s changed=%#v diffs=%#v output=%q", result.Status, result.ChangedFiles, result.FileDiffs, result.Output)
|
||||
}
|
||||
if got := mustReadTestFile(t, targetPath); got != tc.competing {
|
||||
t.Fatalf("raced update overwrote competing content: %q", got)
|
||||
}
|
||||
finalInfo, err := os.Stat(targetPath)
|
||||
if err != nil || !os.SameFile(competingInfo, finalInfo) {
|
||||
t.Fatalf("raced update replaced the competing file identity: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStructuredPatchCreateOnlyFallsBackWhenHardLinksUnsupported(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
workspace, err := os.OpenRoot(root)
|
||||
@@ -978,6 +1085,45 @@ func TestStructuredPatchCopyFailureReportsSurvivingPartialTarget(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIncompleteStructuredPatchNoReplaceWriteReportsOnlyVerifiedTarget(t *testing.T) {
|
||||
for name, content := range map[string]string{
|
||||
"complete publication": "complete content\n",
|
||||
"partial publication": "partial",
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
workspace, err := os.OpenRoot(root)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer workspace.Close()
|
||||
targetPath := filepath.Join(root, "target.txt")
|
||||
writeTestFile(t, targetPath, content)
|
||||
change := structuredPatchChange{
|
||||
kind: structuredPatchAdd,
|
||||
to: structuredPatchTarget{
|
||||
requested: "target.txt",
|
||||
relative: "target.txt",
|
||||
absolute: targetPath,
|
||||
},
|
||||
after: "complete content\n",
|
||||
mode: 0o644,
|
||||
}
|
||||
|
||||
outcome := incompleteStructuredPatchWrite(workspace, change, true)
|
||||
if content == change.after {
|
||||
if len(outcome.completed) != 1 || outcome.completed[0].kind != structuredPatchAdd || len(outcome.incompletePaths) != 0 {
|
||||
t.Fatalf("verified publication outcome = %#v", outcome)
|
||||
}
|
||||
return
|
||||
}
|
||||
if len(outcome.completed) != 0 || !slices.Equal(outcome.incompletePaths, []string{"target.txt"}) {
|
||||
t.Fatalf("partial publication outcome = %#v", outcome)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStructuredPatchPartialFailureLeavesCompletedChangeAndClearsTrackedState(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
trackedPath := filepath.Join(root, "tracked.txt")
|
||||
@@ -1017,7 +1163,7 @@ func TestStructuredPatchPartialFailureLeavesCompletedChangeAndClearsTrackedState
|
||||
},
|
||||
}
|
||||
|
||||
err = applyStructuredPatchChanges(workspace, changes, tracker)
|
||||
_, err = applyStructuredPatchChanges(workspace, ".", changes, tracker)
|
||||
if err == nil || !strings.Contains(err.Error(), "partially applied") {
|
||||
t.Fatalf("second change = %v, want partial-application error", err)
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user