diff --git a/cmd/sin-code/chat_tools.go b/cmd/sin-code/chat_tools.go index 8e8dd768..b5285cde 100644 --- a/cmd/sin-code/chat_tools.go +++ b/cmd/sin-code/chat_tools.go @@ -16,6 +16,7 @@ import ( "strings" "time" + internal "github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal" "github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/agentloop" "github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/meta" "github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/sandbox" @@ -32,13 +33,14 @@ const ( // and network calls. Production defaults point to the real implementations. var ( toolReadFn = toolRead - toolWriteFn = toolWrite - toolEditFn = toolEdit - toolApplyDiffFn = toolApplyDiff - toolGenerateDiffFn = toolGenerateDiff - toolBashFn = toolBash - toolSearchFn = toolSearch - toolBootstrapSkillFn = toolBootstrapSkill + toolWriteFn = toolWrite + toolEditFn = toolEdit + toolReplaceFn = toolReplace + toolApplyDiffFn = toolApplyDiff + toolGenerateDiffFn = toolGenerateDiff + toolBashFn = toolBash + toolSearchFn = toolSearch + toolBootstrapSkillFn = toolBootstrapSkill toolSearchWalkErrFn = func(_ string, err error) error { return nil } metaBootstrapSkillFn = meta.BootstrapSkill ) @@ -57,13 +59,15 @@ func builtinSpecs() []agentloopToolSpecAlias { InputSchema: obj(map[string]any{"path": str("file path")}, "path")}, {Name: "sin_write", Description: "Atomically write content to a file, creating parent dirs.", InputSchema: obj(map[string]any{"path": str("file path"), "content": str("full file content")}, "path", "content")}, - {Name: "sin_edit", Description: "Replace the first exact occurrence of old with new in a file.", + {Name: "sin_edit", Description: "Surgical file edit: replace the first exact occurrence of old with new in a file. Fails if old is ambiguous; use sin_replace for a naive replacement.", + InputSchema: obj(map[string]any{"path": str("file path"), "old": str("exact text to replace"), "new": str("replacement text")}, "path", "old", "new")}, + {Name: "sin_replace", Description: "Naive string replacement: replace the first exact occurrence of old with new in a file (backward-compatible).", InputSchema: obj(map[string]any{"path": str("file path"), "old": str("exact text to replace"), "new": str("replacement text")}, "path", "old", "new")}, {Name: "sin_apply_diff", Description: "Apply a unified diff to a file. Validates each hunk before applying and reports applied/rejected hunks. (issue #365)", InputSchema: obj(map[string]any{"path": str("file path"), "diff": str("unified diff string")}, "path", "diff")}, {Name: "sin_generate_diff", Description: "Generate a unified diff from old and new content. (issue #365)", InputSchema: obj(map[string]any{"old_content": str("original content"), "new_content": str("updated content")}, "old_content", "new_content")}, - {Name: "sin_bash", Description: "Run a shell command in the workspace (120s timeout).", + {Name: "sin_bash", Description: "Run a shell command in the workspace (120s timeout).", InputSchema: obj(map[string]any{"command": str("shell command")}, "command")}, {Name: "sin_search", Description: "Search files for a substring; returns file:line matches.", InputSchema: obj(map[string]any{"pattern": str("substring to search"), "dir": str("directory (default .)")}, "pattern")}, @@ -84,6 +88,8 @@ func builtinTool(ctx context.Context, workspace, name string, args map[string]an return toolWriteFn(argStr(args, "path"), argStr(args, "content")) case "sin_edit": return toolEditFn(argStr(args, "path"), argStr(args, "old"), argStr(args, "new")) + case "sin_replace": + return toolReplaceFn(argStr(args, "path"), argStr(args, "old"), argStr(args, "new")) case "sin_apply_diff": return toolApplyDiffFn(argStr(args, "path"), argStr(args, "diff")) case "sin_generate_diff": @@ -185,19 +191,31 @@ func toolEdit(path, old, new string) (string, error) { if path == "" || old == "" { return "", fmt.Errorf("sin_edit: path and old required") } + if err := internal.EditByString(path, old, new); err != nil { + return "", fmt.Errorf("sin_edit: %w", err) + } + result := "edited " + path + result += maybeGenerateTest(path) + return result, nil +} + +func toolReplace(path, old, new string) (string, error) { + if path == "" || old == "" { + return "", fmt.Errorf("sin_replace: path and old required") + } data, err := os.ReadFile(path) if err != nil { return "", err } content := string(data) if !strings.Contains(content, old) { - return "", fmt.Errorf("sin_edit: old text not found in %s", path) + return "", fmt.Errorf("sin_replace: old text not found in %s", path) } updated := strings.Replace(content, old, new, 1) if err := os.WriteFile(path, []byte(updated), 0o644); err != nil { return "", err } - result := "edited " + path + result := "replaced " + path result += maybeGenerateTest(path) return result, nil } @@ -235,28 +253,44 @@ var sandboxConfig struct { func setSandboxConfig(backend, workspace string) { sandboxConfig.workspace = workspace - if backend == "none" { sandboxConfig.enabled = false } else { sandboxConfig.enabled = true } + if backend == "none" { + sandboxConfig.enabled = false + } else { + sandboxConfig.enabled = true + } } func toolBash(ctx context.Context, command string) (string, error) { - if command == "" { return "", fmt.Errorf("sin_bash: command required") } + if command == "" { + return "", fmt.Errorf("sin_bash: command required") + } cctx, cancel := context.WithTimeout(ctx, bashTimeout) defer cancel() if sandboxConfig.enabled && sandboxConfig.workspace != "" { policy := sandbox.DefaultPolicy(sandboxConfig.workspace, os.TempDir()) cmd, _, err := sandbox.Command(cctx, policy, "sh", "-c", command) - if err != nil { return "", fmt.Errorf("sin_bash sandbox: %v", err) } + if err != nil { + return "", fmt.Errorf("sin_bash sandbox: %v", err) + } out, err := cmd.CombinedOutput() text := string(out) - if len(text) > maxToolOutput { text = text[:maxToolOutput] + "\n[... truncated]" } - if err != nil { return fmt.Sprintf("exit error: %v\n%s", err, text), nil } + if len(text) > maxToolOutput { + text = text[:maxToolOutput] + "\n[... truncated]" + } + if err != nil { + return fmt.Sprintf("exit error: %v\n%s", err, text), nil + } return text, nil } cmd := exec.CommandContext(cctx, "sh", "-c", command) out, err := cmd.CombinedOutput() text := string(out) - if len(text) > maxToolOutput { text = text[:maxToolOutput] + "\n[... truncated]" } - if err != nil { return fmt.Sprintf("exit error: %v\n%s", err, text), nil } + if len(text) > maxToolOutput { + text = text[:maxToolOutput] + "\n[... truncated]" + } + if err != nil { + return fmt.Sprintf("exit error: %v\n%s", err, text), nil + } return text, nil } diff --git a/cmd/sin-code/chat_tools_test.go b/cmd/sin-code/chat_tools_test.go index e763d450..70f0aa52 100644 --- a/cmd/sin-code/chat_tools_test.go +++ b/cmd/sin-code/chat_tools_test.go @@ -297,3 +297,65 @@ func TestToolPropertyNoTests(t *testing.T) { t.Fatalf("expected status in output, got: %s", out) } } + +func TestToolEditRejectsAmbiguousOldString(t *testing.T) { + autoGenerateTests = false + dir := t.TempDir() + path := filepath.Join(dir, "file.txt") + if err := os.WriteFile(path, []byte("foo foo\n"), 0o644); err != nil { + t.Fatal(err) + } + _, err := toolEdit(path, "foo", "bar") + if err == nil { + t.Fatal("expected ambiguous old string to fail") + } + if !strings.Contains(err.Error(), "matches 2 times") { + t.Fatalf("expected ambiguity error, got %v", err) + } +} + +func TestToolEditReplacesUniqueString(t *testing.T) { + autoGenerateTests = false + dir := t.TempDir() + path := filepath.Join(dir, "file.txt") + if err := os.WriteFile(path, []byte("hello world\n"), 0o644); err != nil { + t.Fatal(err) + } + out, err := toolEdit(path, "world", "universe") + if err != nil { + t.Fatalf("toolEdit: %v", err) + } + if !strings.Contains(out, "edited") { + t.Fatalf("expected edited message, got %q", out) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(data) != "hello universe\n" { + t.Fatalf("expected replacement, got %q", data) + } +} + +func TestToolReplaceNaiveFirstOccurrence(t *testing.T) { + autoGenerateTests = false + dir := t.TempDir() + path := filepath.Join(dir, "file.txt") + if err := os.WriteFile(path, []byte("foo foo\n"), 0o644); err != nil { + t.Fatal(err) + } + out, err := toolReplace(path, "foo", "bar") + if err != nil { + t.Fatalf("toolReplace: %v", err) + } + if !strings.Contains(out, "replaced") { + t.Fatalf("expected replaced message, got %q", out) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(data) != "bar foo\n" { + t.Fatalf("expected first occurrence replacement, got %q", data) + } +} diff --git a/cmd/sin-code/internal/edit.go b/cmd/sin-code/internal/edit.go index b26d6934..f3b73ce6 100644 --- a/cmd/sin-code/internal/edit.go +++ b/cmd/sin-code/internal/edit.go @@ -127,6 +127,23 @@ type editResult struct { Diff string `json:"diff"` } +// EditByString performs a single string-mode surgical edit, replacing the first +// exact occurrence of old with new in path. It is exported so the chat tool can +// share the same engine as the MCP sin_edit tool (issue #373). +func EditByString(path, old, new string) error { + if path == "" || old == "" { + return fmt.Errorf("edit: path and old string required") + } + _, err := applyEdit(path, editRequest{ + OldString: old, + NewString: new, + ReplaceAll: false, + Validate: true, + Drift: DefaultDriftWindow, + }) + return err +} + func applyEdit(path string, req editRequest) (*editResult, error) { anchorMode := req.Anchor != "" stringMode := req.OldString != "" diff --git a/cmd/sin-code/internal/permission_defaults.go b/cmd/sin-code/internal/permission_defaults.go index 97b2181d..c3223fff 100644 --- a/cmd/sin-code/internal/permission_defaults.go +++ b/cmd/sin-code/internal/permission_defaults.go @@ -14,7 +14,8 @@ func DefaultPermissionRules() []permission.Rule { {Tool: "sin_read", Policy: "allow"}, {Tool: "sin_write", Policy: "allow"}, {Tool: "sin_edit", Policy: "allow"}, - {Tool: "sin_apply_diff", Policy: "allow"}, // v3.23.0: unified diff editor (issue #365) + {Tool: "sin_replace", Policy: "allow"}, // v3.23.0: naive string replacement (issue #373) + {Tool: "sin_apply_diff", Policy: "allow"}, // v3.23.0: unified diff editor (issue #365) {Tool: "sin_generate_diff", Policy: "allow"}, // v3.23.0: diff generator (issue #365) {Tool: "sin_test", Policy: "allow"}, {Tool: "sin_quality_gate", Policy: "allow"}, // v3.21.0: Test-First Verify-Loop (RFC-test-automation)