From 6691ff3a1dfc1a33eb5210ac9dae75a78a4193d4 Mon Sep 17 00:00:00 2001 From: Reidho Satria Date: Thu, 13 Aug 2026 21:00:59 +0700 Subject: [PATCH 1/2] fix(tools): make file creation results reliable --- internal/agent/guardrail_test.go | 23 +++++++++ internal/agent/prompt.go | 3 ++ internal/tools/file.go | 39 ++++++++++----- internal/tools/file_project_test.go | 64 ++++++++++++++++++++++++ web/src/components/chat/ToolCallCard.tsx | 20 ++++++-- web/src/pages/ChatPage.tsx | 2 + 6 files changed, 133 insertions(+), 18 deletions(-) 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..9447da9 100644 --- a/web/src/components/chat/ToolCallCard.tsx +++ b/web/src/components/chat/ToolCallCard.tsx @@ -48,8 +48,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 } } @@ -169,17 +170,26 @@ 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 + : 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')} + ) : 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 ? ( @@ -258,7 +268,7 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal {open && canExpand ? ( - isDiff ? ( + isDiff && !call.isError ? (
{diff.rows.map((r, ri) => ( diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index 2e188d8..34aca23 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -169,6 +169,7 @@ interface SessionDetail { tool_call_id?: string tool_name?: string attachments?: string + meta?: { is_error?: boolean } | null created_at: string tokens_in: number tokens_out: number @@ -189,6 +190,7 @@ function hydrate(detail: SessionDetail): ChatMessage[] { const call = pending.get(m.tool_call_id ?? '') if (call) { call.result = m.content + call.isError = m.meta?.is_error === true call.running = false } continue From f6fc16fade35383cc6d91a4e7823cfd0c81d9e93 Mon Sep 17 00:00:00 2001 From: Reidho Satria Date: Thu, 13 Aug 2026 22:15:17 +0700 Subject: [PATCH 2/2] fix(chat): distinguish incomplete file writes --- web/src/components/chat/ToolCallCard.tsx | 21 ++++++--- web/src/lib/i18n.tsx | 2 + web/src/lib/toolCallState.test.mjs | 54 ++++++++++++++++++++++++ web/src/lib/toolCallState.ts | 48 +++++++++++++++++++++ web/src/pages/ChatPage.tsx | 30 +++---------- 5 files changed, 124 insertions(+), 31 deletions(-) create mode 100644 web/src/lib/toolCallState.test.mjs create mode 100644 web/src/lib/toolCallState.ts diff --git a/web/src/components/chat/ToolCallCard.tsx b/web/src/components/chat/ToolCallCard.tsx index 9447da9..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 } @@ -151,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 @@ -172,9 +174,11 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal const canExpand = call.isError ? !!output - : isDiff - ? diff.rows.length > 0 - : (call.args && call.args !== '{}') || !!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. @@ -182,6 +186,10 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal {output.trim().split('\n')[0] || t('chat.toolError')} + ) : incomplete ? ( + + {t('chat.toolIncomplete')} + ) : isDiff ? ( {call.running ? : null} @@ -209,7 +217,7 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal
) : null} - {output ? ( + {output || incomplete ? (

{t('chat.toolResult')} @@ -321,9 +329,10 @@ export const ToolCallCard = memo(function ToolCallCard({ call }: { call: ToolCal className={cn( 'max-h-80 overflow-auto whitespace-pre-wrap break-words rounded bg-muted/50 p-2 font-mono text-[11px]', call.isError && 'text-destructive', + incomplete && 'text-[var(--warning)]', )} > - {output} + {output || t('chat.toolIncompleteDetail')}

) : null} diff --git a/web/src/lib/i18n.tsx b/web/src/lib/i18n.tsx index 6acaaa8..eb60f95 100644 --- a/web/src/lib/i18n.tsx +++ b/web/src/lib/i18n.tsx @@ -468,6 +468,8 @@ const en = { 'chat.toolArgs': 'Arguments', 'chat.toolResult': 'Result', 'chat.toolError': 'error', + 'chat.toolIncomplete': 'incomplete', + 'chat.toolIncompleteDetail': 'This tool call did not finish; no result was recorded.', 'chat.nLines': '{n} lines', 'chat.tasks': 'Tasks', 'chat.copyCode': 'Copy code', diff --git a/web/src/lib/toolCallState.test.mjs b/web/src/lib/toolCallState.test.mjs new file mode 100644 index 0000000..8d5af5b --- /dev/null +++ b/web/src/lib/toolCallState.test.mjs @@ -0,0 +1,54 @@ +import { describe, expect, test } from 'bun:test' +import { + changedFilesFromMessages, + isIncompleteToolCall, + isSuccessfulToolCall, +} from './toolCallState.ts' + +describe('tool call completion', () => { + test('requires a non-error result before treating a call as successful', () => { + expect(isSuccessfulToolCall({ result: 'Created file' })).toBe(true) + expect(isSuccessfulToolCall({ result: 'cannot write', isError: true })).toBe(false) + expect(isSuccessfulToolCall({ running: false })).toBe(false) + }) + + test('identifies a persisted call without a result as incomplete', () => { + expect(isIncompleteToolCall({ running: false })).toBe(true) + expect(isIncompleteToolCall({ running: true })).toBe(false) + expect(isIncompleteToolCall({ result: '' })).toBe(false) + expect(isIncompleteToolCall({ result: 'cannot write', isError: true })).toBe(false) + }) +}) + +describe('changed files', () => { + test('excludes failed and incomplete writes', () => { + const files = changedFilesFromMessages([ + { + toolCalls: [ + { name: 'write_file', args: '{"path":"failed.txt"}', result: 'cannot write', isError: true }, + { name: 'write_file', args: '{"path":"interrupted.txt"}' }, + { name: 'write_file', args: '{"path":"empty.txt"}', result: '' }, + ], + }, + ]) + + expect(files).toEqual([{ path: 'empty.txt', tool: 'write_file' }]) + }) + + test('keeps only the newest successful write for each path', () => { + const files = changedFilesFromMessages([ + { toolCalls: [{ name: 'write_file', args: '{"path":"same.txt"}', result: 'Created same.txt' }] }, + { + toolCalls: [ + { name: 'edit_file', args: '{"path":"same.txt"}', result: 'Updated same.txt' }, + { name: 'edit_file', args: '{"path":"other.txt"}', result: 'Updated other.txt' }, + ], + }, + ]) + + expect(files).toEqual([ + { path: 'other.txt', tool: 'edit_file' }, + { path: 'same.txt', tool: 'edit_file' }, + ]) + }) +}) diff --git a/web/src/lib/toolCallState.ts b/web/src/lib/toolCallState.ts new file mode 100644 index 0000000..c3eb458 --- /dev/null +++ b/web/src/lib/toolCallState.ts @@ -0,0 +1,48 @@ +export interface ToolCallState { + result?: string + isError?: boolean + running?: boolean +} + +export interface FileToolCall extends ToolCallState { + name: string + args: string +} + +export interface ToolCallMessage { + toolCalls?: FileToolCall[] +} + +/** A completed result is the only proof that a tool call changed the filesystem. */ +export function isSuccessfulToolCall(call: ToolCallState): boolean { + return !call.isError && call.result !== undefined +} + +/** A persisted call without a completion record was interrupted mid-execution. */ +export function isIncompleteToolCall(call: ToolCallState): boolean { + return !call.running && call.result === undefined +} + +/** Successful write/edit calls, newest first and de-duplicated by path. */ +export function changedFilesFromMessages(messages: readonly ToolCallMessage[]): { path: string; tool: string }[] { + const seen = new Set() + const files: { path: string; tool: string }[] = [] + for (let i = messages.length - 1; i >= 0; i--) { + const calls = messages[i].toolCalls + if (!calls) continue + for (let j = calls.length - 1; j >= 0; j--) { + const call = calls[j] + if ((call.name !== 'write_file' && call.name !== 'edit_file') || !isSuccessfulToolCall(call)) continue + try { + const path = String(JSON.parse(call.args)?.path ?? '').trim() + if (path && !seen.has(path)) { + seen.add(path) + files.push({ path, tool: call.name }) + } + } catch { + /* ignore unparseable arguments */ + } + } + } + return files +} diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index 34aca23..560270f 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -27,6 +27,7 @@ import { import { copyText } from '@/lib/clipboard' import { useI18n, useTimeAgo, type MessageKey } from '@/lib/i18n' import { cn } from '@/lib/utils' +import { changedFilesFromMessages } from '@/lib/toolCallState' import { Button } from '@/components/ui/button' import { Textarea } from '@/components/ui/primitives' import { SkeletonMessage } from '@/components/ui/skeleton' @@ -56,8 +57,10 @@ export interface ToolCallView { args: string result?: string isError?: boolean - progress?: string + // A result is the completion marker. Hydrated calls without one were + // interrupted before the server recorded their outcome. running?: boolean + progress?: string } /** One part of an assistant turn, in the order it happened. */ @@ -481,30 +484,7 @@ export default function ChatPage() { return [...by.values()].sort((a, b) => (b.last ?? '').localeCompare(a.last ?? '') || b.count - a.count) }, [messages]) - // Files the agent wrote/edited this session, newest first and de-duplicated — - // drives the sidebar's Changes tab. Parsed from write_file/edit_file calls. - const changedFiles = useMemo(() => { - const seen = new Set() - const out: { path: string; tool: string }[] = [] - for (let i = messages.length - 1; i >= 0; i--) { - const calls = messages[i].toolCalls - if (!calls) continue - for (let j = calls.length - 1; j >= 0; j--) { - const c = calls[j] - if (c.name !== 'write_file' && c.name !== 'edit_file') continue - try { - const path = String(JSON.parse(c.args)?.path ?? '').trim() - if (path && !seen.has(path)) { - seen.add(path) - out.push({ path, tool: c.name }) - } - } catch { - /* ignore unparseable args */ - } - } - } - return out - }, [messages]) + const changedFiles = useMemo(() => changedFilesFromMessages(messages), [messages]) const abortRef = useRef<(() => void) | null>(null) // The bound project dir tracked in a ref, so a turn fired immediately after