fix(agent): instruct agents to use download_url for attachments (#356)
* fix(agent): instruct agents to use download_url for attachments
Agents were not aware of the signed vs unsigned URL distinction in
attachments, causing failures when trying to read images. Added an
Attachments section to the generated CLAUDE.md/AGENTS.md template that
tells agents to always use `download_url`. Also increased signed URL
expiry from 5 to 30 minutes to better accommodate agent processing time.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(cli): add `multica attachment download` command
Adds a dedicated CLI command for downloading attachments by ID. The
command fetches attachment metadata from the API (which returns a fresh
signed URL), downloads the file, and saves it locally. This eliminates
the need for agents to understand signed vs unsigned URLs.
Changes:
- New `multica attachment download <id>` CLI command
- New `GET /api/attachments/{id}` backend endpoint
- `DownloadFile` helper on APIClient
- Updated CLAUDE.md template to document the command
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* fix(cli): sanitize filename and add download size limit
- Use filepath.Base on attachment filename to prevent path traversal
- Add 100MB size limit to DownloadFile (matches upload limit)
- Include response body in download error messages for debugging
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Devv <devv@Devvs-Mac-mini.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
35b379d688
commit
8eb1caa72b
6 changed files with 147 additions and 2 deletions
87
server/cmd/multica/cmd_attachment.go
Normal file
87
server/cmd/multica/cmd_attachment.go
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"time"
|
||||
|
||||
"github.com/spf13/cobra"
|
||||
|
||||
"github.com/multica-ai/multica/server/internal/cli"
|
||||
)
|
||||
|
||||
var attachmentCmd = &cobra.Command{
|
||||
Use: "attachment",
|
||||
Short: "Manage attachments",
|
||||
}
|
||||
|
||||
var attachmentDownloadCmd = &cobra.Command{
|
||||
Use: "download <attachment-id>",
|
||||
Short: "Download an attachment to a local file",
|
||||
Long: "Fetches the attachment metadata from the API, then downloads the file using its signed URL. Prints the local file path on success.",
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: runAttachmentDownload,
|
||||
}
|
||||
|
||||
func init() {
|
||||
attachmentCmd.AddCommand(attachmentDownloadCmd)
|
||||
|
||||
attachmentDownloadCmd.Flags().StringP("output-dir", "o", ".", "Directory to save the downloaded file")
|
||||
}
|
||||
|
||||
func runAttachmentDownload(cmd *cobra.Command, args []string) error {
|
||||
client, err := newAPIClient(cmd)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// Fetch attachment metadata (includes signed download_url).
|
||||
var att map[string]any
|
||||
if err := client.GetJSON(ctx, "/api/attachments/"+args[0], &att); err != nil {
|
||||
return fmt.Errorf("get attachment: %w", err)
|
||||
}
|
||||
|
||||
downloadURL := strVal(att, "download_url")
|
||||
if downloadURL == "" {
|
||||
return fmt.Errorf("attachment has no download URL")
|
||||
}
|
||||
|
||||
filename := filepath.Base(strVal(att, "filename"))
|
||||
if filename == "" || filename == "." {
|
||||
filename = args[0]
|
||||
}
|
||||
|
||||
// Download the file content.
|
||||
data, err := client.DownloadFile(ctx, downloadURL)
|
||||
if err != nil {
|
||||
return fmt.Errorf("download file: %w", err)
|
||||
}
|
||||
|
||||
// Write to the output directory.
|
||||
outputDir, _ := cmd.Flags().GetString("output-dir")
|
||||
destPath := filepath.Join(outputDir, filename)
|
||||
|
||||
if err := os.WriteFile(destPath, data, 0o644); err != nil {
|
||||
return fmt.Errorf("write file: %w", err)
|
||||
}
|
||||
|
||||
// Print the absolute path so agents can reference the file.
|
||||
abs, err := filepath.Abs(destPath)
|
||||
if err != nil {
|
||||
abs = destPath
|
||||
}
|
||||
fmt.Fprintln(os.Stderr, "Downloaded:", abs)
|
||||
|
||||
// Also print as JSON for --output json compatibility.
|
||||
return cli.PrintJSON(os.Stdout, map[string]any{
|
||||
"id": strVal(att, "id"),
|
||||
"filename": filename,
|
||||
"path": abs,
|
||||
"size": strVal(att, "size_bytes"),
|
||||
})
|
||||
}
|
||||
|
|
@ -32,6 +32,7 @@ func init() {
|
|||
rootCmd.AddCommand(workspaceCmd)
|
||||
rootCmd.AddCommand(configCmd)
|
||||
rootCmd.AddCommand(issueCmd)
|
||||
rootCmd.AddCommand(attachmentCmd)
|
||||
rootCmd.AddCommand(repoCmd)
|
||||
rootCmd.AddCommand(versionCmd)
|
||||
rootCmd.AddCommand(updateCmd)
|
||||
|
|
|
|||
|
|
@ -179,6 +179,7 @@ func NewRouter(pool *pgxpool.Pool, hub *realtime.Hub, bus *events.Bus) chi.Route
|
|||
})
|
||||
|
||||
// Attachments
|
||||
r.Get("/api/attachments/{id}", h.GetAttachmentByID)
|
||||
r.Delete("/api/attachments/{id}", h.DeleteAttachment)
|
||||
|
||||
// Comments
|
||||
|
|
|
|||
|
|
@ -212,6 +212,30 @@ func (c *APIClient) UploadFile(ctx context.Context, fileData []byte, filename st
|
|||
return id, nil
|
||||
}
|
||||
|
||||
// DownloadFile downloads a file from the given URL and returns the response body.
|
||||
// This is used for downloading attachments via their signed download_url.
|
||||
// Downloads are limited to 100 MB to match the upload size limit.
|
||||
func (c *APIClient) DownloadFile(ctx context.Context, downloadURL string) ([]byte, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, downloadURL, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
resp, err := c.HTTPClient.Do(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode >= 400 {
|
||||
body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
|
||||
return nil, fmt.Errorf("download returned %d: %s", resp.StatusCode, strings.TrimSpace(string(body)))
|
||||
}
|
||||
|
||||
const maxDownloadSize = 100 << 20 // 100 MB
|
||||
return io.ReadAll(io.LimitReader(resp.Body, maxDownloadSize))
|
||||
}
|
||||
|
||||
// HealthCheck hits the /health endpoint and returns the response body.
|
||||
func (c *APIClient) HealthCheck(ctx context.Context) (string, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.BaseURL+"/health", nil)
|
||||
|
|
|
|||
|
|
@ -51,7 +51,8 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
|
|||
b.WriteString("- `multica workspace get --output json` — Get workspace details and context\n")
|
||||
b.WriteString("- `multica agent list --output json` — List agents in workspace\n")
|
||||
b.WriteString("- `multica issue runs <issue-id> --output json` — List all execution runs for an issue (status, timestamps, errors)\n")
|
||||
b.WriteString("- `multica issue run-messages <task-id> [--since <seq>] --output json` — List messages for a specific execution run (supports incremental fetch)\n\n")
|
||||
b.WriteString("- `multica issue run-messages <task-id> [--since <seq>] --output json` — List messages for a specific execution run (supports incremental fetch)\n")
|
||||
b.WriteString("- `multica attachment download <id> [-o <dir>]` — Download an attachment file locally by ID\n\n")
|
||||
|
||||
b.WriteString("### Write\n")
|
||||
b.WriteString("- `multica issue comment add <issue-id> --content \"...\" [--parent <comment-id>]` — Post a comment (use --parent to reply to a specific comment)\n")
|
||||
|
|
@ -134,6 +135,13 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string {
|
|||
b.WriteString("- **Agent**: `[@Name](mention://agent/<agent-id>)` — renders as a styled mention\n\n")
|
||||
b.WriteString("Use `multica issue list --output json` to look up issue IDs, and `multica workspace members --output json` for member IDs.\n\n")
|
||||
|
||||
b.WriteString("## Attachments\n\n")
|
||||
b.WriteString("Issues and comments may include file attachments (images, documents, etc.).\n")
|
||||
b.WriteString("Use the download command to fetch attachment files locally:\n\n")
|
||||
b.WriteString("```\nmultica attachment download <attachment-id>\n```\n\n")
|
||||
b.WriteString("This downloads the file to the current directory and prints the local path. Use `-o <dir>` to save elsewhere.\n")
|
||||
b.WriteString("After downloading, you can read the file directly (e.g. view an image, read a document).\n\n")
|
||||
|
||||
b.WriteString("## Output\n\n")
|
||||
b.WriteString("Keep comments concise and natural — state the outcome, not the process.\n")
|
||||
b.WriteString("Good: \"Fixed the login redirect. PR: https://...\"\n")
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ func (h *Handler) attachmentToResponse(a db.Attachment) AttachmentResponse {
|
|||
CreatedAt: a.CreatedAt.Time.Format("2006-01-02T15:04:05Z07:00"),
|
||||
}
|
||||
if h.CFSigner != nil {
|
||||
resp.DownloadURL = h.CFSigner.SignedURL(a.Url, time.Now().Add(5*time.Minute))
|
||||
resp.DownloadURL = h.CFSigner.SignedURL(a.Url, time.Now().Add(30*time.Minute))
|
||||
}
|
||||
if a.IssueID.Valid {
|
||||
s := uuidToString(a.IssueID)
|
||||
|
|
@ -217,6 +217,30 @@ func (h *Handler) ListAttachments(w http.ResponseWriter, r *http.Request) {
|
|||
writeJSON(w, http.StatusOK, resp)
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// GetAttachmentByID — GET /api/attachments/{id}
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func (h *Handler) GetAttachmentByID(w http.ResponseWriter, r *http.Request) {
|
||||
attachmentID := chi.URLParam(r, "id")
|
||||
workspaceID := resolveWorkspaceID(r)
|
||||
if workspaceID == "" {
|
||||
writeError(w, http.StatusBadRequest, "workspace_id is required")
|
||||
return
|
||||
}
|
||||
|
||||
att, err := h.Queries.GetAttachment(r.Context(), db.GetAttachmentParams{
|
||||
ID: parseUUID(attachmentID),
|
||||
WorkspaceID: parseUUID(workspaceID),
|
||||
})
|
||||
if err != nil {
|
||||
writeError(w, http.StatusNotFound, "attachment not found")
|
||||
return
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, h.attachmentToResponse(att))
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// DeleteAttachment — DELETE /api/attachments/{id}
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue