diff --git a/internal/agent/guardrail_test.go b/internal/agent/guardrail_test.go index d06b1cd..00e4bf1 100644 --- a/internal/agent/guardrail_test.go +++ b/internal/agent/guardrail_test.go @@ -7,6 +7,7 @@ import ( "github.com/enowdev/antares/internal/config" "github.com/enowdev/antares/internal/store" + "github.com/enowdev/antares/internal/tools" ) // The tool-call guardrail auto-continue hinges on incompleteTodos: it decides @@ -114,3 +115,25 @@ func TestAbsoluteCeilingDisabledWhenZero(t *testing.T) { t.Fatal("AbsoluteMaxToolCalls should be 0 when disabled") } } + +func TestSystemPromptRequiresSuccessfulWriteBeforeClaimingCreation(t *testing.T) { + cfg := config.Default() + cfg.Memory.Enabled = false + a := agentWithConfig(cfg) + sess := &store.Session{ID: "s", Workspace: "/workspace", Meta: store.Meta{}} + write, ok := tools.Default().Get("write_file") + if !ok { + t.Fatal("write_file is not registered") + } + prompt := a.buildSystemPrompt(context.Background(), Request{}, sess, []tools.Tool{write}) + for _, want := range []string{ + "only after write_file returns a successful result", + "user's statement that a file was created is not evidence", + "check it with read_file first", + "Do not retry the same failed write", + } { + if !strings.Contains(prompt, want) { + t.Fatalf("system prompt missing %q", want) + } + } +} diff --git a/internal/agent/prompt.go b/internal/agent/prompt.go index 6a78da2..6c55c7f 100644 --- a/internal/agent/prompt.go +++ b/internal/agent/prompt.go @@ -108,6 +108,9 @@ help them now — do not block them. b.WriteString("- read_file returns lines as `NUMBER|CONTENT`. The `|` is metadata only. When calling edit_file, copy **only** the content after `|` into old_string/new_string — never the line number. Preserve tabs and spaces exactly (do not expand tabs to spaces). Line endings are matched automatically.\n") b.WriteString("- Before every edit_file call, re-read the region you are editing (read_file with offset/limit on large files). edit_file requires an exact, unique old_string from that fresh read. After any successful edit or write, re-read before making another edit; do not reuse an older block or invent identifiers. If it reports multiple occurrences, include unique neighbouring lines or use replace_all only when every occurrence should change.\n") } + if hasTool(active, "write_file") { + b.WriteString("- File creation is complete only after write_file returns a successful result. A filename in the request, planned arguments, a diff preview, or the user's statement that a file was created is not evidence that it exists. Never turn any of those into your own confirmation. If asked whether a file exists, where it was written, or what it contains without a successful write_file result in this run, check it with read_file first and report the real result. Do not retry the same failed write; explain its actionable error or use a genuinely different valid path/content.\n") + } if hasTool(active, "vps_upload") || hasTool(active, "vps_download") || hasTool(active, "vps_run") { // Without this, models fall back to terminal rsync/scp and never use // the saved-host SFTP tools (credentials and TOFU stay unused). diff --git a/internal/tools/file.go b/internal/tools/file.go index 33bbe61..d3abaab 100644 --- a/internal/tools/file.go +++ b/internal/tools/file.go @@ -236,48 +236,57 @@ type writeFileTool struct{} func (writeFileTool) Name() string { return "write_file" } func (writeFileTool) Description() string { - return "Create or overwrite a file with the given content. Parent directories are created automatically." + return "Create or overwrite a file with the given content. Always provide content; use an empty string only to intentionally create a zero-byte file. Parent directories are created automatically." } func (writeFileTool) RequiresApproval() bool { return true } func (writeFileTool) Schema() map[string]any { return schema(map[string]any{ "path": prop("string", "Destination file path."), - "content": prop("string", "Full file content to write."), + "content": prop("string", "Complete file content. This field must be present; use an empty string only for an intentional zero-byte file."), "append": propDefault("boolean", "Append instead of overwriting.", false), }, "path", "content") } func (writeFileTool) Execute(_ context.Context, in Input) Result { var args struct { - Path string `json:"path"` - Content string `json:"content"` - Append bool `json:"append"` + Path string `json:"path"` + Content *string `json:"content"` + Append bool `json:"append"` } if err := in.Bind(&args); err != nil { return Errorf("%v", err) } + if args.Content == nil { + return Errorf("content is required; provide the complete file content (use an empty string for an intentional empty file)") + } + content := *args.Content path, err := resolveWrite(in, args.Path) if err != nil { return Errorf("%v", err) } + existed := false + if info, statErr := os.Stat(path); statErr == nil { + if info.IsDir() { + return Errorf("cannot write %s: target is a directory; provide a file path", args.Path) + } + existed = true + } else if !os.IsNotExist(statErr) { + return Errorf("cannot inspect %s: %v", args.Path, statErr) + } if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return Errorf("cannot create parent directory: %v", err) } - existed := false - if _, err := os.Stat(path); err == nil { - existed = true - } if args.Append { f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) if err != nil { return Errorf("cannot open %s: %v", args.Path, err) } defer f.Close() - if _, err := f.WriteString(args.Content); err != nil { + if _, err := f.WriteString(content); err != nil { return Errorf("cannot append to %s: %v", args.Path, err) } - } else if err := writeWithCheckpoint(in, path, []byte(args.Content), "write_file"); err != nil { + } else if err := writeWithCheckpoint(in, path, []byte(content), "write_file"); err != nil { return Errorf("cannot write %s: %v", args.Path, err) } @@ -289,9 +298,13 @@ func (writeFileTool) Execute(_ context.Context, in Input) Result { verb = "Appended to" } rel := relTo(in.Workspace, path) + lines := 0 + if content != "" { + lines = strings.Count(content, "\n") + 1 + } return Result{ - Content: fmt.Sprintf("%s %s (%d bytes, %d lines)", verb, rel, len(args.Content), strings.Count(args.Content, "\n")+1), - Meta: map[string]any{"path": rel, "bytes": len(args.Content)}, + Content: fmt.Sprintf("%s %s (%d bytes, %d lines)", verb, rel, len(content), lines), + Meta: map[string]any{"path": rel, "bytes": len(content)}, } } diff --git a/internal/tools/file_project_test.go b/internal/tools/file_project_test.go index 35c29a6..1a5cc87 100644 --- a/internal/tools/file_project_test.go +++ b/internal/tools/file_project_test.go @@ -1,7 +1,11 @@ package tools import ( + "context" + "encoding/json" + "os" "path/filepath" + "strings" "testing" ) @@ -60,3 +64,63 @@ func TestOrdinarySessionStaysConfined(t *testing.T) { t.Fatalf("ordinary read outside workspace must be refused") } } + +func TestWriteFileCreatesEmptyFileAndParents(t *testing.T) { + workspace := t.TempDir() + args, err := json.Marshal(map[string]any{"path": "nested/empty.txt", "content": ""}) + if err != nil { + t.Fatal(err) + } + result := (writeFileTool{}).Execute(context.Background(), Input{Workspace: workspace, Args: args}) + if result.IsError { + t.Fatalf("write_file: %s", result.Content) + } + if !strings.Contains(result.Content, "0 bytes, 0 lines") { + t.Fatalf("empty-file result = %q, want zero bytes and zero lines", result.Content) + } + path := filepath.Join(workspace, "nested", "empty.txt") + info, err := os.Stat(path) + if err != nil { + t.Fatalf("created empty file missing: %v", err) + } + if info.IsDir() || info.Size() != 0 { + t.Fatalf("created empty target = %+v, want a zero-byte file", info) + } +} + +func TestWriteFileRejectsDirectoryTargetWithoutMutation(t *testing.T) { + workspace := t.TempDir() + path := filepath.Join(workspace, "existing") + if err := os.Mkdir(path, 0o755); err != nil { + t.Fatal(err) + } + args, err := json.Marshal(map[string]any{"path": "existing", "content": "should not write"}) + if err != nil { + t.Fatal(err) + } + result := (writeFileTool{}).Execute(context.Background(), Input{Workspace: workspace, Args: args}) + if !result.IsError || !strings.Contains(result.Content, "target is a directory") { + t.Fatalf("directory target result = %+v, want actionable error", result) + } + entries, err := os.ReadDir(path) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Fatalf("directory was mutated: %v", entries) + } +} + +func TestWriteFileRejectsMissingContentBeforeFilesystemMutation(t *testing.T) { + workspace := t.TempDir() + result := (writeFileTool{}).Execute(context.Background(), Input{ + Workspace: workspace, + Args: []byte(`{"path":"nested/missing.txt"}`), + }) + if !result.IsError || !strings.Contains(result.Content, "content is required") { + t.Fatalf("missing-content result = %+v, want actionable error", result) + } + if _, err := os.Stat(filepath.Join(workspace, "nested")); !os.IsNotExist(err) { + t.Fatalf("missing content mutated filesystem: %v", err) + } +} diff --git a/web/src/components/chat/ToolCallCard.tsx b/web/src/components/chat/ToolCallCard.tsx index 4361c71..d0fb8a4 100644 --- a/web/src/components/chat/ToolCallCard.tsx +++ b/web/src/components/chat/ToolCallCard.tsx @@ -18,6 +18,7 @@ import { import { cn } from '@/lib/utils' import type { ToolCallView } from '@/pages/ChatPage' import { useI18n } from '@/lib/i18n' +import { isIncompleteToolCall } from '@/lib/toolCallState' type IconType = React.ComponentType<{ className?: string; weight?: 'regular' | 'fill' }> type Meta = { label: string; Icon: IconType } @@ -48,8 +49,9 @@ type Diff = { rows: DiffRow[]; added: number; removed: number } // Line-based diff (LCS) between old and new source, for the expanded edit view. function lineDiff(oldText: string, newText: string): Diff { - const a = (oldText || '').replace(/\n$/, '').split('\n') - const b = (newText || '').replace(/\n$/, '').split('\n') + if (!oldText && !newText) return { rows: [], added: 0, removed: 0 } + const a = oldText.replace(/\n$/, '').split('\n') + const b = newText.replace(/\n$/, '').split('\n') if (!oldText) { return { rows: b.map((text) => ({ type: 'add', text })), added: b.length, removed: 0 } } @@ -150,6 +152,7 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal const isDiff = isEdit || isCreate const isRead = call.name === 'read_file' const output = call.result ?? call.progress ?? '' + const incomplete = isIncompleteToolCall(call) // Only the file tools render as filename-over-directory. Other tools may also // carry a `path` arg (e.g. view_image with a URL), but for them that is just @@ -169,17 +172,32 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal // Lines read, for the collapsed read summary. const readLines = isRead && output.trim() ? output.trim().split('\n').length : 0 - const canExpand = isDiff ? diff.rows.length > 0 : (call.args && call.args !== '{}') || !!output + const canExpand = call.isError + ? !!output + : incomplete + ? true + : isDiff + ? diff.rows.length > 0 + : (call.args && call.args !== '{}') || !!output // Right-side status: a diff tally for edits, "N lines" for reads, a spinner // while running, else a short result echo. const right = call.isError ? ( - {t('chat.toolError')} + + {output.trim().split('\n')[0] || t('chat.toolError')} + + ) : incomplete ? ( + + {t('chat.toolIncomplete')} + ) : isDiff ? ( {call.running ? : null} {diff.added > 0 ? +{diff.added} : null} {diff.removed > 0 ? -{diff.removed} : null} + {!call.running && diff.added === 0 && diff.removed === 0 ? ( + + ) : null} ) : call.running ? ( @@ -199,7 +217,7 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal