Merge pull request #228 from multica-ai/fix/s3-cleanup-on-delete

fix(upload): clean up S3 objects when attachments are deleted

apps/web/features/issues/components/comment-card.tsx

@@ -33,7 +33,7 @@ interface CommentCardProps {
   entry: TimelineEntry;
   allReplies: Map<string, TimelineEntry[]>;
   currentUserId?: string;
-  onReply: (parentId: string, content: string) => Promise<void>;
+  onReply: (parentId: string, content: string, attachmentIds?: string[]) => Promise<void>;
   onEdit: (commentId: string, content: string) => Promise<void>;
   onDelete: (commentId: string) => void;
   onToggleReaction: (commentId: string, emoji: string) => void;
@@ -375,7 +375,7 @@ function CommentCard({
               size="sm"
               avatarType="member"
               avatarId={currentUserId ?? ""}
-              onSubmit={(content) => onReply(entry.id, content)}
+              onSubmit={(content, attachmentIds) => onReply(entry.id, content, attachmentIds)}
             />
           </div>
         </CollapsibleContent>

apps/web/features/issues/components/comment-input.tsx

@@ -8,17 +8,22 @@ import { useFileUpload } from "@/shared/hooks/use-file-upload";
 
 interface CommentInputProps {
   issueId: string;
-  onSubmit: (content: string) => Promise<void>;
+  onSubmit: (content: string, attachmentIds?: string[]) => Promise<void>;
 }
 
 function CommentInput({ issueId, onSubmit }: CommentInputProps) {
   const editorRef = useRef<RichTextEditorRef>(null);
   const fileInputRef = useRef<HTMLInputElement>(null);
+  const attachmentIdsRef = useRef<string[]>([]);
   const [isEmpty, setIsEmpty] = useState(true);
   const [submitting, setSubmitting] = useState(false);
   const { uploadWithToast, uploading } = useFileUpload();
 
-  const handleUpload = (file: File) => uploadWithToast(file, { issueId });
+  const handleUpload = async (file: File) => {
+    const result = await uploadWithToast(file, { issueId });
+    if (result) attachmentIdsRef.current.push(result.id);
+    return result;
+  };
 
   const handleFileSelect = async (e: React.ChangeEvent<HTMLInputElement>) => {
     const file = e.target.files?.[0];
@@ -35,8 +40,10 @@ function CommentInput({ issueId, onSubmit }: CommentInputProps) {
     if (!content || submitting) return;
     setSubmitting(true);
     try {
-      await onSubmit(content);
+      const ids = attachmentIdsRef.current.length > 0 ? [...attachmentIdsRef.current] : undefined;
+      await onSubmit(content, ids);
       editorRef.current?.clearContent();
+      attachmentIdsRef.current = [];
       setIsEmpty(true);
     } finally {
       setSubmitting(false);

apps/web/features/issues/components/reply-input.tsx

@@ -16,7 +16,7 @@ interface ReplyInputProps {
   placeholder?: string;
   avatarType: string;
   avatarId: string;
-  onSubmit: (content: string) => Promise<void>;
+  onSubmit: (content: string, attachmentIds?: string[]) => Promise<void>;
   size?: "sm" | "default";
 }
 
@@ -34,11 +34,16 @@ function ReplyInput({
 }: ReplyInputProps) {
   const editorRef = useRef<RichTextEditorRef>(null);
   const fileInputRef = useRef<HTMLInputElement>(null);
+  const attachmentIdsRef = useRef<string[]>([]);
   const [isEmpty, setIsEmpty] = useState(true);
   const [submitting, setSubmitting] = useState(false);
   const { uploadWithToast, uploading } = useFileUpload();
 
-  const handleUpload = (file: File) => uploadWithToast(file, { issueId });
+  const handleUpload = async (file: File) => {
+    const result = await uploadWithToast(file, { issueId });
+    if (result) attachmentIdsRef.current.push(result.id);
+    return result;
+  };
 
   const handleFileSelect = async (e: React.ChangeEvent<HTMLInputElement>) => {
     const file = e.target.files?.[0];
@@ -55,8 +60,10 @@ function ReplyInput({
     if (!content || submitting) return;
     setSubmitting(true);
     try {
-      await onSubmit(content);
+      const ids = attachmentIdsRef.current.length > 0 ? [...attachmentIdsRef.current] : undefined;
+      await onSubmit(content, ids);
       editorRef.current?.clearContent();
+      attachmentIdsRef.current = [];
       setIsEmpty(true);
     } finally {
       setSubmitting(false);

apps/web/features/issues/hooks/use-issue-timeline.ts

@@ -174,11 +174,11 @@ export function useIssueTimeline(issueId: string, userId?: string) {
   // --- Mutation functions ---
 
   const submitComment = useCallback(
-    async (content: string) => {
+    async (content: string, attachmentIds?: string[]) => {
       if (!content.trim() || submitting || !userId) return;
       setSubmitting(true);
       try {
-        const comment = await api.createComment(issueId, content);
+        const comment = await api.createComment(issueId, content, undefined, undefined, attachmentIds);
         setTimeline((prev) => {
           if (prev.some((e) => e.id === comment.id)) return prev;
           return [...prev, commentToTimelineEntry(comment)];
@@ -193,10 +193,10 @@ export function useIssueTimeline(issueId: string, userId?: string) {
   );
 
   const submitReply = useCallback(
-    async (parentId: string, content: string) => {
+    async (parentId: string, content: string, attachmentIds?: string[]) => {
       if (!content.trim() || !userId) return;
       try {
-        const comment = await api.createComment(issueId, content, "comment", parentId);
+        const comment = await api.createComment(issueId, content, "comment", parentId, attachmentIds);
         setTimeline((prev) => {
           if (prev.some((e) => e.id === comment.id)) return prev;
           return [...prev, commentToTimelineEntry(comment)];

apps/web/shared/api/client.ts

@@ -209,13 +209,14 @@ export class ApiClient {
     return this.fetch(`/api/issues/${issueId}/comments`);
   }
 
-  async createComment(issueId: string, content: string, type?: string, parentId?: string): Promise<Comment> {
+  async createComment(issueId: string, content: string, type?: string, parentId?: string, attachmentIds?: string[]): Promise<Comment> {
     return this.fetch(`/api/issues/${issueId}/comments`, {
       method: "POST",
       body: JSON.stringify({
         content,
         type: type ?? "comment",
         ...(parentId ? { parent_id: parentId } : {}),
+        ...(attachmentIds?.length ? { attachment_ids: attachmentIds } : {}),
       }),
     });
   }

apps/web/shared/hooks/use-file-upload.ts

@@ -32,6 +32,7 @@ function isAllowedType(type: string): boolean {
 }
 
 export interface UploadResult {
+  id: string;
   filename: string;
   link: string;
 }
@@ -59,7 +60,7 @@ export function useFileUpload() {
           issueId: ctx?.issueId,
           commentId: ctx?.commentId,
         });
-        return { filename: att.filename, link: att.url };
+        return { id: att.id, filename: att.filename, link: att.url };
       } finally {
         setUploading(false);
       }

server/internal/handler/comment.go

@@ -83,9 +83,10 @@ func (h *Handler) ListComments(w http.ResponseWriter, r *http.Request) {
 }
 
 type CreateCommentRequest struct {
-	Content  string  `json:"content"`
-	Type     string  `json:"type"`
-	ParentID *string `json:"parent_id"`
+	Content       string   `json:"content"`
+	Type          string   `json:"type"`
+	ParentID      *string  `json:"parent_id"`
+	AttachmentIDs []string `json:"attachment_ids"`
 }
 
 func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) {
@@ -142,6 +143,11 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Link uploaded attachments to this comment.
+	if len(req.AttachmentIDs) > 0 {
+		h.linkAttachmentsByIDs(r.Context(), comment.ID, issue.ID, req.AttachmentIDs)
+	}
+
 	resp := commentToResponse(comment, nil, nil)
 	slog.Info("comment created", append(logger.RequestAttrs(r), "comment_id", uuidToString(comment.ID), "issue_id", issueID)...)
 	h.publish(protocol.EventCommentCreated, uuidToString(issue.WorkspaceID), authorType, authorID, map[string]any{
@@ -342,11 +348,16 @@ func (h *Handler) DeleteComment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Collect attachment URLs before CASCADE delete removes them.
+	attachmentURLs, _ := h.Queries.ListAttachmentURLsByCommentID(r.Context(), parseUUID(commentId))
+
 	if err := h.Queries.DeleteComment(r.Context(), parseUUID(commentId)); err != nil {
 		slog.Warn("delete comment failed", append(logger.RequestAttrs(r), "error", err, "comment_id", commentId)...)
 		writeError(w, http.StatusInternalServerError, "failed to delete comment")
 		return
 	}
+
+	h.deleteS3Objects(r.Context(), attachmentURLs)
 	slog.Info("comment deleted", append(logger.RequestAttrs(r), "comment_id", commentId, "issue_id", uuidToString(comment.IssueID))...)
 	h.publish(protocol.EventCommentDeleted, workspaceID, actorType, actorID, map[string]any{
 		"comment_id": commentId,

server/internal/handler/file.go

@@ -1,6 +1,7 @@
 package handler
 
 import (
+	"context"
 	"crypto/rand"
 	"encoding/hex"
 	"fmt"
@@ -292,5 +293,46 @@ func (h *Handler) DeleteAttachment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	h.deleteS3Object(r.Context(), att.Url)
 	w.WriteHeader(http.StatusNoContent)
 }
+
+// ---------------------------------------------------------------------------
+// Attachment linking
+// ---------------------------------------------------------------------------
+
+// linkAttachmentsByIDs links the given attachment IDs to a comment.
+// Only updates attachments that belong to the same issue and have no comment_id yet.
+func (h *Handler) linkAttachmentsByIDs(ctx context.Context, commentID, issueID pgtype.UUID, ids []string) {
+	uuids := make([]pgtype.UUID, len(ids))
+	for i, id := range ids {
+		uuids[i] = parseUUID(id)
+	}
+	if err := h.Queries.LinkAttachmentsToComment(ctx, db.LinkAttachmentsToCommentParams{
+		CommentID: commentID,
+		IssueID:   issueID,
+		Column3:   uuids,
+	}); err != nil {
+		slog.Error("failed to link attachments to comment", "error", err)
+	}
+}
+
+// deleteS3Object removes a single file from S3 by its CDN URL.
+func (h *Handler) deleteS3Object(ctx context.Context, url string) {
+	if h.Storage == nil || url == "" {
+		return
+	}
+	h.Storage.Delete(ctx, h.Storage.KeyFromURL(url))
+}
+
+// deleteS3Objects removes multiple files from S3 by their CDN URLs.
+func (h *Handler) deleteS3Objects(ctx context.Context, urls []string) {
+	if h.Storage == nil || len(urls) == 0 {
+		return
+	}
+	keys := make([]string, len(urls))
+	for i, u := range urls {
+		keys[i] = h.Storage.KeyFromURL(u)
+	}
+	h.Storage.DeleteKeys(ctx, keys)
+}

server/internal/handler/issue.go

@@ -548,12 +548,16 @@ func (h *Handler) DeleteIssue(w http.ResponseWriter, r *http.Request) {
 
 	h.TaskService.CancelTasksForIssue(r.Context(), issue.ID)
 
+	// Collect all attachment URLs (issue-level + comment-level) before CASCADE delete.
+	attachmentURLs, _ := h.Queries.ListAttachmentURLsByIssueOrComments(r.Context(), issue.ID)
+
 	err := h.Queries.DeleteIssue(r.Context(), parseUUID(id))
 	if err != nil {
 		writeError(w, http.StatusInternalServerError, "failed to delete issue")
 		return
 	}
 
+	h.deleteS3Objects(r.Context(), attachmentURLs)
 	userID := requestUserID(r)
 	actorType, actorID := h.resolveActor(r, userID, uuidToString(issue.WorkspaceID))
 	h.publish(protocol.EventIssueDeleted, uuidToString(issue.WorkspaceID), actorType, actorID, map[string]any{"issue_id": id})

server/internal/storage/s3.go

@@ -83,6 +83,46 @@ func sanitizeFilename(name string) string {
 	return b.String()
 }
 
+// KeyFromURL extracts the S3 object key from a CDN or bucket URL.
+// e.g. "https://multica-static.copilothub.ai/abc123.png" → "abc123.png"
+func (s *S3Storage) KeyFromURL(rawURL string) string {
+	// Strip the "https://domain/" prefix.
+	for _, prefix := range []string{
+		"https://" + s.cdnDomain + "/",
+		"https://" + s.bucket + "/",
+	} {
+		if strings.HasPrefix(rawURL, prefix) {
+			return strings.TrimPrefix(rawURL, prefix)
+		}
+	}
+	// Fallback: take everything after the last "/".
+	if i := strings.LastIndex(rawURL, "/"); i >= 0 {
+		return rawURL[i+1:]
+	}
+	return rawURL
+}
+
+// Delete removes an object from S3. Errors are logged but not fatal.
+func (s *S3Storage) Delete(ctx context.Context, key string) {
+	if key == "" {
+		return
+	}
+	_, err := s.client.DeleteObject(ctx, &s3.DeleteObjectInput{
+		Bucket: aws.String(s.bucket),
+		Key:    aws.String(key),
+	})
+	if err != nil {
+		slog.Error("s3 DeleteObject failed", "key", key, "error", err)
+	}
+}
+
+// DeleteKeys removes multiple objects from S3. Best-effort, errors are logged.
+func (s *S3Storage) DeleteKeys(ctx context.Context, keys []string) {
+	for _, key := range keys {
+		s.Delete(ctx, key)
+	}
+}
+
 func (s *S3Storage) Upload(ctx context.Context, key string, data []byte, contentType string, filename string) (string, error) {
 	safe := sanitizeFilename(filename)
 	_, err := s.client.PutObject(ctx, &s3.PutObjectInput{

server/pkg/db/generated/attachment.sql.go

@@ -101,6 +101,76 @@ func (q *Queries) GetAttachment(ctx context.Context, arg GetAttachmentParams) (A
 	return i, err
 }
 
+const linkAttachmentsToComment = `-- name: LinkAttachmentsToComment :exec
+UPDATE attachment
+SET comment_id = $1
+WHERE issue_id = $2
+  AND comment_id IS NULL
+  AND id = ANY($3::uuid[])
+`
+
+type LinkAttachmentsToCommentParams struct {
+	CommentID pgtype.UUID   `json:"comment_id"`
+	IssueID   pgtype.UUID   `json:"issue_id"`
+	Column3   []pgtype.UUID `json:"column_3"`
+}
+
+func (q *Queries) LinkAttachmentsToComment(ctx context.Context, arg LinkAttachmentsToCommentParams) error {
+	_, err := q.db.Exec(ctx, linkAttachmentsToComment, arg.CommentID, arg.IssueID, arg.Column3)
+	return err
+}
+
+const listAttachmentURLsByCommentID = `-- name: ListAttachmentURLsByCommentID :many
+SELECT url FROM attachment
+WHERE comment_id = $1
+`
+
+func (q *Queries) ListAttachmentURLsByCommentID(ctx context.Context, commentID pgtype.UUID) ([]string, error) {
+	rows, err := q.db.Query(ctx, listAttachmentURLsByCommentID, commentID)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	items := []string{}
+	for rows.Next() {
+		var url string
+		if err := rows.Scan(&url); err != nil {
+			return nil, err
+		}
+		items = append(items, url)
+	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+	return items, nil
+}
+
+const listAttachmentURLsByIssueOrComments = `-- name: ListAttachmentURLsByIssueOrComments :many
+SELECT a.url FROM attachment a
+WHERE a.issue_id = $1
+   OR a.comment_id IN (SELECT c.id FROM comment c WHERE c.issue_id = $1)
+`
+
+func (q *Queries) ListAttachmentURLsByIssueOrComments(ctx context.Context, issueID pgtype.UUID) ([]string, error) {
+	rows, err := q.db.Query(ctx, listAttachmentURLsByIssueOrComments, issueID)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	items := []string{}
+	for rows.Next() {
+		var url string
+		if err := rows.Scan(&url); err != nil {
+			return nil, err
+		}
+		items = append(items, url)
+	}
+	if err := rows.Err(); err != nil {
+		return nil, err
+	}
+	return items, nil
+}
+
 const listAttachmentsByComment = `-- name: ListAttachmentsByComment :many
 SELECT id, workspace_id, issue_id, comment_id, uploader_type, uploader_id, filename, url, content_type, size_bytes, created_at FROM attachment
 WHERE comment_id = $1 AND workspace_id = $2

server/pkg/db/generated/models.go

@@ -127,24 +127,6 @@ type DaemonConnection struct {
 	UpdatedAt       pgtype.Timestamptz `json:"updated_at"`
 }
 
-type DaemonPairingSession struct {
-	ID             pgtype.UUID        `json:"id"`
-	Token          string             `json:"token"`
-	DaemonID       string             `json:"daemon_id"`
-	DeviceName     string             `json:"device_name"`
-	RuntimeName    string             `json:"runtime_name"`
-	RuntimeType    string             `json:"runtime_type"`
-	RuntimeVersion string             `json:"runtime_version"`
-	WorkspaceID    pgtype.UUID        `json:"workspace_id"`
-	ApprovedBy     pgtype.UUID        `json:"approved_by"`
-	Status         string             `json:"status"`
-	ApprovedAt     pgtype.Timestamptz `json:"approved_at"`
-	ClaimedAt      pgtype.Timestamptz `json:"claimed_at"`
-	ExpiresAt      pgtype.Timestamptz `json:"expires_at"`
-	CreatedAt      pgtype.Timestamptz `json:"created_at"`
-	UpdatedAt      pgtype.Timestamptz `json:"updated_at"`
-}
-
 type DaemonToken struct {
 	ID          pgtype.UUID        `json:"id"`
 	TokenHash   string             `json:"token_hash"`

server/pkg/db/queries/attachment.sql

@@ -22,5 +22,21 @@ SELECT * FROM attachment
 WHERE comment_id = ANY($1::uuid[])
 ORDER BY created_at ASC;
 
+-- name: ListAttachmentURLsByIssueOrComments :many
+SELECT a.url FROM attachment a
+WHERE a.issue_id = $1
+   OR a.comment_id IN (SELECT c.id FROM comment c WHERE c.issue_id = $1);
+
+-- name: ListAttachmentURLsByCommentID :many
+SELECT url FROM attachment
+WHERE comment_id = $1;
+
+-- name: LinkAttachmentsToComment :exec
+UPDATE attachment
+SET comment_id = $1
+WHERE issue_id = $2
+  AND comment_id IS NULL
+  AND id = ANY($3::uuid[]);
+
 -- name: DeleteAttachment :exec
 DELETE FROM attachment WHERE id = $1 AND workspace_id = $2;