From 3c9bf8295febb51c5075744c308b703ac9e2e225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 4 Aug 2026 22:11:13 +0100 Subject: [PATCH] feat: support 100 MiB prompt attachments (#2261) --- .../runtime/agentctl/client_attachments.go | 103 +++++ .../agent/runtime/lifecycle/manager.go | 9 + .../agent/runtime/lifecycle/session.go | 89 ++++- .../lifecycle/session_attachments_test.go | 115 ++++++ .../internal/agent/runtime/lifecycle/types.go | 10 + .../adapter/transport/acp/adapter_prompt.go | 36 ++ .../transport/acp/adapter_prompt_test.go | 73 ++++ .../adapter/transport/shared/attachments.go | 161 +++++++- .../transport/shared/attachments_test.go | 55 +++ .../agentctl/server/api/attachments.go | 205 ++++++++++ .../agentctl/server/api/attachments_test.go | 198 +++++++++ .../internal/agentctl/server/api/server.go | 1 + apps/backend/internal/backendapp/adapters.go | 2 + apps/backend/internal/backendapp/gateway.go | 1 + apps/backend/internal/backendapp/helpers.go | 6 + apps/backend/internal/backendapp/main.go | 2 + .../internal/backendapp/orchestrator.go | 10 + apps/backend/internal/backendapp/services.go | 1 + .../backendapp/storage_maintenance.go | 28 ++ .../orchestrator/event_handlers_agent.go | 2 + .../orchestrator/event_handlers_workflow.go | 4 + .../orchestrator/executor/executor.go | 14 + .../executor/executor_interaction.go | 74 +++- .../executor_passthrough_prompt_test.go | 88 ++++ .../orchestrator/handlers/queue_handlers.go | 184 ++++++++- .../handlers/queue_handlers_test.go | 82 ++++ .../orchestrator/messagequeue/service.go | 17 + .../orchestrator/messagequeue/types.go | 2 + apps/backend/internal/orchestrator/service.go | 15 + .../task/handlers/attachment_handlers.go | 167 ++++++++ .../task/handlers/attachment_handlers_test.go | 29 ++ .../task/handlers/message_handlers.go | 3 + .../task/handlers/task_http_handlers.go | 50 ++- .../internal/task/models/attachments.go | 18 + apps/backend/internal/task/models/models.go | 30 ++ .../internal/task/repository/interface.go | 14 + .../task/repository/sqlite/attachment.go | 377 ++++++++++++++++++ .../sqlite/attachment_postgres_test.go | 86 ++++ .../task/repository/sqlite/attachment_test.go | 147 +++++++ .../task/repository/sqlite/base_schema.go | 37 ++ .../task/service/attachment_service.go | 337 ++++++++++++++++ .../task/service/attachment_service_test.go | 45 +++ .../internal/task/service/document_service.go | 1 - apps/backend/internal/task/service/service.go | 24 ++ .../task/service/service_attachments.go | 69 ++++ .../internal/task/service/service_tasks.go | 12 + apps/backend/pkg/api/v1/task.go | 2 + .../task-create-dialog-form-body.test.tsx | 2 + .../task-create-dialog-form-body.tsx | 4 + .../components/task-create-dialog-helpers.ts | 30 +- .../task-create-dialog-prop-builders.ts | 3 +- .../task-create-dialog-selectors.test.tsx | 11 +- .../task-create-dialog-selectors.tsx | 105 ++++- .../components/task-create-dialog-state.ts | 6 + .../task-create-dialog-submit.test.tsx | 1 + .../components/task-create-dialog-submit.tsx | 109 +++-- .../components/task-create-dialog-types.ts | 11 +- .../components/task-create-dialog.test.tsx | 2 + apps/web/components/task-create-dialog.tsx | 23 +- .../task/chat/chat-input-body.test.tsx | 1 + .../components/task/chat/chat-input-body.tsx | 8 +- .../task/chat/chat-input-container.tsx | 21 +- .../context-items/file-attachment-item.tsx | 48 ++- .../task/chat/context-items/image-item.tsx | 140 ++++--- .../components/task/chat/file-attachment.ts | 46 ++- .../task/chat/messages/chat-message.test.tsx | 21 + .../task/chat/messages/chat-message.tsx | 54 ++- .../chat/use-chat-input-container.test.ts | 43 +- .../task/chat/use-chat-input-container.ts | 37 +- .../task/chat/use-chat-input-state.test.ts | 4 +- .../task/chat/use-chat-input-state.ts | 148 +++++-- .../task/new-session-dialog.test.tsx | 4 + .../components/task/new-session-dialog.tsx | 33 +- .../task/new-session-form-actions.test.ts | 40 ++ .../task/new-session-form-actions.ts | 9 +- .../components/task/new-subtask-dialog.tsx | 1 + .../task/new-subtask-form-parts.test.tsx | 1 + .../components/task/new-subtask-form-state.ts | 2 + .../task/passthrough-chat-composer.tsx | 16 +- .../task/passthrough-toolbar.test.tsx | 4 + .../components/task/session-dialog-shared.tsx | 80 +++- .../web/components/task/sessions-dropdown.tsx | 14 +- .../task/use-subtask-submit.test.ts | 49 ++- .../web/components/task/use-subtask-submit.ts | 19 +- .../e2e/helpers/mixed-attachment-layout.ts | 2 +- .../mobile-attachment-size-warning.spec.ts | 4 +- .../create-task-attachment-warning.spec.ts | 4 +- apps/web/hooks/domains/session/use-queue.ts | 4 +- apps/web/hooks/use-message-handler.ts | 3 +- .../lib/api/domains/attachment-api.test.ts | 63 +++ apps/web/lib/api/domains/attachment-api.ts | 71 ++++ apps/web/lib/api/domains/kanban-api.ts | 4 +- apps/web/lib/api/domains/queue-api.ts | 8 +- apps/web/lib/local-storage.ts | 56 ++- .../lib/services/session-launch-service.ts | 4 +- apps/web/lib/types/context.ts | 1 + apps/web/src/locales/en/chat.json | 13 + apps/web/src/locales/pseudo/chat.json | 13 + ...26-08-04-file-backed-prompt-attachments.md | 86 ++++ docs/decisions/INDEX.md | 1 + docs/plans/prompt-attachments/plan.md | 237 +++++++++++ .../task-01-attachment-storage-api.md | 71 ++++ .../task-02-agent-delivery-and-queue.md | 74 ++++ .../task-03-frontend-staged-uploads.md | 82 ++++ .../task-04-attachment-e2e.md | 69 ++++ .../prompt-attachments/task-05-public-docs.md | 63 +++ docs/public/tasks-and-workflows.md | 2 +- docs/public/websocket-api.md | 16 +- docs/specs/INDEX.md | 1 + docs/specs/office/live-updates.md | 2 +- docs/specs/tasks/prompt-attachments.md | 180 +++++++++ 111 files changed, 5082 insertions(+), 237 deletions(-) create mode 100644 apps/backend/internal/agent/runtime/agentctl/client_attachments.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go create mode 100644 apps/backend/internal/agentctl/server/api/attachments.go create mode 100644 apps/backend/internal/agentctl/server/api/attachments_test.go create mode 100644 apps/backend/internal/task/handlers/attachment_handlers.go create mode 100644 apps/backend/internal/task/handlers/attachment_handlers_test.go create mode 100644 apps/backend/internal/task/models/attachments.go create mode 100644 apps/backend/internal/task/repository/sqlite/attachment.go create mode 100644 apps/backend/internal/task/repository/sqlite/attachment_postgres_test.go create mode 100644 apps/backend/internal/task/repository/sqlite/attachment_test.go create mode 100644 apps/backend/internal/task/service/attachment_service.go create mode 100644 apps/backend/internal/task/service/attachment_service_test.go create mode 100644 apps/backend/internal/task/service/service_attachments.go create mode 100644 apps/web/lib/api/domains/attachment-api.test.ts create mode 100644 apps/web/lib/api/domains/attachment-api.ts create mode 100644 docs/decisions/2026-08-04-file-backed-prompt-attachments.md create mode 100644 docs/plans/prompt-attachments/plan.md create mode 100644 docs/plans/prompt-attachments/task-01-attachment-storage-api.md create mode 100644 docs/plans/prompt-attachments/task-02-agent-delivery-and-queue.md create mode 100644 docs/plans/prompt-attachments/task-03-frontend-staged-uploads.md create mode 100644 docs/plans/prompt-attachments/task-04-attachment-e2e.md create mode 100644 docs/plans/prompt-attachments/task-05-public-docs.md create mode 100644 docs/specs/tasks/prompt-attachments.md diff --git a/apps/backend/internal/agent/runtime/agentctl/client_attachments.go b/apps/backend/internal/agent/runtime/agentctl/client_attachments.go new file mode 100644 index 0000000000..b4c431dc79 --- /dev/null +++ b/apps/backend/internal/agent/runtime/agentctl/client_attachments.go @@ -0,0 +1,103 @@ +package client + +import ( + "context" + "encoding/json" + "fmt" + "io" + "mime/multipart" + "net/http" + "strconv" + "strings" + + "github.com/kandev/kandev/internal/task/models" +) + +// MaterializedAttachment is the agentctl-side filename selected for a +// descriptor. The name is relative to the session attachment directory. +type MaterializedAttachment struct { + Name string `json:"name"` + SizeBytes int64 `json:"size_bytes"` +} + +// MaterializeAttachment streams one claimed attachment to the agentctl +// session. The request body is multipart and remains bounded by the backend's +// descriptor limit; the client never base64-encodes the file. +func (c *Client) MaterializeAttachment( + ctx context.Context, + sessionID, attachmentID, name, mimeType string, + sizeBytes int64, + content io.Reader, +) (*MaterializedAttachment, error) { + if strings.TrimSpace(sessionID) == "" || strings.TrimSpace(attachmentID) == "" || content == nil { + return nil, fmt.Errorf("attachment session, id, and content are required") + } + if sizeBytes < 0 || sizeBytes > models.MaxMessageAttachmentBytes { + return nil, fmt.Errorf("attachment size exceeds maximum") + } + pipeReader, pipeWriter := io.Pipe() + multipartWriter := multipart.NewWriter(pipeWriter) + go func() { + if err := writeMaterializedAttachmentMultipart(multipartWriter, sessionID, attachmentID, name, mimeType, sizeBytes, content); err != nil { + _ = pipeWriter.CloseWithError(err) + return + } + _ = pipeWriter.Close() + }() + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+"/api/v1/attachments/materialize", pipeReader) + if err != nil { + _ = pipeReader.Close() + return nil, fmt.Errorf("create attachment materialization request: %w", err) + } + req.Header.Set("Content-Type", multipartWriter.FormDataContentType()) + resp, err := c.longRunningHTTPClient.Do(req) + if err != nil { + _ = pipeReader.Close() + return nil, fmt.Errorf("materialize attachment: %w", err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 16*1024)) + return nil, fmt.Errorf("materialize attachment failed with status %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + } + var result MaterializedAttachment + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("decode materialized attachment: %w", err) + } + if strings.TrimSpace(result.Name) == "" || result.SizeBytes != sizeBytes { + return nil, fmt.Errorf("materialize attachment returned invalid metadata") + } + return &result, nil +} + +func writeMaterializedAttachmentMultipart( + writer *multipart.Writer, + sessionID, attachmentID, name, mimeType string, + sizeBytes int64, + content io.Reader, +) error { + fields := [...]struct { + key string + value string + }{ + {key: "session_id", value: sessionID}, + {key: "attachment_id", value: attachmentID}, + {key: "name", value: name}, + {key: "mime_type", value: mimeType}, + {key: "size_bytes", value: strconv.FormatInt(sizeBytes, 10)}, + } + for _, field := range fields { + if err := writer.WriteField(field.key, field.value); err != nil { + return err + } + } + part, err := writer.CreateFormFile("file", name) + if err != nil { + return err + } + if _, err := io.Copy(part, content); err != nil { + return err + } + return writer.Close() +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager.go b/apps/backend/internal/agent/runtime/lifecycle/manager.go index c3c787bf60..81e4761209 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager.go @@ -379,6 +379,15 @@ func (m *Manager) SetSessionAccessChecker(check func(ctx context.Context, sessio m.sessionAccessCheck = check } +// SetAttachmentReader wires the backend attachment reader used by prompt +// dispatch. Claimed descriptors are streamed into the active agentctl +// session immediately before an ACP prompt is sent. +func (m *Manager) SetAttachmentReader(reader AttachmentReader) { + if m.sessionManager != nil { + m.sessionManager.SetAttachmentReader(reader) + } +} + // SetEnvironmentAccessChecker installs the per-user environment visibility // check used by GetOrEnsureExecutionForEnvironment (terminal env-shell route). func (m *Manager) SetEnvironmentAccessChecker(check func(ctx context.Context, environmentID string) error) { diff --git a/apps/backend/internal/agent/runtime/lifecycle/session.go b/apps/backend/internal/agent/runtime/lifecycle/session.go index 971d794c71..e5db82b464 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/session.go +++ b/apps/backend/internal/agent/runtime/lifecycle/session.go @@ -23,7 +23,11 @@ import ( "go.opentelemetry.io/otel/trace" ) -const modelConfigOptionID = "model" +const ( + modelConfigOptionID = "model" + attachmentDeliveryPath = "path" + attachmentDeliveryPrompt = "prompt" +) // SessionManager handles ACP session initialization and management type SessionManager struct { @@ -34,6 +38,7 @@ type SessionManager struct { promptStarter func(executionID string) (uint64, error) initialPromptFailure func(executionID string) historyManager *SessionHistoryManager + attachmentReader AttachmentReader stopCh <-chan struct{} // For graceful shutdown coordination } @@ -54,6 +59,12 @@ func (sm *SessionManager) SetDependencies(ep *EventPublisher, strm *StreamManage sm.historyManager = history } +// SetAttachmentReader wires the backend attachment reader used to materialize +// claimed file descriptors into the active agentctl session before prompts. +func (sm *SessionManager) SetAttachmentReader(reader AttachmentReader) { + sm.attachmentReader = reader +} + func (sm *SessionManager) SetPromptStarter(starter func(executionID string) (uint64, error)) { sm.promptStarter = starter } @@ -653,10 +664,12 @@ func convertAttachments(attachments []MessageAttachment) []v1.MessageAttachment result := make([]v1.MessageAttachment, 0, len(attachments)) for _, att := range attachments { result = append(result, v1.MessageAttachment{ + AttachmentID: att.AttachmentID, Type: att.Type, Data: att.Data, MimeType: att.MimeType, Name: att.Name, + SizeBytes: att.SizeBytes, DeliveryMode: att.DeliveryMode, }) } @@ -912,12 +925,84 @@ func (sm *SessionManager) sendPrompt( if err != nil { return nil, err } - if err := sm.triggerPrompt(preparedCtx, execution, effectivePrompt, attachments, promptGeneration); err != nil { + materializedAttachments, err := sm.materializeAttachments(preparedCtx, execution, attachments) + if err != nil { + return nil, err + } + if err := sm.triggerPrompt(preparedCtx, execution, effectivePrompt, materializedAttachments, promptGeneration); err != nil { return nil, err } return sm.finishAcceptedPrompt(preparedCtx, execution, dispatchOnly, onDispatched, promptGeneration) } +// materializeAttachments resolves claimed backend descriptors into the active +// agentctl session. The prompt and queue protocols carry only bounded +// descriptors; bytes are streamed directly from backend storage to agentctl. +// Materialized descriptors preserve the requested delivery mode. Path-mode +// files stay on disk; prompt-mode images are converted to native ACP content +// by agentctl from the materialized file. Legacy inline attachments are left +// intact. +func (sm *SessionManager) materializeAttachments( + ctx context.Context, + execution *AgentExecution, + attachments []v1.MessageAttachment, +) ([]v1.MessageAttachment, error) { + if len(attachments) == 0 { + return attachments, nil + } + result := append([]v1.MessageAttachment(nil), attachments...) + for i, attachment := range result { + if attachment.AttachmentID == "" { + continue + } + if sm.attachmentReader == nil { + return nil, fmt.Errorf("attachment %q cannot be delivered: attachment storage is unavailable", attachment.AttachmentID) + } + if execution.ACPSessionID == "" { + return nil, fmt.Errorf("attachment %q cannot be delivered: ACP session is not ready", attachment.AttachmentID) + } + reader, name, mimeType, sizeBytes, err := sm.attachmentReader.OpenClaimed( + ctx, attachment.AttachmentID, execution.TaskID, execution.SessionID, + ) + if err != nil { + return nil, fmt.Errorf("open attachment %q: %w", attachment.AttachmentID, err) + } + materialized, materializeErr := execution.agentctl.MaterializeAttachment( + ctx, + execution.ACPSessionID, + attachment.AttachmentID, + name, + mimeType, + sizeBytes, + reader, + ) + closeErr := reader.Close() + if materializeErr != nil { + return nil, fmt.Errorf("materialize attachment %q: %w", attachment.AttachmentID, materializeErr) + } + if closeErr != nil { + return nil, fmt.Errorf("close attachment %q: %w", attachment.AttachmentID, closeErr) + } + if materialized == nil || strings.TrimSpace(materialized.Name) == "" { + return nil, fmt.Errorf("materialize attachment %q: agentctl returned no filename", attachment.AttachmentID) + } + result[i].Name = materialized.Name + result[i].MimeType = mimeType + result[i].SizeBytes = sizeBytes + result[i].Data = "" + if result[i].DeliveryMode == "" { + result[i].DeliveryMode = attachmentDeliveryPrompt + } + if result[i].DeliveryMode != attachmentDeliveryPath { + sm.logger.Debug("preserving native delivery for staged attachment", + zap.String("attachment_id", attachment.AttachmentID), + zap.String("requested_delivery_mode", attachment.DeliveryMode), + zap.Int64("size_bytes", sizeBytes)) + } + } + return result, nil +} + func (sm *SessionManager) preparePrompt( ctx context.Context, execution *AgentExecution, diff --git a/apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go b/apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go new file mode 100644 index 0000000000..e79c8d130c --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go @@ -0,0 +1,115 @@ +package lifecycle + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "strings" + "testing" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + "github.com/kandev/kandev/internal/common/logger" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +type testAttachmentReader struct{} + +func (testAttachmentReader) OpenClaimed(context.Context, string, string, string) (io.ReadCloser, string, string, int64, error) { + return io.NopCloser(strings.NewReader("attachment bytes")), "bundle.zip", "application/zip", 16, nil +} + +func TestMaterializeAttachmentsStreamsClaimedDescriptor(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/attachments/materialize" { + t.Fatalf("path = %q", r.URL.Path) + } + if err := r.ParseMultipartForm(1 << 20); err != nil { + t.Fatalf("parse multipart form: %v", err) + } + if got := r.FormValue("session_id"); got != "acp-session" { + t.Errorf("session_id = %q", got) + } + file, _, err := r.FormFile("file") + if err != nil { + t.Fatalf("form file: %v", err) + } + defer func() { _ = file.Close() }() + body, err := io.ReadAll(file) + if err != nil { + t.Fatalf("read file: %v", err) + } + if string(body) != "attachment bytes" { + t.Errorf("body = %q", body) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"name":"bundle.zip","size_bytes":16}`)) + })) + defer server.Close() + + parsed, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + port, err := strconv.Atoi(parsed.Port()) + if err != nil { + t.Fatal(err) + } + sm := NewSessionManager(logger.Default(), nil) + sm.SetAttachmentReader(testAttachmentReader{}) + execution := &AgentExecution{ + TaskID: "task-1", + SessionID: "session-1", + ACPSessionID: "acp-session", + agentctl: agentctl.NewClient(parsed.Hostname(), port, logger.Default()), + } + + attachments, err := sm.materializeAttachments(context.Background(), execution, []v1.MessageAttachment{ + {AttachmentID: "attachment-1", Type: "resource", Name: "bundle.zip", SizeBytes: 16}, + }) + if err != nil { + t.Fatal(err) + } + if len(attachments) != 1 { + t.Fatalf("attachments = %+v", attachments) + } + if attachments[0].DeliveryMode != "prompt" || attachments[0].Data != "" || attachments[0].Name != "bundle.zip" { + t.Fatalf("materialized attachment = %+v", attachments[0]) + } +} + +func TestMaterializeAttachmentsPreservesPromptDeliveryMode(t *testing.T) { + sm := NewSessionManager(logger.Default(), nil) + sm.SetAttachmentReader(testAttachmentReader{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := r.ParseMultipartForm(1 << 20); err != nil { + t.Fatalf("parse multipart form: %v", err) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"name":"photo.png","size_bytes":16}`)) + })) + defer server.Close() + parsed, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + port, err := strconv.Atoi(parsed.Port()) + if err != nil { + t.Fatal(err) + } + execution := &AgentExecution{ + TaskID: "task-1", SessionID: "session-1", ACPSessionID: "acp-session", + agentctl: agentctl.NewClient(parsed.Hostname(), port, logger.Default()), + } + attachments, err := sm.materializeAttachments(context.Background(), execution, []v1.MessageAttachment{ + {AttachmentID: "attachment-1", Type: "image", Name: "photo.png", DeliveryMode: "prompt"}, + }) + if err != nil { + t.Fatal(err) + } + if len(attachments) != 1 || attachments[0].DeliveryMode != "prompt" { + t.Fatalf("materialized attachment = %+v, want prompt delivery", attachments) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index 23a6f2dd49..9236a4d145 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -4,6 +4,7 @@ package lifecycle import ( "context" + "io" "strings" "sync" "sync/atomic" @@ -743,13 +744,22 @@ func (r *LaunchRequest) RepoSpecs() []RepoLaunchSpec { // MessageAttachment represents an image or file attachment for agent prompts. type MessageAttachment struct { + AttachmentID string // file-backed descriptor ID; Data is empty for staged files Type string // "image", "audio", or "resource" Data string // base64-encoded data MimeType string // MIME type Name string // optional filename for resource attachments + SizeBytes int64 // raw byte size for file-backed descriptors DeliveryMode string // "prompt" (native/default) or "path" } +// AttachmentReader opens an authorized, claimed backend attachment for +// streaming into the active agent execution. Implementations must enforce the +// task/session ownership checks before returning any bytes. +type AttachmentReader interface { + OpenClaimed(ctx context.Context, id, taskID, sessionID string) (io.ReadCloser, string, string, int64, error) +} + // CredentialsManager interface for credential retrieval type CredentialsManager interface { GetCredentialValue(ctx context.Context, key string) (value string, err error) diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go index a34199431a..f541cf0fd1 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go @@ -2,8 +2,10 @@ package acp import ( "context" + "encoding/base64" "errors" "fmt" + "os" "github.com/coder/acp-go-sdk" "github.com/kandev/kandev/internal/agentctl/server/adapter/transport/shared" @@ -13,6 +15,8 @@ import ( "go.uber.org/zap" ) +const maxNativeAttachmentBytes int64 = 8 << 20 + // Prompt sends a prompt to the agent. // If pending context is set (from SetPendingContext), it will be prepended to the message. // Attachments (images) are converted to ACP ImageBlocks and included in the prompt. @@ -260,6 +264,19 @@ func (a *Adapter) buildPromptContentBlocks(message string, attachments []v1.Mess contentBlocks = append(contentBlocks, acp.TextBlock(shared.BuildAttachmentPrompt(saved, true))) continue } + if att.AttachmentID != "" && att.Data == "" && (att.Type == contentTypeImage || att.Type == contentTypeAudio) { + data, saved, ok := a.loadMaterializedAttachmentData(att) + switch { + case ok: + att.Data = data + case len(saved) > 0: + contentBlocks = append(contentBlocks, acp.TextBlock(shared.BuildAttachmentPrompt(saved, false))) + continue + default: + a.logger.Warn("failed to load materialized prompt attachment", + zap.String("attachment_id", att.AttachmentID), zap.String("name", att.Name)) + } + } switch att.Type { case contentTypeImage: @@ -286,6 +303,25 @@ func (a *Adapter) buildPromptContentBlocks(message string, attachments []v1.Mess return contentBlocks } +func (a *Adapter) loadMaterializedAttachmentData(att v1.MessageAttachment) (string, []shared.SavedAttachment, bool) { + if a.attachMgr == nil { + return "", nil, false + } + saved, err := a.attachMgr.SaveAttachments([]v1.MessageAttachment{att}) + if err != nil || len(saved) == 0 { + return "", nil, false + } + info, err := os.Stat(saved[0].AbsPath) + if err != nil || info.Size() > maxNativeAttachmentBytes { + return "", saved, false + } + data, err := os.ReadFile(saved[0].AbsPath) + if err != nil { + return "", saved, false + } + return base64.StdEncoding.EncodeToString(data), saved, true +} + func buildAttachmentFallbackBlock(att v1.MessageAttachment) acp.ContentBlock { switch att.Type { case contentTypeImage: diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_test.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_test.go index afb0a84f8d..f95dde5685 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_test.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_test.go @@ -164,6 +164,79 @@ func TestBuildPromptContentBlocks_PathModeAttachmentSavesWritableFile(t *testing } } +func TestBuildPromptContentBlocks_PromptModeMaterializedImageUsesFileBytes(t *testing.T) { + a := newTestAdapter() + t.Cleanup(func() { _ = a.Close() }) + + workDir := t.TempDir() + dir := filepath.Join(workDir, ".kandev", "attachments", "sess-prompt") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "shot.png"), []byte("materialized image"), 0o600); err != nil { + t.Fatal(err) + } + a.attachMgr = shared.NewAttachmentManager(workDir, a.logger.Zap()) + a.attachMgr.SetSessionID("sess-prompt") + a.capabilities = acp.AgentCapabilities{PromptCapabilities: acp.PromptCapabilities{Image: true}} + + blocks := a.buildPromptContentBlocks("inspect this", []v1.MessageAttachment{{ + Type: "image", + AttachmentID: "attachment-1", + MimeType: "image/png", + Name: "shot.png", + DeliveryMode: "prompt", + }}) + + if len(blocks) != 2 || blocks[1].Image == nil { + t.Fatalf("expected materialized prompt image block, got %#v", blocks) + } + if blocks[1].Image.Data != base64.StdEncoding.EncodeToString([]byte("materialized image")) { + t.Fatalf("image data = %q, want materialized bytes", blocks[1].Image.Data) + } +} + +func TestBuildPromptContentBlocks_LargeMaterializedImageUsesReadOnlyPath(t *testing.T) { + a := newTestAdapter() + t.Cleanup(func() { _ = a.Close() }) + + workDir := t.TempDir() + dir := filepath.Join(workDir, ".kandev", "attachments", "sess-large-prompt") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "large.png") + file, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + if err := file.Truncate(maxNativeAttachmentBytes + 1); err != nil { + _ = file.Close() + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + a.attachMgr = shared.NewAttachmentManager(workDir, a.logger.Zap()) + a.attachMgr.SetSessionID("sess-large-prompt") + a.capabilities = acp.AgentCapabilities{PromptCapabilities: acp.PromptCapabilities{Image: true}} + + blocks := a.buildPromptContentBlocks("inspect this", []v1.MessageAttachment{{ + Type: "image", + AttachmentID: "attachment-large", + MimeType: "image/png", + Name: "large.png", + DeliveryMode: "prompt", + }}) + + if len(blocks) != 2 || blocks[1].Text == nil { + t.Fatalf("expected large prompt image to use a path block, got %#v", blocks) + } + if blocks[1].Image != nil || !strings.Contains(blocks[1].Text.Text, "large.png") { + t.Fatalf("expected read-only path fallback, got %#v", blocks[1]) + } +} + func TestBuildPromptContentBlocks_PathModeSaveFailureFallsBackToNativeBlock(t *testing.T) { a := newTestAdapter() t.Cleanup(func() { _ = a.Close() }) diff --git a/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments.go b/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments.go index 5b40088741..5c79de3d94 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "errors" "fmt" + "io" "os" "path/filepath" "strings" @@ -35,6 +36,8 @@ type AttachmentManager struct { logger *zap.Logger } +var ErrMaterializedAttachmentMissing = errors.New("materialized attachment is missing") + // NewAttachmentManager creates a new AttachmentManager. // sessionID can be empty initially and set later via SetSessionID. func NewAttachmentManager(workDir string, logger *zap.Logger) *AttachmentManager { @@ -58,8 +61,14 @@ func (m *AttachmentManager) SaveAttachments(attachments []v1.MessageAttachment) if m.workDir == "" || m.sessionID == "" { return nil, fmt.Errorf("workDir or sessionID not set") } + if !isSafeAttachmentComponent(m.sessionID) { + return nil, fmt.Errorf("invalid sessionID") + } - dir := filepath.Join(m.workDir, ".kandev", "attachments", m.sessionID) + dir, err := safeAttachmentPath(m.workDir, ".kandev", "attachments", m.sessionID) + if err != nil { + return nil, err + } if err := os.MkdirAll(dir, 0o755); err != nil { return nil, fmt.Errorf("create attachments dir: %w", err) } @@ -71,12 +80,33 @@ func (m *AttachmentManager) SaveAttachments(attachments []v1.MessageAttachment) if name == "" { name = m.generateName(att) } - // Sanitize: strip directory components to prevent path traversal attacks. name = filepath.Base(name) - if name == "." || name == ".." || name == string(filepath.Separator) { + if !isSafeAttachmentComponent(name) { m.logger.Warn("skipping attachment with invalid name", zap.String("original_name", att.Name)) continue } + if att.AttachmentID != "" && att.Data == "" { + // Descriptor attachments are materialized by the backend before the + // prompt reaches agentctl. Reuse that file instead of trying to + // decode an empty base64 payload (which would create a zero-byte file). + absPath, pathErr := safeAttachmentPath(dir, name) + if pathErr != nil { + m.logger.Warn("skipping attachment with invalid path", zap.String("name", name), zap.Error(pathErr)) + continue + } + info, statErr := os.Stat(absPath) + if statErr != nil || !info.Mode().IsRegular() { + return nil, fmt.Errorf("%w: %s", ErrMaterializedAttachmentMissing, att.AttachmentID) + } + usedNames[name] = true + relPath := filepath.Join(".kandev", "attachments", m.sessionID, name) + saved = append(saved, SavedAttachment{ + RelPath: relPath, AbsPath: absPath, Name: name, + MimeType: att.MimeType, Type: att.Type, + }) + m.logger.Debug("reused materialized attachment", zap.String("path", relPath), zap.Int64("size", info.Size())) + continue + } decoded, err := base64.StdEncoding.DecodeString(att.Data) if err != nil { m.logger.Warn("failed to decode attachment", zap.String("name", name), zap.Error(err)) @@ -105,13 +135,65 @@ func (m *AttachmentManager) SaveAttachments(attachments []v1.MessageAttachment) return saved, nil } +// SaveAttachmentStream writes one claimed attachment without buffering its +// contents in memory. The caller owns src and must close it. +func (m *AttachmentManager) SaveAttachmentStream(att v1.MessageAttachment, src io.Reader) (SavedAttachment, error) { + if src == nil { + return SavedAttachment{}, fmt.Errorf("attachment stream is nil") + } + if m.workDir == "" || m.sessionID == "" { + return SavedAttachment{}, fmt.Errorf("workDir or sessionID not set") + } + if !isSafeAttachmentComponent(m.sessionID) { + return SavedAttachment{}, fmt.Errorf("invalid sessionID") + } + if att.SizeBytes < 0 { + return SavedAttachment{}, fmt.Errorf("attachment size is invalid") + } + + dir, err := safeAttachmentPath(m.workDir, ".kandev", "attachments", m.sessionID) + if err != nil { + return SavedAttachment{}, err + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return SavedAttachment{}, fmt.Errorf("create attachments dir: %w", err) + } + + name := att.Name + if name == "" { + name = m.generateName(att) + } + name = filepath.Base(name) + if !isSafeAttachmentComponent(name) { + return SavedAttachment{}, fmt.Errorf("invalid attachment name") + } + + name, absPath, err := writeUniqueAttachmentStream(dir, name, make(map[string]bool), src, att.SizeBytes) + if err != nil { + return SavedAttachment{}, err + } + relPath := filepath.Join(".kandev", "attachments", m.sessionID, name) + saved := SavedAttachment{ + RelPath: relPath, + AbsPath: absPath, + Name: name, + MimeType: att.MimeType, + Type: att.Type, + } + m.logger.Debug("saved streamed attachment", zap.String("path", relPath), zap.Int64("size", att.SizeBytes)) + return saved, nil +} + // Cleanup removes the session's attachment directory. // Safe to call multiple times or with empty sessionID (no-op). func (m *AttachmentManager) Cleanup() { if m.sessionID == "" || m.workDir == "" { return } - dir := filepath.Join(m.workDir, ".kandev", "attachments", m.sessionID) + dir, err := safeAttachmentPath(m.workDir, ".kandev", "attachments", m.sessionID) + if err != nil { + return + } if err := os.RemoveAll(dir); err != nil { m.logger.Debug("failed to clean attachments dir", zap.String("dir", dir), zap.Error(err)) } @@ -164,7 +246,10 @@ func writeUniqueAttachmentFile(dir, name string, used map[string]bool, data []by continue } - absPath := filepath.Join(dir, candidate) + absPath, err := safeAttachmentPath(dir, candidate) + if err != nil { + return "", "", err + } file, err := os.OpenFile(absPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) if errors.Is(err, os.ErrExist) { used[candidate] = true @@ -188,6 +273,72 @@ func writeUniqueAttachmentFile(dir, name string, used map[string]bool, data []by } } +func writeUniqueAttachmentStream(dir, name string, used map[string]bool, src io.Reader, expectedSize int64) (string, string, error) { + ext := filepath.Ext(name) + base := strings.TrimSuffix(name, ext) + for i := 1; ; i++ { + candidate := name + if i > 1 { + candidate = fmt.Sprintf("%s-%d%s", base, i, ext) + } + if used[candidate] { + continue + } + + absPath, err := safeAttachmentPath(dir, candidate) + if err != nil { + return "", "", err + } + file, err := os.OpenFile(absPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if errors.Is(err, os.ErrExist) { + used[candidate] = true + continue + } + if err != nil { + return "", absPath, err + } + + written, copyErr := io.Copy(file, io.LimitReader(src, expectedSize+1)) + closeErr := file.Close() + if copyErr == nil && written != expectedSize { + copyErr = fmt.Errorf("attachment size mismatch: wrote %d bytes, expected %d", written, expectedSize) + } + if copyErr != nil { + _ = os.Remove(absPath) + return "", absPath, copyErr + } + if closeErr != nil { + _ = os.Remove(absPath) + return "", absPath, closeErr + } + return candidate, absPath, nil + } +} + +func isSafeAttachmentComponent(value string) bool { + return value != "" && filepath.IsLocal(value) && filepath.Base(value) == value && !strings.ContainsAny(value, "/\\\x00") +} + +func safeAttachmentPath(root string, components ...string) (string, error) { + if root == "" { + return "", fmt.Errorf("attachment root is required") + } + parts := make([]string, 0, len(components)+1) + parts = append(parts, root) + for _, component := range components { + if !isSafeAttachmentComponent(component) { + return "", fmt.Errorf("invalid attachment path component") + } + parts = append(parts, component) + } + path := filepath.Join(parts...) + rel, err := filepath.Rel(root, path) + if err != nil || filepath.IsAbs(rel) || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("attachment path escapes root") + } + return path, nil +} + func sanitizePromptValue(value string) string { replacer := strings.NewReplacer("\r", " ", "\n", " ", "<", "(", ">", ")", "`", "'") return strings.TrimSpace(replacer.Replace(value)) diff --git a/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments_test.go b/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments_test.go index 7017dd20f5..33b5d4fb25 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments_test.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments_test.go @@ -2,6 +2,7 @@ package shared import ( "encoding/base64" + "errors" "fmt" "os" "path/filepath" @@ -69,6 +70,16 @@ func TestSaveAttachments_MissingWorkDir(t *testing.T) { } } +func TestSaveAttachments_RejectsPathTraversalSession(t *testing.T) { + mgr := NewAttachmentManager(t.TempDir(), testLogger()) + mgr.SetSessionID("../escape") + + _, err := mgr.SaveAttachments([]v1.MessageAttachment{{Type: "resource", Data: "", MimeType: "text/plain", Name: "note.txt"}}) + if err == nil { + t.Fatal("expected unsafe session id to be rejected") + } +} + func TestSaveAttachments_ImageAttachment(t *testing.T) { workDir := t.TempDir() mgr := NewAttachmentManager(workDir, testLogger()) @@ -147,6 +158,50 @@ func TestSaveAttachments_ResourceAttachment(t *testing.T) { } } +func TestSaveAttachments_ReusesMaterializedDescriptor(t *testing.T) { + workDir := t.TempDir() + dir := filepath.Join(workDir, ".kandev", "attachments", "sess-materialized") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + want := []byte("already streamed by backend") + path := filepath.Join(dir, "bundle.zip") + if err := os.WriteFile(path, want, 0o600); err != nil { + t.Fatal(err) + } + + mgr := NewAttachmentManager(workDir, testLogger()) + mgr.SetSessionID("sess-materialized") + saved, err := mgr.SaveAttachments([]v1.MessageAttachment{ + {AttachmentID: "attachment-1", Type: "resource", MimeType: "application/zip", Name: "bundle.zip"}, + }) + if err != nil { + t.Fatal(err) + } + if len(saved) != 1 || saved[0].AbsPath != path { + t.Fatalf("saved = %+v, want materialized path %q", saved, path) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != string(want) { + t.Fatalf("materialized file changed: %q", got) + } +} + +func TestSaveAttachments_ReturnsTypedErrorForMissingMaterializedDescriptor(t *testing.T) { + mgr := NewAttachmentManager(t.TempDir(), testLogger()) + mgr.SetSessionID("sess-missing") + + _, err := mgr.SaveAttachments([]v1.MessageAttachment{{ + AttachmentID: "missing", Type: "resource", MimeType: "text/plain", Name: "missing.txt", + }}) + if !errors.Is(err, ErrMaterializedAttachmentMissing) { + t.Fatalf("error = %v, want ErrMaterializedAttachmentMissing", err) + } +} + func TestSaveAttachments_MultipleAttachments(t *testing.T) { workDir := t.TempDir() mgr := NewAttachmentManager(workDir, testLogger()) diff --git a/apps/backend/internal/agentctl/server/api/attachments.go b/apps/backend/internal/agentctl/server/api/attachments.go new file mode 100644 index 0000000000..571994e6f3 --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/attachments.go @@ -0,0 +1,205 @@ +package api + +import ( + "errors" + "fmt" + "io" + "mime/multipart" + "net/http" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/task/models" + "go.uber.org/zap" +) + +const maxMaterializedAttachmentRequestBytes int64 = models.MaxMessageAttachmentBytes + 4*1024*1024 + +type materializedAttachmentResponse struct { + Name string `json:"name"` + SizeBytes int64 `json:"size_bytes"` +} + +type materializedAttachmentUpload struct { + sessionID string + attachmentID string + name string + mimeType string + declaredSize int64 + fileHeader *multipart.FileHeader +} + +func (s *Server) handleMaterializeAttachment(c *gin.Context) { + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxMaterializedAttachmentRequestBytes) + if err := c.Request.ParseMultipartForm(16 << 20); err != nil { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "attachment request is too large"}) + return + } + upload, status, message := parseMaterializedAttachmentUpload(c) + if status != 0 { + c.JSON(status, gin.H{"error": message}) + return + } + response, err := s.persistMaterializedAttachment(upload) + if err != nil { + if status, message, ok := materializedAttachmentError(err); ok { + c.JSON(status, gin.H{"error": message}) + return + } + s.logger.Error("materialize attachment", zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{"error": "attachment storage unavailable"}) + return + } + c.JSON(http.StatusCreated, response) +} + +func parseMaterializedAttachmentUpload(c *gin.Context) (materializedAttachmentUpload, int, string) { + upload := materializedAttachmentUpload{ + sessionID: strings.TrimSpace(c.PostForm("session_id")), + attachmentID: strings.TrimSpace(c.PostForm("attachment_id")), + name: strings.TrimSpace(c.PostForm("name")), + mimeType: strings.TrimSpace(c.PostForm("mime_type")), + } + declaredSize, err := strconv.ParseInt(strings.TrimSpace(c.PostForm("size_bytes")), 10, 64) + if upload.sessionID == "" || upload.attachmentID == "" || upload.name == "" || upload.mimeType == "" || err != nil || declaredSize < 0 || declaredSize > models.MaxMessageAttachmentBytes { + return upload, http.StatusBadRequest, "attachment metadata is invalid" + } + upload.declaredSize = declaredSize + if !isSafeAttachmentComponent(upload.sessionID) || !isSafeAttachmentComponent(upload.attachmentID) { + return upload, http.StatusBadRequest, "attachment identity is invalid" + } + upload.fileHeader, err = c.FormFile("file") + if err != nil { + return upload, http.StatusBadRequest, "file is required" + } + return upload, 0, "" +} + +func (s *Server) persistMaterializedAttachment(upload materializedAttachmentUpload) (materializedAttachmentResponse, error) { + file, err := upload.fileHeader.Open() + if err != nil { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusBadRequest, "file is unreadable") + } + defer func() { _ = file.Close() }() + + dir, err := safeAttachmentPath(s.cfg.WorkDir, ".kandev", "attachments", upload.sessionID) + if err != nil { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusBadRequest, "attachment identity is invalid") + } + if err := os.MkdirAll(dir, 0o700); err != nil { + s.logger.Error("create agent attachment directory", zap.Error(err)) + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusInternalServerError, "attachment storage unavailable") + } + tmp, err := os.CreateTemp(dir, ".upload-*") + if err != nil { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusInternalServerError, "attachment storage unavailable") + } + tmpPath := tmp.Name() + cleanup := func() { + _ = tmp.Close() + _ = os.Remove(tmpPath) + } + defer cleanup() + n, copyErr := io.Copy(tmp, io.LimitReader(file, models.MaxMessageAttachmentBytes+1)) + if copyErr != nil { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusBadRequest, "attachment upload failed") + } + if n != upload.declaredSize || n > models.MaxMessageAttachmentBytes { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusRequestEntityTooLarge, "attachment size does not match its descriptor") + } + if err := tmp.Close(); err != nil { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusInternalServerError, "attachment storage unavailable") + } + if err := os.Chmod(tmpPath, 0o600); err != nil { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusInternalServerError, "attachment storage unavailable") + } + destinationName := safeAttachmentName(upload.name) + if destinationName == "" { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusBadRequest, "attachment name is invalid") + } + destinationName, err = installMaterializedAttachment(dir, tmpPath, destinationName) + if err != nil { + return materializedAttachmentResponse{}, newMaterializedAttachmentError(http.StatusInternalServerError, "attachment storage unavailable") + } + return materializedAttachmentResponse{Name: destinationName, SizeBytes: n}, nil +} + +type materializedAttachmentHTTPError struct { + status int + message string +} + +func (e *materializedAttachmentHTTPError) Error() string { return e.message } + +func newMaterializedAttachmentError(status int, message string) error { + return &materializedAttachmentHTTPError{status: status, message: message} +} + +func materializedAttachmentError(err error) (int, string, bool) { + var httpErr *materializedAttachmentHTTPError + if !errors.As(err, &httpErr) { + return 0, "", false + } + return httpErr.status, httpErr.message, true +} + +func safeAttachmentName(name string) string { + if !isSafeAttachmentComponent(name) { + return "" + } + return name +} + +func isSafeAttachmentComponent(value string) bool { + return value != "" && filepath.IsLocal(value) && filepath.Base(value) == value && !strings.ContainsAny(value, "/\\\x00") +} + +func safeAttachmentPath(root string, components ...string) (string, error) { + if root == "" { + return "", fmt.Errorf("attachment root is required") + } + parts := make([]string, 0, len(components)+1) + parts = append(parts, root) + for _, component := range components { + if !isSafeAttachmentComponent(component) { + return "", fmt.Errorf("invalid attachment path component") + } + parts = append(parts, component) + } + path := filepath.Join(parts...) + rel, err := filepath.Rel(root, path) + if err != nil || filepath.IsAbs(rel) || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("attachment path escapes root") + } + return path, nil +} + +func installMaterializedAttachment(dir, tmpPath, name string) (string, error) { + ext := filepath.Ext(name) + base := strings.TrimSuffix(name, ext) + for i := 1; i <= 10000; i++ { + candidate := name + if i > 1 { + candidate = fmt.Sprintf("%s-%d%s", base, i, ext) + } + candidatePath, err := safeAttachmentPath(dir, candidate) + if err != nil { + return "", err + } + if err := os.Link(tmpPath, candidatePath); err != nil { + if errors.Is(err, os.ErrExist) { + continue + } + return "", err + } + if err := os.Remove(tmpPath); err != nil { + _ = os.Remove(candidatePath) + return "", err + } + return candidate, nil + } + return "", fmt.Errorf("no available attachment destination") +} diff --git a/apps/backend/internal/agentctl/server/api/attachments_test.go b/apps/backend/internal/agentctl/server/api/attachments_test.go new file mode 100644 index 0000000000..6d76efd450 --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/attachments_test.go @@ -0,0 +1,198 @@ +package api + +import ( + "bytes" + "encoding/json" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strconv" + "sync" + "testing" + + "github.com/kandev/kandev/internal/agentctl/server/config" + "github.com/kandev/kandev/internal/agentctl/server/process" +) + +func TestMaterializeAttachment_StreamsIntoSessionDirectory(t *testing.T) { + workDir := t.TempDir() + log := newTestLogger() + cfg := &config.InstanceConfig{Port: 0, WorkDir: workDir, AuthToken: "test-token"} + server := NewServer(cfg, process.NewManager(cfg, log), nil, nil, log) + + content := []byte("diagnostic bytes") + var body bytes.Buffer + writer := multipart.NewWriter(&body) + for key, value := range map[string]string{ + "session_id": "acp-session", + "attachment_id": "attachment-1", + "name": "diagnostic.zip", + "mime_type": "application/zip", + "size_bytes": "16", + } { + if err := writer.WriteField(key, value); err != nil { + t.Fatal(err) + } + } + part, err := writer.CreateFormFile("file", "diagnostic.zip") + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(content); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, "/api/v1/attachments/materialize", &body) + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("Content-Type", writer.FormDataContentType()) + response := httptest.NewRecorder() + server.Router().ServeHTTP(response, req) + + if response.Code != http.StatusCreated { + t.Fatalf("status = %d, body = %s", response.Code, response.Body.String()) + } + var result materializedAttachmentResponse + if err := json.Unmarshal(response.Body.Bytes(), &result); err != nil { + t.Fatal(err) + } + if result.Name != "diagnostic.zip" || result.SizeBytes != int64(len(content)) { + t.Fatalf("response = %+v", result) + } + path := filepath.Join(workDir, ".kandev", "attachments", "acp-session", result.Name) + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, content) { + t.Fatalf("materialized content = %q, want %q", got, content) + } +} + +func TestMaterializeAttachment_RejectsDescriptorSizeMismatch(t *testing.T) { + workDir := t.TempDir() + log := newTestLogger() + cfg := &config.InstanceConfig{Port: 0, WorkDir: workDir, AuthToken: "test-token"} + server := NewServer(cfg, process.NewManager(cfg, log), nil, nil, log) + + var body bytes.Buffer + writer := multipart.NewWriter(&body) + for key, value := range map[string]string{ + "session_id": "acp-session", + "attachment_id": "attachment-1", + "name": "diagnostic.zip", + "mime_type": "application/zip", + "size_bytes": "17", + } { + if err := writer.WriteField(key, value); err != nil { + t.Fatal(err) + } + } + part, err := writer.CreateFormFile("file", "diagnostic.zip") + if err != nil { + t.Fatal(err) + } + _, _ = part.Write([]byte("diagnostic bytes")) + if err := writer.Close(); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, "/api/v1/attachments/materialize", &body) + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("Content-Type", writer.FormDataContentType()) + response := httptest.NewRecorder() + server.Router().ServeHTTP(response, req) + + if response.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, body = %s", response.Code, response.Body.String()) + } +} + +func TestMaterializeAttachment_RejectsPathTraversalIdentity(t *testing.T) { + workDir := t.TempDir() + log := newTestLogger() + cfg := &config.InstanceConfig{Port: 0, WorkDir: workDir, AuthToken: "test-token"} + server := NewServer(cfg, process.NewManager(cfg, log), nil, nil, log) + + var body bytes.Buffer + writer := multipart.NewWriter(&body) + for key, value := range map[string]string{ + "session_id": "../escape", + "attachment_id": "attachment-1", + "name": "diagnostic.zip", + "mime_type": "application/zip", + "size_bytes": "0", + } { + if err := writer.WriteField(key, value); err != nil { + t.Fatal(err) + } + } + part, err := writer.CreateFormFile("file", "diagnostic.zip") + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(nil); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + + req := httptest.NewRequest(http.MethodPost, "/api/v1/attachments/materialize", &body) + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("Content-Type", writer.FormDataContentType()) + response := httptest.NewRecorder() + server.Router().ServeHTTP(response, req) + + if response.Code != http.StatusBadRequest { + t.Fatalf("status = %d, body = %s", response.Code, response.Body.String()) + } +} + +func TestInstallMaterializedAttachment_NeverOverwritesConcurrentNames(t *testing.T) { + dir := t.TempDir() + type result struct { + name string + err error + data []byte + } + results := make(chan result, 8) + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + tmpPath := filepath.Join(dir, "upload-"+strconv.Itoa(i)) + content := []byte("content-" + strconv.Itoa(i)) + if err := os.WriteFile(tmpPath, content, 0o600); err != nil { + results <- result{err: err} + return + } + name, err := installMaterializedAttachment(dir, tmpPath, "same.txt") + var got []byte + if err == nil { + got, err = os.ReadFile(filepath.Join(dir, name)) + } + results <- result{name: name, err: err, data: got} + }(i) + } + wg.Wait() + close(results) + seen := make(map[string]struct{}) + for got := range results { + if got.err != nil { + t.Fatal(got.err) + } + if _, exists := seen[got.name]; exists { + t.Fatalf("duplicate destination name %q", got.name) + } + seen[got.name] = struct{}{} + if len(got.data) == 0 { + t.Fatalf("empty installed file for %q", got.name) + } + } +} diff --git a/apps/backend/internal/agentctl/server/api/server.go b/apps/backend/internal/agentctl/server/api/server.go index 4b8382077b..0fb679c33e 100644 --- a/apps/backend/internal/agentctl/server/api/server.go +++ b/apps/backend/internal/agentctl/server/api/server.go @@ -138,6 +138,7 @@ func (s *Server) setupRoutes() { // Docker, Sprites — to seed the workspace with gitignored config // after the in-container clone). api.POST("/workspace/copy-files", s.handleWorkspaceCopyFiles) + api.POST("/attachments/materialize", s.handleMaterializeAttachment) api.POST("/workspace/diagnostics/:id", s.handleWorkspaceDiagnostics) api.POST("/workspace/materialize-repository", s.handleWorkspaceMaterializeRepository) api.POST("/workspace/materialize-repository/remove", s.handleWorkspaceRemoveMaterializedRepository) diff --git a/apps/backend/internal/backendapp/adapters.go b/apps/backend/internal/backendapp/adapters.go index c2e5a1fe65..3e6c796f70 100644 --- a/apps/backend/internal/backendapp/adapters.go +++ b/apps/backend/internal/backendapp/adapters.go @@ -271,10 +271,12 @@ func convertToLifecycleAttachments(attachments []v1.MessageAttachment) []lifecyc result := make([]lifecycle.MessageAttachment, len(attachments)) for i, att := range attachments { result[i] = lifecycle.MessageAttachment{ + AttachmentID: att.AttachmentID, Type: att.Type, Data: att.Data, MimeType: att.MimeType, Name: att.Name, + SizeBytes: att.SizeBytes, DeliveryMode: att.DeliveryMode, } } diff --git a/apps/backend/internal/backendapp/gateway.go b/apps/backend/internal/backendapp/gateway.go index e0a7bbb338..7f78876884 100644 --- a/apps/backend/internal/backendapp/gateway.go +++ b/apps/backend/internal/backendapp/gateway.go @@ -160,6 +160,7 @@ func provideGateway( taskSvc, referenceValidator, ) + queueHandlers.SetAttachmentClaimer(taskSvc) queueHandlers.RegisterHandlers(gateway.Dispatcher) if lifecycleMgr != nil && agentRegistry != nil { diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index b9b78291f1..62f8b86ccf 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -544,6 +544,7 @@ type routeParams struct { httpPort int features config.FeaturesConfig voice config.VoiceConfig + homeDir string interimSettingsInterlockToken string log *logger.Logger } @@ -963,6 +964,11 @@ func resolveRepositoryIDForSessionSubpath(ctx context.Context, taskRepo *sqliter // registerTaskRoutes registers all task-related HTTP and WebSocket routes. func registerTaskRoutes(p routeParams, planService *taskservice.PlanService, handoffSvc *taskservice.HandoffService) { + if attachmentSvc := p.taskSvc.AttachmentService(); attachmentSvc != nil { + taskhandlers.RegisterAttachmentRoutes(p.router, attachmentSvc, p.log) + } else { + p.log.Warn("prompt attachment routes disabled: attachment service is unavailable") + } taskhandlers.RegisterWorkspaceRoutes(p.router, p.gateway.Dispatcher, p.taskSvc, p.log) if p.services != nil { registerMentionRoutes(p.router, p.services.Mentions) diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index 8eec41f9cd..a2a8e87fbf 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -829,6 +829,7 @@ func startGatewayAndServe( }) storageComposition, err := provideStorageComposition( cfg, dbPool, systemSvc.Jobs, lifecycleMgr, services.WorktreeMgr, services.Task, + log, func(message string, err error) { log.Error(message, zap.Error(err)) }, ) if err != nil { @@ -1838,6 +1839,7 @@ func buildHTTPServer( httpPort: resolvedHTTPPort(cfg), features: cfg.Features, voice: cfg.Voice, + homeDir: cfg.ResolvedHomeDir(), interimSettingsInterlockToken: interimSettingsInterlockToken, log: log, }) diff --git a/apps/backend/internal/backendapp/orchestrator.go b/apps/backend/internal/backendapp/orchestrator.go index 0ad783d0f9..c580f794d7 100644 --- a/apps/backend/internal/backendapp/orchestrator.go +++ b/apps/backend/internal/backendapp/orchestrator.go @@ -94,8 +94,18 @@ func provideOrchestrator( msgQueue := messagequeue.NewService(queueRepo, maxPerSession, log) log.Info("Message queue initialized", zap.Int("max_per_session", maxPerSession)) + if taskSvc.AttachmentService() == nil && taskSvc.AttachmentRepository() != nil { + attachmentSvc, attachmentErr := taskservice.NewAttachmentService( + taskSvc.AttachmentRepository(), cfg.ResolvedHomeDir(), taskSvc.AuthorizeWorkspaceAccess, log, + ) + if attachmentErr != nil { + return nil, nil, fmt.Errorf("initialize prompt attachment storage: %w", attachmentErr) + } + taskSvc.SetAttachmentService(attachmentSvc) + } orchestratorSvc := orchestrator.NewService(serviceCfg, eventBus, agentManagerClient, taskRepoAdapter, taskRepo, userSvc, secretStore, msgQueue, log) + orchestratorSvc.SetAttachmentReader(taskSvc.AttachmentService()) if githubSvc != nil { orchestratorSvc.SetGitHubCredentialBroker( githubExecutorCredentialLeaseAdapter{service: githubSvc}, diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index 2d72b6e82d..b2bb69d34c 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -70,6 +70,7 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories WorkspaceFolders: repos.Task, Workflows: repos.Task, Messages: repos.Task, + Attachments: repos.Task, Turns: repos.Task, Sessions: repos.Task, GitSnapshots: repos.Task, diff --git a/apps/backend/internal/backendapp/storage_maintenance.go b/apps/backend/internal/backendapp/storage_maintenance.go index c4aeed5f16..ce7176ceb3 100644 --- a/apps/backend/internal/backendapp/storage_maintenance.go +++ b/apps/backend/internal/backendapp/storage_maintenance.go @@ -12,6 +12,7 @@ import ( "github.com/kandev/kandev/internal/agent/runtime/activity" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/common/config" + "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/db" "github.com/kandev/kandev/internal/system/jobs" systemsettings "github.com/kandev/kandev/internal/system/settings" @@ -36,6 +37,7 @@ func provideStorageComposition( lifecycleMgr *lifecycle.Manager, worktreeMgr *worktree.Manager, taskSvc *taskservice.Service, + log *logger.Logger, logError func(string, error), ) (*storageComposition, error) { rawSettings, err := systemsettings.NewStore(pool) @@ -76,6 +78,21 @@ func provideStorageComposition( activity: coordinator, } providers := storageCleanupProviders(settings, workspaceFactory, goCache, dockerProvider, quarantine) + if taskSvc.AttachmentService() == nil && taskSvc.AttachmentRepository() != nil { + attachmentSvc, attachmentErr := taskservice.NewAttachmentService( + taskSvc.AttachmentRepository(), cfg.ResolvedHomeDir(), taskSvc.AuthorizeWorkspaceAccess, log, + ) + if attachmentErr != nil { + return nil, fmt.Errorf("initialize prompt attachment storage: %w", attachmentErr) + } + taskSvc.SetAttachmentService(attachmentSvc) + } + if attachmentSvc := taskSvc.AttachmentService(); attachmentSvc != nil { + if lifecycleMgr != nil { + lifecycleMgr.SetAttachmentReader(attachmentSvc) + } + providers = append(providers, attachmentCleanupProvider{service: attachmentSvc}) + } runner := storagepkg.NewRunner(storagepkg.RunnerConfig{ Activity: coordinator, Store: store, Providers: providers, Overview: cachedOverview, }) @@ -101,6 +118,17 @@ type taskCleanupActivityGate struct { coordinator *activity.Coordinator } +type attachmentCleanupProvider struct { + service *taskservice.AttachmentService +} + +func (p attachmentCleanupProvider) Name() string { return "prompt_attachments" } + +func (p attachmentCleanupProvider) Cleanup(ctx context.Context) (map[string]any, error) { + deleted, err := p.service.CleanupExpired(ctx) + return map[string]any{"deleted": deleted}, err +} + func (g *taskCleanupActivityGate) AcquireTaskResourceCleanup( ctx context.Context, ) (taskservice.TaskResourceCleanupActivityLease, error) { diff --git a/apps/backend/internal/orchestrator/event_handlers_agent.go b/apps/backend/internal/orchestrator/event_handlers_agent.go index d8b274b042..a3c08e9357 100644 --- a/apps/backend/internal/orchestrator/event_handlers_agent.go +++ b/apps/backend/internal/orchestrator/event_handlers_agent.go @@ -643,9 +643,11 @@ func (s *Service) executeQueuedMessage(callerSessionID string, queuedMsg *messag for i, att := range queuedMsg.Attachments { attachments[i] = v1.MessageAttachment{ Type: att.Type, + AttachmentID: att.AttachmentID, Data: att.Data, MimeType: att.MimeType, Name: att.Name, + SizeBytes: att.SizeBytes, DeliveryMode: att.DeliveryMode, } } diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index f341056fb9..a821bc305b 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -1879,9 +1879,11 @@ func (s *Service) takeAndMergeHandoffMessage(ctx context.Context, sessionID, bas for _, a := range msg.Attachments { attachments = append(attachments, v1.MessageAttachment{ Type: a.Type, + AttachmentID: a.AttachmentID, Data: a.Data, MimeType: a.MimeType, Name: a.Name, + SizeBytes: a.SizeBytes, DeliveryMode: a.DeliveryMode, }) } @@ -1964,9 +1966,11 @@ func toQueuedAttachments(attachments []v1.MessageAttachment) []messagequeue.Mess for _, attachment := range attachments { queued = append(queued, messagequeue.MessageAttachment{ Type: attachment.Type, + AttachmentID: attachment.AttachmentID, Data: attachment.Data, MimeType: attachment.MimeType, Name: attachment.Name, + SizeBytes: attachment.SizeBytes, DeliveryMode: attachment.DeliveryMode, }) } diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index d1657603c5..06e9c53bbf 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "io" "strings" "sync" "time" @@ -645,6 +646,12 @@ type ExecutorTypeCapabilities interface { ShouldApplyPreferredShell(executorType string) bool } +// AttachmentReader is the narrow attachment-store seam used to stream a +// claimed descriptor into a passthrough workspace. +type AttachmentReader interface { + OpenClaimed(ctx context.Context, id, taskID, sessionID string) (io.ReadCloser, string, string, int64, error) +} + // GitLabCredentialResolver returns the configured origin and credential for // exactly one workspace. Implementations must not fall back across workspaces. type GitLabCredentialResolver interface { @@ -654,6 +661,7 @@ type GitLabCredentialResolver interface { // Executor manages agent execution for tasks type Executor struct { agentManager AgentManagerClient + attachmentReader AttachmentReader repo executorStore secretStore secrets.SecretStore shellPrefs ShellPreferenceProvider @@ -839,6 +847,12 @@ func NewExecutor(agentManager AgentManagerClient, repo executorStore, log *logge } } +// SetAttachmentReader wires the backend attachment store used to stream +// claimed descriptors into passthrough workspaces. +func (e *Executor) SetAttachmentReader(reader AttachmentReader) { + e.attachmentReader = reader +} + // SetOnTaskStateChange sets a callback for task state changes. // This allows the orchestrator to route state changes through the task service layer // which publishes WebSocket events. Without this, async goroutines would only update diff --git a/apps/backend/internal/orchestrator/executor/executor_interaction.go b/apps/backend/internal/orchestrator/executor/executor_interaction.go index 647e093161..c747c9be59 100644 --- a/apps/backend/internal/orchestrator/executor/executor_interaction.go +++ b/apps/backend/internal/orchestrator/executor/executor_interaction.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "path/filepath" "strings" "time" @@ -386,7 +387,7 @@ func (e *Executor) buildPassthroughPromptWithAttachments(ctx context.Context, se } attachMgr := agentctlshared.NewAttachmentManager(workDir, e.logger.Zap()) attachMgr.SetSessionID(session.ID) - saved, err := attachMgr.SaveAttachments(attachments) + saved, err := e.savePassthroughAttachments(ctx, session, attachMgr, attachments) if err != nil { return "", fmt.Errorf("save passthrough attachments: %w", err) } @@ -406,6 +407,77 @@ func (e *Executor) buildPassthroughPromptWithAttachments(ctx context.Context, se return prompt + "\n\n" + attachmentPrompt, nil } +func (e *Executor) savePassthroughAttachments( + ctx context.Context, + session *models.TaskSession, + attachMgr *agentctlshared.AttachmentManager, + attachments []v1.MessageAttachment, +) ([]agentctlshared.SavedAttachment, error) { + var saved []agentctlshared.SavedAttachment + rollback := func() { + for _, attachment := range saved { + if attachment.AbsPath == "" { + continue + } + if err := os.Remove(attachment.AbsPath); err != nil && !errors.Is(err, os.ErrNotExist) { + e.logger.Debug("failed to roll back passthrough attachment", + zap.String("path", attachment.AbsPath), zap.Error(err)) + } + } + } + for _, attachment := range attachments { + if attachment.AttachmentID != "" && attachment.Data == "" { + streamed, err := e.savePassthroughAttachment(ctx, session, attachMgr, attachment) + if streamed.AbsPath != "" { + saved = append(saved, streamed) + } + if err != nil { + rollback() + return nil, err + } + continue + } + + inline, inlineErr := attachMgr.SaveAttachments([]v1.MessageAttachment{attachment}) + saved = append(saved, inline...) + if inlineErr != nil { + rollback() + return nil, inlineErr + } + } + return saved, nil +} + +func (e *Executor) savePassthroughAttachment( + ctx context.Context, + session *models.TaskSession, + attachMgr *agentctlshared.AttachmentManager, + attachment v1.MessageAttachment, +) (agentctlshared.SavedAttachment, error) { + if e.attachmentReader == nil { + return agentctlshared.SavedAttachment{}, fmt.Errorf("attachment reader is unavailable for %q", attachment.AttachmentID) + } + reader, name, mimeType, sizeBytes, err := e.attachmentReader.OpenClaimed( + ctx, attachment.AttachmentID, session.TaskID, session.ID, + ) + if err != nil { + return agentctlshared.SavedAttachment{}, fmt.Errorf("open attachment %q: %w", attachment.AttachmentID, err) + } + materialized := attachment + materialized.Name = name + materialized.MimeType = mimeType + materialized.SizeBytes = sizeBytes + streamed, saveErr := attachMgr.SaveAttachmentStream(materialized, reader) + closeErr := reader.Close() + if saveErr != nil { + return streamed, fmt.Errorf("write attachment %q: %w", attachment.AttachmentID, saveErr) + } + if closeErr != nil { + return streamed, fmt.Errorf("close attachment %q: %w", attachment.AttachmentID, closeErr) + } + return streamed, nil +} + func (e *Executor) passthroughAttachmentWorkspace(ctx context.Context, session *models.TaskSession) string { if workDir := strings.TrimSpace(session.WorkspacePath); workDir != "" { return workDir diff --git a/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go b/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go index 41b87b5015..77a4fb9141 100644 --- a/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_passthrough_prompt_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "errors" + "io" "os" "path/filepath" "strings" @@ -15,6 +16,27 @@ import ( v1 "github.com/kandev/kandev/pkg/api/v1" ) +type passthroughAttachmentReader struct{} + +func (passthroughAttachmentReader) OpenClaimed(context.Context, string, string, string) (io.ReadCloser, string, string, int64, error) { + const body = "descriptor attachment body" + return io.NopCloser(strings.NewReader(body)), "passthrough-upload.txt", "text/plain", int64(len(body)), nil +} + +var _ AttachmentReader = passthroughAttachmentReader{} + +type failingPassthroughAttachmentReader struct{} + +func (failingPassthroughAttachmentReader) OpenClaimed(_ context.Context, id, _ string, _ string) (io.ReadCloser, string, string, int64, error) { + if id == "attachment-1" { + const body = "first attachment body" + return io.NopCloser(strings.NewReader(body)), "first-upload.txt", "text/plain", int64(len(body)), nil + } + return io.NopCloser(strings.NewReader("short")), "second-upload.txt", "text/plain", int64(len("second attachment body")), nil +} + +var _ AttachmentReader = failingPassthroughAttachmentReader{} + // seedPassthroughSession installs a task + session into the mock repo and wires // the agent manager to claim a valid execution ID, so Executor.Prompt reaches // the IsPassthroughSession branch instead of bailing on lookup errors. @@ -173,6 +195,72 @@ func TestExecutor_Prompt_PassthroughWithAttachmentsSavesFilesAndReferencesPaths( } } +func TestExecutor_Prompt_PassthroughWithClaimedAttachmentStreamsFileAndReferencesPath(t *testing.T) { + repo := newMockRepository() + agentManager := &mockAgentManager{ + isPassthroughSessionFunc: func(_ context.Context, _ string) bool { return true }, + } + seedPassthroughSession(t, repo, agentManager, "task-1", "sess-1", "exec-1") + workDir := t.TempDir() + repo.sessions["sess-1"].WorkspacePath = workDir + exec := newTestExecutor(t, agentManager, repo) + exec.SetAttachmentReader(passthroughAttachmentReader{}) + + const body = "descriptor attachment body" + atts := []v1.MessageAttachment{{ + AttachmentID: "attachment-1", + Type: "resource", + MimeType: "text/plain", + Name: "passthrough-upload.txt", + SizeBytes: int64(len(body)), + DeliveryMode: "path", + }} + result, err := exec.Prompt(context.Background(), "task-1", "sess-1", "read the attached file", atts, false) + if err != nil { + t.Fatalf("Prompt with a claimed attachment should succeed: %v", err) + } + if result == nil || result.StopReason != "passthrough_dispatched" { + t.Fatalf("expected passthrough_dispatched result, got %+v", result) + } + + relPath := filepath.Join(".kandev", "attachments", "sess-1", "passthrough-upload.txt") + data := agentManager.writePassthroughStdinCalls[0].Data + if !strings.Contains(data, "read the attached file") || !strings.Contains(data, relPath) { + t.Fatalf("PTY write should reference the claimed attachment path, got %q", data) + } + contents, err := os.ReadFile(filepath.Join(workDir, relPath)) + if err != nil { + t.Fatalf("expected streamed attachment file: %v", err) + } + if string(contents) != body { + t.Fatalf("streamed attachment contents = %q, want %q", contents, body) + } +} + +func TestExecutor_Prompt_PassthroughWithClaimedAttachmentFailureRollsBackPreviousFiles(t *testing.T) { + repo := newMockRepository() + agentManager := &mockAgentManager{ + isPassthroughSessionFunc: func(_ context.Context, _ string) bool { return true }, + } + seedPassthroughSession(t, repo, agentManager, "task-1", "sess-1", "exec-1") + workDir := t.TempDir() + repo.sessions["sess-1"].WorkspacePath = workDir + exec := newTestExecutor(t, agentManager, repo) + exec.SetAttachmentReader(failingPassthroughAttachmentReader{}) + + atts := []v1.MessageAttachment{ + {AttachmentID: "attachment-1", Type: "resource", DeliveryMode: "path"}, + {AttachmentID: "attachment-2", Type: "resource", DeliveryMode: "path"}, + } + if _, err := exec.Prompt(context.Background(), "task-1", "sess-1", "read both attached files", atts, false); err == nil { + t.Fatal("Prompt should fail when the second attachment size does not match") + } + firstPath := filepath.Join(workDir, ".kandev", "attachments", "sess-1", "first-upload.txt") + if _, err := os.Stat(firstPath); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("first attachment should be removed after batch failure, stat error = %v", err) + } +} + func TestExecutor_Prompt_PassthroughAttachmentOnlyMessageSendsPathPrompt(t *testing.T) { repo := newMockRepository() agentManager := &mockAgentManager{ diff --git a/apps/backend/internal/orchestrator/handlers/queue_handlers.go b/apps/backend/internal/orchestrator/handlers/queue_handlers.go index 7a2e9aae6d..a40b1f3a65 100644 --- a/apps/backend/internal/orchestrator/handlers/queue_handlers.go +++ b/apps/backend/internal/orchestrator/handlers/queue_handlers.go @@ -2,6 +2,7 @@ package handlers import ( "context" + "encoding/base64" "errors" "github.com/kandev/kandev/internal/common/logger" @@ -10,6 +11,7 @@ import ( "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/orchestrator" "github.com/kandev/kandev/internal/orchestrator/messagequeue" + "github.com/kandev/kandev/internal/task/models" v1 "github.com/kandev/kandev/pkg/api/v1" ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" @@ -40,6 +42,7 @@ const ( type QueueService interface { QueueMessageWithMetadata(ctx context.Context, sessionID, taskID, content, model, userID string, planMode bool, attachments []messagequeue.MessageAttachment, metadata map[string]interface{}) (*messagequeue.QueuedMessage, error) AppendContent(ctx context.Context, sessionID, taskID, content, model, userID string, planMode bool, attachments []messagequeue.MessageAttachment) (*messagequeue.QueuedMessage, bool, error) + GetEntry(ctx context.Context, sessionID, entryID string) (*messagequeue.QueuedMessage, error) UpdateMessageWithMetadata(ctx context.Context, sessionID, entryID, content string, attachments []messagequeue.MessageAttachment, metadataUpdates map[string]interface{}, queuedBy string) error RemoveEntry(ctx context.Context, sessionID, entryID string) error MergeIntoAbove(ctx context.Context, sessionID, entryID, queuedBy string) (*messagequeue.QueuedMessage, error) @@ -58,6 +61,20 @@ type QueueAccessAuthorizer interface { AuthorizeTaskSessionAccess(ctx context.Context, taskID, sessionID string) error } +// QueueAttachmentClaimer binds staged file descriptors to the task/session +// after a queue entry has been durably accepted. +type QueueAttachmentClaimer interface { + ClaimMessageAttachments(ctx context.Context, taskID, sessionID string, attachments []v1.MessageAttachment) error +} + +type QueueAttachmentReleaser interface { + ReleaseMessageAttachments(ctx context.Context, taskID, sessionID string, attachments []v1.MessageAttachment) error +} + +type queueEntryTaker interface { + TakeQueuedEntry(context.Context, string, string) (*messagequeue.QueuedMessage, bool, error) +} + // QueueHandlers handles WebSocket message-queue operations. type QueueHandlers struct { queueService QueueService @@ -66,6 +83,13 @@ type QueueHandlers struct { eventBus bus.EventBus logger *logger.Logger referenceValidator entityrefs.SubmissionValidator + attachmentClaimer QueueAttachmentClaimer +} + +// SetAttachmentClaimer wires the task attachment registry into queue adds. +// It is optional so queue-focused tests and non-task consumers remain small. +func (h *QueueHandlers) SetAttachmentClaimer(claimer QueueAttachmentClaimer) { + h.attachmentClaimer = claimer } // NewQueueHandlers creates a new QueueHandlers instance. @@ -136,6 +160,10 @@ func (h *QueueHandlers) wsQueueMessage(ctx context.Context, msg *ws.Message) (*w return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "attachment delivery_mode must be prompt or path", map[string]interface{}{"attachment_index": invalid}) } + if invalid := firstInvalidAttachment(req.Attachments); invalid >= 0 { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "attachment metadata is invalid", + map[string]interface{}{"attachment_index": invalid}) + } if messagequeue.IsReservedQueuedBy(req.UserID) { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, reservedIdentityError(req.UserID), nil) } @@ -166,6 +194,14 @@ func (h *QueueHandlers) wsQueueMessage(ctx context.Context, msg *ws.Message) (*w h.logger.Error("failed to queue message", zap.Error(err)) return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to queue message", nil) } + if h.attachmentClaimer != nil && len(req.Attachments) > 0 { + if err := h.attachmentClaimer.ClaimMessageAttachments(ctx, req.TaskID, req.SessionID, queueAttachmentsToV1(req.Attachments)); err != nil { + if rollbackErr := h.rollbackQueuedAttachmentClaim(ctx, req.SessionID, queued.ID); rollbackErr != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "Failed to roll back queued attachment", nil) + } + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "Attachment is no longer available", nil) + } + } h.publishStatus(ctx, req.SessionID) return ws.NewResponse(msg.ID, msg.Action, queued) @@ -292,6 +328,10 @@ func (h *QueueHandlers) wsUpdateMessage(ctx context.Context, msg *ws.Message) (* return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "attachment delivery_mode must be prompt or path", map[string]interface{}{"attachment_index": invalid}) } + if invalid := firstInvalidAttachment(req.Attachments); invalid >= 0 { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "attachment metadata is invalid", + map[string]interface{}{"attachment_index": invalid}) + } // Reject any client-supplied identity that would impersonate the agent. // Without this guard a hostile WS client could send user_id="agent" to @@ -322,17 +362,84 @@ func (h *QueueHandlers) wsUpdateMessage(ctx context.Context, msg *ws.Message) (* } metadataUpdates = map[string]interface{}{messagequeue.MetadataEntityReferences: referenceMetadata} } + var previous *messagequeue.QueuedMessage + var releaseClaims QueueAttachmentReleaser + var newlyAdded []messagequeue.MessageAttachment + if h.attachmentClaimer != nil { + var err error + previous, err = h.queueService.GetEntry(ctx, req.SessionID, req.EntryID) + if err != nil { + if errors.Is(err, messagequeue.ErrEntryNotFound) { + return ws.NewError(msg.ID, msg.Action, queueErrorCodeEntryNotFound, "Queue entry was already drained or not owned by caller", nil) + } + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, err.Error(), nil) + } + newlyAdded = newlyAddedQueueAttachments(previous.Attachments, req.Attachments) + if err := h.attachmentClaimer.ClaimMessageAttachments(ctx, previous.TaskID, req.SessionID, queueAttachmentsToV1(newlyAdded)); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "Attachment is no longer available", nil) + } + releaseClaims, _ = h.attachmentClaimer.(QueueAttachmentReleaser) + } if err := h.queueService.UpdateMessageWithMetadata(ctx, req.SessionID, req.EntryID, req.Content, req.Attachments, metadataUpdates, queuedBy); err != nil { + if releaseClaims != nil && previous != nil { + if releaseErr := releaseClaims.ReleaseMessageAttachments(ctx, previous.TaskID, req.SessionID, queueAttachmentsToV1(newlyAdded)); releaseErr != nil { + h.logger.Warn("failed to release attachments after queue update failure", zap.Error(releaseErr)) + } + } if errors.Is(err, messagequeue.ErrEntryNotFound) { return ws.NewError(msg.ID, msg.Action, queueErrorCodeEntryNotFound, "Queue entry was already drained or not owned by caller", nil) } return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, err.Error(), nil) } - + if releaseClaims != nil && previous != nil { + if superseded := supersededQueueAttachments(previous.Attachments, req.Attachments); len(superseded) > 0 { + if err := releaseClaims.ReleaseMessageAttachments(ctx, previous.TaskID, req.SessionID, queueAttachmentsToV1(superseded)); err != nil { + h.logger.Warn("failed to release superseded queue attachments", zap.Error(err)) + } + } + } h.publishStatus(ctx, req.SessionID) return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{fieldEntryID: req.EntryID}) } +func newlyAddedQueueAttachments(previous, replacement []messagequeue.MessageAttachment) []messagequeue.MessageAttachment { + retained := make(map[string]struct{}, len(previous)) + for _, attachment := range previous { + if attachment.AttachmentID != "" { + retained[attachment.AttachmentID] = struct{}{} + } + } + var newlyAdded []messagequeue.MessageAttachment + for _, attachment := range replacement { + if attachment.AttachmentID == "" { + continue + } + if _, ok := retained[attachment.AttachmentID]; !ok { + newlyAdded = append(newlyAdded, attachment) + } + } + return newlyAdded +} + +func supersededQueueAttachments(previous, replacement []messagequeue.MessageAttachment) []messagequeue.MessageAttachment { + retained := make(map[string]struct{}, len(replacement)) + for _, attachment := range replacement { + if attachment.AttachmentID != "" { + retained[attachment.AttachmentID] = struct{}{} + } + } + var superseded []messagequeue.MessageAttachment + for _, attachment := range previous { + if attachment.AttachmentID == "" { + continue + } + if _, ok := retained[attachment.AttachmentID]; !ok { + superseded = append(superseded, attachment) + } + } + return superseded +} + func (h *QueueHandlers) validateSubmittedReferences( ctx context.Context, sessionID, taskID string, @@ -347,6 +454,21 @@ func (h *QueueHandlers) validateSubmittedReferences( return h.referenceValidator.ValidateForSubmission(ctx, sessionID, taskID, references) } +func (h *QueueHandlers) rollbackQueuedAttachmentClaim(ctx context.Context, sessionID, entryID string) error { + if err := h.queueService.RemoveEntry(ctx, sessionID, entryID); err == nil { + return nil + } else { + h.logger.Error("failed to remove queue entry after attachment claim failure", + zap.String("entry_id", entryID), zap.Error(err)) + } + taker, ok := h.queueService.(queueEntryTaker) + if !ok { + return errors.New("queue service cannot atomically remove a queued entry") + } + _, _, err := taker.TakeQueuedEntry(ctx, sessionID, entryID) + return err +} + func firstInvalidDeliveryMode(attachments []messagequeue.MessageAttachment) int { for i, att := range attachments { if att.DeliveryMode != "" && att.DeliveryMode != "prompt" && att.DeliveryMode != "path" { @@ -356,6 +478,66 @@ func firstInvalidDeliveryMode(attachments []messagequeue.MessageAttachment) int return -1 } +func firstInvalidAttachment(attachments []messagequeue.MessageAttachment) int { + if len(attachments) > models.MaxMessageAttachmentCount { + return models.MaxMessageAttachmentCount + } + var total int64 + for i, attachment := range attachments { + if attachment.Type != "image" && attachment.Type != "audio" && attachment.Type != "resource" { + return i + } + bytes, valid := attachmentPayloadBytes(attachment) + if !valid { + return i + } + total += bytes + if total > models.MaxMessageAttachmentBytes { + return i + } + } + return -1 +} + +func attachmentPayloadBytes(attachment messagequeue.MessageAttachment) (int64, bool) { + if attachment.AttachmentID != "" { + if attachment.Data != "" || attachment.Name == "" || attachment.MimeType == "" { + return 0, false + } + if attachment.SizeBytes < 0 || attachment.SizeBytes > models.MaxMessageAttachmentBytes { + return 0, false + } + return attachment.SizeBytes, true + } + if attachment.Data == "" || len(attachment.Data) > 10*1024*1024 { + return 0, false + } + decoded, err := base64.StdEncoding.DecodeString(attachment.Data) + if err != nil { + return 0, false + } + return int64(len(decoded)), true +} + +func queueAttachmentsToV1(attachments []messagequeue.MessageAttachment) []v1.MessageAttachment { + if len(attachments) == 0 { + return nil + } + converted := make([]v1.MessageAttachment, 0, len(attachments)) + for _, attachment := range attachments { + converted = append(converted, v1.MessageAttachment{ + AttachmentID: attachment.AttachmentID, + Type: attachment.Type, + Data: attachment.Data, + MimeType: attachment.MimeType, + Name: attachment.Name, + SizeBytes: attachment.SizeBytes, + DeliveryMode: attachment.DeliveryMode, + }) + } + return converted +} + type wsRemoveEntryRequest struct { SessionID string `json:"session_id"` EntryID string `json:"entry_id"` diff --git a/apps/backend/internal/orchestrator/handlers/queue_handlers_test.go b/apps/backend/internal/orchestrator/handlers/queue_handlers_test.go index d09d35fd2b..d7b36688c2 100644 --- a/apps/backend/internal/orchestrator/handlers/queue_handlers_test.go +++ b/apps/backend/internal/orchestrator/handlers/queue_handlers_test.go @@ -2,6 +2,7 @@ package handlers import ( "context" + "encoding/base64" "encoding/json" "errors" "testing" @@ -12,6 +13,7 @@ import ( "github.com/kandev/kandev/internal/events/bus" "github.com/kandev/kandev/internal/orchestrator" "github.com/kandev/kandev/internal/orchestrator/messagequeue" + "github.com/kandev/kandev/internal/task/models" v1 "github.com/kandev/kandev/pkg/api/v1" ws "github.com/kandev/kandev/pkg/websocket" "github.com/stretchr/testify/assert" @@ -78,6 +80,30 @@ type fakeReferenceSubmissionValidator struct { err error } +type recordingQueueAttachmentClaimer struct { + claims []string + releases []string + claimErr error +} + +func (f *recordingQueueAttachmentClaimer) ClaimMessageAttachments(_ context.Context, _ string, _ string, attachments []v1.MessageAttachment) error { + for _, attachment := range attachments { + if attachment.AttachmentID != "" { + f.claims = append(f.claims, attachment.AttachmentID) + } + } + return f.claimErr +} + +func (f *recordingQueueAttachmentClaimer) ReleaseMessageAttachments(_ context.Context, _ string, _ string, attachments []v1.MessageAttachment) error { + for _, attachment := range attachments { + if attachment.AttachmentID != "" { + f.releases = append(f.releases, attachment.AttachmentID) + } + } + return nil +} + func (f *fakeReferenceSubmissionValidator) ValidateForSubmission( _ context.Context, sessionID, taskID string, @@ -409,6 +435,15 @@ func TestFirstInvalidDeliveryMode(t *testing.T) { } } +func TestFirstInvalidAttachmentCountsInlineDataInAggregate(t *testing.T) { + inline := base64.StdEncoding.EncodeToString([]byte("inline")) + attachments := []messagequeue.MessageAttachment{ + {Type: "resource", AttachmentID: "descriptor", Name: "large.bin", MimeType: "application/octet-stream", SizeBytes: models.MaxMessageAttachmentBytes}, + {Type: "resource", Data: inline, Name: "inline.txt", MimeType: "text/plain"}, + } + assert.Equal(t, 1, firstInvalidAttachment(attachments)) +} + func TestWsCancelAll(t *testing.T) { t.Run("clears the queue", func(t *testing.T) { handlers, svc := setupQueueHandlers(t) @@ -553,6 +588,53 @@ func TestWsGetQueueStatus(t *testing.T) { } func TestWsUpdateMessage(t *testing.T) { + t.Run("claims replacement attachments and releases superseded claims", func(t *testing.T) { + handlers, svc := setupQueueHandlers(t) + claimer := &recordingQueueAttachmentClaimer{} + handlers.SetAttachmentClaimer(claimer) + ctx := context.Background() + queued, err := svc.QueueMessage(ctx, "s", "task-1", "original", "", "u", false, []messagequeue.MessageAttachment{{ + Type: "resource", AttachmentID: "old-attachment", Name: "old.txt", MimeType: "text/plain", + }}) + require.NoError(t, err) + + response, err := handlers.wsUpdateMessage(ctx, + createTestMessage(t, ws.ActionMessageQueueUpdate, map[string]interface{}{ + "session_id": "s", "entry_id": queued.ID, "content": "edited", "user_id": "u", + "attachments": []messagequeue.MessageAttachment{{ + Type: "resource", AttachmentID: "new-attachment", Name: "new.txt", MimeType: "text/plain", + }}, + })) + require.NoError(t, err) + assert.Equal(t, ws.MessageTypeResponse, response.Type) + assert.Equal(t, []string{"new-attachment"}, claimer.claims) + assert.Equal(t, []string{"old-attachment"}, claimer.releases) + }) + + t.Run("releases only newly claimed attachments when replacement fails", func(t *testing.T) { + handlers, svc := setupQueueHandlers(t) + claimer := &recordingQueueAttachmentClaimer{} + handlers.SetAttachmentClaimer(claimer) + ctx := context.Background() + queued, err := svc.QueueMessage(ctx, "s", "task-1", "original", "", "u", false, []messagequeue.MessageAttachment{{ + Type: "resource", AttachmentID: "old-attachment", Name: "old.txt", MimeType: "text/plain", + }}) + require.NoError(t, err) + + response, err := handlers.wsUpdateMessage(ctx, + createTestMessage(t, ws.ActionMessageQueueUpdate, map[string]interface{}{ + "session_id": "s", "entry_id": queued.ID, "content": "edited", "user_id": "other", + "attachments": []messagequeue.MessageAttachment{ + {Type: "resource", AttachmentID: "old-attachment", Name: "old.txt", MimeType: "text/plain"}, + {Type: "resource", AttachmentID: "new-attachment", Name: "new.txt", MimeType: "text/plain"}, + }, + })) + require.NoError(t, err) + assert.Equal(t, ws.MessageTypeError, response.Type) + assert.Equal(t, []string{"new-attachment"}, claimer.claims) + assert.Equal(t, []string{"new-attachment"}, claimer.releases) + }) + t.Run("updates an entry", func(t *testing.T) { handlers, svc := setupQueueHandlers(t) ctx := context.Background() diff --git a/apps/backend/internal/orchestrator/messagequeue/service.go b/apps/backend/internal/orchestrator/messagequeue/service.go index 4b13e850d4..7b2cf83863 100644 --- a/apps/backend/internal/orchestrator/messagequeue/service.go +++ b/apps/backend/internal/orchestrator/messagequeue/service.go @@ -392,6 +392,23 @@ func (s *Service) UpdateMessage(ctx context.Context, sessionID, entryID, content return s.UpdateMessageWithMetadata(ctx, sessionID, entryID, content, attachments, nil, queuedBy) } +// GetEntry returns a pending queue entry scoped to its session. It is used by +// lifecycle-aware callers that must inspect the trusted task ID and previous +// attachment descriptors before replacing an entry. +func (s *Service) GetEntry(ctx context.Context, sessionID, entryID string) (*QueuedMessage, error) { + entries, err := s.repo.ListBySession(ctx, sessionID) + if err != nil { + return nil, err + } + for i := range entries { + if entries[i].ID == entryID { + entry := entries[i] + return &entry, nil + } + } + return nil, ErrEntryNotFound +} + // UpdateMessageWithMetadata atomically edits queue content and applies // metadata replacements while retaining unrelated metadata keys. func (s *Service) UpdateMessageWithMetadata(ctx context.Context, sessionID, entryID, content string, attachments []MessageAttachment, metadataUpdates map[string]interface{}, queuedBy string) error { diff --git a/apps/backend/internal/orchestrator/messagequeue/types.go b/apps/backend/internal/orchestrator/messagequeue/types.go index b9542baa8e..7c786cff03 100644 --- a/apps/backend/internal/orchestrator/messagequeue/types.go +++ b/apps/backend/internal/orchestrator/messagequeue/types.go @@ -170,8 +170,10 @@ func clearReservedMetadata(metadata map[string]interface{}) map[string]interface type MessageAttachment struct { Type string `json:"type"` Data string `json:"data"` + AttachmentID string `json:"attachment_id,omitempty"` MimeType string `json:"mime_type"` Name string `json:"name,omitempty"` + SizeBytes int64 `json:"size_bytes,omitempty"` DeliveryMode string `json:"delivery_mode,omitempty"` } diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index b81cffd8b2..6853159437 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -15,6 +15,7 @@ import ( "database/sql" "errors" "fmt" + "io" "regexp" "strconv" "strings" @@ -61,6 +62,14 @@ type ServiceConfig struct { ClaudeBackgroundPromptHandoff bool } +// AttachmentReader is the narrow attachment-store seam needed when the +// orchestrator delivers claimed descriptors to a passthrough agent. +// Keeping this interface local avoids coupling the coordinator to lifecycle's +// concrete package while preserving the same structural contract. +type AttachmentReader interface { + OpenClaimed(ctx context.Context, id, taskID, sessionID string) (io.ReadCloser, string, string, int64, error) +} + // DefaultServiceConfig returns default configuration func DefaultServiceConfig() ServiceConfig { return ServiceConfig{ @@ -812,6 +821,12 @@ func (s *Service) SetMessageCreator(mc MessageCreator) { s.messageCreator = mc } +// SetAttachmentReader wires the backend attachment store into passthrough +// prompt delivery so claimed descriptors can be streamed into the workspace. +func (s *Service) SetAttachmentReader(reader AttachmentReader) { + s.executor.SetAttachmentReader(reader) +} + // SetOnPrimarySessionSet sets a callback on the executor for when the first session // of a task is marked primary. Used to publish a task.updated event so the frontend // receives the primary_session_id. diff --git a/apps/backend/internal/task/handlers/attachment_handlers.go b/apps/backend/internal/task/handlers/attachment_handlers.go new file mode 100644 index 0000000000..3efd155e51 --- /dev/null +++ b/apps/backend/internal/task/handlers/attachment_handlers.go @@ -0,0 +1,167 @@ +package handlers + +import ( + "errors" + "fmt" + "mime/multipart" + "net/http" + "path/filepath" + "strconv" + "strings" + + "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/service" + "go.uber.org/zap" +) + +const maxAttachmentRequestBytes int64 = service.MaxAttachmentBytes + 4*1024*1024 +const attachmentMultipartMaxMemory = 1 << 20 + +// AttachmentHandlers exposes the authenticated staging/download/delete API. +type AttachmentHandlers struct { + service *service.AttachmentService + log *logger.Logger +} + +func NewAttachmentHandlers(svc *service.AttachmentService, log *logger.Logger) *AttachmentHandlers { + return &AttachmentHandlers{service: svc, log: log.WithFields(zap.String("component", "attachment-http-handlers"))} +} + +func RegisterAttachmentRoutes(router *gin.Engine, svc *service.AttachmentService, log *logger.Logger) *AttachmentHandlers { + h := NewAttachmentHandlers(svc, log) + api := router.Group("/api/v1") + api.POST("/attachments", h.upload) + api.GET("/attachments/:id/content", h.download) + api.DELETE("/attachments/:id", h.remove) + return h +} + +func (h *AttachmentHandlers) owner(c *gin.Context) (string, bool) { + identity, ok := authn.FromGin(c) + if !ok || strings.TrimSpace(identity.UserID) == "" { + c.JSON(http.StatusUnauthorized, gin.H{"error": "authentication required"}) + return "", false + } + return identity.UserID, true +} + +func (h *AttachmentHandlers) upload(c *gin.Context) { + ownerID, ok := h.owner(c) + if !ok { + return + } + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxAttachmentRequestBytes) + if err := c.Request.ParseMultipartForm(attachmentMultipartMaxMemory); err != nil { + if isAttachmentRequestTooLarge(err) { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "attachment exceeds maximum size"}) + return + } + c.JSON(http.StatusBadRequest, gin.H{"error": "multipart upload is required"}) + return + } + defer func() { _ = c.Request.MultipartForm.RemoveAll() }() + workspaceID := strings.TrimSpace(c.PostForm("workspace_id")) + if workspaceID == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "workspace_id is required"}) + return + } + fileHeader, err := c.FormFile("file") + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "file is required"}) + return + } + file, err := fileHeader.Open() + if err != nil { + h.log.Error("open multipart attachment", zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read attachment"}) + return + } + defer func() { _ = file.Close() }() + mimeType := strings.TrimSpace(c.PostForm("mime_type")) + if mimeType == "" { + mimeType = fileHeader.Header.Get("Content-Type") + } + if mimeType == "" { + mimeType = "application/octet-stream" + } + kind := strings.TrimSpace(c.PostForm("kind")) + if kind == "" { + kind = attachmentKind(mimeType) + } + deliveryMode := strings.TrimSpace(c.PostForm("delivery_mode")) + if deliveryMode == "" { + if kind == "image" { + deliveryMode = "prompt" + } else { + deliveryMode = "path" + } + } + attachment, err := h.service.Stage(c.Request.Context(), ownerID, workspaceID, filepath.Base(fileHeader.Filename), mimeType, kind, deliveryMode, file) + if err != nil { + h.writeError(c, err) + return + } + c.JSON(http.StatusCreated, h.service.Descriptor(attachment)) +} + +func isAttachmentRequestTooLarge(err error) bool { + var maxBytesErr *http.MaxBytesError + return errors.As(err, &maxBytesErr) || errors.Is(err, multipart.ErrMessageTooLarge) +} + +func attachmentKind(mimeType string) string { + if strings.HasPrefix(strings.ToLower(mimeType), "image/") { + return "image" + } + if strings.HasPrefix(strings.ToLower(mimeType), "audio/") { + return "audio" + } + return "resource" +} + +func (h *AttachmentHandlers) download(c *gin.Context) { + ownerID, ok := h.owner(c) + if !ok { + return + } + attachment, file, err := h.service.Open(c.Request.Context(), ownerID, c.Param("id")) + if err != nil { + h.writeError(c, err) + return + } + defer func() { _ = file.Close() }() + c.Header("Content-Type", attachment.MimeType) + c.Header("Content-Length", strconv.FormatInt(attachment.SizeBytes, 10)) + c.Header("Content-Disposition", fmt.Sprintf("inline; filename=%q", strings.ReplaceAll(attachment.Name, `"`, ""))) + http.ServeContent(c.Writer, c.Request, attachment.Name, attachment.CreatedAt, file) +} + +func (h *AttachmentHandlers) remove(c *gin.Context) { + ownerID, ok := h.owner(c) + if !ok { + return + } + if err := h.service.Delete(c.Request.Context(), ownerID, c.Param("id")); err != nil { + h.writeError(c, err) + return + } + c.Status(http.StatusNoContent) +} + +func (h *AttachmentHandlers) writeError(c *gin.Context, err error) { + switch { + case errors.Is(err, service.ErrAttachmentTooLarge): + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "attachment exceeds maximum size"}) + case errors.Is(err, service.ErrAttachmentForbidden), errors.Is(err, service.ErrAttachmentNotFound), errors.Is(err, models.ErrAttachmentClaimConflict): + c.JSON(http.StatusNotFound, gin.H{"error": "attachment not found"}) + case errors.Is(err, service.ErrAttachmentInvalid), errors.Is(err, service.ErrAttachmentTotalTooLarge), errors.Is(err, service.ErrTooManyAttachments): + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + default: + h.log.Error("attachment request failed", zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{"error": "attachment request failed"}) + } +} diff --git a/apps/backend/internal/task/handlers/attachment_handlers_test.go b/apps/backend/internal/task/handlers/attachment_handlers_test.go new file mode 100644 index 0000000000..450a9f659e --- /dev/null +++ b/apps/backend/internal/task/handlers/attachment_handlers_test.go @@ -0,0 +1,29 @@ +package handlers + +import ( + "errors" + "mime/multipart" + "net/http" + "testing" +) + +func TestIsAttachmentRequestTooLarge(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + want bool + }{ + {name: "max bytes", err: &http.MaxBytesError{Limit: 10}, want: true}, + {name: "multipart form limit", err: multipart.ErrMessageTooLarge, want: true}, + {name: "other error", err: errors.New("multipart: malformed"), want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isAttachmentRequestTooLarge(tt.err); got != tt.want { + t.Fatalf("isAttachmentRequestTooLarge(%v) = %v, want %v", tt.err, got, tt.want) + } + }) + } +} diff --git a/apps/backend/internal/task/handlers/message_handlers.go b/apps/backend/internal/task/handlers/message_handlers.go index 71f0cfe2dd..fea01c0a94 100644 --- a/apps/backend/internal/task/handlers/message_handlers.go +++ b/apps/backend/internal/task/handlers/message_handlers.go @@ -434,6 +434,9 @@ func (h *MessageHandlers) wsAddMessage(ctx context.Context, msg *ws.Message) (*w } } req.Content = storedContent + if err := h.service.ClaimMessageAttachments(ctx, req.TaskID, req.TaskSessionID, req.Attachments); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) + } createRequest := &service.CreateMessageRequest{ TaskSessionID: req.TaskSessionID, diff --git a/apps/backend/internal/task/handlers/task_http_handlers.go b/apps/backend/internal/task/handlers/task_http_handlers.go index 3bf5705436..bcc7baef57 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers.go +++ b/apps/backend/internal/task/handlers/task_http_handlers.go @@ -8,6 +8,7 @@ import ( "net/http" "strconv" "strings" + "time" "github.com/gin-gonic/gin" @@ -753,7 +754,8 @@ func validateAttachments(items []v1.MessageAttachment) error { if len(items) > maxCreateTaskAttachments { return fmt.Errorf("too many attachments (max %d)", maxCreateTaskAttachments) } - var totalSize int + var totalSize int64 + var legacyTotalSize int for i, a := range items { typ := strings.TrimSpace(a.Type) if _, ok := allowedAttachmentTypes[typ]; !ok { @@ -762,18 +764,35 @@ func validateAttachments(items []v1.MessageAttachment) error { if strings.TrimSpace(a.MimeType) == "" { return fmt.Errorf("attachment[%d] mime_type is required", i) } + if !a.HasValidDeliveryMode() { + return fmt.Errorf("attachment[%d] delivery_mode must be prompt or path", i) + } + if a.AttachmentID != "" { + if a.Data != "" { + return fmt.Errorf("attachment[%d] descriptors cannot include inline data", i) + } + if strings.TrimSpace(a.Name) == "" || strings.TrimSpace(a.MimeType) == "" { + return fmt.Errorf("attachment[%d] descriptor name and mime_type are required", i) + } + if err := service.ValidateAttachmentSize(a.SizeBytes); err != nil { + return fmt.Errorf("attachment[%d]: %w", i, err) + } + totalSize += a.SizeBytes + continue + } if len(a.Data) == 0 { return fmt.Errorf("attachment[%d] data is required", i) } if len(a.Data) > maxAttachmentDataBytes { return fmt.Errorf("attachment[%d] data exceeds size limit", i) } - if !a.HasValidDeliveryMode() { - return fmt.Errorf("attachment[%d] delivery_mode must be prompt or path", i) - } - totalSize += len(a.Data) + totalSize += int64(len(a.Data)) + legacyTotalSize += len(a.Data) } - if totalSize > maxAttachmentDataBytes { + if legacyTotalSize > maxAttachmentDataBytes { + return fmt.Errorf("total legacy attachment size exceeds limit") + } + if totalSize > service.MaxAttachmentBytes { return fmt.Errorf("total attachment size exceeds limit") } return nil @@ -865,6 +884,25 @@ func (h *TaskHandlers) httpCreateTask(c *gin.Context) { handleNotFound(c, h.logger, err, "task not created") return } + if err := h.service.ClaimMessageAttachments(c.Request.Context(), task.ID, "", body.Attachments); err != nil { + rollbackCtx, cancel := context.WithTimeout(context.WithoutCancel(c.Request.Context()), 10*time.Second) + defer cancel() + if deleteErr := h.service.DeleteTask(rollbackCtx, task.ID); deleteErr != nil { + h.logger.Warn("failed to roll back task after attachment claim", zap.String("task_id", task.ID), zap.Error(deleteErr)) + } + switch { + case errors.Is(err, service.ErrAttachmentTooLarge): + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": err.Error()}) + case errors.Is(err, service.ErrAttachmentForbidden), errors.Is(err, service.ErrAttachmentNotFound), errors.Is(err, service.ErrAttachmentClaimConflict): + c.JSON(http.StatusNotFound, gin.H{"error": "attachment not found"}) + case errors.Is(err, service.ErrAttachmentInvalid), errors.Is(err, service.ErrAttachmentTotalTooLarge), errors.Is(err, service.ErrTooManyAttachments): + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + default: + h.logger.Error("failed to claim task attachments", zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to claim task attachments"}) + } + return + } if h.handoffSvc != nil && wsPolicy.NeedsAttachment() { if attachErr := h.handoffSvc.AttachWorkspacePolicy(c.Request.Context(), task.ID, body.ParentID, wsPolicy); attachErr != nil { diff --git a/apps/backend/internal/task/models/attachments.go b/apps/backend/internal/task/models/attachments.go new file mode 100644 index 0000000000..4f310383c4 --- /dev/null +++ b/apps/backend/internal/task/models/attachments.go @@ -0,0 +1,18 @@ +package models + +import "errors" + +const ( + MaxMessageAttachmentBytes int64 = 100 * 1024 * 1024 + MaxMessageAttachmentCount = 10 +) + +var ( + ErrAttachmentTooLarge = errors.New("attachment exceeds maximum size") + ErrAttachmentTotalTooLarge = errors.New("total attachment size exceeds maximum") + ErrTooManyAttachments = errors.New("too many attachments") + ErrAttachmentClaimConflict = errors.New("attachment claim conflict") + ErrAttachmentNotFound = errors.New("attachment not found") + ErrAttachmentForbidden = errors.New("attachment access denied") + ErrAttachmentInvalid = errors.New("invalid attachment") +) diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 7e0c3eaba3..193d9d2ddc 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -1712,6 +1712,36 @@ type TaskDocumentRevision struct { UpdatedAt time.Time `json:"updated_at" db:"updated_at"` } +// TaskMessageAttachment is the durable registry row for a prompt attachment. +// Message and queue metadata carry only the public descriptor; the bytes stay +// in private backend storage addressed by StorageKey. +type TaskMessageAttachment struct { + ID string `json:"id" db:"id"` + OwnerID string `json:"owner_id" db:"owner_id"` + WorkspaceID string `json:"workspace_id" db:"workspace_id"` + TaskID string `json:"task_id,omitempty" db:"task_id"` + SessionID string `json:"session_id,omitempty" db:"session_id"` + MessageID string `json:"message_id,omitempty" db:"message_id"` + QueueID string `json:"queue_id,omitempty" db:"queue_id"` + Name string `json:"name" db:"name"` + MimeType string `json:"mime_type" db:"mime_type"` + Kind string `json:"kind" db:"kind"` + DeliveryMode string `json:"delivery_mode" db:"delivery_mode"` + SizeBytes int64 `json:"size_bytes" db:"size_bytes"` + StorageKey string `json:"-" db:"storage_key"` + State string `json:"state" db:"state"` + ExpiresAt time.Time `json:"expires_at" db:"expires_at"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + UpdatedAt time.Time `json:"updated_at" db:"updated_at"` +} + +const ( + AttachmentStateStaged = "staged" + AttachmentStateClaimed = "claimed" + AttachmentStateExpired = "expired" + AttachmentStateDeleted = "deleted" +) + // SessionFileReview tracks per-file review state within a session type SessionFileReview struct { ID string `json:"id"` diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index 08ec60b9d1..4f66f4944d 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -190,6 +190,20 @@ type MessageRepository interface { DeleteMessage(ctx context.Context, id string) error } +// AttachmentRepository stores file-backed prompt attachment descriptors. +// Implementations must keep ownership and aggregate-claim checks in the same +// transaction as state transitions so a retry cannot partially claim a batch. +type AttachmentRepository interface { + CreateMessageAttachment(ctx context.Context, attachment *models.TaskMessageAttachment) error + GetMessageAttachment(ctx context.Context, id string) (*models.TaskMessageAttachment, error) + ListMessageAttachments(ctx context.Context, ids []string) ([]*models.TaskMessageAttachment, error) + ClaimMessageAttachments(ctx context.Context, ids []string, ownerID, workspaceID, taskID, sessionID string) error + DeleteClaimedMessageAttachments(ctx context.Context, ids []string, ownerID, taskID, sessionID string) ([]*models.TaskMessageAttachment, error) + DeleteMessageAttachmentsByTask(ctx context.Context, taskID string) ([]*models.TaskMessageAttachment, error) + DeleteMessageAttachment(ctx context.Context, id, ownerID string) error + MarkExpiredMessageAttachments(ctx context.Context, now time.Time) ([]*models.TaskMessageAttachment, error) +} + // TurnRepository handles conversation turn persistence. type TurnRepository interface { CreateTurn(ctx context.Context, turn *models.Turn) error diff --git a/apps/backend/internal/task/repository/sqlite/attachment.go b/apps/backend/internal/task/repository/sqlite/attachment.go new file mode 100644 index 0000000000..c61afe4c0e --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/attachment.go @@ -0,0 +1,377 @@ +package sqlite + +import ( + "context" + "database/sql" + "fmt" + "strings" + "time" + + "github.com/google/uuid" + "github.com/jmoiron/sqlx" + "github.com/kandev/kandev/internal/task/models" +) + +const attachmentSelectColumns = `id, owner_id, workspace_id, task_id, session_id, + message_id, queue_id, name, mime_type, kind, delivery_mode, size_bytes, + storage_key, state, expires_at, created_at, updated_at` + +func (r *Repository) CreateMessageAttachment(ctx context.Context, attachment *models.TaskMessageAttachment) error { + if attachment.ID == "" { + attachment.ID = uuid.NewString() + } + if attachment.StorageKey == "" { + attachment.StorageKey = attachment.ID + } + now := time.Now().UTC() + if attachment.CreatedAt.IsZero() { + attachment.CreatedAt = now + } + attachment.UpdatedAt = now + if attachment.State == "" { + attachment.State = models.AttachmentStateStaged + } + _, err := r.db.ExecContext(ctx, r.db.Rebind(` + INSERT INTO task_message_attachments + (id, owner_id, workspace_id, task_id, session_id, message_id, queue_id, + name, mime_type, kind, delivery_mode, size_bytes, storage_key, state, + expires_at, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `), attachment.ID, attachment.OwnerID, attachment.WorkspaceID, attachment.TaskID, + attachment.SessionID, attachment.MessageID, attachment.QueueID, attachment.Name, + attachment.MimeType, attachment.Kind, attachment.DeliveryMode, attachment.SizeBytes, + attachment.StorageKey, attachment.State, attachment.ExpiresAt, attachment.CreatedAt, + attachment.UpdatedAt) + if err != nil { + return fmt.Errorf("create message attachment: %w", err) + } + return nil +} + +func (r *Repository) GetMessageAttachment(ctx context.Context, id string) (*models.TaskMessageAttachment, error) { + attachment := &models.TaskMessageAttachment{} + err := r.ro.QueryRowxContext(ctx, r.ro.Rebind(` + SELECT `+attachmentSelectColumns+` FROM task_message_attachments WHERE id = ? + `), id).StructScan(attachment) + if err == sql.ErrNoRows { + return nil, models.ErrAttachmentNotFound + } + if err != nil { + return nil, fmt.Errorf("get message attachment: %w", err) + } + return attachment, nil +} + +func (r *Repository) ListMessageAttachments(ctx context.Context, ids []string) ([]*models.TaskMessageAttachment, error) { + if len(ids) == 0 { + return nil, nil + } + placeholders := strings.TrimSuffix(strings.Repeat("?,", len(ids)), ",") + args := make([]interface{}, len(ids)) + for i := range ids { + args[i] = ids[i] + } + rows, err := r.ro.QueryxContext(ctx, r.ro.Rebind(` + SELECT `+attachmentSelectColumns+` FROM task_message_attachments + WHERE id IN (`+placeholders+`) + `), args...) + if err != nil { + return nil, fmt.Errorf("list message attachments: %w", err) + } + defer func() { _ = rows.Close() }() + var out []*models.TaskMessageAttachment + for rows.Next() { + attachment := &models.TaskMessageAttachment{} + if err := rows.StructScan(attachment); err != nil { + return nil, fmt.Errorf("scan message attachment: %w", err) + } + out = append(out, attachment) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate message attachments: %w", err) + } + return out, nil +} + +func (r *Repository) ClaimMessageAttachments(ctx context.Context, ids []string, ownerID, workspaceID, taskID, sessionID string) error { + if len(ids) == 0 { + return nil + } + if len(ids) > models.MaxMessageAttachmentCount { + return models.ErrTooManyAttachments + } + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return fmt.Errorf("begin attachment claim: %w", err) + } + defer func() { _ = tx.Rollback() }() + + now := time.Now().UTC() + selection, err := r.selectAttachmentsForClaim(ctx, tx, ids, ownerID, workspaceID, taskID, sessionID, now) + if err != nil { + return err + } + if selection.selectedSize > models.MaxMessageAttachmentBytes { + return models.ErrAttachmentTotalTooLarge + } + if err := r.markAttachmentsClaimed(ctx, tx, selection.claimIDs, ownerID, workspaceID, taskID, sessionID); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("commit attachment claim: %w", err) + } + return nil +} + +type attachmentClaimSelection struct { + claimIDs []string + selectedSize int64 +} + +func (r *Repository) selectAttachmentsForClaim(ctx context.Context, tx *sqlx.Tx, ids []string, ownerID, workspaceID, taskID, sessionID string, now time.Time) (attachmentClaimSelection, error) { + selection := attachmentClaimSelection{} + seen := make(map[string]struct{}, len(ids)) + for _, id := range ids { + if _, ok := seen[id]; ok { + return attachmentClaimSelection{}, models.ErrAttachmentClaimConflict + } + seen[id] = struct{}{} + var attachment models.TaskMessageAttachment + err := tx.GetContext(ctx, &attachment, tx.Rebind(` + SELECT `+attachmentSelectColumns+` FROM task_message_attachments WHERE id = ? + `), id) + if errorsIsNoRows(err) { + return attachmentClaimSelection{}, models.ErrAttachmentNotFound + } + if err != nil { + return attachmentClaimSelection{}, fmt.Errorf("load attachment for claim: %w", err) + } + if attachment.OwnerID != ownerID || attachment.WorkspaceID != workspaceID { + return attachmentClaimSelection{}, models.ErrAttachmentClaimConflict + } + if err := addAttachmentToClaim(&selection, &attachment, id, taskID, sessionID, now); err != nil { + return attachmentClaimSelection{}, err + } + } + return selection, nil +} + +func addAttachmentToClaim(selection *attachmentClaimSelection, attachment *models.TaskMessageAttachment, id, taskID, sessionID string, now time.Time) error { + switch attachment.State { + case models.AttachmentStateStaged: + if !attachment.ExpiresAt.IsZero() && !attachment.ExpiresAt.After(now) { + return models.ErrAttachmentClaimConflict + } + if attachment.SizeBytes < 0 || attachment.SizeBytes > models.MaxMessageAttachmentBytes { + return models.ErrAttachmentTooLarge + } + selection.selectedSize += attachment.SizeBytes + selection.claimIDs = append(selection.claimIDs, id) + return nil + case models.AttachmentStateClaimed: + if attachment.TaskID != taskID || attachment.SessionID != sessionID { + return models.ErrAttachmentClaimConflict + } + return nil + default: + return models.ErrAttachmentClaimConflict + } +} + +func (r *Repository) markAttachmentsClaimed(ctx context.Context, tx *sqlx.Tx, ids []string, ownerID, workspaceID, taskID, sessionID string) error { + now := time.Now().UTC() + for _, id := range ids { + result, err := tx.ExecContext(ctx, tx.Rebind(` + UPDATE task_message_attachments + SET task_id = ?, session_id = ?, state = ?, updated_at = ? + WHERE id = ? AND owner_id = ? AND workspace_id = ? AND state = ? + `), taskID, sessionID, models.AttachmentStateClaimed, now, id, ownerID, workspaceID, models.AttachmentStateStaged) + if err != nil { + return fmt.Errorf("claim attachment: %w", err) + } + affected, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("claim attachment rows affected: %w", err) + } + if affected != 1 { + return models.ErrAttachmentClaimConflict + } + } + return nil +} + +func (r *Repository) DeleteMessageAttachment(ctx context.Context, id, ownerID string) error { + result, err := r.db.ExecContext(ctx, r.db.Rebind(` + DELETE FROM task_message_attachments WHERE id = ? AND owner_id = ? + `), id, ownerID) + if err != nil { + return fmt.Errorf("delete message attachment: %w", err) + } + rows, _ := result.RowsAffected() + if rows == 0 { + return models.ErrAttachmentNotFound + } + return nil +} + +func (r *Repository) DeleteClaimedMessageAttachments(ctx context.Context, ids []string, ownerID, taskID, sessionID string) ([]*models.TaskMessageAttachment, error) { + if len(ids) == 0 { + return nil, nil + } + placeholders := strings.TrimSuffix(strings.Repeat("?,", len(ids)), ",") + args := []interface{}{ownerID, taskID, sessionID, models.AttachmentStateClaimed} + args = append(args, idsToInterfaces(ids)...) + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return nil, fmt.Errorf("begin claimed attachment release: %w", err) + } + defer func() { _ = tx.Rollback() }() + rows, err := tx.QueryxContext(ctx, tx.Rebind(` + SELECT `+attachmentSelectColumns+` FROM task_message_attachments + WHERE owner_id = ? AND task_id = ? AND session_id = ? AND state = ? + AND id IN (`+placeholders+`) + `), args...) + if err != nil { + return nil, fmt.Errorf("list claimed attachments for release: %w", err) + } + var released []*models.TaskMessageAttachment + for rows.Next() { + attachment := &models.TaskMessageAttachment{} + if err := rows.StructScan(attachment); err != nil { + _ = rows.Close() + return nil, fmt.Errorf("scan claimed attachment for release: %w", err) + } + released = append(released, attachment) + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return nil, fmt.Errorf("iterate claimed attachments for release: %w", err) + } + if err := rows.Close(); err != nil { + return nil, fmt.Errorf("close claimed attachments for release: %w", err) + } + for _, attachment := range released { + result, err := tx.ExecContext(ctx, tx.Rebind(` + DELETE FROM task_message_attachments + WHERE id = ? AND owner_id = ? AND task_id = ? AND session_id = ? AND state = ? + `), attachment.ID, ownerID, taskID, sessionID, models.AttachmentStateClaimed) + if err != nil { + return nil, fmt.Errorf("release claimed attachment: %w", err) + } + affected, err := result.RowsAffected() + if err != nil { + return nil, fmt.Errorf("release claimed attachment rows affected: %w", err) + } + if affected != 1 { + return nil, models.ErrAttachmentClaimConflict + } + } + if err := tx.Commit(); err != nil { + return nil, fmt.Errorf("commit claimed attachment release: %w", err) + } + return released, nil +} + +func (r *Repository) DeleteMessageAttachmentsByTask(ctx context.Context, taskID string) ([]*models.TaskMessageAttachment, error) { + if taskID == "" { + return nil, nil + } + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return nil, fmt.Errorf("begin task attachment cleanup: %w", err) + } + defer func() { _ = tx.Rollback() }() + rows, err := tx.QueryxContext(ctx, tx.Rebind(` + SELECT `+attachmentSelectColumns+` FROM task_message_attachments WHERE task_id = ? + `), taskID) + if err != nil { + return nil, fmt.Errorf("list task attachments for cleanup: %w", err) + } + var attachments []*models.TaskMessageAttachment + for rows.Next() { + attachment := &models.TaskMessageAttachment{} + if err := rows.StructScan(attachment); err != nil { + _ = rows.Close() + return nil, fmt.Errorf("scan task attachment for cleanup: %w", err) + } + attachments = append(attachments, attachment) + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return nil, fmt.Errorf("iterate task attachments for cleanup: %w", err) + } + if err := rows.Close(); err != nil { + return nil, fmt.Errorf("close task attachments for cleanup: %w", err) + } + if _, err := tx.ExecContext(ctx, tx.Rebind(`DELETE FROM task_message_attachments WHERE task_id = ?`), taskID); err != nil { + return nil, fmt.Errorf("delete task attachments: %w", err) + } + if err := tx.Commit(); err != nil { + return nil, fmt.Errorf("commit task attachment cleanup: %w", err) + } + return attachments, nil +} + +func (r *Repository) MarkExpiredMessageAttachments(ctx context.Context, now time.Time) ([]*models.TaskMessageAttachment, error) { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return nil, fmt.Errorf("begin expired attachment cleanup: %w", err) + } + defer func() { _ = tx.Rollback() }() + rows, err := tx.QueryxContext(ctx, tx.Rebind(` + SELECT `+attachmentSelectColumns+` FROM task_message_attachments + WHERE state = ? AND expires_at <= ? + `), models.AttachmentStateStaged, now) + if err != nil { + return nil, fmt.Errorf("list expired attachments: %w", err) + } + var expired []*models.TaskMessageAttachment + for rows.Next() { + attachment := &models.TaskMessageAttachment{} + if err := rows.StructScan(attachment); err != nil { + _ = rows.Close() + return nil, fmt.Errorf("scan expired attachment: %w", err) + } + expired = append(expired, attachment) + } + if err := rows.Err(); err != nil { + _ = rows.Close() + return nil, fmt.Errorf("iterate expired attachments: %w", err) + } + if err := rows.Close(); err != nil { + return nil, fmt.Errorf("close expired attachments: %w", err) + } + transitioned := make([]*models.TaskMessageAttachment, 0, len(expired)) + for _, attachment := range expired { + result, err := tx.ExecContext(ctx, tx.Rebind(` + UPDATE task_message_attachments SET state = ?, updated_at = ? + WHERE id = ? AND state = ? AND expires_at <= ? + `), models.AttachmentStateExpired, now, attachment.ID, models.AttachmentStateStaged, now) + if err != nil { + return nil, fmt.Errorf("mark expired attachment: %w", err) + } + affected, err := result.RowsAffected() + if err != nil { + return nil, fmt.Errorf("mark expired attachment rows affected: %w", err) + } + if affected == 1 { + attachment.State = models.AttachmentStateExpired + attachment.UpdatedAt = now + transitioned = append(transitioned, attachment) + } + } + if err := tx.Commit(); err != nil { + return nil, fmt.Errorf("commit expired attachment cleanup: %w", err) + } + return transitioned, nil +} + +func errorsIsNoRows(err error) bool { return err == sql.ErrNoRows } + +func idsToInterfaces(ids []string) []interface{} { + args := make([]interface{}, len(ids)) + for i, id := range ids { + args[i] = id + } + return args +} diff --git a/apps/backend/internal/task/repository/sqlite/attachment_postgres_test.go b/apps/backend/internal/task/repository/sqlite/attachment_postgres_test.go new file mode 100644 index 0000000000..8b30d94740 --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/attachment_postgres_test.go @@ -0,0 +1,86 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/testutil" +) + +func TestPostgresMessageAttachmentLifecycle(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "workspace-attachments-pg", Name: "Attachments PG"}); err != nil { + t.Fatalf("seed workspace: %v", err) + } + + t.Run("claim and release", func(t *testing.T) { + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "attachment-release-pg", OwnerID: "owner-pg", WorkspaceID: "workspace-attachments-pg", + Name: "release.bin", MimeType: "application/octet-stream", Kind: "resource", DeliveryMode: "path", + SizeBytes: 1, StorageKey: "attachment-release-pg", State: models.AttachmentStateStaged, + ExpiresAt: now.Add(time.Hour), CreatedAt: now, + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatalf("create attachment: %v", err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-pg", "workspace-attachments-pg", "task-pg", "session-pg"); err != nil { + t.Fatalf("claim attachment: %v", err) + } + released, err := repo.DeleteClaimedMessageAttachments(ctx, []string{attachment.ID}, "owner-pg", "task-pg", "session-pg") + if err != nil { + t.Fatalf("release attachment: %v", err) + } + if len(released) != 1 || released[0].ID != attachment.ID { + t.Fatalf("released = %+v", released) + } + }) + + t.Run("task cleanup", func(t *testing.T) { + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "attachment-task-cleanup-pg", OwnerID: "owner-pg", WorkspaceID: "workspace-attachments-pg", + TaskID: "task-cleanup-pg", SessionID: "session-pg", Name: "cleanup.bin", + MimeType: "application/octet-stream", Kind: "resource", DeliveryMode: "path", SizeBytes: 1, + StorageKey: "attachment-task-cleanup-pg", State: models.AttachmentStateClaimed, + ExpiresAt: now.Add(time.Hour), CreatedAt: now, + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatalf("create attachment: %v", err) + } + removed, err := repo.DeleteMessageAttachmentsByTask(ctx, attachment.TaskID) + if err != nil { + t.Fatalf("delete task attachments: %v", err) + } + if len(removed) != 1 || removed[0].ID != attachment.ID { + t.Fatalf("removed = %+v", removed) + } + }) + + t.Run("expiry", func(t *testing.T) { + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "attachment-expired-pg", OwnerID: "owner-pg", WorkspaceID: "workspace-attachments-pg", + Name: "expired.bin", MimeType: "application/octet-stream", Kind: "resource", DeliveryMode: "path", + SizeBytes: 1, StorageKey: "attachment-expired-pg", State: models.AttachmentStateStaged, + ExpiresAt: now.Add(-time.Minute), CreatedAt: now.Add(-time.Hour), + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatalf("create attachment: %v", err) + } + expired, err := repo.MarkExpiredMessageAttachments(ctx, now) + if err != nil { + t.Fatalf("mark expired attachments: %v", err) + } + if len(expired) != 1 || expired[0].ID != attachment.ID || expired[0].State != models.AttachmentStateExpired { + t.Fatalf("expired = %+v", expired) + } + }) +} diff --git a/apps/backend/internal/task/repository/sqlite/attachment_test.go b/apps/backend/internal/task/repository/sqlite/attachment_test.go new file mode 100644 index 0000000000..5a5d671cbe --- /dev/null +++ b/apps/backend/internal/task/repository/sqlite/attachment_test.go @@ -0,0 +1,147 @@ +package sqlite + +import ( + "context" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestClaimMessageAttachments_IsIdempotentForSameTaskSession(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-attachments") + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "attachment-idempotent", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", + Name: "bundle.zip", MimeType: "application/zip", Kind: "resource", DeliveryMode: "path", + SizeBytes: 16, StorageKey: "attachment-idempotent", State: models.AttachmentStateStaged, + ExpiresAt: now.Add(time.Hour), CreatedAt: now, + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatal(err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-1", "workspace-attachments", "task-1", "session-1"); err != nil { + t.Fatalf("initial claim: %v", err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-1", "workspace-attachments", "task-1", "session-1"); err != nil { + t.Fatalf("idempotent claim: %v", err) + } + got, err := repo.GetMessageAttachment(ctx, attachment.ID) + if err != nil { + t.Fatal(err) + } + if got.State != models.AttachmentStateClaimed || got.TaskID != "task-1" || got.SessionID != "session-1" { + t.Fatalf("claimed attachment = %+v", got) + } +} + +func TestClaimMessageAttachments_AllowsSeparateSubmissionsToReachTheLimit(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-attachments") + now := time.Now().UTC() + first := &models.TaskMessageAttachment{ + ID: "attachment-first", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", + Name: "first.bin", MimeType: "application/octet-stream", Kind: "resource", DeliveryMode: "path", + SizeBytes: models.MaxMessageAttachmentBytes * 3 / 5, StorageKey: "attachment-first", State: models.AttachmentStateStaged, + ExpiresAt: now.Add(time.Hour), CreatedAt: now, + } + second := *first + second.ID = "attachment-second" + second.Name = "second.bin" + second.StorageKey = second.ID + for _, attachment := range []*models.TaskMessageAttachment{first, &second} { + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatal(err) + } + } + + if err := repo.ClaimMessageAttachments(ctx, []string{first.ID}, "owner-1", "workspace-attachments", "task-1", "session-1"); err != nil { + t.Fatalf("first claim: %v", err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{second.ID}, "owner-1", "workspace-attachments", "task-1", "session-1"); err != nil { + t.Fatalf("second claim: %v", err) + } +} + +func TestClaimMessageAttachments_RejectsExpiredStagedDescriptor(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-attachments") + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "attachment-expired", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", + Name: "expired.bin", MimeType: "application/octet-stream", Kind: "resource", DeliveryMode: "path", + SizeBytes: 1, StorageKey: "attachment-expired", State: models.AttachmentStateStaged, + ExpiresAt: now.Add(-time.Minute), CreatedAt: now.Add(-time.Hour), + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatal(err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-1", "workspace-attachments", "task-1", "session-1"); err == nil { + t.Fatal("expected expired attachment claim to fail") + } + got, err := repo.GetMessageAttachment(ctx, attachment.ID) + if err != nil { + t.Fatal(err) + } + if got.State != models.AttachmentStateStaged { + t.Fatalf("expired attachment state = %q, want staged", got.State) + } +} + +func TestDeleteClaimedMessageAttachments_RemovesOnlyMatchingClaims(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-attachments") + now := time.Now().UTC() + attachments := []*models.TaskMessageAttachment{ + {ID: "release-one", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", TaskID: "task-1", SessionID: "session-1", Name: "one", MimeType: "text/plain", Kind: "resource", DeliveryMode: "path", SizeBytes: 1, StorageKey: "release-one", State: models.AttachmentStateClaimed, ExpiresAt: now.Add(time.Hour), CreatedAt: now}, + {ID: "release-other-task", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", TaskID: "task-2", SessionID: "session-1", Name: "two", MimeType: "text/plain", Kind: "resource", DeliveryMode: "path", SizeBytes: 1, StorageKey: "release-other-task", State: models.AttachmentStateClaimed, ExpiresAt: now.Add(time.Hour), CreatedAt: now}, + } + for _, attachment := range attachments { + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatal(err) + } + } + released, err := repo.DeleteClaimedMessageAttachments(ctx, []string{"release-one", "release-other-task"}, "owner-1", "task-1", "session-1") + if err != nil { + t.Fatal(err) + } + if len(released) != 1 || released[0].ID != "release-one" { + t.Fatalf("released = %+v", released) + } + if _, err := repo.GetMessageAttachment(ctx, "release-one"); err == nil { + t.Fatal("released attachment still exists") + } + if _, err := repo.GetMessageAttachment(ctx, "release-other-task"); err != nil { + t.Fatalf("unmatched claim was removed: %v", err) + } +} + +func TestDeleteMessageAttachmentsByTask_RemovesRegistryRows(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-attachments") + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "task-cleanup-attachment", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", TaskID: "task-cleanup", + Name: "cleanup.bin", MimeType: "application/octet-stream", Kind: "resource", DeliveryMode: "path", SizeBytes: 1, + StorageKey: "task-cleanup-attachment", State: models.AttachmentStateClaimed, ExpiresAt: now.Add(time.Hour), CreatedAt: now, + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatal(err) + } + removed, err := repo.DeleteMessageAttachmentsByTask(ctx, "task-cleanup") + if err != nil { + t.Fatal(err) + } + if len(removed) != 1 || removed[0].ID != attachment.ID { + t.Fatalf("removed = %+v", removed) + } + if _, err := repo.GetMessageAttachment(ctx, attachment.ID); err == nil { + t.Fatal("task attachment still exists") + } +} diff --git a/apps/backend/internal/task/repository/sqlite/base_schema.go b/apps/backend/internal/task/repository/sqlite/base_schema.go index 6febcf56ba..1174b77a1c 100644 --- a/apps/backend/internal/task/repository/sqlite/base_schema.go +++ b/apps/backend/internal/task/repository/sqlite/base_schema.go @@ -20,6 +20,7 @@ func (r *Repository) initSchema() error { r.initWalkthroughsSchema, r.initDocumentsSchema, r.initSessionSchema, + r.initAttachmentsSchema, r.initTaskResourceCleanupSchema, r.initGitSchema, r.initReviewSchema, @@ -555,6 +556,42 @@ func (r *Repository) initDocumentsSchema() error { return nil } +// initAttachmentsSchema creates the descriptor registry for file-backed prompt +// attachments. The storage key is intentionally opaque and never exposed in +// API responses; bytes are owned by the attachment service. +func (r *Repository) initAttachmentsSchema() error { + _, err := r.db.Exec(` + CREATE TABLE IF NOT EXISTS task_message_attachments ( + id TEXT PRIMARY KEY, + owner_id TEXT NOT NULL DEFAULT '', + workspace_id TEXT NOT NULL, + task_id TEXT NOT NULL DEFAULT '', + session_id TEXT NOT NULL DEFAULT '', + message_id TEXT NOT NULL DEFAULT '', + queue_id TEXT NOT NULL DEFAULT '', + name TEXT NOT NULL DEFAULT '', + mime_type TEXT NOT NULL DEFAULT 'application/octet-stream', + kind TEXT NOT NULL DEFAULT 'resource', + delivery_mode TEXT NOT NULL DEFAULT 'prompt', + size_bytes INTEGER NOT NULL, + storage_key TEXT NOT NULL UNIQUE, + state TEXT NOT NULL DEFAULT 'staged', + expires_at TIMESTAMP NOT NULL, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + FOREIGN KEY (workspace_id) REFERENCES workspaces(id) ON DELETE CASCADE + ); + CREATE INDEX IF NOT EXISTS idx_task_message_attachments_owner_state + ON task_message_attachments(owner_id, state, expires_at); + CREATE INDEX IF NOT EXISTS idx_task_message_attachments_scope + ON task_message_attachments(workspace_id, task_id, session_id, state); + `) + if err != nil { + return fmt.Errorf("init attachments schema: %w", err) + } + return nil +} + func (r *Repository) initSessionSchema() error { if err := r.initSessionWorktreeSchema(); err != nil { return err diff --git a/apps/backend/internal/task/service/attachment_service.go b/apps/backend/internal/task/service/attachment_service.go new file mode 100644 index 0000000000..428f97a5b3 --- /dev/null +++ b/apps/backend/internal/task/service/attachment_service.go @@ -0,0 +1,337 @@ +package service + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "time" + + "github.com/google/uuid" + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/task/repository" +) + +const ( + // MaxAttachmentBytes is the raw (not base64-encoded) size accepted for a + // single prompt attachment. + MaxAttachmentBytes int64 = models.MaxMessageAttachmentBytes + // MaxAttachmentCount bounds the number of files in one prompt submission. + MaxAttachmentCount = models.MaxMessageAttachmentCount + attachmentDeliveryModePrompt = "prompt" + attachmentDeliveryModePath = "path" +) + +var ( + ErrAttachmentTooLarge = models.ErrAttachmentTooLarge + ErrAttachmentTotalTooLarge = models.ErrAttachmentTotalTooLarge + ErrTooManyAttachments = models.ErrTooManyAttachments + ErrAttachmentClaimConflict = models.ErrAttachmentClaimConflict + ErrAttachmentNotFound = models.ErrAttachmentNotFound + ErrAttachmentForbidden = models.ErrAttachmentForbidden + ErrAttachmentInvalid = models.ErrAttachmentInvalid +) + +const AttachmentStagedTTL = 24 * time.Hour + +// AttachmentDescriptor is the safe client-facing representation of an +// attachment. It deliberately contains no storage path or inline bytes. +type AttachmentDescriptor struct { + ID string `json:"attachment_id"` + Name string `json:"name"` + MimeType string `json:"mime_type"` + Kind string `json:"kind"` + DeliveryMode string `json:"delivery_mode"` + SizeBytes int64 `json:"size_bytes"` + State string `json:"state"` + ExpiresAt time.Time `json:"expires_at,omitempty"` +} + +// AttachmentService owns private attachment bytes and their durable registry. +type AttachmentService struct { + repo repository.AttachmentRepository + root string + authorizeWorkspace func(context.Context, string) error + log *logger.Logger +} + +func NewAttachmentService(repo repository.AttachmentRepository, root string, authorizeWorkspace func(context.Context, string) error, log *logger.Logger) (*AttachmentService, error) { + if repo == nil { + return nil, errors.New("attachment repository is required") + } + if strings.TrimSpace(root) == "" { + return nil, errors.New("attachment storage root is required") + } + if log == nil { + return nil, errors.New("attachment logger is required") + } + return &AttachmentService{repo: repo, root: filepath.Join(root, "attachments"), authorizeWorkspace: authorizeWorkspace, log: log.WithFields(zap.String("component", "attachment-service"))}, nil +} + +func (s *AttachmentService) ensureRoot() error { + return os.MkdirAll(s.root, 0o700) +} + +// Stage writes one raw file to a temporary file, validates its exact byte +// count, atomically renames it, then commits the descriptor row. Any failure +// removes both the temporary and committed file so callers never observe a +// usable row without bytes. +func (s *AttachmentService) Stage(ctx context.Context, ownerID, workspaceID, name, mimeType, kind, deliveryMode string, src io.Reader) (*models.TaskMessageAttachment, error) { + if workspaceID == "" || ownerID == "" || src == nil { + return nil, ErrAttachmentInvalid + } + if s.authorizeWorkspace != nil { + if err := s.authorizeWorkspace(ctx, workspaceID); err != nil { + return nil, err + } + } + if deliveryMode == "" { + deliveryMode = attachmentDeliveryModePrompt + } + if err := ValidateAttachmentMetadata(name, mimeType, kind, deliveryMode); err != nil { + return nil, err + } + if err := s.ensureRoot(); err != nil { + return nil, fmt.Errorf("create attachment storage: %w", err) + } + id := uuid.NewString() + storageKey := id + dest, n, err := s.writeAttachmentBytes(storageKey, src) + if err != nil { + return nil, err + } + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: id, OwnerID: ownerID, WorkspaceID: workspaceID, Name: filepath.Base(name), + MimeType: mimeType, Kind: kind, DeliveryMode: deliveryMode, SizeBytes: n, + StorageKey: storageKey, State: models.AttachmentStateStaged, + ExpiresAt: now.Add(AttachmentStagedTTL), CreatedAt: now, UpdatedAt: now, + } + if err := s.repo.CreateMessageAttachment(ctx, attachment); err != nil { + _ = os.Remove(dest) + return nil, err + } + return attachment, nil +} + +func (s *AttachmentService) writeAttachmentBytes(storageKey string, src io.Reader) (string, int64, error) { + tmp, err := os.CreateTemp(s.root, ".upload-*") + if err != nil { + return "", 0, fmt.Errorf("create attachment temp file: %w", err) + } + tmpPath := tmp.Name() + renamed := false + defer func() { + if !renamed { + _ = tmp.Close() + _ = os.Remove(tmpPath) + } + }() + n, copyErr := io.Copy(tmp, io.LimitReader(src, MaxAttachmentBytes+1)) + if copyErr != nil { + return "", 0, fmt.Errorf("write attachment: %w", copyErr) + } + if err := tmp.Close(); err != nil { + return "", 0, fmt.Errorf("close attachment: %w", err) + } + if err := ValidateAttachmentSize(n); err != nil { + return "", 0, err + } + dest := filepath.Join(s.root, storageKey) + if err := os.Chmod(tmpPath, 0o600); err != nil { + return "", 0, fmt.Errorf("secure attachment temp file: %w", err) + } + if err := os.Rename(tmpPath, dest); err != nil { + return "", 0, fmt.Errorf("commit attachment bytes: %w", err) + } + renamed = true + return dest, n, nil +} + +func ValidateAttachmentMetadata(name, mimeType, kind, deliveryMode string) error { + if strings.TrimSpace(name) == "" || strings.ContainsAny(name, "/\\\x00") { + return fmt.Errorf("%w: invalid name", ErrAttachmentInvalid) + } + if strings.TrimSpace(mimeType) == "" { + return fmt.Errorf("%w: mime type is required", ErrAttachmentInvalid) + } + switch kind { + case "image", "audio", "resource": + default: + return fmt.Errorf("%w: unsupported kind %q", ErrAttachmentInvalid, kind) + } + if deliveryMode == "" { + deliveryMode = attachmentDeliveryModePrompt + } + if deliveryMode != attachmentDeliveryModePrompt && deliveryMode != attachmentDeliveryModePath { + return fmt.Errorf("%w: delivery mode must be prompt or path", ErrAttachmentInvalid) + } + return nil +} + +func (s *AttachmentService) Get(ctx context.Context, ownerID, id string) (*models.TaskMessageAttachment, error) { + attachment, err := s.repo.GetMessageAttachment(ctx, id) + if err != nil { + return nil, err + } + if attachment == nil || attachment.OwnerID != ownerID { + return nil, ErrAttachmentForbidden + } + return attachment, nil +} + +func (s *AttachmentService) Open(ctx context.Context, ownerID, id string) (*models.TaskMessageAttachment, *os.File, error) { + attachment, err := s.Get(ctx, ownerID, id) + if err != nil { + return nil, nil, err + } + if filepath.Base(attachment.StorageKey) != attachment.StorageKey || attachment.StorageKey == "" { + return nil, nil, ErrAttachmentInvalid + } + file, err := os.Open(filepath.Join(s.root, attachment.StorageKey)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil, ErrAttachmentNotFound + } + return nil, nil, fmt.Errorf("open attachment: %w", err) + } + return attachment, file, nil +} + +// OpenClaimed opens a descriptor for the internal lifecycle delivery path. +// It deliberately authorizes by the claimed task/session binding rather than +// accepting a caller-supplied owner, and never exposes the storage key. +func (s *AttachmentService) OpenClaimed(ctx context.Context, id, taskID, sessionID string) (io.ReadCloser, string, string, int64, error) { + attachment, err := s.repo.GetMessageAttachment(ctx, id) + if err != nil { + return nil, "", "", 0, err + } + if attachment == nil || attachment.State != models.AttachmentStateClaimed || + attachment.TaskID != taskID || (attachment.SessionID != "" && attachment.SessionID != sessionID) { + return nil, "", "", 0, ErrAttachmentForbidden + } + if filepath.Base(attachment.StorageKey) != attachment.StorageKey || attachment.StorageKey == "" { + return nil, "", "", 0, ErrAttachmentInvalid + } + file, err := os.Open(filepath.Join(s.root, attachment.StorageKey)) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, "", "", 0, ErrAttachmentNotFound + } + return nil, "", "", 0, fmt.Errorf("open claimed attachment: %w", err) + } + return file, attachment.Name, attachment.MimeType, attachment.SizeBytes, nil +} + +func (s *AttachmentService) Delete(ctx context.Context, ownerID, id string) error { + attachment, err := s.Get(ctx, ownerID, id) + if err != nil { + return err + } + if attachment.State != models.AttachmentStateStaged { + return ErrAttachmentClaimConflict + } + if err := s.repo.DeleteMessageAttachment(ctx, id, ownerID); err != nil { + return err + } + s.removeBytes(attachment) + return nil +} + +func (s *AttachmentService) Claim(ctx context.Context, ownerID, workspaceID, taskID, sessionID string, ids []string) error { + if s.authorizeWorkspace != nil { + if err := s.authorizeWorkspace(ctx, workspaceID); err != nil { + return err + } + } + return s.repo.ClaimMessageAttachments(ctx, ids, ownerID, workspaceID, taskID, sessionID) +} + +// Release removes claimed descriptors that are no longer referenced by a +// queued message. It is used after an atomic queue replacement succeeds. +func (s *AttachmentService) Release(ctx context.Context, ownerID, taskID, sessionID string, ids []string) error { + attachments, err := s.repo.DeleteClaimedMessageAttachments(ctx, ids, ownerID, taskID, sessionID) + if err != nil { + return err + } + for _, attachment := range attachments { + s.removeBytes(attachment) + } + return nil +} + +// DeleteByTask removes all attachment registry rows and private bytes owned by +// a task. Task deletion must clean claimed rows as well as staged rows because +// only staged rows participate in expiry maintenance. +func (s *AttachmentService) DeleteByTask(ctx context.Context, taskID string) error { + attachments, err := s.repo.DeleteMessageAttachmentsByTask(ctx, taskID) + if err != nil { + return err + } + for _, attachment := range attachments { + s.removeBytes(attachment) + } + return nil +} + +func (s *AttachmentService) Descriptor(attachment *models.TaskMessageAttachment) AttachmentDescriptor { + return AttachmentDescriptor{ + ID: attachment.ID, Name: attachment.Name, MimeType: attachment.MimeType, + Kind: attachment.Kind, DeliveryMode: attachment.DeliveryMode, + SizeBytes: attachment.SizeBytes, State: attachment.State, ExpiresAt: attachment.ExpiresAt, + } +} + +// CleanupExpired marks staged descriptors expired and removes their bytes. +func (s *AttachmentService) CleanupExpired(ctx context.Context) (int, error) { + expired, err := s.repo.MarkExpiredMessageAttachments(ctx, time.Now().UTC()) + if err != nil { + return 0, err + } + for _, attachment := range expired { + s.removeBytes(attachment) + } + return len(expired), nil +} + +func (s *AttachmentService) removeBytes(attachment *models.TaskMessageAttachment) { + if attachment == nil || attachment.StorageKey == "" { + return + } + if err := os.Remove(filepath.Join(s.root, filepath.Base(attachment.StorageKey))); err != nil && !errors.Is(err, os.ErrNotExist) { + s.log.Warn("remove attachment bytes failed", zap.String("attachment_id", attachment.ID), zap.Error(err)) + } +} + +// ValidateAttachmentSize applies the raw-byte per-file limit. The boundary is +// inclusive so a file exactly MaxAttachmentBytes is valid. +func ValidateAttachmentSize(size int64) error { + if size < 0 || size > MaxAttachmentBytes { + return fmt.Errorf("%w: %d bytes (maximum %d)", ErrAttachmentTooLarge, size, MaxAttachmentBytes) + } + return nil +} + +// ValidateAttachmentBatch applies the shared count and aggregate limits. +func ValidateAttachmentBatch(sizes []int64) error { + if len(sizes) > MaxAttachmentCount { + return fmt.Errorf("%w: maximum %d", ErrTooManyAttachments, MaxAttachmentCount) + } + var total int64 + for _, size := range sizes { + if err := ValidateAttachmentSize(size); err != nil { + return err + } + total += size + if total > MaxAttachmentBytes { + return fmt.Errorf("%w: maximum %d bytes", ErrAttachmentTotalTooLarge, MaxAttachmentBytes) + } + } + return nil +} diff --git a/apps/backend/internal/task/service/attachment_service_test.go b/apps/backend/internal/task/service/attachment_service_test.go new file mode 100644 index 0000000000..9e11cf6eda --- /dev/null +++ b/apps/backend/internal/task/service/attachment_service_test.go @@ -0,0 +1,45 @@ +package service + +import ( + "errors" + "testing" +) + +func TestValidateAttachmentSizeAcceptsExact100MiB(t *testing.T) { + if err := ValidateAttachmentSize(MaxAttachmentBytes); err != nil { + t.Fatalf("exact maximum should be accepted: %v", err) + } +} + +func TestValidateAttachmentSizeRejectsOneByteOver100MiB(t *testing.T) { + if err := ValidateAttachmentSize(MaxAttachmentBytes + 1); !errors.Is(err, ErrAttachmentTooLarge) { + t.Fatalf("expected ErrAttachmentTooLarge, got %v", err) + } +} + +func TestValidateAttachmentBatchEnforcesCountAndAggregate(t *testing.T) { + if err := ValidateAttachmentBatch([]int64{MaxAttachmentBytes / 2, MaxAttachmentBytes / 2}); err != nil { + t.Fatalf("two fitting files should be accepted: %v", err) + } + if err := ValidateAttachmentBatch([]int64{MaxAttachmentBytes / 2, MaxAttachmentBytes/2 + 1}); !errors.Is(err, ErrAttachmentTotalTooLarge) { + t.Fatalf("expected aggregate limit error, got %v", err) + } + tooMany := make([]int64, MaxAttachmentCount+1) + for i := range tooMany { + tooMany[i] = 1 + } + if err := ValidateAttachmentBatch(tooMany); !errors.Is(err, ErrTooManyAttachments) { + t.Fatalf("expected attachment count error, got %v", err) + } +} + +func TestValidateAttachmentMetadataAcceptsXAndZeroAndRejectsNUL(t *testing.T) { + for _, name := range []string{"x-ray0.png", "index0.log"} { + if err := ValidateAttachmentMetadata(name, "image/png", "image", "prompt"); err != nil { + t.Fatalf("name %q rejected: %v", name, err) + } + } + if err := ValidateAttachmentMetadata("bad"+string(rune(0))+"name", "image/png", "image", "prompt"); !errors.Is(err, ErrAttachmentInvalid) { + t.Fatalf("NUL name error = %v, want ErrAttachmentInvalid", err) + } +} diff --git a/apps/backend/internal/task/service/document_service.go b/apps/backend/internal/task/service/document_service.go index 90a2bc2541..bb4d8382e4 100644 --- a/apps/backend/internal/task/service/document_service.go +++ b/apps/backend/internal/task/service/document_service.go @@ -21,7 +21,6 @@ var ( ErrDocumentNotFound = errors.New("document not found") ErrDocumentKeyRequired = errors.New("document key is required") ErrDocumentTaskRequired = errors.New("task_id is required") - ErrAttachmentTooLarge = errors.New("attachment exceeds maximum size") ErrInvalidPathComponent = errors.New("invalid path component") ) diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index 2e8bc0a984..c02fe5cd4a 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -214,6 +214,7 @@ type Repos struct { WorkspaceFolders repository.TaskWorkspaceFolderRepository Workflows repository.WorkflowRepository Messages repository.MessageRepository + Attachments repository.AttachmentRepository Turns repository.TurnRepository Sessions repository.SessionRepository GitSnapshots repository.GitSnapshotRepository @@ -235,6 +236,7 @@ type Service struct { workspaceFolders repository.TaskWorkspaceFolderRepository workflows repository.WorkflowRepository messages repository.MessageRepository + attachments repository.AttachmentRepository turns repository.TurnRepository sessions repository.SessionRepository gitSnapshots repository.GitSnapshotRepository @@ -246,6 +248,7 @@ type Service struct { reviews repository.ReviewRepository resourceCleanups repository.TaskResourceCleanupRepository statusSummaries repository.TaskStatusSummaryRepository + attachmentSvc *AttachmentService statusSummaryPRs TaskStatusSummaryPRReader eventBus bus.EventBus logger *logger.Logger @@ -311,6 +314,26 @@ type Service struct { repoResolveMu sync.Mutex } +// SetAttachmentService wires the file-backed prompt attachment owner into the +// task service. It is optional for focused unit-test harnesses that never send +// file-backed descriptors. +func (s *Service) SetAttachmentService(attachments *AttachmentService) { + s.attachmentSvc = attachments +} + +// AttachmentService returns the optional file-backed attachment owner wired +// into this task service. It lets route and maintenance composition reuse the +// same storage boundary instead of creating competing service instances. +func (s *Service) AttachmentService() *AttachmentService { + return s.attachmentSvc +} + +// AttachmentRepository returns the attachment registry repository used by the +// task service. It is exposed for composition of the storage maintenance hook. +func (s *Service) AttachmentRepository() repository.AttachmentRepository { + return s.attachments +} + // NewService creates a new task service func NewService(repos Repos, eventBus bus.EventBus, log *logger.Logger, discoveryConfig RepositoryDiscoveryConfig) *Service { return &Service{ @@ -320,6 +343,7 @@ func NewService(repos Repos, eventBus bus.EventBus, log *logger.Logger, discover workspaceFolders: repos.WorkspaceFolders, workflows: repos.Workflows, messages: repos.Messages, + attachments: repos.Attachments, turns: repos.Turns, sessions: repos.Sessions, gitSnapshots: repos.GitSnapshots, diff --git a/apps/backend/internal/task/service/service_attachments.go b/apps/backend/internal/task/service/service_attachments.go new file mode 100644 index 0000000000..80aff4affc --- /dev/null +++ b/apps/backend/internal/task/service/service_attachments.go @@ -0,0 +1,69 @@ +package service + +import ( + "context" + "errors" + + "github.com/kandev/kandev/internal/auth/authn" + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// ClaimMessageAttachments binds staged descriptors to a task/session after +// the normal task and session authorization checks have completed. +func (s *Service) ClaimMessageAttachments(ctx context.Context, taskID, sessionID string, attachments []v1.MessageAttachment) error { + if len(attachments) == 0 { + return nil + } + if s.attachmentSvc == nil { + for _, attachment := range attachments { + if attachment.AttachmentID != "" { + return errors.New("file-backed attachments are unavailable") + } + } + return nil + } + ids := make([]string, 0, len(attachments)) + for _, attachment := range attachments { + if attachment.AttachmentID != "" { + ids = append(ids, attachment.AttachmentID) + } + } + if len(ids) == 0 { + return nil + } + task, err := s.GetTask(ctx, taskID) + if err != nil { + return err + } + identity, ok := authn.IdentityFromContext(ctx) + if !ok || identity.UserID == "" { + return models.ErrAttachmentForbidden + } + return s.attachmentSvc.Claim(ctx, identity.UserID, task.WorkspaceID, taskID, sessionID, ids) +} + +// ReleaseMessageAttachments removes claimed descriptors that a queue edit no +// longer references. The caller has already authorized the task session. +func (s *Service) ReleaseMessageAttachments(ctx context.Context, taskID, sessionID string, attachments []v1.MessageAttachment) error { + if len(attachments) == 0 || s.attachmentSvc == nil { + return nil + } + ids := make([]string, 0, len(attachments)) + for _, attachment := range attachments { + if attachment.AttachmentID != "" { + ids = append(ids, attachment.AttachmentID) + } + } + if len(ids) == 0 { + return nil + } + if _, err := s.GetTask(ctx, taskID); err != nil { + return err + } + identity, ok := authn.IdentityFromContext(ctx) + if !ok || identity.UserID == "" { + return models.ErrAttachmentForbidden + } + return s.attachmentSvc.Release(ctx, identity.UserID, taskID, sessionID, ids) +} diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index aefaccf4fa..a68a59dcb5 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -1789,6 +1789,12 @@ func (s *Service) deleteTaskWithReasonAndDBDelete( s.resolveTaskResourceCleanupAfterMutationError(ctx, cleanupJob) return false, nil } + if s.attachmentSvc != nil { + if err := s.attachmentSvc.DeleteByTask(context.WithoutCancel(ctx), id); err != nil { + s.logger.Warn("failed to remove task attachment bytes", + zap.String("task_id", id), zap.Error(err)) + } + } // 5. Publish event (sync, fast) - frontend removes task immediately var extra map[string]interface{} @@ -1853,6 +1859,12 @@ func (s *Service) deleteTaskStopTargets(ctx context.Context, id string) ([]taskS // for delete cascade, false for archive — archive preserves the row). Runtime // inventory failures abort cleanup so durable stop handles remain retryable. func (s *Service) CleanupTaskResources(ctx context.Context, taskID string, deleteEnvRow bool) { + if deleteEnvRow && s.attachmentSvc != nil { + if err := s.attachmentSvc.DeleteByTask(context.WithoutCancel(ctx), taskID); err != nil { + s.logger.Warn("failed to remove task attachment bytes during resource cleanup", + zap.String("task_id", taskID), zap.Error(err)) + } + } sessions, err := s.sessions.ListTaskSessions(ctx, taskID) if err != nil { s.logger.Warn("failed to list sessions for cascade cleanup", diff --git a/apps/backend/pkg/api/v1/task.go b/apps/backend/pkg/api/v1/task.go index 4777ce3f6d..c882eab50d 100644 --- a/apps/backend/pkg/api/v1/task.go +++ b/apps/backend/pkg/api/v1/task.go @@ -201,9 +201,11 @@ type TaskEvent struct { // "resource" → ResourceBlock (text or blob based on MIME type). type MessageAttachment struct { Type string `json:"type"` // "image", "audio", "resource" + AttachmentID string `json:"attachment_id,omitempty"` // File-backed attachment descriptor ID Data string `json:"data,omitempty"` // Base64-encoded data MimeType string `json:"mime_type,omitempty"` // MIME type (e.g., "image/png") Name string `json:"name,omitempty"` // Display name (e.g., filename) + SizeBytes int64 `json:"size_bytes,omitempty"` // Raw byte size for file-backed descriptors DeliveryMode string `json:"delivery_mode,omitempty"` // "prompt" (native/default) or "path" } diff --git a/apps/web/components/task-create-dialog-form-body.test.tsx b/apps/web/components/task-create-dialog-form-body.test.tsx index b99b7ecff9..5efa6347a3 100644 --- a/apps/web/components/task-create-dialog-form-body.test.tsx +++ b/apps/web/components/task-create-dialog-form-body.test.tsx @@ -68,6 +68,8 @@ function makeFs(): DialogFormState { setHasTitle: () => {}, hasDescription: false, setHasDescription: () => {}, + hasPendingAttachmentUploads: false, + setHasPendingAttachmentUploads: () => {}, draftDescription: "", openCycle: 0, currentDefaults: { name: "", description: "" }, diff --git a/apps/web/components/task-create-dialog-form-body.tsx b/apps/web/components/task-create-dialog-form-body.tsx index 6476e6e176..9dd75b79ed 100644 --- a/apps/web/components/task-create-dialog-form-body.tsx +++ b/apps/web/components/task-create-dialog-form-body.tsx @@ -362,6 +362,7 @@ export type DialogPromptSectionProps = { isTaskStarted: boolean; initialDescription: string; fs: DialogFormState; + onPendingAttachmentUploadsChange?: (pending: boolean) => void; handleKeyDown: ReturnType; enhance?: DialogPromptEnhance; workspaceId?: string | null; @@ -401,6 +402,7 @@ export function DialogPromptSection({ isTaskStarted, initialDescription, fs, + onPendingAttachmentUploadsChange, handleKeyDown, enhance, workspaceId, @@ -421,9 +423,11 @@ export function DialogPromptSection({ attachment.file && !attachment.attachmentId); +} + /** Converts FileAttachment array to MessageAttachment array for the launch request. */ export function toMessageAttachments( attachments: FileAttachment[], ): MessageAttachment[] | undefined { if (attachments.length === 0) return undefined; - return attachments.map((att) => - att.isImage - ? { - type: "image" as const, - data: att.data, - mime_type: att.mimeType, - name: att.fileName, - ...(att.deliveryMode === "path" && { delivery_mode: "path" as const }), - } - : { - type: "resource" as const, - data: att.data, - mime_type: att.mimeType, - name: att.fileName, - delivery_mode: "path" as const, - }, - ); + return attachments.map((att) => ({ + type: att.isImage ? ("image" as const) : ("resource" as const), + mime_type: att.mimeType, + name: att.fileName, + size_bytes: att.size, + ...(att.attachmentId ? { attachment_id: att.attachmentId } : { data: att.data ?? "" }), + ...(att.deliveryMode === "path" && { delivery_mode: "path" as const }), + })); } export function autoSelectBranch( diff --git a/apps/web/components/task-create-dialog-prop-builders.ts b/apps/web/components/task-create-dialog-prop-builders.ts index da5315da10..cb4b7d49b8 100644 --- a/apps/web/components/task-create-dialog-prop-builders.ts +++ b/apps/web/components/task-create-dialog-prop-builders.ts @@ -90,6 +90,7 @@ export function buildDialogFormBodyProps( export function buildDialogFooterProps( setup: ReturnType, props: TaskCreateDialogProps, + pendingAttachmentUploadReason?: string | null, ) { const { fs, computed, submitHandlers } = setup; return { @@ -114,6 +115,6 @@ export function buildDialogFooterProps( onUpdateWithoutAgent: submitHandlers.handleUpdateWithoutAgent, onCreateWithoutAgent: submitHandlers.handleCreateWithoutAgent, onCreateWithPlanMode: submitHandlers.handleCreateWithPlanMode, - submitBlockedReason: props.submitBlockedReason, + submitBlockedReason: props.submitBlockedReason ?? pendingAttachmentUploadReason, }; } diff --git a/apps/web/components/task-create-dialog-selectors.test.tsx b/apps/web/components/task-create-dialog-selectors.test.tsx index 7a39d57322..ef62f092d0 100644 --- a/apps/web/components/task-create-dialog-selectors.test.tsx +++ b/apps/web/components/task-create-dialog-selectors.test.tsx @@ -3,7 +3,12 @@ import { act, cleanup, fireEvent, render, screen } from "@testing-library/react" import { afterEach, describe, expect, it, vi } from "vitest"; import { TooltipProvider } from "@kandev/ui/tooltip"; import { ToastProvider } from "@/components/toast-provider"; -import { MAX_FILES, MAX_TOTAL_SIZE, processFile } from "@/components/task/chat/file-attachment"; +import { + MAX_FILES, + MAX_FILE_SIZE, + MAX_TOTAL_SIZE, + processFile, +} from "@/components/task/chat/file-attachment"; import { formatBytes } from "@/lib/utils/format-bytes"; import { TaskFormInputs } from "./task-create-dialog-selectors"; import type { TaskFormInputsHandle } from "./task-create-dialog-types"; @@ -300,7 +305,7 @@ describe("TaskFormInputs attachment feedback", () => { it("warns when a pasted image exceeds the attachment limit", async () => { const { textarea } = renderTaskFormInputs(""); const image = new File(["image"], "copied-image.png", { type: "image/png" }); - Object.defineProperty(image, "size", { value: 14 * 1024 * 1024 }); + Object.defineProperty(image, "size", { value: MAX_FILE_SIZE + 1 }); fireEvent.paste(textarea, { clipboardData: { @@ -313,7 +318,7 @@ describe("TaskFormInputs attachment feedback", () => { const warning = await screen.findByTestId(TOAST_MESSAGE_TEST_ID); expect(warning.textContent).toContain("Attachment is too large"); expect(warning.textContent).toContain( - "copied-image.png is 14.0 MB. The maximum file size is 10.0 MB.", + `copied-image.png is ${formatBytes(MAX_FILE_SIZE + 1)}. The maximum file size is ${formatBytes(MAX_FILE_SIZE)}.`, ); }); diff --git a/apps/web/components/task-create-dialog-selectors.tsx b/apps/web/components/task-create-dialog-selectors.tsx index 6060b2ff3e..c9fd58aab8 100644 --- a/apps/web/components/task-create-dialog-selectors.tsx +++ b/apps/web/components/task-create-dialog-selectors.tsx @@ -38,6 +38,8 @@ import type { LinearIssue } from "@/lib/types/linear"; import { useTaskCreatePromptMention } from "@/hooks/use-task-create-prompt-mention"; import { cn } from "@/lib/utils"; import { clampTaskTitleInput } from "@/lib/task-title"; +import { deleteAttachment, uploadAttachment } from "@/lib/api/domains/attachment-api"; +import { ApiError } from "@/lib/api/client"; const CURSOR_POINTER_CLASS = "cursor-pointer"; @@ -322,9 +324,11 @@ export const InlineTaskName = memo(function InlineTaskName({ // Memoized description input to prevent re-rendering the entire dialog on every keystroke type TaskFormInputsProps = { isSessionMode: boolean; + workspaceId?: string | null; autoFocus?: boolean; initialDescription: string; onDescriptionChange: (hasContent: boolean) => void; + onPendingAttachmentUploadsChange?: (pending: boolean) => void; onKeyDown: (e: React.KeyboardEvent) => void; descriptionValueRef: React.RefObject; disabled?: boolean; @@ -350,15 +354,74 @@ type TaskFormInputsProps = { onVoiceAutoSend?: () => void; }; -function useFileAttachments() { +// eslint-disable-next-line max-lines-per-function +function useFileAttachments( + workspaceId: string | null | undefined, + onPendingAttachmentUploadsChange?: (pending: boolean) => void, +) { const [attachments, setAttachments] = useState([]); const attachmentsRef = useRef([]); + + const updateAttachment = useCallback((id: string, update: Partial) => { + setAttachments((prev) => { + const next = prev.map((attachment) => + attachment.id === id ? { ...attachment, ...update } : attachment, + ); + attachmentsRef.current = next; + return next; + }); + }, []); + + const uploadPendingAttachment = useCallback( + async (attachment: FileAttachment) => { + if (!workspaceId || !attachment.file || attachment.attachmentId) return; + updateAttachment(attachment.id, { uploadStatus: "uploading" }); + try { + const uploaded = await uploadAttachment(attachment.file, { + workspaceId, + kind: attachment.isImage ? "image" : "resource", + deliveryMode: attachment.deliveryMode, + }); + if (!attachmentsRef.current.some((current) => current.id === attachment.id)) { + void deleteAttachment(uploaded.attachment_id).catch(() => undefined); + return; + } + updateAttachment(attachment.id, { + attachmentId: uploaded.attachment_id, + uploadStatus: "ready", + size: uploaded.size_bytes, + }); + } catch (error) { + updateAttachment(attachment.id, { + uploadStatus: "failed", + uploadError: error instanceof ApiError ? error.message : "Upload failed", + }); + } + }, + [updateAttachment, workspaceId], + ); + + useEffect(() => { + if (!workspaceId) return; + for (const attachment of attachmentsRef.current) { + if (attachment.file && !attachment.attachmentId && attachment.uploadStatus !== "uploading") { + void uploadPendingAttachment(attachment); + } + } + }, [uploadPendingAttachment, workspaceId]); + const [isDragging, setIsDragging] = useState(false); const warnAttachmentCountLimit = useAttachmentCountFeedback(); const rejectOversizedFile = useAttachmentFileFeedback(); const warnAttachmentTotalSizeLimit = useAttachmentTotalSizeFeedback(); const warnUnreadablePastedImage = useUnreadablePastedImageFeedback(); + useEffect(() => { + onPendingAttachmentUploadsChange?.( + attachments.some((attachment) => attachment.file && !attachment.attachmentId), + ); + }, [attachments, onPendingAttachmentUploadsChange, workspaceId]); + const addFiles = useCallback( async (files: File[], issue?: ImagePasteIssue) => { if (issue === "unreadable-image") { @@ -387,22 +450,41 @@ function useFileAttachments() { const next = [...attachmentsRef.current, ...accepted]; attachmentsRef.current = next; setAttachments(next); + for (const attachment of accepted) void uploadPendingAttachment(attachment); }, [ rejectOversizedFile, warnAttachmentCountLimit, warnAttachmentTotalSizeLimit, warnUnreadablePastedImage, + uploadPendingAttachment, ], ); const handleRemoveAttachment = useCallback((id: string) => { + const removed = attachmentsRef.current.find((attachment) => attachment.id === id); const next = attachmentsRef.current.filter((att) => att.id !== id); attachmentsRef.current = next; setAttachments(next); + if (removed?.attachmentId) void deleteAttachment(removed.attachmentId).catch(() => undefined); }, []); - return { attachments, isDragging, setIsDragging, addFiles, handleRemoveAttachment }; + const handleRetryAttachment = useCallback( + (id: string) => { + const attachment = attachmentsRef.current.find((item) => item.id === id); + if (attachment) void uploadPendingAttachment(attachment); + }, + [uploadPendingAttachment], + ); + + return { + attachments, + isDragging, + setIsDragging, + addFiles, + handleRemoveAttachment, + handleRetryAttachment, + }; } function useAttachmentHandlers( @@ -470,6 +552,7 @@ function useAttachmentHandlers( function toContextItems( attachments: FileAttachment[], onRemove: (id: string) => void, + onRetry: (id: string) => void, ): ContextItem[] { return attachments.map((att) => att.isImage @@ -479,6 +562,7 @@ function toContextItems( label: `Image (${formatBytes(att.size)})`, attachment: att, onRemove: () => onRemove(att.id), + onRetry: () => onRetry(att.id), } as ImageContextItem) : ({ kind: "file-attachment" as const, @@ -486,6 +570,7 @@ function toContextItems( label: att.fileName, attachment: att, onRemove: () => onRemove(att.id), + onRetry: () => onRetry(att.id), } as FileAttachmentContextItem), ); } @@ -737,10 +822,12 @@ function DraggingOverlay({ isDragging }: { isDragging: boolean }) { } export const TaskFormInputs = memo(function TaskFormInputs({ + workspaceId, isSessionMode, autoFocus, initialDescription, onDescriptionChange, + onPendingAttachmentUploadsChange, onKeyDown, descriptionValueRef, disabled, @@ -752,16 +839,22 @@ export const TaskFormInputs = memo(function TaskFormInputs({ linearImport, onVoiceAutoSend, }: TaskFormInputsProps) { - const { attachments, isDragging, setIsDragging, addFiles, handleRemoveAttachment } = - useFileAttachments(); + const { + attachments, + isDragging, + setIsDragging, + addFiles, + handleRemoveAttachment, + handleRetryAttachment, + } = useFileAttachments(workspaceId, onPendingAttachmentUploadsChange); const { handlePaste, handleDragOver, handleDragLeave, handleDrop } = useAttachmentHandlers( disabled, addFiles, setIsDragging, ); const contextItems = useMemo( - () => toContextItems(attachments, handleRemoveAttachment), - [attachments, handleRemoveAttachment], + () => toContextItems(attachments, handleRemoveAttachment, handleRetryAttachment), + [attachments, handleRemoveAttachment, handleRetryAttachment], ); const { description, textareaRef, setDescriptionValue, insertAtCursor } = useDescriptionInput( initialDescription, diff --git a/apps/web/components/task-create-dialog-state.ts b/apps/web/components/task-create-dialog-state.ts index cadec4ad21..c1bdc4bb6d 100644 --- a/apps/web/components/task-create-dialog-state.ts +++ b/apps/web/components/task-create-dialog-state.ts @@ -41,6 +41,7 @@ type FormResetters = { setTaskName: (v: string) => void; setHasTitle: (v: boolean) => void; setHasDescription: (v: boolean) => void; + setHasPendingAttachmentUploads: (v: boolean) => void; setRepositories: (v: TaskRepoRow[]) => void; setRemoteRepos: (v: TaskRemoteRepoRow[]) => void; setAgentProfileId: (v: string) => void; @@ -170,6 +171,7 @@ function resetTaskForm( resetters.setTaskName(name); resetters.setHasTitle(name.trim().length > 0); resetters.setHasDescription(description.trim().length > 0); + resetters.setHasPendingAttachmentUploads(false); // Seed the unified repos list from initialValues. A repo + branch pre-fill // becomes a single row; nothing seeds an empty list (the auto-select // effect later picks the user's last-used repo or the first workspace one). @@ -318,6 +320,7 @@ function useFormStateValues(workflowId: string | null) { const [taskName, setTaskName] = useState(""); const [hasTitle, setHasTitle] = useState(false); const [hasDescription, setHasDescription] = useState(false); + const [hasPendingAttachmentUploads, setHasPendingAttachmentUploads] = useState(false); const [draftDescription, setDraftDescription] = useState(""); const descriptionInputRef = useRef(null); @@ -340,6 +343,8 @@ function useFormStateValues(workflowId: string | null) { setHasTitle, hasDescription, setHasDescription, + hasPendingAttachmentUploads, + setHasPendingAttachmentUploads, draftDescription, setDraftDescription, descriptionInputRef, @@ -416,6 +421,7 @@ export function useDialogFormState( setTaskName: form.setTaskName, setHasTitle: form.setHasTitle, setHasDescription: form.setHasDescription, + setHasPendingAttachmentUploads: form.setHasPendingAttachmentUploads, setRepositories: repos.setRepositories, setRemoteRepos: remoteRepos.setRemoteRepos, setAgentProfileId: form.setAgentProfileId, diff --git a/apps/web/components/task-create-dialog-submit.test.tsx b/apps/web/components/task-create-dialog-submit.test.tsx index 0cce420885..afaef11858 100644 --- a/apps/web/components/task-create-dialog-submit.test.tsx +++ b/apps/web/components/task-create-dialog-submit.test.tsx @@ -77,6 +77,7 @@ vi.mock("@/components/task-create-dialog-helpers", () => ({ editingTask.state !== "CREATED", ), findDuplicateRemoteRepo: () => null, + hasPendingAttachmentUploads: () => false, validateCreateInputs: (...args: unknown[]) => validateCreateInputsMock(...args), toMessageAttachments: () => [], })); diff --git a/apps/web/components/task-create-dialog-submit.tsx b/apps/web/components/task-create-dialog-submit.tsx index e177c0c29b..06930ddb42 100644 --- a/apps/web/components/task-create-dialog-submit.tsx +++ b/apps/web/components/task-create-dialog-submit.tsx @@ -21,6 +21,7 @@ import { computeIsTaskStarted, findDuplicateRemoteRepo, validateCreateInputs, + hasPendingAttachmentUploads, toMessageAttachments, } from "@/components/task-create-dialog-helpers"; @@ -38,6 +39,24 @@ function notifyQueuedTask( }); } +type NoAgentTaskRequirements = { + description: string; + workflowStepId: string; + workspaceId: string; + workflowId: string; +}; + +function hasNoAgentTaskRequirements(input: { + description: string; + workflowStepId: string | null; + workspaceId: string | null; + workflowId: string | null; +}): input is NoAgentTaskRequirements { + return Boolean( + input.description && input.workflowStepId && input.workspaceId && input.workflowId, + ); +} + // eslint-disable-next-line max-lines-per-function export function useTaskSubmitHandlers({ isSessionMode, @@ -202,11 +221,17 @@ export function useTaskSubmitHandlers({ const description = descriptionInputRef.current?.getValue() ?? ""; const trimmedDescription = description.trim(); const attachments = descriptionInputRef.current?.getAttachments() ?? []; + if (hasPendingAttachmentUploads(attachments)) return; if (!agentProfileId) return; if (!trimmedDescription) return; if (onCreateSession) { - onCreateSession({ prompt: trimmedDescription, agentProfileId, executorId }); + onCreateSession({ + prompt: trimmedDescription, + agentProfileId, + executorId, + attachments: toMessageAttachments(attachments), + }); onOpenChange(false); return; } @@ -412,13 +437,18 @@ export function useTaskSubmitHandlers({ ); const handleCreatePlanMode = useCallback( - (trimmedTitle: string, consented: string[]) => + ( + trimmedTitle: string, + consented: string[], + attachments?: ReturnType, + ) => performCreate({ trimmedTitle, trimmedDescription: "", consented, withAgent: false, planMode: true, + attachments, }), [performCreate], ); @@ -477,7 +507,9 @@ export function useTaskSubmitHandlers({ const trimmedTitle = taskName.trim(); const description = descriptionInputRef.current?.getValue() ?? ""; const trimmedDescription = description.trim(); - const attachments = toMessageAttachments(descriptionInputRef.current?.getAttachments() ?? []); + const selectedAttachments = descriptionInputRef.current?.getAttachments() ?? []; + if (hasPendingAttachmentUploads(selectedAttachments)) return; + const attachments = toMessageAttachments(selectedAttachments); if (!validateForCreate(trimmedTitle, trimmedDescription)) return; if (checkRemoteDuplicates()) return; const consent = await ensureFreshBranchConsent(); @@ -514,17 +546,18 @@ export function useTaskSubmitHandlers({ setIsCreatingTask, ]); - const handleCreateSubmit = useCallback(async () => { - const trimmedTitle = taskName.trim(); - const description = descriptionInputRef.current?.getValue() ?? ""; - const trimmedDescription = description.trim(); - const attachments = toMessageAttachments(descriptionInputRef.current?.getAttachments() ?? []); - if (!validateForCreate(trimmedTitle, trimmedDescription)) return; - if (checkRemoteDuplicates()) return; - const consent = await ensureFreshBranchConsent(); - if (consent === null) return; - setIsCreatingTask(true); - try { + const submitCreateTask = useCallback( + async ({ + trimmedTitle, + trimmedDescription, + consent, + attachments, + }: { + trimmedTitle: string; + trimmedDescription: string; + consent: string[]; + attachments: ReturnType; + }) => { if (trimmedDescription) { const finalDescription = transformDescriptionBeforeSubmit ? await transformDescriptionBeforeSubmit(trimmedDescription) @@ -536,9 +569,29 @@ export function useTaskSubmitHandlers({ withAgent: true, attachments, }); - } else if (!autoTitle) { - await handleCreatePlanMode(trimmedTitle, consent); + return; } + if (!autoTitle) { + await handleCreatePlanMode(trimmedTitle, consent, attachments); + } + }, + [autoTitle, handleCreatePlanMode, performCreate, transformDescriptionBeforeSubmit], + ); + + const handleCreateSubmit = useCallback(async () => { + const trimmedTitle = taskName.trim(); + const description = descriptionInputRef.current?.getValue() ?? ""; + const trimmedDescription = description.trim(); + const selectedAttachments = descriptionInputRef.current?.getAttachments() ?? []; + if (hasPendingAttachmentUploads(selectedAttachments)) return; + const attachments = toMessageAttachments(selectedAttachments); + if (!validateForCreate(trimmedTitle, trimmedDescription)) return; + if (checkRemoteDuplicates()) return; + const consent = await ensureFreshBranchConsent(); + if (consent === null) return; + setIsCreatingTask(true); + try { + await submitCreateTask({ trimmedTitle, trimmedDescription, consent, attachments }); } catch (error) { toast({ title: "Failed to create task", @@ -550,13 +603,10 @@ export function useTaskSubmitHandlers({ } }, [ taskName, - autoTitle, validateForCreate, checkRemoteDuplicates, ensureFreshBranchConsent, - performCreate, - handleCreatePlanMode, - transformDescriptionBeforeSubmit, + submitCreateTask, toast, descriptionInputRef, setIsCreatingTask, @@ -565,9 +615,17 @@ export function useTaskSubmitHandlers({ const handleCreateWithoutAgent = useCallback(async () => { const trimmedTitle = taskName.trim(); const trimmedDescription = (descriptionInputRef.current?.getValue() ?? "").trim(); + const selectedAttachments = descriptionInputRef.current?.getAttachments() ?? []; + if (hasPendingAttachmentUploads(selectedAttachments)) return; + const attachments = toMessageAttachments(selectedAttachments); if (!validateForCreate(trimmedTitle, trimmedDescription)) return; - if (!trimmedDescription || !effectiveDefaultStepId || !workspaceId || !effectiveWorkflowId) - return; + const requirements = { + description: trimmedDescription, + workflowStepId: effectiveDefaultStepId, + workspaceId, + workflowId: effectiveWorkflowId, + }; + if (!hasNoAgentTaskRequirements(requirements)) return; if (checkRemoteDuplicates()) return; const consent = await ensureFreshBranchConsent(); @@ -577,8 +635,8 @@ export function useTaskSubmitHandlers({ let submittedPayload: ReturnType | null = null; const buildPayload = (c: string[]) => { const p = buildCreateTaskPayload({ - workspaceId, - effectiveWorkflowId, + workspaceId: requirements.workspaceId, + effectiveWorkflowId: requirements.workflowId, trimmedTitle, trimmedDescription, autoTitle, @@ -587,9 +645,10 @@ export function useTaskSubmitHandlers({ executorId, executorProfileId, withAgent: false, + attachments, workspacePath: noRepository ? workspacePath.trim() || undefined : undefined, }); - p.workflow_step_id = effectiveDefaultStepId; + p.workflow_step_id = requirements.workflowStepId; submittedPayload = p; return p; }; diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index 3e06852f4a..59cb5fd3dc 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -232,6 +232,8 @@ export type DialogFormState = { setHasTitle: (v: boolean) => void; hasDescription: boolean; setHasDescription: (v: boolean) => void; + hasPendingAttachmentUploads: boolean; + setHasPendingAttachmentUploads: (v: boolean) => void; /** Restored draft description, used as initialDescription for TaskFormInputs */ draftDescription: string; /** Cycle counter incremented each time dialog opens - used in key for remount */ @@ -361,7 +363,14 @@ export type SubmitHandlersDeps = { mode: "create" | "edit", meta?: { taskSessionId?: string | null; willNavigate?: boolean }, ) => void; - onCreateSession?: (data: { prompt: string; agentProfileId: string; executorId: string }) => void; + onCreateSession?: (data: { + prompt: string; + agentProfileId: string; + executorId: string; + attachments?: ReturnType< + typeof import("@/components/task-create-dialog-helpers").toMessageAttachments + >; + }) => void; onOpenChange: (open: boolean) => void; preserveTaskCreateLastUsedOnClose?: () => void; taskId: string | null; diff --git a/apps/web/components/task-create-dialog.test.tsx b/apps/web/components/task-create-dialog.test.tsx index d8a645dfbf..699ebab231 100644 --- a/apps/web/components/task-create-dialog.test.tsx +++ b/apps/web/components/task-create-dialog.test.tsx @@ -236,6 +236,8 @@ function buildMockFs(initialDescription = ORIGINAL_PROMPT): DialogFormState { setHasTitle: () => undefined, hasDescription: true, setHasDescription: setHasDescriptionMock, + hasPendingAttachmentUploads: false, + setHasPendingAttachmentUploads: () => undefined, draftDescription: initialDescription, openCycle: 0, currentDefaults: { name: "Task title", description: initialDescription }, diff --git a/apps/web/components/task-create-dialog.tsx b/apps/web/components/task-create-dialog.tsx index 50232534d8..d292b3aaef 100644 --- a/apps/web/components/task-create-dialog.tsx +++ b/apps/web/components/task-create-dialog.tsx @@ -1,6 +1,7 @@ "use client"; import { FormEvent, useCallback, useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; import { Dialog, DialogContent, DialogHeader, DialogFooter } from "@kandev/ui/dialog"; import type { Task } from "@/lib/types/http"; import type { TaskCreateLastUsedState } from "@/lib/state/slices/settings/types"; @@ -59,7 +60,14 @@ export interface TaskCreateDialogProps { mode: "create" | "edit", meta?: { taskSessionId?: string | null; willNavigate?: boolean }, ) => void; - onCreateSession?: (data: { prompt: string; agentProfileId: string; executorId: string }) => void; + onCreateSession?: (data: { + prompt: string; + agentProfileId: string; + executorId: string; + attachments?: ReturnType< + typeof import("@/components/task-create-dialog-helpers").toMessageAttachments + >; + }) => void; initialValues?: TaskCreateDialogInitialValues; taskId?: string | null; parentTaskId?: string; @@ -157,6 +165,7 @@ function CreateModeBody(props: DialogFormBodyProps) { isTaskStarted={isTaskStarted} initialDescription={props.initialDescription} fs={fs} + onPendingAttachmentUploadsChange={fs.setHasPendingAttachmentUploads} handleKeyDown={props.handleKeyDown} enhance={props.enhance} workspaceId={workspaceId} @@ -196,6 +205,7 @@ function SessionModeBody(props: DialogFormBodyProps) { isTaskStarted={props.isTaskStarted} initialDescription={props.initialDescription} fs={props.fs} + onPendingAttachmentUploadsChange={props.fs.setHasPendingAttachmentUploads} handleKeyDown={props.handleKeyDown} enhance={props.enhance} workspaceId={props.workspaceId} @@ -246,6 +256,7 @@ function DialogFormBody(props: DialogFormBodyProps) { const VOICE_SUBMIT_EVENT = { preventDefault: () => {} } as unknown as FormEvent; export function TaskCreateDialog(props: TaskCreateDialogProps) { + const { t } = useTranslation("chat"); const syncedTaskCreateLastUsed = useAppStore((state) => state.userSettings.taskCreateLastUsed); const preserveQueuedLastUsedOnCloseRef = useRef<{ syncedSettings: TaskCreateLastUsedState | null | undefined; @@ -314,7 +325,15 @@ export function TaskCreateDialog(props: TaskCreateDialogProps) { onVoiceAutoSend={handleVoiceAutoSend} /> - + diff --git a/apps/web/components/task/chat/chat-input-body.test.tsx b/apps/web/components/task/chat/chat-input-body.test.tsx index 359db5d85b..514d57a4e3 100644 --- a/apps/web/components/task/chat/chat-input-body.test.tsx +++ b/apps/web/components/task/chat/chat-input-body.test.tsx @@ -54,6 +54,7 @@ function props(overrides: Partial = {}): ChatInputBodyProps inputPlaceholder: "Ask to make changes", isDisabled: false, submitDisabled: false, + hasPendingAttachmentUploads: false, planModeEnabled: false, planModeAvailable: true, mcpServers: [], diff --git a/apps/web/components/task/chat/chat-input-body.tsx b/apps/web/components/task/chat/chat-input-body.tsx index 192e1e460b..f56e759a17 100644 --- a/apps/web/components/task/chat/chat-input-body.tsx +++ b/apps/web/components/task/chat/chat-input-body.tsx @@ -1,6 +1,7 @@ "use client"; import { useState, useCallback } from "react"; +import { useTranslation } from "react-i18next"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { cn } from "@/lib/utils"; import { TipTapInput } from "./tiptap-input"; @@ -22,6 +23,7 @@ export type ChatInputEditorAreaProps = { isDisabled: boolean; submitDisabled: boolean; submitDisabledReason?: string; + hasPendingAttachmentUploads: boolean; planModeEnabled: boolean; planModeAvailable: boolean; mcpServers: string[]; @@ -127,6 +129,7 @@ function FileInput({ } export function ChatInputEditorArea(p: ChatInputEditorAreaProps) { + const { t } = useTranslation("chat"); const { inputRef, value, handleChange, handleSubmitWithReset, inputPlaceholder } = p; const { isDisabled, planModeEnabled, planModeAvailable, mcpServers } = p; const { submitKey, setIsInputFocused, sessionId, taskId, planContextEnabled } = p; @@ -144,6 +147,9 @@ export function ChatInputEditorArea(p: ChatInputEditorAreaProps) { const hasContent = value.trim().length > 0 || userContextCount > 0; // Block submit while enhancing prompt, but keep editor editable for programmatic updates const wrappedSubmit = isEnhancingPrompt || p.submitDisabled ? () => {} : handleSubmitWithReset; + const submitDisabledReason = p.hasPendingAttachmentUploads + ? t("chat:attachmentUploadPendingSubmit") + : p.submitDisabledReason; const handleAttachFiles = useCallback(() => fileInputRef.current?.click(), [fileInputRef]); return (
@@ -189,7 +195,7 @@ export function ChatInputEditorArea(p: ChatInputEditorAreaProps) { canCancelAgent={p.canCancelAgent} hasContent={hasContent} isDisabled={p.submitDisabled} - submitDisabledReason={p.submitDisabledReason} + submitDisabledReason={submitDisabledReason} isSending={isSending} onCancel={onCancel} onSubmit={wrappedSubmit} diff --git a/apps/web/components/task/chat/chat-input-container.tsx b/apps/web/components/task/chat/chat-input-container.tsx index 501a700b44..712136385e 100644 --- a/apps/web/components/task/chat/chat-input-container.tsx +++ b/apps/web/components/task/chat/chat-input-container.tsx @@ -31,9 +31,11 @@ export type { ImageAttachment } from "./image-attachment-preview"; // Type for message attachments sent to backend export type MessageAttachment = { type: "image" | "audio" | "resource"; - data: string; + data?: string; + attachment_id?: string; mime_type: string; name?: string; + size_bytes?: number; delivery_mode?: "prompt" | "path"; }; @@ -126,11 +128,15 @@ async function requestSessionRecover( } } +// The recovery banner keeps resume/fresh-start controls together for desktop +// and mobile layouts; its line count is intentionally bounded by that UI. +// eslint-disable-next-line max-lines-per-function function FailedSessionBanner({ showDialog, onShowDialog, taskId, sessionId, + workspaceId, message = "This agent has stopped.", detail, resumeLabel = "Resume", @@ -140,6 +146,7 @@ function FailedSessionBanner({ onShowDialog: (open: boolean) => void; taskId: string | null; sessionId: string | null; + workspaceId?: string | null; message?: string; detail?: string; resumeLabel?: string; @@ -221,7 +228,14 @@ function FailedSessionBanner({
- {taskId && } + {taskId && ( + + )} ); } @@ -294,6 +308,7 @@ function buildEditorAreaProps( isDisabled: s.isDisabled, submitDisabled: s.submitDisabled, submitDisabledReason: s.submitDisabledReason, + hasPendingAttachmentUploads: s.hasPendingAttachmentUploads, planModeEnabled: p.planModeEnabled, planModeAvailable: p.planModeAvailable ?? true, mcpServers: p.mcpServers ?? [], @@ -423,6 +438,7 @@ export const ChatInputContainer = forwardRef ); diff --git a/apps/web/components/task/chat/context-items/file-attachment-item.tsx b/apps/web/components/task/chat/context-items/file-attachment-item.tsx index bc892d53c3..274091f0fe 100644 --- a/apps/web/components/task/chat/context-items/file-attachment-item.tsx +++ b/apps/web/components/task/chat/context-items/file-attachment-item.tsx @@ -1,6 +1,9 @@ "use client"; -import { memo } from "react"; +import { memo, type ReactNode } from "react"; +import { IconLoader2, IconRefresh } from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; +import { Button } from "@kandev/ui/button"; import type { FileAttachmentContextItem } from "@/lib/types/context"; import { formatBytes } from "@/lib/utils/format-bytes"; import { ContextChip } from "./context-chip"; @@ -10,9 +13,48 @@ export const FileAttachmentItem = memo(function FileAttachmentItem({ }: { item: FileAttachmentContextItem; }) { + const { t } = useTranslation("chat"); + const status = item.attachment.uploadStatus; + let statusIndicator: ReactNode = null; + if (status === "pending" || status === "uploading") { + statusIndicator = ( + + + ); + } else if (status === "failed") { + statusIndicator = ( + + {t("chat:attachmentUploadFailed")} + {item.onRetry && ( + + )} + + ); + } + const preview = ( -
- {item.attachment.fileName} ({formatBytes(item.attachment.size)}) +
+ + {item.attachment.fileName} ({formatBytes(item.attachment.size)}) + + {statusIndicator}
); diff --git a/apps/web/components/task/chat/context-items/image-item.tsx b/apps/web/components/task/chat/context-items/image-item.tsx index c96964f5e7..938f0c9d81 100644 --- a/apps/web/components/task/chat/context-items/image-item.tsx +++ b/apps/web/components/task/chat/context-items/image-item.tsx @@ -1,7 +1,8 @@ "use client"; -import { memo, useState, useCallback } from "react"; -import { IconFile, IconPhoto } from "@tabler/icons-react"; +import { memo, useState, useCallback, type ReactNode } from "react"; +import { IconFile, IconLoader2, IconPhoto, IconRefresh } from "@tabler/icons-react"; +import { useTranslation } from "react-i18next"; import { Button } from "@kandev/ui/button"; import { Dialog, DialogContent } from "@kandev/ui/dialog"; import type { ImageContextItem } from "@/lib/types/context"; @@ -11,66 +12,109 @@ import { } from "@/components/task/chat/image-preview-dialog"; import { ContextChip } from "./context-chip"; +// The image chip intentionally owns the preview, delivery toggle, and retry +// affordance so they stay synchronized on narrow/mobile composers. +// eslint-disable-next-line max-lines-per-function, complexity export const ImageItem = memo(function ImageItem({ item }: { item: ImageContextItem }) { + const { t } = useTranslation("chat"); const [dialogOpen, setDialogOpen] = useState(false); const previewSrc = item.attachment.preview; const deliveryMode = item.attachment.deliveryMode; + const uploadStatus = item.attachment.uploadStatus; const deliveryDescription = deliveryMode === "path" - ? "Upload into the workspace so the agent can read or edit the file." - : "Send as prompt context for visual understanding. The agent will not get a file path."; + ? t("chat:attachmentDeliveryPathDescription") + : t("chat:attachmentDeliveryPromptDescription"); let leadingIcon; const handleClick = useCallback(() => { setDialogOpen(true); }, []); - const preview = previewSrc ? ( -
- Preview - {item.onDeliveryModeChange && ( -
-
+
- ) : undefined; + )} +
+ ) : undefined; if (deliveryMode === "path") { leadingIcon = (