opencode/picoclaw
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: keep URL-downloaded temp file alive for MediaStore resolution

Browse source 
MediaStore.Store() only saves a path reference, not a copy.
The previous defer os.Remove() deleted the file before Telegram
could read it, causing silent send failures.

Now downloads go to media.TempDir()/sendfile/ and persist until
CleanupTempFiles() removes them after 1 hour.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
dj-oyu 2026-03-22 11:51:33 +09:00
parent 2716b47f9a
commit 19eb88dd38
2 changed files with 40 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
pkg/tools/send_file.go
View file

@ -21,11 +21,17 @@ import (
)
const (
sendFileTempDir = "/tmp/picoclaw-sendfile"
sendFileTempMaxAge = 1 * time.Hour
sendFileDownloadLimit = 50 * 1024 * 1024 // 50 MB download limit
)
// sendFileTempDir returns the directory for URL-downloaded temp files.
// Uses the shared media temp directory so that MediaStore can resolve paths
// after Execute returns (MediaStore only stores a path reference, not a copy).
func sendFileTempDir() string {
return filepath.Join(media.TempDir(), "sendfile")
}
// SendFileTool allows the LLM to send a local file (image, document, etc.)
// to the user on the current chat channel via the MediaStore pipeline.
type SendFileTool struct {
@ -118,15 +124,17 @@ func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
// Handle URL downloads
var resolved string
var tempFile string // non-empty when we downloaded a URL
var isURL bool
if isHTTPURL(path) {
var err error
resolved, err = downloadToTemp(ctx, path)
if err != nil {
return ErrorResult(fmt.Sprintf("download failed: %v", err))
}
tempFile = resolved
defer os.Remove(tempFile)
isURL = true
// Do NOT delete the temp file here — MediaStore only stores a path
// reference, so the file must survive until the channel sends it.
// CleanupTempFiles() handles stale files periodically.
} else {
var err error
resolved, err = validatePathWithAllowPaths(path, t.workspace, t.restrict, t.allowPaths)
@ -151,7 +159,7 @@ func (t *SendFileTool) Execute(ctx context.Context, args map[string]any) *ToolRe
filename, _ := args["filename"].(string)
if filename == "" {
if tempFile != "" {
if isURL {
filename = filenameFromURL(path)
} else {
filename = filepath.Base(resolved)
@ -178,10 +186,12 @@ func isHTTPURL(path string) bool {
return strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://")
}
// downloadToTemp downloads a URL to a temporary file under sendFileTempDir.
// The caller is responsible for removing the returned file.
// downloadToTemp downloads a URL to a temporary file under sendFileTempDir().
// The file must persist until the channel has sent it; cleanup is handled by
// CleanupTempFiles (periodic) rather than the caller.
func downloadToTemp(ctx context.Context, rawURL string) (string, error) {
if err := os.MkdirAll(sendFileTempDir, 0o700); err != nil {
dir := sendFileTempDir()
if err := os.MkdirAll(dir, 0o700); err != nil {
return "", fmt.Errorf("create temp dir: %w", err)
}
@ -206,7 +216,7 @@ func downloadToTemp(ctx context.Context, rawURL string) (string, error) {
return "", err
}
ext := extFromURL(rawURL)
tmpPath := filepath.Join(sendFileTempDir, fmt.Sprintf("dl_%x%s", randBytes, ext))
tmpPath := filepath.Join(dir, fmt.Sprintf("dl_%x%s", randBytes, ext))
f, err := os.OpenFile(tmpPath, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0o600)
if err != nil {
@ -254,7 +264,8 @@ func filenameFromURL(rawURL string) string {
// CleanupTempFiles removes old temp files from the sendfile temp directory.
// Files older than sendFileTempMaxAge are deleted.
func CleanupTempFiles() {
entries, err := os.ReadDir(sendFileTempDir)
dir := sendFileTempDir()
entries, err := os.ReadDir(dir)
if err != nil {
return
}
@ -268,7 +279,7 @@ func CleanupTempFiles() {
continue
}
if info.ModTime().Before(cutoff) {
os.Remove(filepath.Join(sendFileTempDir, e.Name()))
os.Remove(filepath.Join(dir, e.Name()))
}
}
}

31
pkg/tools/send_file_test.go
View file

@ -239,7 +239,7 @@ func TestSendFileTool_URLDownloadHTTPError(t *testing.T) {
}
}
func TestSendFileTool_URLTempFileCleanedUp(t *testing.T) {
func TestSendFileTool_URLTempFilePersistsForMediaStore(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte("data"))
}))
@ -249,31 +249,32 @@ func TestSendFileTool_URLTempFileCleanedUp(t *testing.T) {
tool := NewSendFileTool(t.TempDir(), false, 0, store)
tool.SetContext("telegram", "chat123")
// Execute should download and clean up temp file
tool.Execute(context.Background(), map[string]any{
result := tool.Execute(context.Background(), map[string]any{
"path": srv.URL + "/photo.jpg",
})
if result.IsError {
t.Fatalf("unexpected error: %s", result.ForLLM)
}
// Verify no temp files remain with dl_ prefix in the temp dir
entries, _ := os.ReadDir(sendFileTempDir)
for _, e := range entries {
if strings.HasPrefix(e.Name(), "dl_") {
info, _ := e.Info()
// Only flag files created very recently (within this test)
if info != nil && time.Since(info.ModTime()) < 5*time.Second {
t.Errorf("temp file not cleaned up: %s", e.Name())
}
// The temp file must still exist so MediaStore can resolve and read it
ref := result.Media[0]
resolved, err := store.Resolve(ref)
if err != nil {
t.Fatalf("Resolve failed: %v", err)
}
if _, err := os.Stat(resolved); err != nil {
t.Errorf("temp file should persist for MediaStore, but got: %v", err)
}
t.Cleanup(func() { os.Remove(resolved) })
}
func TestCleanupTempFiles(t *testing.T) {
if err := os.MkdirAll(sendFileTempDir, 0o700); err != nil {
if err := os.MkdirAll(sendFileTempDir(), 0o700); err != nil {
t.Fatal(err)
}
// Create an "old" temp file
oldFile := filepath.Join(sendFileTempDir, "dl_test_old.tmp")
oldFile := filepath.Join(sendFileTempDir(), "dl_test_old.tmp")
if err := os.WriteFile(oldFile, []byte("old"), 0o600); err != nil {
t.Fatal(err)
}
@ -282,7 +283,7 @@ func TestCleanupTempFiles(t *testing.T) {
os.Chtimes(oldFile, past, past)
// Create a "new" temp file
newFile := filepath.Join(sendFileTempDir, "dl_test_new.tmp")
newFile := filepath.Join(sendFileTempDir(), "dl_test_new.tmp")
if err := os.WriteFile(newFile, []byte("new"), 0o600); err != nil {
t.Fatal(err)
}