diff --git a/cmd/gateway_builtin_tools.go b/cmd/gateway_builtin_tools.go index 548fe1bc..04c1e92b 100644 --- a/cmd/gateway_builtin_tools.go +++ b/cmd/gateway_builtin_tools.go @@ -90,6 +90,7 @@ func builtinToolSeedData() []store.BuiltinToolDef { // messaging {Name: "message", DisplayName: "Message", Description: "Send a proactive message to a user on a connected channel (Telegram, Discord, etc.)", Category: "messaging", Enabled: true}, + {Name: "send_file", DisplayName: "Send File", Description: "Send an existing workspace file as an attachment in the current chat (does not create or modify the file)", Category: "messaging", Enabled: true}, // scheduling {Name: "cron", DisplayName: "Cron Scheduler", Description: "Schedule or manage recurring tasks using cron expressions, at-times, or intervals", Category: "scheduling", Enabled: true, diff --git a/cmd/gateway_setup.go b/cmd/gateway_setup.go index dfa3fdaf..0d270f1a 100644 --- a/cmd/gateway_setup.go +++ b/cmd/gateway_setup.go @@ -273,6 +273,11 @@ func setupToolRegistry( t.DenyPaths(internalDenyPaths...) } } + if sf, ok := toolsReg.Get("send_file"); ok { + if t, ok := sf.(*tools.SendFileTool); ok { + t.DenyPaths(internalDenyPaths...) + } + } return } diff --git a/cmd/gateway_tools_wiring.go b/cmd/gateway_tools_wiring.go index 192255c4..b385cac0 100644 --- a/cmd/gateway_tools_wiring.go +++ b/cmd/gateway_tools_wiring.go @@ -49,9 +49,11 @@ func wireExtraTools( // Message tool (send to channels) toolsReg.Register(tools.NewMessageTool(workspace, agentCfg.RestrictToWorkspace)) + // Send file tool (deliver existing workspace file as attachment) + toolsReg.Register(tools.NewSendFileTool(workspace, agentCfg.RestrictToWorkspace)) // Group members tool (list members in group chats) toolsReg.Register(tools.NewListGroupMembersTool()) - slog.Info("session + message tools registered") + slog.Info("session + message + send_file tools registered") // Register legacy tool aliases (backward-compat names from policy.go). for alias, canonical := range tools.LegacyToolAliases() { @@ -117,6 +119,12 @@ func wireExtraTools( pa.AllowPaths(userAllowPaths...) } } + if sendFileTool, ok := toolsReg.Get("send_file"); ok { + if pa, ok := sendFileTool.(tools.PathAllowable); ok { + pa.AllowPaths(skillsAllowPaths...) + pa.AllowPaths(userAllowPaths...) + } + } // Memory tools are PG-backed; always available. hasMemory = true diff --git a/docs/03-tools-system.md b/docs/03-tools-system.md index 5280efc9..7f11ab60 100644 --- a/docs/03-tools-system.md +++ b/docs/03-tools-system.md @@ -134,6 +134,7 @@ Memory layers: L1 (`memory_search`) returns ranked abstracts; L2 (`memory_expand | Tool | Description | |---|---| | `message` | Send a message to a channel | +| `send_file` | Send an existing workspace file as a chat attachment (with optional caption); marks `DeliveredMedia` to prevent duplicate delivery | | `create_forum_topic` | Create a Telegram forum topic | | `list_group_members` | List members in a group chat (Feishu/Lark) | @@ -313,7 +314,7 @@ flowchart TD | Group | Members | |---|---| -| `fs` | `read_file`, `write_file`, `list_files`, `edit` | +| `fs` | `read_file`, `write_file`, `list_files`, `edit`, `send_file` | | `runtime` | `exec` | | `web` | `web_search`, `web_fetch` | | `memory` | `memory_search`, `memory_get` | diff --git a/docs/project-changelog.md b/docs/project-changelog.md index 388bc30e..4fb1c6fa 100644 --- a/docs/project-changelog.md +++ b/docs/project-changelog.md @@ -35,6 +35,18 @@ Implementation is evidence-backed against the native ChatGPT Responses API event --- +## 2026-04-20 + +### Tools: `send_file` — explicit workspace file delivery + +**Features** + +- **`send_file` tool** (`internal/tools/send_file.go`): dedicated tool for sending existing workspace files as chat attachments. Takes `path` (required) and `caption` (optional). Replaces implicit `message(MEDIA:path)` convention for re-delivering already-created files. Marks `DeliveredMedia` on success to prevent duplicate delivery. +- **`DeliveredMedia` mark on `message(MEDIA:)` sends** (`internal/tools/message.go`): patched to call `IsDelivered` / mark after successful MEDIA upload — closes the cross-tool duplicate-delivery gap where a file sent via `message(MEDIA:)` was not tracked and could be re-sent by `send_file`. +- Registered as builtin tool in `cmd/gateway_tools_wiring.go` and seeded in `cmd/gateway_builtin_tools.go`. + +--- + ## 2026-04-19 ### TTS: Gemini provider + ProviderCapabilities schema engine diff --git a/internal/agent/systemprompt.go b/internal/agent/systemprompt.go index 4a299f0a..031bfc1a 100644 --- a/internal/agent/systemprompt.go +++ b/internal/agent/systemprompt.go @@ -175,7 +175,8 @@ func (cfg SystemPromptConfig) sectionContent(id string, defaultFn func() []strin // Shown in the ## Tooling section of the system prompt. var coreToolSummaries = map[string]string{ "read_file": "Read file contents — only accesses your agent workspace. For docs returned by vault_search (shared/personal/team vault), use vault_read instead", - "write_file": "Create or overwrite files", + "write_file": "Create or overwrite files (set deliver=true to also send as chat attachment)", + "send_file": "Send an EXISTING workspace file as a chat attachment — use to resend/share files; does NOT create or modify the file (use write_file for that)", "list_files": "List directory contents", "exec": "Run shell commands", "memory_search": "Search indexed memory files (MEMORY.md + memory/*.md)", diff --git a/internal/tools/filesystem_write_deliver_test.go b/internal/tools/filesystem_write_deliver_test.go new file mode 100644 index 00000000..0116074b --- /dev/null +++ b/internal/tools/filesystem_write_deliver_test.go @@ -0,0 +1,216 @@ +package tools + +// Characterization tests for write_file deliver=true/false behavior. +// These pin the delivery chain side-effects so phase 03 (send_file tool) cannot +// silently regress Result.Media population or DeliveredMedia.Mark() call. +// +// Gap noted: message.go (line 123) only READS IsDelivered — it does NOT call Mark. +// Mark is called exclusively by write_file (filesystem_write.go:236, 281). +// Phase 03 will add send_file which must also call Mark. Tests here lock that contract. + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/bus" +) + +// TestWriteFileDeliverTrue_PopulatesResultMedia asserts that write_file with +// deliver=true sets Result.Media with the resolved path and filename. +func TestWriteFileDeliverTrue_PopulatesResultMedia(t *testing.T) { + workspace := t.TempDir() + workspaceCanonical, _ := filepath.EvalSymlinks(workspace) + + tool := NewWriteFileTool(workspaceCanonical, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": "report.csv", + "content": "col1,col2\n1,2\n", + "deliver": true, + }) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if len(result.Media) != 1 { + t.Fatalf("expected 1 Media entry, got %d", len(result.Media)) + } + gotPath := result.Media[0].Path + wantPath := filepath.Join(workspaceCanonical, "report.csv") + if gotPath != wantPath { + t.Errorf("Media[0].Path = %q, want %q", gotPath, wantPath) + } + if result.Media[0].Filename != "report.csv" { + t.Errorf("Media[0].Filename = %q, want %q", result.Media[0].Filename, "report.csv") + } +} + +// TestWriteFileDeliverTrue_MarksDeliveredMedia asserts that write_file with +// deliver=true calls dm.Mark(resolved) so message tool's self-send guard can detect it. +func TestWriteFileDeliverTrue_MarksDeliveredMedia(t *testing.T) { + workspace := t.TempDir() + workspaceCanonical, _ := filepath.EvalSymlinks(workspace) + + tool := NewWriteFileTool(workspaceCanonical, true) + + dm := NewDeliveredMedia() + ctx := WithDeliveredMedia(context.Background(), dm) + + result := tool.Execute(ctx, map[string]any{ + "path": "output.pdf", + "content": "%PDF-1.4", + "deliver": true, + }) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + + resolvedPath := filepath.Join(workspaceCanonical, "output.pdf") + if !dm.IsDelivered(resolvedPath) { + t.Errorf("expected dm.IsDelivered(%q) = true after write_file deliver=true, got false", resolvedPath) + } +} + +// TestWriteFileDeliverFalse_NoMediaNoMark asserts that write_file with +// deliver=false leaves Result.Media empty and does NOT call dm.Mark. +func TestWriteFileDeliverFalse_NoMediaNoMark(t *testing.T) { + workspace := t.TempDir() + workspaceCanonical, _ := filepath.EvalSymlinks(workspace) + + tool := NewWriteFileTool(workspaceCanonical, true) + + dm := NewDeliveredMedia() + ctx := WithDeliveredMedia(context.Background(), dm) + + result := tool.Execute(ctx, map[string]any{ + "path": "temp.json", + "content": `{"key":"val"}`, + "deliver": false, + }) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if len(result.Media) != 0 { + t.Errorf("expected 0 Media entries for deliver=false, got %d", len(result.Media)) + } + + resolvedPath := filepath.Join(workspaceCanonical, "temp.json") + if dm.IsDelivered(resolvedPath) { + t.Errorf("expected dm.IsDelivered(%q) = false for deliver=false, got true", resolvedPath) + } +} + +// TestWriteFileDeliverDefault_IsTrue asserts that omitting the deliver arg +// defaults to deliver=true (per filesystem_write.go:110). +func TestWriteFileDeliverDefault_IsTrue(t *testing.T) { + workspace := t.TempDir() + workspaceCanonical, _ := filepath.EvalSymlinks(workspace) + + tool := NewWriteFileTool(workspaceCanonical, true) + + dm := NewDeliveredMedia() + ctx := WithDeliveredMedia(context.Background(), dm) + + // No "deliver" key in args — default should be true. + result := tool.Execute(ctx, map[string]any{ + "path": "data.txt", + "content": "hello", + }) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if len(result.Media) != 1 { + t.Errorf("expected 1 Media entry when deliver omitted (default true), got %d", len(result.Media)) + } + + resolvedPath := filepath.Join(workspaceCanonical, "data.txt") + if !dm.IsDelivered(resolvedPath) { + t.Errorf("expected dm.IsDelivered(%q) = true when deliver omitted, got false", resolvedPath) + } +} + +// TestWriteFileThenMessageBlocked characterizes the full delivery chain: +// write_file(deliver=true) → message(MEDIA:same_path) → blocked. +// This is the key regression guard for phase 03 — if send_file breaks Mark(), +// the message block will stop working. +func TestWriteFileThenMessageBlocked(t *testing.T) { + workspace := t.TempDir() + workspaceCanonical, _ := filepath.EvalSymlinks(workspace) + + writeTool := NewWriteFileTool(workspaceCanonical, true) + msgTool := NewMessageTool(workspaceCanonical, true) + msgTool.SetMessageBus(nil) // no bus — we only care about the block error + + dm := NewDeliveredMedia() + ctx := context.Background() + ctx = WithDeliveredMedia(ctx, dm) + ctx = WithToolChannel(ctx, "telegram") + ctx = WithToolChatID(ctx, "chat-42") + + // Step 1: write_file delivers the file and marks it. + writeResult := writeTool.Execute(ctx, map[string]any{ + "path": "invoice.pdf", + "content": "%PDF-1.4 body", + "deliver": true, + }) + if writeResult.IsError { + t.Fatalf("write_file failed: %s", writeResult.ForLLM) + } + + // Step 2: message(MEDIA:path) for same file must be blocked. + resolvedPath := filepath.Join(workspaceCanonical, "invoice.pdf") + msgResult := msgTool.Execute(ctx, map[string]any{ + "action": "send", + "channel": "telegram", + "target": "chat-42", + "message": "MEDIA:" + resolvedPath, + }) + if !msgResult.IsError { + t.Fatal("expected message(MEDIA:path) to be blocked after write_file deliver=true, but it was allowed") + } +} + +// TestMessageMediaMarksDelivered verifies that message(MEDIA:path) calls dm.Mark() +// on the file it sends (patched in phase 03). This closes the cross-tool duplicate +// gap: send_file after message(MEDIA:) now correctly detects the duplicate. +func TestMessageMediaMarksDelivered(t *testing.T) { + workspace := t.TempDir() + workspaceCanonical, _ := filepath.EvalSymlinks(workspace) + + // Create a real file so MEDIA: resolution succeeds. + filePath := filepath.Join(workspaceCanonical, "attachment.csv") + if err := os.WriteFile(filePath, []byte("a,b\n1,2\n"), 0o644); err != nil { + t.Fatal(err) + } + + msgTool := NewMessageTool(workspaceCanonical, true) + // Need a message bus for sendMedia to proceed past the nil-bus guard. + msgTool.SetMessageBus(bus.New()) + + dm := NewDeliveredMedia() + ctx := context.Background() + ctx = WithDeliveredMedia(ctx, dm) + // Unbound session (no channel/chatID) — MEDIA send is allowed. + + result := msgTool.Execute(ctx, map[string]any{ + "action": "send", + "channel": "telegram", + "target": "chat-99", + "message": "MEDIA:" + filePath, + }) + + // message(MEDIA:) should succeed and mark the file as delivered. + if result.IsError { + t.Fatalf("expected message(MEDIA:path) to succeed, got error: %s", result.ForLLM) + } + if !dm.IsDelivered(filePath) { + t.Errorf("expected dm.IsDelivered(%q) = true after message(MEDIA:path), got false — "+ + "check message.go sendMedia: dm.Mark call missing", filePath) + } +} diff --git a/internal/tools/message.go b/internal/tools/message.go index 2d8ebba1..b0a6eca0 100644 --- a/internal/tools/message.go +++ b/internal/tools/message.go @@ -200,6 +200,12 @@ func (t *MessageTool) Execute(ctx context.Context, args map[string]any) *Result outMsg.Metadata = map[string]string{"group_id": target} } t.msgBus.PublishOutbound(outMsg) + // Mark each embedded media path as delivered. + if dm := DeliveredMediaFromCtx(ctx); dm != nil { + for _, att := range embeddedMedia { + dm.Mark(att.URL) + } + } return noticeOnSuccess(SilentResult(fmt.Sprintf(`{"status":"sent","channel":"%s","target":"%s"}`, channel, target))) } @@ -287,6 +293,10 @@ func (t *MessageTool) sendMedia(ctx context.Context, channel, target, filePath s Media: []bus.MediaAttachment{{URL: filePath, ContentType: mimeFromPath(filePath)}}, Metadata: meta, }) + // Mark delivered so subsequent send_file or message(MEDIA:) calls detect the duplicate. + if dm := DeliveredMediaFromCtx(ctx); dm != nil { + dm.Mark(filePath) + } out, _ := json.Marshal(map[string]string{ "status": "sent", "channel": channel, diff --git a/internal/tools/send_file.go b/internal/tools/send_file.go new file mode 100644 index 00000000..8057723e --- /dev/null +++ b/internal/tools/send_file.go @@ -0,0 +1,125 @@ +package tools + +import ( + "context" + "fmt" + "os" + "path/filepath" + + "github.com/nextlevelbuilder/goclaw/internal/bus" +) + +// SendFileTool delivers an existing workspace file as a media attachment in the +// current chat session. It does NOT create or modify files — use write_file for that. +type SendFileTool struct { + workspace string + restrict bool + allowedPrefixes []string + deniedPrefixes []string // path prefixes to deny access to (e.g. memory.db, config.json) +} + +// NewSendFileTool creates a SendFileTool bound to the given workspace. +func NewSendFileTool(workspace string, restrict bool) *SendFileTool { + return &SendFileTool{workspace: workspace, restrict: restrict} +} + +// AllowPaths adds extra path prefixes that bypass restrict=true workspace boundary. +// Implements PathAllowable for consistent wiring with read_file, write_file, edit. +func (t *SendFileTool) AllowPaths(prefixes ...string) { + t.allowedPrefixes = append(t.allowedPrefixes, prefixes...) +} + +// DenyPaths adds path prefixes that send_file must reject (e.g. internal DB files). +// Implements PathDenyable for consistent wiring with read_file, write_file, edit, list_files. +func (t *SendFileTool) DenyPaths(prefixes ...string) { + t.deniedPrefixes = append(t.deniedPrefixes, prefixes...) +} + +func (t *SendFileTool) Name() string { return "send_file" } + +func (t *SendFileTool) Description() string { + return "Send an existing workspace file as an attachment in the current chat. " + + "Use when the user asks to share or resend a file that already exists. " + + "Does NOT create or modify the file — use write_file(deliver=true) to create and send a new file." +} + +func (t *SendFileTool) Parameters() map[string]any { + return map[string]any{ + "type": "object", + "properties": map[string]any{ + "path": map[string]any{ + "type": "string", + "description": "Path to the file to send (relative to workspace, or absolute)", + }, + "caption": map[string]any{ + "type": "string", + "description": "Optional text message accompanying the file", + }, + }, + "required": []string{"path"}, + } +} + +// Execute resolves and validates the path, checks for duplicate delivery, then +// returns a Result with Media populated for downstream pipeline delivery. +func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *Result { + path := argString(args, "path") + if path == "" { + return ErrorResult("path is required") + } + + // Per-request workspace (multi-tenant: each user has own workspace in context). + workspace := ToolWorkspaceFromCtx(ctx) + if workspace == "" { + workspace = t.workspace + } + + // Resolve path with allowed-prefixes support (mirrors write_file pattern). + allowed := allowedWithTeamWorkspace(ctx, t.allowedPrefixes) + resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, t.restrict), allowed) + if err != nil { + return ErrorResult("cannot access path: " + err.Error()) + } + + // Deny-paths guard: reject access to internal files (memory.db, config.json, etc.). + if err := checkDeniedPath(resolved, workspace, t.deniedPrefixes); err != nil { + return ErrorResult(err.Error()) + } + + // Stat: file must exist and be a regular file (not a directory or device). + fi, err := os.Stat(resolved) + if err != nil { + return ErrorResult(fmt.Sprintf("file not found: %s", path)) + } + if !fi.Mode().IsRegular() { + return ErrorResult(fmt.Sprintf("path is not a regular file: %s", path)) + } + + // Duplicate-delivery guard: block if already delivered in this turn. + if dm := DeliveredMediaFromCtx(ctx); dm != nil && dm.IsDelivered(resolved) { + return ErrorResult(fmt.Sprintf( + "file already delivered in this turn: %s. Do not re-send the same file. "+ + "If user explicitly asked to resend, the next turn will reset delivery state.", + filepath.Base(resolved))) + } + + // Build result — caption overrides default message if provided. + filename := filepath.Base(resolved) + msg := fmt.Sprintf("Sent file: %s", filename) + if caption := argString(args, "caption"); caption != "" { + msg = caption + } + result := SilentResult(msg) + result.Media = []bus.MediaFile{{ + Path: resolved, + Filename: filename, + MimeType: mimeFromPath(resolved), + }} + + // Mark delivered so subsequent send_file or message(MEDIA:) calls detect the duplicate. + if dm := DeliveredMediaFromCtx(ctx); dm != nil { + dm.Mark(resolved) + } + + return result +} diff --git a/internal/tools/send_file_test.go b/internal/tools/send_file_test.go new file mode 100644 index 00000000..17dfc62c --- /dev/null +++ b/internal/tools/send_file_test.go @@ -0,0 +1,464 @@ +package tools + +// TDD red-state tests for send_file tool. +// NewSendFileTool does NOT exist yet — this file will fail to compile until +// phase 03 adds send_file.go. That compile failure is the intended red state. +// +// Run to confirm red state: +// go test -v ./internal/tools/... -run TestSendFile 2>&1 | grep -i 'undefined\|SendFileTool' +// Run to confirm no regression: +// go test ./internal/tools/... -run 'TestMessage|TestWriteFile|TestDeliveredMedia' + +import ( + "context" + "crypto/rand" + "os" + "path/filepath" + "strings" + "testing" +) + +// mkSendFileWorkspace creates a temp workspace with a small set of test files. +// Returns (workspaceCanonical, reportPDF, subFile, subDir). +func mkSendFileWorkspace(t *testing.T) (ws, reportPDF, subFile, subDir string) { + t.Helper() + workspace := t.TempDir() + workspaceCanonical, _ := filepath.EvalSymlinks(workspace) + + // workspace/report.pdf + reportPDF = filepath.Join(workspaceCanonical, "report.pdf") + if err := os.WriteFile(reportPDF, []byte("%PDF-1.4 body"), 0o644); err != nil { + t.Fatal(err) + } + + // workspace/subdir/file.txt + subDir = filepath.Join(workspaceCanonical, "subdir") + if err := os.MkdirAll(subDir, 0o755); err != nil { + t.Fatal(err) + } + subFile = filepath.Join(subDir, "file.txt") + if err := os.WriteFile(subFile, []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + + return workspaceCanonical, reportPDF, subFile, subDir +} + +// TestSendFile_T1_HappyPath verifies that send_file on an existing file succeeds +// with Result.Media populated, correct path, filename, and MIME type. +func TestSendFile_T1_HappyPath(t *testing.T) { + ws, reportPDF, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": reportPDF, + }) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if len(result.Media) != 1 { + t.Fatalf("expected 1 Media entry, got %d", len(result.Media)) + } + if result.Media[0].Path != reportPDF { + t.Errorf("Media[0].Path = %q, want %q", result.Media[0].Path, reportPDF) + } + if result.Media[0].Filename != "report.pdf" { + t.Errorf("Media[0].Filename = %q, want %q", result.Media[0].Filename, "report.pdf") + } + if result.Media[0].MimeType != "application/pdf" { + t.Errorf("Media[0].MimeType = %q, want %q", result.Media[0].MimeType, "application/pdf") + } +} + +// TestSendFile_T2_WithCaption verifies that a caption appears in ForLLM or ForUser +// and that Media is still populated. +func TestSendFile_T2_WithCaption(t *testing.T) { + ws, reportPDF, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": reportPDF, + "caption": "Q4 report", + }) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if len(result.Media) != 1 { + t.Fatalf("expected 1 Media entry, got %d", len(result.Media)) + } + caption := result.ForLLM + result.ForUser + if !strings.Contains(caption, "Q4 report") { + t.Errorf("expected caption %q in ForLLM/ForUser, got ForLLM=%q ForUser=%q", + "Q4 report", result.ForLLM, result.ForUser) + } +} + +// TestSendFile_T3_MissingPath verifies that omitting the "path" param returns an error. +func TestSendFile_T3_MissingPath(t *testing.T) { + ws, _, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{}) + + if !result.IsError { + t.Fatal("expected error for missing path, got success") + } + if !strings.Contains(strings.ToLower(result.ForLLM), "path") { + t.Errorf("expected error to mention 'path', got: %s", result.ForLLM) + } +} + +// TestSendFile_T4_FileNotFound verifies that a non-existent path returns an error. +func TestSendFile_T4_FileNotFound(t *testing.T) { + ws, _, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": filepath.Join(ws, "nonexistent.pdf"), + }) + + if !result.IsError { + t.Fatal("expected error for non-existent file, got success") + } + msg := strings.ToLower(result.ForLLM) + if !strings.Contains(msg, "not found") && !strings.Contains(msg, "does not exist") && + !strings.Contains(msg, "no such file") { + t.Errorf("expected error to mention 'not found' or similar, got: %s", result.ForLLM) + } +} + +// TestSendFile_T5_DirectoryRejected verifies that passing a directory path returns an error. +func TestSendFile_T5_DirectoryRejected(t *testing.T) { + ws, _, _, subDir := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": subDir, + }) + + if !result.IsError { + t.Fatal("expected error for directory path, got success") + } + msg := strings.ToLower(result.ForLLM) + if !strings.Contains(msg, "regular file") && !strings.Contains(msg, "directory") && + !strings.Contains(msg, "not a file") { + t.Errorf("expected error to mention directory/regular-file, got: %s", result.ForLLM) + } +} + +// TestSendFile_T6_PathTraversalBlocked verifies that path traversal with restrict=true is blocked. +func TestSendFile_T6_PathTraversalBlocked(t *testing.T) { + ws, _, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + // Try to escape workspace using traversal. + traversalPath := filepath.Join(ws, "..", "..", "etc", "passwd") + result := tool.Execute(ctx, map[string]any{ + "path": traversalPath, + }) + + if !result.IsError { + t.Fatal("expected error for path traversal, got success") + } +} + +// TestSendFile_T7_RelativePathResolvesAgainstWorkspace verifies that a relative path +// like "subdir/file.txt" resolves against the workspace root. +func TestSendFile_T7_RelativePathResolvesAgainstWorkspace(t *testing.T) { + ws, _, subFile, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": "subdir/file.txt", + }) + + if result.IsError { + t.Fatalf("expected success for relative path, got error: %s", result.ForLLM) + } + if len(result.Media) != 1 { + t.Fatalf("expected 1 Media entry, got %d", len(result.Media)) + } + // Canonical form comparison — both paths should point to same file. + gotCanonical, _ := filepath.EvalSymlinks(result.Media[0].Path) + wantCanonical, _ := filepath.EvalSymlinks(subFile) + if gotCanonical != wantCanonical { + t.Errorf("Media[0].Path resolved to %q, want %q", gotCanonical, wantCanonical) + } +} + +// TestSendFile_T8_AbsoluteInsideWorkspaceAllowed verifies that an absolute path +// inside the workspace succeeds with restrict=true. +func TestSendFile_T8_AbsoluteInsideWorkspaceAllowed(t *testing.T) { + ws, reportPDF, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": reportPDF, // absolute path inside workspace + }) + + if result.IsError { + t.Fatalf("expected success for absolute path inside workspace, got error: %s", result.ForLLM) + } + if len(result.Media) != 1 { + t.Fatalf("expected 1 Media entry, got %d", len(result.Media)) + } +} + +// TestSendFile_T9_DuplicateBlocksSameToolCall verifies that calling send_file twice +// with the same path in the same ctx causes the second call to return an error. +func TestSendFile_T9_DuplicateBlocksSameToolCall(t *testing.T) { + ws, reportPDF, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + dm := NewDeliveredMedia() + ctx := WithDeliveredMedia(context.Background(), dm) + + // First call — should succeed. + first := tool.Execute(ctx, map[string]any{ + "path": reportPDF, + }) + if first.IsError { + t.Fatalf("first send_file: expected success, got error: %s", first.ForLLM) + } + + // Second call — same path, same ctx — must be blocked. + second := tool.Execute(ctx, map[string]any{ + "path": reportPDF, + }) + if !second.IsError { + t.Fatal("second send_file (dup): expected error (already delivered), got success") + } + msg := strings.ToLower(second.ForLLM) + if !strings.Contains(msg, "already") { + t.Errorf("expected error to mention 'already delivered/sent', got: %s", second.ForLLM) + } +} + +// TestSendFile_T10_MarksDeliveredMedia verifies that after send_file succeeds, +// dm.IsDelivered(resolvedPath) returns true. +func TestSendFile_T10_MarksDeliveredMedia(t *testing.T) { + ws, reportPDF, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + dm := NewDeliveredMedia() + ctx := WithDeliveredMedia(context.Background(), dm) + + result := tool.Execute(ctx, map[string]any{ + "path": reportPDF, + }) + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + + if !dm.IsDelivered(reportPDF) { + t.Errorf("expected dm.IsDelivered(%q) = true after send_file, got false", reportPDF) + } +} + +// TestSendFile_T11_SubsequentMessageMediaBlocked verifies that after send_file +// marks a path, a subsequent message(MEDIA:path) self-send is blocked. +func TestSendFile_T11_SubsequentMessageMediaBlocked(t *testing.T) { + ws, reportPDF, _, _ := mkSendFileWorkspace(t) + sendTool := NewSendFileTool(ws, true) + msgTool := NewMessageTool(ws, true) + msgTool.SetMessageBus(nil) + + dm := NewDeliveredMedia() + ctx := context.Background() + ctx = WithDeliveredMedia(ctx, dm) + ctx = WithToolChannel(ctx, "telegram") + ctx = WithToolChatID(ctx, "chat-42") + + // Step 1: send_file delivers and marks the file. + sendResult := sendTool.Execute(ctx, map[string]any{ + "path": reportPDF, + }) + if sendResult.IsError { + t.Fatalf("send_file step failed: %s", sendResult.ForLLM) + } + + // Step 2: message(MEDIA:path) for same file — must be blocked by self-send guard. + msgResult := msgTool.Execute(ctx, map[string]any{ + "action": "send", + "channel": "telegram", + "target": "chat-42", + "message": "MEDIA:" + reportPDF, + }) + if !msgResult.IsError { + t.Fatal("expected message(MEDIA:path) to be blocked after send_file, but it was allowed") + } +} + +// TestSendFile_T11b_BlocksAfterMessageMediaMarked verifies that send_file returns +// an error when the path was already marked by message(MEDIA:) success. +// +// NOTE: This test simulates the post-phase-03 state where message.go calls dm.Mark() +// on MEDIA: success. Currently message.go does NOT call Mark (documented gap in +// TestMessageMediaNoMark_DocumentedGap). Once phase 03 patches message.go, the +// manual dm.Mark call below can be replaced by running the message tool directly. +func TestSendFile_T11b_BlocksAfterMessageMediaMarked(t *testing.T) { + ws, reportPDF, _, _ := mkSendFileWorkspace(t) + tool := NewSendFileTool(ws, true) + + dm := NewDeliveredMedia() + // Simulating post-phase-03 mark: once message.go is patched, this test can + // call the message tool directly and remove this manual Mark call. + dm.Mark(reportPDF) + ctx := WithDeliveredMedia(context.Background(), dm) + + result := tool.Execute(ctx, map[string]any{ + "path": reportPDF, + }) + if !result.IsError { + t.Fatal("expected send_file to block path already marked by message(MEDIA:), got success") + } + msg := strings.ToLower(result.ForLLM) + if !strings.Contains(msg, "already") { + t.Errorf("expected error to mention 'already delivered/sent', got: %s", result.ForLLM) + } +} + +// TestSendFile_T11c_BlocksAfterWriteFileDeliverTrue verifies that send_file blocks +// when the path was already delivered via write_file(deliver=true). +func TestSendFile_T11c_BlocksAfterWriteFileDeliverTrue(t *testing.T) { + ws, _, _, _ := mkSendFileWorkspace(t) + writeTool := NewWriteFileTool(ws, true) + sendTool := NewSendFileTool(ws, true) + + dm := NewDeliveredMedia() + ctx := WithDeliveredMedia(context.Background(), dm) + + // Step 1: write_file deliver=true creates + marks the file. + writeResult := writeTool.Execute(ctx, map[string]any{ + "path": "data.csv", + "content": "col1,col2\n1,2\n", + "deliver": true, + }) + if writeResult.IsError { + t.Fatalf("write_file step failed: %s", writeResult.ForLLM) + } + resolvedPath := filepath.Join(ws, "data.csv") + + // Step 2: send_file for same path — must be blocked. + sendResult := sendTool.Execute(ctx, map[string]any{ + "path": resolvedPath, + }) + if !sendResult.IsError { + t.Fatal("expected send_file to block path already delivered by write_file deliver=true, got success") + } + msg := strings.ToLower(sendResult.ForLLM) + if !strings.Contains(msg, "already") { + t.Errorf("expected error to mention 'already delivered/sent', got: %s", sendResult.ForLLM) + } +} + +// TestSendFile_T12_MimeDetection is a table-driven test for MIME type detection +// from file extension. +func TestSendFile_T12_MimeDetection(t *testing.T) { + ws := t.TempDir() + wsCanonical, _ := filepath.EvalSymlinks(ws) + tool := NewSendFileTool(wsCanonical, true) + + cases := []struct { + ext string + wantMime string + }{ + {".pdf", "application/pdf"}, + {".png", "image/png"}, + {".unknown", "application/octet-stream"}, + } + + for _, tc := range cases { + t.Run(tc.ext, func(t *testing.T) { + name := "testfile" + tc.ext + fpath := filepath.Join(wsCanonical, name) + if err := os.WriteFile(fpath, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": fpath, + }) + if result.IsError { + t.Fatalf("expected success for %s, got error: %s", tc.ext, result.ForLLM) + } + if len(result.Media) != 1 { + t.Fatalf("expected 1 Media entry, got %d", len(result.Media)) + } + if result.Media[0].MimeType != tc.wantMime { + t.Errorf("MimeType for %s = %q, want %q", tc.ext, result.Media[0].MimeType, tc.wantMime) + } + }) + } +} + +// TestSendFile_T14_DenyPathsBlocked verifies that send_file rejects paths covered by DenyPaths. +func TestSendFile_T14_DenyPathsBlocked(t *testing.T) { + ws := t.TempDir() + wsCanonical, _ := filepath.EvalSymlinks(ws) + tool := NewSendFileTool(wsCanonical, true) + tool.DenyPaths("memory.db", "config.json") + + // Create a denied file inside the workspace. + deniedFile := filepath.Join(wsCanonical, "memory.db") + if err := os.WriteFile(deniedFile, []byte("db-data"), 0o644); err != nil { + t.Fatal(err) + } + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": "memory.db", + }) + + if !result.IsError { + t.Fatal("expected error for denied path memory.db, got success") + } + msg := strings.ToLower(result.ForLLM) + if !strings.Contains(msg, "denied") && !strings.Contains(msg, "restricted") { + t.Errorf("expected error to mention 'denied' or 'restricted', got: %s", result.ForLLM) + } +} + +// TestSendFile_T13_BinaryFileLargeNoContentRead verifies that a large binary file +// is handled efficiently — the tool must not read the file contents, only stat + path. +// Test should complete well under 1 second even for 10MB. +func TestSendFile_T13_BinaryFileLargeNoContentRead(t *testing.T) { + ws := t.TempDir() + wsCanonical, _ := filepath.EvalSymlinks(ws) + tool := NewSendFileTool(wsCanonical, true) + + // Write a 10MB random binary file. + bigFile := filepath.Join(wsCanonical, "bigdata.bin") + const size = 10 * 1024 * 1024 + data := make([]byte, size) + _, _ = rand.Read(data) + if err := os.WriteFile(bigFile, data, 0o644); err != nil { + t.Fatal(err) + } + + ctx := context.Background() + result := tool.Execute(ctx, map[string]any{ + "path": bigFile, + }) + + if result.IsError { + t.Fatalf("expected success for large binary, got error: %s", result.ForLLM) + } + if len(result.Media) != 1 { + t.Fatalf("expected 1 Media entry, got %d", len(result.Media)) + } + if result.Media[0].Path != bigFile { + t.Errorf("Media[0].Path = %q, want %q", result.Media[0].Path, bigFile) + } +}