diff --git a/README.md b/README.md index e917a18606..fca8f5096d 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Manage and run tasks in parallel. Orchestrate agents. Review changes. Ship value Kandev Demo

-[See all screenshots](docs/screenshots.md) +[See desktop and mobile screenshots](docs/screenshots.md) ## What diff --git a/apps/backend/cmd/kandev/adapters.go b/apps/backend/cmd/kandev/adapters.go index 3580e8fd7e..bbaee6b21f 100644 --- a/apps/backend/cmd/kandev/adapters.go +++ b/apps/backend/cmd/kandev/adapters.go @@ -216,10 +216,11 @@ func convertToLifecycleAttachments(attachments []v1.MessageAttachment) []lifecyc result := make([]lifecycle.MessageAttachment, len(attachments)) for i, att := range attachments { result[i] = lifecycle.MessageAttachment{ - Type: att.Type, - Data: att.Data, - MimeType: att.MimeType, - Name: att.Name, + Type: att.Type, + Data: att.Data, + MimeType: att.MimeType, + Name: att.Name, + DeliveryMode: att.DeliveryMode, } } return result diff --git a/apps/backend/internal/agent/runtime/lifecycle/session.go b/apps/backend/internal/agent/runtime/lifecycle/session.go index 83a7e04969..caf59e9632 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/session.go +++ b/apps/backend/internal/agent/runtime/lifecycle/session.go @@ -411,10 +411,11 @@ func convertAttachments(attachments []MessageAttachment) []v1.MessageAttachment result := make([]v1.MessageAttachment, 0, len(attachments)) for _, att := range attachments { result = append(result, v1.MessageAttachment{ - Type: att.Type, - Data: att.Data, - MimeType: att.MimeType, - Name: att.Name, + Type: att.Type, + Data: att.Data, + MimeType: att.MimeType, + Name: att.Name, + DeliveryMode: att.DeliveryMode, }) } return result diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index bc7e8cac5b..598ce616ba 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -425,10 +425,11 @@ func (r *LaunchRequest) RepoSpecs() []RepoLaunchSpec { // MessageAttachment represents an image or file attachment for agent prompts. type MessageAttachment struct { - Type string // "image", "audio", or "resource" - Data string // base64-encoded data - MimeType string // MIME type - Name string // optional filename for resource attachments + Type string // "image", "audio", or "resource" + Data string // base64-encoded data + MimeType string // MIME type + Name string // optional filename for resource attachments + DeliveryMode string // "prompt" (native/default) or "path" } // CredentialsManager interface for credential retrieval 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 11e9f736eb..955f1199c4 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 @@ -77,32 +77,7 @@ func (a *Adapter) sendPrompt(ctx context.Context, message string, attachments [] zap.Int("context_length", len(pendingContext))) } - // Build content blocks: text first, then images - contentBlocks := []acp.ContentBlock{acp.TextBlock(finalMessage)} - - // Add media attachments as typed content blocks - for _, att := range attachments { - switch att.Type { - case contentTypeImage: - contentBlocks = append(contentBlocks, acp.ImageBlock(att.Data, att.MimeType)) - case contentTypeAudio: - contentBlocks = append(contentBlocks, acp.AudioBlock(att.Data, att.MimeType)) - case contentTypeResource: - if a.capabilities.PromptCapabilities.EmbeddedContext { - contentBlocks = append(contentBlocks, buildResourceBlock(att)) - } else { - // Agent doesn't support embedded resources — save to workspace and reference in text - saved, saveErr := a.attachMgr.SaveAttachments([]v1.MessageAttachment{att}) - if saveErr != nil || len(saved) == 0 { - a.logger.Warn("failed to save attachment to workspace, falling back to resource block", - zap.String("name", att.Name), zap.Error(saveErr)) - contentBlocks = append(contentBlocks, buildResourceBlock(att)) - } else { - contentBlocks = append(contentBlocks, acp.TextBlock(shared.BuildAttachmentPrompt(saved))) - } - } - } - } + contentBlocks := a.buildPromptContentBlocks(finalMessage, attachments) // Start prompt span — notification spans become children via getPromptTraceCtx() traceCtx, promptSpan := shared.TraceProtocolRequest(ctx, shared.ProtocolACP, a.agentID, "prompt") @@ -225,12 +200,63 @@ func (a *Adapter) sendPrompt(ctx context.Context, message string, attachments [] Usage: usage, }) - // Clean up saved attachments — agent has finished reading them - a.attachMgr.Cleanup() - return nil } +func (a *Adapter) buildPromptContentBlocks(message string, attachments []v1.MessageAttachment) []acp.ContentBlock { + contentBlocks := []acp.ContentBlock{acp.TextBlock(message)} + + for _, att := range attachments { + if att.DeliveryMode == "path" { + saved, saveErr := a.attachMgr.SaveAttachments([]v1.MessageAttachment{att}) + if saveErr != nil || len(saved) == 0 { + a.logger.Warn("failed to save path-mode attachment to workspace", + zap.String("name", att.Name), zap.Error(saveErr)) + contentBlocks = append(contentBlocks, buildAttachmentFallbackBlock(att)) + continue + } + contentBlocks = append(contentBlocks, acp.TextBlock(shared.BuildAttachmentPrompt(saved, true))) + continue + } + + switch att.Type { + case contentTypeImage: + contentBlocks = append(contentBlocks, acp.ImageBlock(att.Data, att.MimeType)) + case contentTypeAudio: + contentBlocks = append(contentBlocks, acp.AudioBlock(att.Data, att.MimeType)) + case contentTypeResource: + if a.capabilities.PromptCapabilities.EmbeddedContext { + contentBlocks = append(contentBlocks, buildResourceBlock(att)) + } else { + // Agent doesn't support embedded resources — save to workspace and reference in text. + saved, saveErr := a.attachMgr.SaveAttachments([]v1.MessageAttachment{att}) + if saveErr != nil || len(saved) == 0 { + a.logger.Warn("failed to save attachment to workspace, falling back to resource block", + zap.String("name", att.Name), zap.Error(saveErr)) + contentBlocks = append(contentBlocks, buildResourceBlock(att)) + } else { + contentBlocks = append(contentBlocks, acp.TextBlock(shared.BuildAttachmentPrompt(saved, false))) + } + } + } + } + + return contentBlocks +} + +func buildAttachmentFallbackBlock(att v1.MessageAttachment) acp.ContentBlock { + switch att.Type { + case contentTypeImage: + return acp.ImageBlock(att.Data, att.MimeType) + case contentTypeAudio: + return acp.AudioBlock(att.Data, att.MimeType) + case contentTypeResource: + return buildResourceBlock(att) + default: + return acp.TextBlock("The user attached a file, but Kandev could not save it to the workspace.") + } +} + // fireWakeup is invoked by wakeupScheduler when a ScheduleWakeup timer // elapses. It issues a synthetic session/prompt so the upstream // @agentclientprotocol/claude-agent-acp bridge drains the SDK's queued wakeup 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 38a753a605..9e4ffb4c0b 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 @@ -1,10 +1,16 @@ package acp import ( + "encoding/base64" + "os" + "path/filepath" + "strings" "testing" - "github.com/coder/acp-go-sdk" + acp "github.com/coder/acp-go-sdk" + "github.com/kandev/kandev/internal/agentctl/server/adapter/transport/shared" "github.com/kandev/kandev/internal/agentctl/types/streams" + v1 "github.com/kandev/kandev/pkg/api/v1" ) // TestCancelActiveToolCalls_PreservesSubagentTask pins the fix for the @@ -50,6 +56,124 @@ func TestCancelActiveToolCalls_PreservesSubagentTask(t *testing.T) { } } +func TestBuildPromptContentBlocks_PathModeAttachmentSavesWritableFile(t *testing.T) { + a := newTestAdapter() + t.Cleanup(func() { _ = a.Close() }) + + workDir := t.TempDir() + a.attachMgr = shared.NewAttachmentManager(workDir, a.logger.Zap()) + a.attachMgr.SetSessionID("sess-path") + a.capabilities = acp.AgentCapabilities{ + PromptCapabilities: acp.PromptCapabilities{ + Image: true, + EmbeddedContext: true, + }, + } + + encoded := base64.StdEncoding.EncodeToString([]byte("remote bytes")) + blocks := a.buildPromptContentBlocks("inspect this", []v1.MessageAttachment{{ + Type: "image", + Data: encoded, + MimeType: "image/png", + Name: "shot.png", + DeliveryMode: "path", + }}) + + if len(blocks) != 2 { + t.Fatalf("expected text prompt plus attachment path prompt, got %d blocks", len(blocks)) + } + if blocks[1].Text == nil { + t.Fatalf("expected path-mode attachment to be sent as text, got %#v", blocks[1]) + } + if blocks[1].Image != nil { + t.Fatalf("path-mode attachment should not be sent as an image block") + } + text := blocks[1].Text.Text + if !strings.Contains(text, ".kandev/attachments/sess-path/shot.png") { + t.Fatalf("attachment prompt did not include saved path: %q", text) + } + if !strings.Contains(text, "writable") { + t.Fatalf("attachment prompt did not include writable contract: %q", text) + } + + savedPath := filepath.Join(workDir, ".kandev", "attachments", "sess-path", "shot.png") + data, err := os.ReadFile(savedPath) + if err != nil { + t.Fatalf("expected attachment to be written to workspace: %v", err) + } + if string(data) != "remote bytes" { + t.Fatalf("saved attachment = %q, want %q", string(data), "remote bytes") + } +} + +func TestBuildPromptContentBlocks_PathModeSaveFailureFallsBackToNativeBlock(t *testing.T) { + a := newTestAdapter() + t.Cleanup(func() { _ = a.Close() }) + + a.attachMgr = shared.NewAttachmentManager("", a.logger.Zap()) + a.attachMgr.SetSessionID("sess-path") + a.capabilities = acp.AgentCapabilities{ + PromptCapabilities: acp.PromptCapabilities{ + Image: true, + }, + } + + encoded := base64.StdEncoding.EncodeToString([]byte("image bytes")) + blocks := a.buildPromptContentBlocks("inspect this", []v1.MessageAttachment{{ + Type: "image", + Data: encoded, + MimeType: "image/png", + Name: "shot.png", + DeliveryMode: "path", + }}) + + if len(blocks) != 2 { + t.Fatalf("expected text prompt plus native fallback block, got %d blocks", len(blocks)) + } + if blocks[1].Image == nil { + t.Fatalf("expected path-mode save failure to fall back to an image block, got %#v", blocks[1]) + } + if blocks[1].Text != nil { + t.Fatalf("fallback image should not be replaced by a text path prompt") + } +} + +func TestBuildPromptContentBlocks_UnsupportedEmbeddedResourceSavesReadOnlyFile(t *testing.T) { + a := newTestAdapter() + t.Cleanup(func() { _ = a.Close() }) + + workDir := t.TempDir() + a.attachMgr = shared.NewAttachmentManager(workDir, a.logger.Zap()) + a.attachMgr.SetSessionID("sess-resource") + a.capabilities = acp.AgentCapabilities{ + PromptCapabilities: acp.PromptCapabilities{ + EmbeddedContext: false, + }, + } + + encoded := base64.StdEncoding.EncodeToString([]byte("doc bytes")) + blocks := a.buildPromptContentBlocks("read this", []v1.MessageAttachment{{ + Type: "resource", + Data: encoded, + MimeType: "application/pdf", + Name: "doc.pdf", + }}) + + if len(blocks) != 2 { + t.Fatalf("expected text prompt plus attachment path prompt, got %d blocks", len(blocks)) + } + if blocks[1].Text == nil { + t.Fatalf("expected unsupported embedded resource to be sent as text path prompt, got %#v", blocks[1]) + } + text := blocks[1].Text.Text + if !strings.Contains(text, ".kandev/attachments/sess-resource/doc.pdf") { + t.Fatalf("attachment prompt did not include saved path: %q", text) + } + if strings.Contains(text, "writable") || strings.Contains(text, "modify") { + t.Fatalf("fallback resource prompt should be read-only, got: %q", text) + } +} + // TestCancelActiveToolCalls_SubagentLateUpdate_LandsAsComplete drives the full // race scenario end-to-end through the real adapter methods (no mocks): // 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 11e31c0880..5b40088741 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/shared/attachments.go @@ -2,6 +2,7 @@ package shared import ( "encoding/base64" + "errors" "fmt" "os" "path/filepath" @@ -64,6 +65,7 @@ func (m *AttachmentManager) SaveAttachments(attachments []v1.MessageAttachment) } var saved []SavedAttachment + usedNames := make(map[string]bool, len(attachments)) for _, att := range attachments { name := att.Name if name == "" { @@ -75,18 +77,18 @@ func (m *AttachmentManager) SaveAttachments(attachments []v1.MessageAttachment) m.logger.Warn("skipping attachment with invalid name", zap.String("original_name", att.Name)) 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)) continue } - absPath := filepath.Join(dir, name) - if err := os.WriteFile(absPath, decoded, 0o644); err != nil { + name, absPath, err := writeUniqueAttachmentFile(dir, name, usedNames, decoded) + if err != nil { m.logger.Warn("failed to write attachment", zap.String("path", absPath), zap.Error(err)) continue } + usedNames[name] = true relPath := filepath.Join(".kandev", "attachments", m.sessionID, name) saved = append(saved, SavedAttachment{ @@ -117,7 +119,7 @@ func (m *AttachmentManager) Cleanup() { // BuildAttachmentPrompt generates prompt text referencing saved attachment files. // Used by adapters that don't support native multimodal content. -func BuildAttachmentPrompt(saved []SavedAttachment) string { +func BuildAttachmentPrompt(saved []SavedAttachment, writable bool) string { if len(saved) == 0 { return "" } @@ -125,17 +127,72 @@ func BuildAttachmentPrompt(saved []SavedAttachment) string { var sb strings.Builder if len(saved) == 1 { s := saved[0] - fmt.Fprintf(&sb, "The user attached a file: %s (saved to %s in the workspace). Use your file reading tools to access it.\n\n", s.Name, s.RelPath) - } else { - sb.WriteString("The user attached files that you should read and analyze:\n") - for _, s := range saved { - fmt.Fprintf(&sb, "- %s (saved to %s)\n", s.Name, s.RelPath) + name := sanitizePromptValue(s.Name) + relPath := sanitizePromptValue(s.RelPath) + if writable { + fmt.Fprintf(&sb, "The user attached a writable file: %s (saved to %s in the workspace). Use your file reading tools to access or modify it.\n\n", name, relPath) + } else { + fmt.Fprintf(&sb, "The user attached a file: %s (saved to %s in the workspace). Use your file reading tools to access it.\n\n", name, relPath) + } + } else { + if writable { + sb.WriteString("The user attached writable files that you should read and analyze:\n") + } else { + sb.WriteString("The user attached files that you should read and analyze:\n") + } + for _, s := range saved { + fmt.Fprintf(&sb, "- %s (saved to %s)\n", sanitizePromptValue(s.Name), sanitizePromptValue(s.RelPath)) + } + if writable { + sb.WriteString("\nUse your file reading tools to access or modify them.\n\n") + } else { + sb.WriteString("\nUse your file reading tools to access them.\n\n") } - sb.WriteString("\nUse your file reading tools to access them.\n\n") } return sb.String() } +func writeUniqueAttachmentFile(dir, name string, used map[string]bool, data []byte) (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 := filepath.Join(dir, candidate) + 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 + } + + _, writeErr := file.Write(data) + closeErr := file.Close() + if writeErr != nil { + _ = os.Remove(absPath) + return "", absPath, writeErr + } + if closeErr != nil { + _ = os.Remove(absPath) + return "", absPath, closeErr + } + return candidate, absPath, nil + } +} + +func sanitizePromptValue(value string) string { + replacer := strings.NewReplacer("\r", " ", "\n", " ", "<", "(", ">", ")", "`", "'") + return strings.TrimSpace(replacer.Replace(value)) +} + // generateName creates a filename for attachments that don't have a Name field. func (m *AttachmentManager) generateName(att v1.MessageAttachment) string { ext := extensionFromMimeType(att.MimeType) 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 daf459e8c6..7017dd20f5 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,8 +2,10 @@ package shared import ( "encoding/base64" + "fmt" "os" "path/filepath" + "sync" "testing" v1 "github.com/kandev/kandev/pkg/api/v1" @@ -172,6 +174,157 @@ func TestSaveAttachments_MultipleAttachments(t *testing.T) { } } +func TestSaveAttachments_DuplicateNamesDoNotClobber(t *testing.T) { + workDir := t.TempDir() + mgr := NewAttachmentManager(workDir, testLogger()) + mgr.SetSessionID("sess-dupe") + + first := base64.StdEncoding.EncodeToString([]byte("first")) + second := base64.StdEncoding.EncodeToString([]byte("second")) + third := base64.StdEncoding.EncodeToString([]byte("third")) + + saved, err := mgr.SaveAttachments([]v1.MessageAttachment{ + {Type: "resource", Data: first, MimeType: "text/plain", Name: "report.txt"}, + {Type: "resource", Data: second, MimeType: "text/plain", Name: "report.txt"}, + {Type: "resource", Data: third, MimeType: "text/plain", Name: "report.txt"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(saved) != 3 { + t.Fatalf("expected 3 saved, got %d", len(saved)) + } + + wantNames := []string{"report.txt", "report-2.txt", "report-3.txt"} + wantContents := []string{"first", "second", "third"} + for i := range saved { + if saved[i].Name != wantNames[i] { + t.Errorf("saved[%d].Name = %q, want %q", i, saved[i].Name, wantNames[i]) + } + data, err := os.ReadFile(saved[i].AbsPath) + if err != nil { + t.Fatalf("failed to read saved[%d]: %v", i, err) + } + if string(data) != wantContents[i] { + t.Errorf("saved[%d] content = %q, want %q", i, string(data), wantContents[i]) + } + } +} + +func TestSaveAttachments_DuplicateNamesAcrossCallsDoNotClobber(t *testing.T) { + workDir := t.TempDir() + mgr := NewAttachmentManager(workDir, testLogger()) + mgr.SetSessionID("sess-dupe-calls") + + first := base64.StdEncoding.EncodeToString([]byte("first")) + second := base64.StdEncoding.EncodeToString([]byte("second")) + + firstSaved, err := mgr.SaveAttachments([]v1.MessageAttachment{ + {Type: "resource", Data: first, MimeType: "text/plain", Name: "report.txt"}, + }) + if err != nil { + t.Fatalf("unexpected first save error: %v", err) + } + secondSaved, err := mgr.SaveAttachments([]v1.MessageAttachment{ + {Type: "resource", Data: second, MimeType: "text/plain", Name: "report.txt"}, + }) + if err != nil { + t.Fatalf("unexpected second save error: %v", err) + } + + if firstSaved[0].Name != "report.txt" { + t.Fatalf("first name = %q, want report.txt", firstSaved[0].Name) + } + if secondSaved[0].Name != "report-2.txt" { + t.Fatalf("second name = %q, want report-2.txt", secondSaved[0].Name) + } + + firstData, err := os.ReadFile(firstSaved[0].AbsPath) + if err != nil { + t.Fatalf("failed to read first file: %v", err) + } + secondData, err := os.ReadFile(secondSaved[0].AbsPath) + if err != nil { + t.Fatalf("failed to read second file: %v", err) + } + if string(firstData) != "first" { + t.Errorf("first content = %q, want first", string(firstData)) + } + if string(secondData) != "second" { + t.Errorf("second content = %q, want second", string(secondData)) + } +} + +func TestSaveAttachments_ConcurrentDuplicateNamesDoNotClobber(t *testing.T) { + workDir := t.TempDir() + const workers = 20 + + var wg sync.WaitGroup + savedCh := make(chan SavedAttachment, workers) + errCh := make(chan error, workers) + + for i := 0; i < workers; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + mgr := NewAttachmentManager(workDir, testLogger()) + mgr.SetSessionID("sess-concurrent") + + content := fmt.Sprintf("payload-%02d", i) + saved, err := mgr.SaveAttachments([]v1.MessageAttachment{ + { + Type: "resource", + Data: base64.StdEncoding.EncodeToString([]byte(content)), + MimeType: "text/plain", + Name: "report.txt", + }, + }) + if err != nil { + errCh <- err + return + } + if len(saved) != 1 { + errCh <- fmt.Errorf("saved count = %d, want 1", len(saved)) + return + } + savedCh <- saved[0] + }() + } + + wg.Wait() + close(savedCh) + close(errCh) + + for err := range errCh { + if err != nil { + t.Fatalf("unexpected save error: %v", err) + } + } + + names := make(map[string]bool, workers) + contents := make(map[string]bool, workers) + for saved := range savedCh { + if names[saved.Name] { + t.Fatalf("duplicate saved name %q", saved.Name) + } + names[saved.Name] = true + + data, err := os.ReadFile(saved.AbsPath) + if err != nil { + t.Fatalf("failed to read %s: %v", saved.AbsPath, err) + } + contents[string(data)] = true + } + if len(names) != workers { + t.Fatalf("saved unique names = %d, want %d", len(names), workers) + } + if len(contents) != workers { + t.Fatalf("saved unique contents = %d, want %d", len(contents), workers) + } +} + func TestSaveAttachments_NoName_GeneratesFromMime(t *testing.T) { workDir := t.TempDir() mgr := NewAttachmentManager(workDir, testLogger()) @@ -303,27 +456,48 @@ func TestCleanup_SessionIsolation(t *testing.T) { } func TestBuildAttachmentPrompt_Empty(t *testing.T) { - result := BuildAttachmentPrompt(nil) + result := BuildAttachmentPrompt(nil, true) if result != "" { t.Errorf("expected empty string, got %q", result) } } -func TestBuildAttachmentPrompt_SingleFile(t *testing.T) { +func TestBuildAttachmentPrompt_SingleWritableFile(t *testing.T) { saved := []SavedAttachment{ {RelPath: ".kandev/attachments/s1/report.pdf", Name: "report.pdf"}, } - result := BuildAttachmentPrompt(saved) + result := BuildAttachmentPrompt(saved, true) if result == "" { t.Fatal("expected non-empty result") } - // Should mention the file name and path if !contains(result, "report.pdf") { t.Errorf("result should contain filename, got: %q", result) } if !contains(result, ".kandev/attachments/s1/report.pdf") { t.Errorf("result should contain path, got: %q", result) } + if !contains(result, "writable") { + t.Errorf("result should tell the agent the file is writable, got: %q", result) + } +} + +func TestBuildAttachmentPrompt_SingleReadOnlyFile(t *testing.T) { + saved := []SavedAttachment{ + {RelPath: ".kandev/attachments/s1/report.pdf", Name: "report.pdf"}, + } + result := BuildAttachmentPrompt(saved, false) + if result == "" { + t.Fatal("expected non-empty result") + } + if contains(result, "writable") { + t.Errorf("read-only result should not call the file writable, got: %q", result) + } + if contains(result, "modify") { + t.Errorf("read-only result should not tell the agent to modify the file, got: %q", result) + } + if !contains(result, "access it") { + t.Errorf("read-only result should tell the agent to access the file, got: %q", result) + } } func TestBuildAttachmentPrompt_MultipleFiles(t *testing.T) { @@ -331,7 +505,7 @@ func TestBuildAttachmentPrompt_MultipleFiles(t *testing.T) { {RelPath: ".kandev/attachments/s1/a.pdf", Name: "a.pdf"}, {RelPath: ".kandev/attachments/s1/b.png", Name: "b.png"}, } - result := BuildAttachmentPrompt(saved) + result := BuildAttachmentPrompt(saved, true) if !contains(result, "a.pdf") { t.Errorf("result should contain a.pdf, got: %q", result) } @@ -340,6 +514,25 @@ func TestBuildAttachmentPrompt_MultipleFiles(t *testing.T) { } } +func TestBuildAttachmentPrompt_SanitizesPromptValues(t *testing.T) { + saved := []SavedAttachment{ + { + RelPath: ".kandev/attachments/s1/\npath`v2`.pdf", + Name: "report\nplease`now`.pdf", + }, + } + result := BuildAttachmentPrompt(saved, true) + if contains(result, "<") || contains(result, ">") || contains(result, "`") || contains(result, "\nplease") { + t.Errorf("result contains unsanitized prompt values: %q", result) + } + if !contains(result, "report(one) please'now'.pdf") { + t.Errorf("result should contain sanitized filename, got: %q", result) + } + if !contains(result, ".kandev/attachments/s1/(bad) path'v2'.pdf") { + t.Errorf("result should contain sanitized path, got: %q", result) + } +} + func TestExtensionFromMimeType(t *testing.T) { tests := []struct { mimeType string diff --git a/apps/backend/internal/orchestrator/event_handlers_agent.go b/apps/backend/internal/orchestrator/event_handlers_agent.go index e6fc4dc443..14af47e667 100644 --- a/apps/backend/internal/orchestrator/event_handlers_agent.go +++ b/apps/backend/internal/orchestrator/event_handlers_agent.go @@ -354,9 +354,11 @@ func (s *Service) executeQueuedMessage(callerSessionID string, queuedMsg *messag attachments := make([]v1.MessageAttachment, len(queuedMsg.Attachments)) for i, att := range queuedMsg.Attachments { attachments[i] = v1.MessageAttachment{ - Type: att.Type, - Data: att.Data, - MimeType: att.MimeType, + Type: att.Type, + Data: att.Data, + MimeType: att.MimeType, + Name: att.Name, + DeliveryMode: att.DeliveryMode, } } diff --git a/apps/backend/internal/orchestrator/event_handlers_workflow.go b/apps/backend/internal/orchestrator/event_handlers_workflow.go index c80fbbc2a4..4b2451e709 100644 --- a/apps/backend/internal/orchestrator/event_handlers_workflow.go +++ b/apps/backend/internal/orchestrator/event_handlers_workflow.go @@ -1415,9 +1415,11 @@ func (s *Service) takeAndMergeHandoffMessage(ctx context.Context, sessionID, bas attachments = make([]v1.MessageAttachment, 0, len(msg.Attachments)) for _, a := range msg.Attachments { attachments = append(attachments, v1.MessageAttachment{ - Type: a.Type, - Data: a.Data, - MimeType: a.MimeType, + Type: a.Type, + Data: a.Data, + MimeType: a.MimeType, + Name: a.Name, + DeliveryMode: a.DeliveryMode, }) } } @@ -1495,9 +1497,11 @@ func toQueuedAttachments(attachments []v1.MessageAttachment) []messagequeue.Mess queued := make([]messagequeue.MessageAttachment, 0, len(attachments)) for _, attachment := range attachments { queued = append(queued, messagequeue.MessageAttachment{ - Type: attachment.Type, - Data: attachment.Data, - MimeType: attachment.MimeType, + Type: attachment.Type, + Data: attachment.Data, + MimeType: attachment.MimeType, + Name: attachment.Name, + DeliveryMode: attachment.DeliveryMode, }) } return queued diff --git a/apps/backend/internal/orchestrator/handlers/queue_handlers.go b/apps/backend/internal/orchestrator/handlers/queue_handlers.go index 4b8b5f0867..06fa145b22 100644 --- a/apps/backend/internal/orchestrator/handlers/queue_handlers.go +++ b/apps/backend/internal/orchestrator/handlers/queue_handlers.go @@ -100,6 +100,10 @@ func (h *QueueHandlers) wsQueueMessage(ctx context.Context, msg *ws.Message) (*w if req.Content == "" && len(req.Attachments) == 0 { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "content or attachments are required", nil) } + if invalid := firstInvalidDeliveryMode(req.Attachments); invalid >= 0 { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "attachment delivery_mode must be prompt or path", + map[string]interface{}{"attachment_index": invalid}) + } if req.UserID == messagequeue.QueuedByAgent { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "user_id may not impersonate the agent identity", nil) } @@ -233,6 +237,10 @@ func (h *QueueHandlers) wsUpdateMessage(ctx context.Context, msg *ws.Message) (* if req.Content == "" && len(req.Attachments) == 0 { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "content or attachments are required", nil) } + if invalid := firstInvalidDeliveryMode(req.Attachments); invalid >= 0 { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "attachment delivery_mode must be prompt or path", + 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 @@ -260,6 +268,15 @@ func (h *QueueHandlers) wsUpdateMessage(ctx context.Context, msg *ws.Message) (* return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{fieldEntryID: req.EntryID}) } +func firstInvalidDeliveryMode(attachments []messagequeue.MessageAttachment) int { + for i, att := range attachments { + if att.DeliveryMode != "" && att.DeliveryMode != "prompt" && att.DeliveryMode != "path" { + return i + } + } + return -1 +} + 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 49472e58a1..9c9669465b 100644 --- a/apps/backend/internal/orchestrator/handlers/queue_handlers_test.go +++ b/apps/backend/internal/orchestrator/handlers/queue_handlers_test.go @@ -151,6 +151,41 @@ func TestWsQueueMessage(t *testing.T) { }) } +func TestFirstInvalidDeliveryMode(t *testing.T) { + tests := []struct { + name string + attachments []messagequeue.MessageAttachment + want int + }{ + {name: "empty list", attachments: nil, want: -1}, + { + name: "valid modes", + attachments: []messagequeue.MessageAttachment{ + {DeliveryMode: ""}, + {DeliveryMode: "prompt"}, + {DeliveryMode: "path"}, + }, + want: -1, + }, + { + name: "first invalid mode", + attachments: []messagequeue.MessageAttachment{ + {DeliveryMode: "prompt"}, + {DeliveryMode: "inline"}, + {DeliveryMode: "path"}, + }, + want: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := firstInvalidDeliveryMode(tt.attachments) + assert.Equal(t, tt.want, got) + }) + } +} + func TestWsCancelAll(t *testing.T) { t.Run("clears the queue", func(t *testing.T) { handlers, svc := setupQueueHandlers(t) diff --git a/apps/backend/internal/orchestrator/messagequeue/types.go b/apps/backend/internal/orchestrator/messagequeue/types.go index 1e809d673f..72566c9f04 100644 --- a/apps/backend/internal/orchestrator/messagequeue/types.go +++ b/apps/backend/internal/orchestrator/messagequeue/types.go @@ -51,9 +51,11 @@ type QueuedMessage struct { // MessageAttachment represents an attachment (image) in a queued message. type MessageAttachment struct { - Type string `json:"type"` - Data string `json:"data"` - MimeType string `json:"mime_type"` + Type string `json:"type"` + Data string `json:"data"` + MimeType string `json:"mime_type"` + Name string `json:"name,omitempty"` + DeliveryMode string `json:"delivery_mode,omitempty"` } // QueueStatus is the per-session view returned to clients: full ordered list of diff --git a/apps/backend/internal/task/handlers/message_handlers.go b/apps/backend/internal/task/handlers/message_handlers.go index d25d626e1c..30220f00ef 100644 --- a/apps/backend/internal/task/handlers/message_handlers.go +++ b/apps/backend/internal/task/handlers/message_handlers.go @@ -298,6 +298,9 @@ func validateAddMessageRequest(req wsAddMessageRequest) string { if req.Content == "" && len(req.Attachments) == 0 { return "content or attachments are required" } + if err := validateAttachments(req.Attachments); err != nil { + return err.Error() + } return "" } diff --git a/apps/backend/internal/task/handlers/task_http_handlers.go b/apps/backend/internal/task/handlers/task_http_handlers.go index e4c171399d..1e288ca31e 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers.go +++ b/apps/backend/internal/task/handlers/task_http_handlers.go @@ -461,6 +461,9 @@ func validateAttachments(items []v1.MessageAttachment) error { 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) } if totalSize > maxAttachmentDataBytes { diff --git a/apps/backend/internal/task/handlers/task_http_handlers_test.go b/apps/backend/internal/task/handlers/task_http_handlers_test.go index 0be3deccf2..e18aceaaba 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers_test.go +++ b/apps/backend/internal/task/handlers/task_http_handlers_test.go @@ -19,6 +19,7 @@ import ( "github.com/kandev/kandev/internal/orchestrator" "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/service" + v1 "github.com/kandev/kandev/pkg/api/v1" ) // captureOrchestrator records every LaunchSession request so tests can assert @@ -185,6 +186,40 @@ func TestHTTPCreateTask_ProjectIDReachesOfficePath(t *testing.T) { assert.Equal(t, "wf-office", repo.captured.WorkflowID, "office workflow should be auto-resolved") } +func TestValidateAttachments_DeliveryMode(t *testing.T) { + base := v1.MessageAttachment{ + Type: "resource", + MimeType: "text/plain", + Data: "dGVzdA==", + } + + tests := []struct { + name string + deliveryMode string + wantErr string + }{ + {name: "empty uses default", deliveryMode: ""}, + {name: "prompt is valid", deliveryMode: "prompt"}, + {name: "path is valid", deliveryMode: "path"}, + {name: "inline is rejected", deliveryMode: "inline", wantErr: "delivery_mode must be prompt or path"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + attachment := base + attachment.DeliveryMode = tt.deliveryMode + + err := validateAttachments([]v1.MessageAttachment{attachment}) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} + func newTestLogger(t *testing.T) *logger.Logger { t.Helper() log, err := logger.NewLogger(logger.LoggingConfig{ diff --git a/apps/backend/pkg/api/v1/task.go b/apps/backend/pkg/api/v1/task.go index 6d1816de23..1c9d8ff74e 100644 --- a/apps/backend/pkg/api/v1/task.go +++ b/apps/backend/pkg/api/v1/task.go @@ -138,10 +138,15 @@ type TaskEvent struct { // Type determines the ACP content block: "image" → ImageBlock, "audio" → AudioBlock, // "resource" → ResourceBlock (text or blob based on MIME type). type MessageAttachment struct { - Type string `json:"type"` // "image", "audio", "resource" - 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) + Type string `json:"type"` // "image", "audio", "resource" + 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) + DeliveryMode string `json:"delivery_mode,omitempty"` // "prompt" (native/default) or "path" +} + +func (a MessageAttachment) HasValidDeliveryMode() bool { + return a.DeliveryMode == "" || a.DeliveryMode == "prompt" || a.DeliveryMode == "path" } // ContextFileMeta represents a context file reference attached to a message diff --git a/apps/backend/pkg/api/v1/task_test.go b/apps/backend/pkg/api/v1/task_test.go new file mode 100644 index 0000000000..481274d131 --- /dev/null +++ b/apps/backend/pkg/api/v1/task_test.go @@ -0,0 +1,25 @@ +package v1 + +import "testing" + +func TestMessageAttachmentHasValidDeliveryMode(t *testing.T) { + tests := []struct { + name string + deliveryMode string + want bool + }{ + {name: "empty defaults to prompt", deliveryMode: "", want: true}, + {name: "prompt", deliveryMode: "prompt", want: true}, + {name: "path", deliveryMode: "path", want: true}, + {name: "invalid", deliveryMode: "inline", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := (MessageAttachment{DeliveryMode: tt.deliveryMode}).HasValidDeliveryMode() + if got != tt.want { + t.Fatalf("HasValidDeliveryMode() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/apps/web/components/task-create-dialog-helpers.ts b/apps/web/components/task-create-dialog-helpers.ts index 61367db5f4..d5efc6312d 100644 --- a/apps/web/components/task-create-dialog-helpers.ts +++ b/apps/web/components/task-create-dialog-helpers.ts @@ -30,12 +30,19 @@ export function toMessageAttachments( if (attachments.length === 0) return undefined; return attachments.map((att) => att.isImage - ? { type: "image" as const, data: att.data, mime_type: att.mimeType } + ? { + 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, }, ); } diff --git a/apps/web/components/task/changes-panel-file-row.test.tsx b/apps/web/components/task/changes-panel-file-row.test.tsx index 2a08eb137f..a3da1200a9 100644 --- a/apps/web/components/task/changes-panel-file-row.test.tsx +++ b/apps/web/components/task/changes-panel-file-row.test.tsx @@ -107,6 +107,36 @@ describe("FileRow truncation (regression: path overlaps diff stats in narrow pan repositoryName: "frontend", }); }); + + it("opens image files in the file preview instead of the diff viewer", () => { + const onOpenDiff = vi.fn(); + const onEditFile = vi.fn(); + const { container } = render( + +
    + +
+ , + ); + + const row = container.querySelector( + "[data-changes-file='docs/screenshots/mobile-chat.webp']", + ) as HTMLElement | null; + expect(row).not.toBeNull(); + fireEvent.click(row!); + + expect(onEditFile).toHaveBeenCalledWith("docs/screenshots/mobile-chat.webp"); + expect(onOpenDiff).not.toHaveBeenCalled(); + }); }); describe("FileRow hover swap (stats <-> actions occupy same cell)", () => { diff --git a/apps/web/components/task/changes-panel-file-row.tsx b/apps/web/components/task/changes-panel-file-row.tsx index c8650b45a1..fd63957f54 100644 --- a/apps/web/components/task/changes-panel-file-row.tsx +++ b/apps/web/components/task/changes-panel-file-row.tsx @@ -14,6 +14,7 @@ import { cn } from "@/lib/utils"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { LineStat } from "@/components/diff-stat"; import { FileIcon } from "@/components/ui/file-icon"; +import { getFileCategory } from "@/lib/utils/file-types"; import { FileStatusIcon } from "./file-status-icon"; import type { ChangedFile } from "./changes-panel-helpers"; import type { OpenDiffOptions } from "./changes-diff-target"; @@ -74,6 +75,10 @@ export function FileRow({ if (e.button === 2) return; const consumed = onSelect?.(file.path, e); if (!consumed) { + if (getFileCategory(file.path) === "image") { + onEditFile(file.path); + return; + } onOpenDiff(file.path, { source: "uncommitted", repositoryName: file.repositoryName, diff --git a/apps/web/components/task/chat/chat-input-container.tsx b/apps/web/components/task/chat/chat-input-container.tsx index 22ffd9cd9d..d2caa0d445 100644 --- a/apps/web/components/task/chat/chat-input-container.tsx +++ b/apps/web/components/task/chat/chat-input-container.tsx @@ -29,6 +29,7 @@ export type MessageAttachment = { data: string; mime_type: string; name?: string; + delivery_mode?: "prompt" | "path"; }; export type ChatInputContainerHandle = { diff --git a/apps/web/components/task/chat/context-items/context-chip.tsx b/apps/web/components/task/chat/context-items/context-chip.tsx index 3655b633ac..1a076f7fec 100644 --- a/apps/web/components/task/chat/context-items/context-chip.tsx +++ b/apps/web/components/task/chat/context-items/context-chip.tsx @@ -33,6 +33,7 @@ type ContextChipProps = { preview?: ReactNode; /** Data URL to render as a tiny thumbnail instead of the default icon */ thumbnail?: string; + leadingIcon?: ReactNode; onClick?: () => void; onUnpin?: () => void; onRemove?: () => void; @@ -44,23 +45,30 @@ export const ContextChip = memo(function ContextChip({ pinned, preview, thumbnail, + leadingIcon, onClick, onUnpin, onRemove, }: ContextChipProps) { const Icon = ICON_BY_KIND[kind]; + let iconNode: ReactNode; + if (leadingIcon) { + iconNode = leadingIcon; + } else if (thumbnail) { + iconNode = ( + // eslint-disable-next-line @next/next/no-img-element -- base64 thumbnail + + ); + } else { + iconNode = ; + } const chip = (
- {thumbnail ? ( - // eslint-disable-next-line @next/next/no-img-element -- base64 thumbnail - - ) : ( - - )} + {iconNode} {label} {pinned && onUnpin && ( + +
+

{deliveryDescription}

+ + )} ) : undefined; + if (deliveryMode === "path") { + leadingIcon = ( + + ); + } else if (!previewSrc) { + leadingIcon = ; + } return ( <> { expect(attachment!.id).toMatch(UUID_V4_REGEX); expect(attachment!.isImage).toBe(false); expect(attachment!.fileName).toBe("notes.txt"); + expect(attachment!.deliveryMode).toBe("path"); }); it("assigns a fallback UUID to image attachments when crypto.randomUUID is unavailable", async () => { @@ -56,6 +57,7 @@ describe("processFile in insecure context (HTTP, no crypto.randomUUID)", () => { expect(attachment).not.toBeNull(); expect(attachment!.id).toMatch(UUID_V4_REGEX); expect(attachment!.isImage).toBe(true); + expect(attachment!.deliveryMode).toBe("prompt"); expect(attachment!.preview).toMatch(/^data:image\/png;base64,/); }); diff --git a/apps/web/components/task/chat/file-attachment.ts b/apps/web/components/task/chat/file-attachment.ts index 1c835378cc..d33b9789f8 100644 --- a/apps/web/components/task/chat/file-attachment.ts +++ b/apps/web/components/task/chat/file-attachment.ts @@ -13,6 +13,7 @@ export type FileAttachment = { size: number; // File size in bytes preview?: string; // Data URL for image preview (only for images) isImage: boolean; // Whether this is a previewable image + deliveryMode: "prompt" | "path"; // Native prompt block or writable executor file path }; export const PREVIEWABLE_IMAGE_TYPES = ["image/png", "image/jpeg", "image/gif", "image/webp"]; @@ -79,6 +80,7 @@ export function processFile(file: File): Promise { size: file.size, preview: dataUrl, isImage: true, + deliveryMode: "prompt", }); }; img.onerror = () => resolve(null); @@ -91,6 +93,7 @@ export function processFile(file: File): Promise { fileName: file.name, size: file.size, isImage: false, + deliveryMode: "path", }); } }; diff --git a/apps/web/components/task/chat/use-chat-input-state.ts b/apps/web/components/task/chat/use-chat-input-state.ts index fded7ac142..20d35e5e84 100644 --- a/apps/web/components/task/chat/use-chat-input-state.ts +++ b/apps/web/components/task/chat/use-chat-input-state.ts @@ -52,8 +52,20 @@ function collectComments( function toMessageAttachments(attachments: FileAttachment[]): MessageAttachment[] { return attachments.map((att) => att.isImage - ? { type: "image" as const, data: att.data, mime_type: att.mimeType } - : { type: "resource" as const, data: att.data, mime_type: att.mimeType, name: att.fileName }, + ? { + 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, + }, ); } @@ -117,7 +129,19 @@ function useAttachments(sessionId: string | null) { ); const handleRemoveAttachment = useCallback((id: string) => { - setAttachments((prev) => prev.filter((att) => att.id !== id)); + setAttachments((prev) => { + const next = prev.filter((att) => att.id !== id); + attachmentsRef.current = next; + return next; + }); + }, []); + + const handleDeliveryModeChange = useCallback((id: string, deliveryMode: "prompt" | "path") => { + setAttachments((prev) => { + const next = prev.map((att) => (att.id === id ? { ...att, deliveryMode } : att)); + attachmentsRef.current = next; + return next; + }); }, []); const getAttachments = useCallback( @@ -131,6 +155,7 @@ function useAttachments(sessionId: string | null) { setAttachments, addFiles, handleRemoveAttachment, + handleDeliveryModeChange, getAttachments, }; } @@ -158,6 +183,7 @@ export function useChatInputState({ setAttachments, addFiles, handleRemoveAttachment, + handleDeliveryModeChange, getAttachments, } = useAttachments(sessionId); @@ -225,6 +251,7 @@ export function useChatInputState({ label: `Image (${formatBytes(att.size)})`, attachment: att, onRemove: () => handleRemoveAttachment(att.id), + onDeliveryModeChange: (mode) => handleDeliveryModeChange(att.id, mode), } as ImageContextItem) : ({ kind: "file-attachment" as const, @@ -235,7 +262,7 @@ export function useChatInputState({ } as FileAttachmentContextItem), ); return [...contextItems, ...attachmentItems]; - }, [contextItems, attachments, handleRemoveAttachment]); + }, [contextItems, attachments, handleRemoveAttachment, handleDeliveryModeChange]); // prettier-ignore return { value, attachments, inputRef, addFiles, handleChange, handleSubmit, allItems, getAttachments }; diff --git a/apps/web/components/task/new-session-form-actions.test.ts b/apps/web/components/task/new-session-form-actions.test.ts index d23a2806fa..b4b0831bbb 100644 --- a/apps/web/components/task/new-session-form-actions.test.ts +++ b/apps/web/components/task/new-session-form-actions.test.ts @@ -41,6 +41,7 @@ const ATTACHMENT: FileAttachment = { fileName: "notes.txt", size: 42, isImage: false, + deliveryMode: "path", }; const TASK_ID = "task-1"; const PROFILE_ID = "profile-a"; diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index f1482a7c22..b21556156f 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -84,6 +84,8 @@ type CreateTaskOpts = { repositories?: Array<{ repository_id: string; base_branch?: string; checkout_branch?: string }>; plan_mode?: boolean; metadata?: Record; + parent_id?: string; + attachments?: MessageAttachmentInput[]; }; function buildTaskMetadata(opts: CreateTaskOpts): Record | undefined { @@ -112,11 +114,20 @@ function buildCreateTaskBody( "repositories", opts?.repositories ?? opts?.repository_ids?.map((id) => ({ repository_id: id })), ); + setIf(body, "attachments", opts?.attachments); if (opts?.plan_mode) body.plan_mode = true; setIf(body, "parent_id", opts?.parent_id); return body; } +type MessageAttachmentInput = { + type: string; + data: string; + mime_type: string; + name?: string; + delivery_mode?: "prompt" | "path"; +}; + type OptionalAgentTaskOpts = { workflow_id?: string; workflow_step_id?: string; @@ -126,6 +137,7 @@ type OptionalAgentTaskOpts = { executor_profile_id?: string; metadata?: Record; parent_id?: string; + attachments?: MessageAttachmentInput[]; }; /** `repositories` (with per-entry branches) takes precedence over the shorthand @@ -147,6 +159,7 @@ function buildOptionalAgentTaskFields(opts?: OptionalAgentTaskOpts): Record; /** Parent task ID for subtasks. */ parent_id?: string; + attachments?: MessageAttachmentInput[]; }, ): Promise { return this.request("POST", "/api/v1/tasks", buildCreateTaskBody(workspaceId, title, opts)); @@ -415,6 +429,7 @@ export class ApiClient { metadata?: Record; /** Parent task ID for subtasks. */ parent_id?: string; + attachments?: MessageAttachmentInput[]; }, ): Promise { return this.request("POST", "/api/v1/tasks", { @@ -1311,11 +1326,31 @@ export class ApiClient { * Use after the auto-started agent finishes so the session is in a state * that accepts new prompts. */ - async addUserMessage(taskId: string, sessionId: string, content: string): Promise { + async addUserMessage( + taskId: string, + sessionId: string, + content: string, + attachments?: MessageAttachmentInput[], + ): Promise { await this.wsRequest("message.add", { task_id: taskId, session_id: sessionId, content, + attachments, + }); + } + + async queueMessage( + taskId: string, + sessionId: string, + content: string, + attachments?: MessageAttachmentInput[], + ): Promise { + await this.wsRequest("message.queue.add", { + task_id: taskId, + session_id: sessionId, + content, + attachments, }); } diff --git a/apps/web/e2e/tests/chat/attachment-delivery-mode.spec.ts b/apps/web/e2e/tests/chat/attachment-delivery-mode.spec.ts new file mode 100644 index 0000000000..b8d11bca19 --- /dev/null +++ b/apps/web/e2e/tests/chat/attachment-delivery-mode.spec.ts @@ -0,0 +1,124 @@ +import fs from "node:fs"; +import path from "node:path"; +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { openTaskSession, waitForLatestSessionDone } from "../../helpers/session"; + +type PersistedAttachment = { + type?: string; + name?: string; + mime_type?: string; + delivery_mode?: string; +}; + +async function waitForUserAttachment( + apiClient: ApiClient, + sessionId: string, + content: string, +): Promise { + await expect + .poll( + async () => { + const { messages } = await apiClient.listSessionMessages(sessionId); + const message = messages.find( + (m) => m.author_type === "user" && m.content.includes(content), + ); + const attachments = message?.metadata?.attachments; + return Array.isArray(attachments) ? attachments.length : 0; + }, + { timeout: 15_000, message: `Wait for persisted attachment on "${content}"` }, + ) + .toBeGreaterThan(0); + + const { messages } = await apiClient.listSessionMessages(sessionId); + const message = messages.find((m) => m.author_type === "user" && m.content.includes(content)); + const attachments = message?.metadata?.attachments; + if (!Array.isArray(attachments)) throw new Error("Persisted user message has no attachments"); + return attachments[0] as PersistedAttachment; +} + +test.describe("chat attachment delivery mode", () => { + test("shows a Prompt/File selector for supported images and sends unsupported files by path", async ({ + testPage, + apiClient, + seedData, + }, testInfo) => { + test.setTimeout(90_000); + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Attachment delivery selector", + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id"); + await waitForLatestSessionDone(apiClient, task.id, 1, "Wait for initial attachment task"); + const session = await openTaskSession(testPage, task.id); + await session.waitForChatIdle({ timeout: 30_000 }); + fs.mkdirSync(testInfo.outputDir, { recursive: true }); + + const imagePath = path.join(testInfo.outputDir, "tiny.png"); + fs.writeFileSync( + imagePath, + Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMB/6X3pZQAAAAASUVORK5CYII=", + "base64", + ), + ); + + await testPage.locator('input[type="file"]').setInputFiles(imagePath); + const imageChip = testPage.getByText(/Image \(/).first(); + await expect(imageChip).toBeVisible({ timeout: 10_000 }); + await imageChip.hover(); + + const promptButton = testPage.getByTestId("attachment-delivery-prompt"); + const pathButton = testPage.getByTestId("attachment-delivery-path"); + await expect(promptButton).toBeVisible({ timeout: 10_000 }); + await expect(pathButton).toBeVisible(); + await expect(promptButton).toHaveAttribute("data-selected", "true"); + + await pathButton.click(); + await expect(pathButton).toHaveAttribute("data-selected", "true"); + await session.sendMessageViaButton("send image as file path"); + const imageAttachment = await waitForUserAttachment( + apiClient, + task.session_id, + "send image as file path", + ); + expect(imageAttachment).toMatchObject({ + type: "image", + name: "tiny.png", + mime_type: "image/png", + delivery_mode: "path", + }); + + await session.waitForChatIdle({ timeout: 30_000 }); + const textPath = path.join(testInfo.outputDir, "notes.txt"); + fs.writeFileSync(textPath, "plain text attachment"); + + await testPage.locator('input[type="file"]').setInputFiles(textPath); + await expect(testPage.getByText("notes.txt", { exact: true })).toBeVisible({ + timeout: 10_000, + }); + await testPage.getByText("notes.txt", { exact: true }).hover(); + await expect(testPage.getByTestId("attachment-delivery-prompt")).toHaveCount(0); + await expect(testPage.getByTestId("attachment-delivery-path")).toHaveCount(0); + + await session.sendMessageViaButton("send unsupported file"); + const textAttachment = await waitForUserAttachment( + apiClient, + task.session_id, + "send unsupported file", + ); + expect(textAttachment).toMatchObject({ + type: "resource", + name: "notes.txt", + mime_type: "text/plain", + delivery_mode: "path", + }); + }); +}); diff --git a/apps/web/e2e/tests/ssh/launch-task.spec.ts b/apps/web/e2e/tests/ssh/launch-task.spec.ts index 80dd9d0633..cafee06daf 100644 --- a/apps/web/e2e/tests/ssh/launch-task.spec.ts +++ b/apps/web/e2e/tests/ssh/launch-task.spec.ts @@ -1,5 +1,10 @@ import { test, expect } from "../../fixtures/ssh-test-base"; -import { listRemoteDir, readRemoteFile, remotePathExists } from "../../helpers/ssh"; +import { + execInContainer, + listRemoteDir, + readRemoteFile, + remotePathExists, +} from "../../helpers/ssh"; import { waitForLatestSessionDone } from "../../helpers/session"; /** @@ -165,6 +170,121 @@ test.describe("ssh executor — task launch", () => { expect(row!.remote_agentctl_port).toBeGreaterThan(0); }); + test("uploads path-mode attachments into the remote executor workspace", async ({ + apiClient, + seedData, + }) => { + test.setTimeout(180_000); + const content = "remote attachment bytes"; + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "H6 attachment upload", + seedData.agentProfileId, + { + description: "Inspect the attached file.", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + executor_profile_id: seedData.sshExecutorProfileId, + attachments: [ + { + type: "resource", + data: Buffer.from(content, "utf8").toString("base64"), + mime_type: "text/plain", + name: "remote-note.txt", + delivery_mode: "path", + }, + ], + }, + ); + await waitForLatestSessionDone(apiClient, task.id, 1, "Wait for attachment upload"); + + await expect + .poll( + async () => { + const sessions = await apiClient.listSSHSessions(seedData.sshExecutorId); + return sessions.find((s) => s.task_id === task.id)?.remote_task_dir ?? ""; + }, + { timeout: 60_000, message: "Wait for SSH session row" }, + ) + .not.toBe(""); + + const sessions = await apiClient.listSSHSessions(seedData.sshExecutorId); + const row = sessions.find((s) => s.task_id === task.id); + expect(row?.remote_task_dir).toBeTruthy(); + const savedPath = execInContainer(seedData.sshTarget, [ + "sh", + "-c", + `find ${JSON.stringify(row!.remote_task_dir)}/.kandev/attachments -name remote-note.txt -type f -print -quit`, + ]).trim(); + expect(savedPath).toContain("/.kandev/attachments/"); + expect(readRemoteFile(seedData.sshTarget, savedPath)).toBe(content); + }); + + test("uploads path-mode attachments queued while the remote task is running", async ({ + apiClient, + seedData, + }) => { + test.setTimeout(180_000); + const content = "remote follow-up attachment bytes"; + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "H6 running attachment upload", + seedData.agentProfileId, + { + description: "/slow 20s", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + executor_profile_id: seedData.sshExecutorProfileId, + }, + ); + if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id"); + + await apiClient.queueMessage(task.id, task.session_id, "Inspect the follow-up file.", [ + { + type: "resource", + data: Buffer.from(content, "utf8").toString("base64"), + mime_type: "text/plain", + name: "remote-followup.txt", + delivery_mode: "path", + }, + ]); + + await expect + .poll( + async () => { + const sessions = await apiClient.listSSHSessions(seedData.sshExecutorId); + return sessions.find((s) => s.task_id === task.id)?.remote_task_dir ?? ""; + }, + { timeout: 60_000, message: "Wait for SSH session row" }, + ) + .not.toBe(""); + + const sessions = await apiClient.listSSHSessions(seedData.sshExecutorId); + const row = sessions.find((s) => s.task_id === task.id); + const taskDir = row?.remote_task_dir; + expect(taskDir).toBeTruthy(); + + const findAttachment = () => + execInContainer(seedData.sshTarget, [ + "sh", + "-c", + `find ${JSON.stringify(taskDir)}/.kandev/attachments -name remote-followup.txt -type f -print -quit 2>/dev/null || true`, + ]).trim(); + + await expect + .poll(findAttachment, { + timeout: 120_000, + message: "Wait for queued attachment to upload remotely", + }) + .not.toBe(""); + + const attachmentPath = findAttachment(); + expect(attachmentPath).toContain("/.kandev/attachments/"); + expect(readRemoteFile(seedData.sshTarget, attachmentPath)).toBe(content); + }); + test("stopping the session cleans up the session runtime dir but leaves the task dir", async ({ apiClient, seedData, diff --git a/apps/web/hooks/domains/session/use-queue.ts b/apps/web/hooks/domains/session/use-queue.ts index c4b841b15b..3336d485bf 100644 --- a/apps/web/hooks/domains/session/use-queue.ts +++ b/apps/web/hooks/domains/session/use-queue.ts @@ -17,6 +17,8 @@ export type MessageAttachment = { type: string; data: string; mime_type: string; + name?: string; + delivery_mode?: "prompt" | "path"; }; /** Selectors over the queue slice for one session. */ diff --git a/apps/web/hooks/use-message-handler.ts b/apps/web/hooks/use-message-handler.ts index 25bcf4e0e5..2983fec930 100644 --- a/apps/web/hooks/use-message-handler.ts +++ b/apps/web/hooks/use-message-handler.ts @@ -227,6 +227,7 @@ export function useMessageHandler({ data: att.data, mime_type: att.mime_type, name: att.name, + delivery_mode: att.delivery_mode, })); await queue(taskId, finalMessage, modelToSend, planModeEnabled, queueAttachments); return; diff --git a/apps/web/lib/api/domains/kanban-api.ts b/apps/web/lib/api/domains/kanban-api.ts index c378adde9b..2d867c723d 100644 --- a/apps/web/lib/api/domains/kanban-api.ts +++ b/apps/web/lib/api/domains/kanban-api.ts @@ -71,7 +71,13 @@ export async function createTask( executor_id?: string; executor_profile_id?: string; plan_mode?: boolean; - attachments?: Array<{ type: string; data: string; mime_type: string; name?: string }>; + attachments?: Array<{ + type: string; + data: string; + mime_type: string; + name?: string; + delivery_mode?: "prompt" | "path"; + }>; parent_id?: string; workspace_path?: string; priority?: string; diff --git a/apps/web/lib/api/domains/queue-api.ts b/apps/web/lib/api/domains/queue-api.ts index 2dfd9fbc5a..a3ace66edb 100644 --- a/apps/web/lib/api/domains/queue-api.ts +++ b/apps/web/lib/api/domains/queue-api.ts @@ -68,7 +68,13 @@ export type QueueMessageParams = { content: string; model?: string; plan_mode?: boolean; - attachments?: Array<{ type: string; data: string; mime_type: string }>; + attachments?: Array<{ + type: string; + data: string; + mime_type: string; + name?: string; + delivery_mode?: "prompt" | "path"; + }>; user_id?: string; }; @@ -140,7 +146,13 @@ export async function updateQueuedMessage(params: { session_id: string; entry_id: string; content: string; - attachments?: Array<{ type: string; data: string; mime_type: string }>; + attachments?: Array<{ + type: string; + data: string; + mime_type: string; + name?: string; + delivery_mode?: "prompt" | "path"; + }>; user_id?: string; }): Promise<{ entry_id: string }> { const client = getWebSocketClient(); diff --git a/apps/web/lib/local-storage.test.ts b/apps/web/lib/local-storage.test.ts index ade72f0ed0..07c3fde030 100644 --- a/apps/web/lib/local-storage.test.ts +++ b/apps/web/lib/local-storage.test.ts @@ -5,6 +5,7 @@ import { getGlobalSidebarWidth, markPRClosedBannerDismissed, markPRMergedBannerDismissed, + restoreAttachmentPreview, setGlobalSidebarWidth, wasPRClosedBannerDismissed, wasPRMergedBannerDismissed, @@ -103,3 +104,34 @@ describe("global sidebar width storage", () => { expect(getGlobalSidebarWidth()).toBe(320); }); }); + +describe("chat draft attachment storage", () => { + it("normalizes invalid restored image delivery modes to prompt", () => { + const restored = restoreAttachmentPreview({ + id: "att-1", + data: "abc", + mimeType: "image/png", + fileName: "shot.png", + size: 3, + isImage: true, + deliveryMode: "inline" as "prompt", + }); + + expect(restored.deliveryMode).toBe("prompt"); + expect(restored.preview).toBe("data:image/png;base64,abc"); + }); + + it("normalizes invalid restored file delivery modes to path", () => { + const restored = restoreAttachmentPreview({ + id: "att-2", + data: "abc", + mimeType: "application/pdf", + fileName: "doc.pdf", + size: 3, + isImage: false, + deliveryMode: "inline" as "path", + }); + + expect(restored.deliveryMode).toBe("path"); + }); +}); diff --git a/apps/web/lib/local-storage.ts b/apps/web/lib/local-storage.ts index 69c73e2d9b..abb17c2c58 100644 --- a/apps/web/lib/local-storage.ts +++ b/apps/web/lib/local-storage.ts @@ -542,6 +542,7 @@ type StoredFileAttachment = { fileName: string; size: number; isImage: boolean; + deliveryMode?: "prompt" | "path"; }; export function getChatDraftText(sessionId: string): string { @@ -576,6 +577,13 @@ export function getChatDraftAttachments(sessionId: string): StoredFileAttachment ); } +function normalizeAttachmentDeliveryMode( + value: unknown, + fallback: "prompt" | "path", +): "prompt" | "path" { + return value === "prompt" || value === "path" ? value : fallback; +} + export function setChatDraftAttachments( sessionId: string, attachments: Array<{ @@ -585,6 +593,7 @@ export function setChatDraftAttachments( fileName: string; size: number; isImage: boolean; + deliveryMode?: "prompt" | "path"; preview?: string; }>, ): void { @@ -593,13 +602,14 @@ export function setChatDraftAttachments( } else { // Strip `preview` to halve storage cost — reconstructed on load for images const stored: StoredFileAttachment[] = attachments.map( - ({ id, data, mimeType, fileName, size, isImage }) => ({ + ({ id, data, mimeType, fileName, size, isImage, deliveryMode }) => ({ id, data, mimeType, fileName, size, isImage, + deliveryMode, }), ); setSessionStorage(`${CHAT_DRAFT_ATTACHMENTS_KEY}.${sessionId}`, stored); @@ -611,11 +621,15 @@ export function setChatDraftAttachments( */ export function restoreAttachmentPreview( att: StoredFileAttachment, -): StoredFileAttachment & { preview?: string } { +): StoredFileAttachment & { deliveryMode: "prompt" | "path"; preview?: string } { if (att.isImage) { - return { ...att, preview: `data:${att.mimeType};base64,${att.data}` }; + return { + ...att, + deliveryMode: normalizeAttachmentDeliveryMode(att.deliveryMode, "prompt"), + preview: `data:${att.mimeType};base64,${att.data}`, + }; } - return att; + return { ...att, deliveryMode: normalizeAttachmentDeliveryMode(att.deliveryMode, "path") }; } export function getChatInputHeight(sessionId: string): number | null { diff --git a/apps/web/lib/services/session-launch-service.ts b/apps/web/lib/services/session-launch-service.ts index cd13546718..b7e276cd0e 100644 --- a/apps/web/lib/services/session-launch-service.ts +++ b/apps/web/lib/services/session-launch-service.ts @@ -13,6 +13,7 @@ export type MessageAttachment = { data: string; mime_type: string; name?: string; + delivery_mode?: "prompt" | "path"; }; export type LaunchSessionRequest = { diff --git a/apps/web/lib/state/slices/session/types.ts b/apps/web/lib/state/slices/session/types.ts index 8576aa7b44..81b2db2a87 100644 --- a/apps/web/lib/state/slices/session/types.ts +++ b/apps/web/lib/state/slices/session/types.ts @@ -102,7 +102,13 @@ export type QueuedMessage = { content: string; model?: string; plan_mode: boolean; - attachments?: Array<{ type: string; data: string; mime_type: string }>; + attachments?: Array<{ + type: string; + data: string; + mime_type: string; + name?: string; + delivery_mode?: "prompt" | "path"; + }>; metadata?: QueuedMessageMetadata; queued_at: string; queued_by?: string; diff --git a/apps/web/lib/types/context.ts b/apps/web/lib/types/context.ts index c6391dcb39..75446a3942 100644 --- a/apps/web/lib/types/context.ts +++ b/apps/web/lib/types/context.ts @@ -44,6 +44,7 @@ export type PlanCommentContextItem = ContextItemBase & { export type ImageContextItem = ContextItemBase & { kind: "image"; attachment: FileAttachment; + onDeliveryModeChange?: (mode: "prompt" | "path") => void; }; export type FileAttachmentContextItem = ContextItemBase & { diff --git a/docs/screenshots.md b/docs/screenshots.md index be0e7cba31..56627a875e 100644 --- a/docs/screenshots.md +++ b/docs/screenshots.md @@ -1,5 +1,17 @@ # Screenshots +## Mobile + + + + + + + +
Mobile Chat
Mobile Chat		Mobile Changes
Mobile Changes		Mobile Diff Review
Mobile Diff Review
+ +## Desktop + diff --git a/docs/screenshots/mobile-changes.webp b/docs/screenshots/mobile-changes.webp new file mode 100644 index 0000000000..2ea5285bf9 Binary files /dev/null and b/docs/screenshots/mobile-changes.webp differ diff --git a/docs/screenshots/mobile-chat.webp b/docs/screenshots/mobile-chat.webp new file mode 100644 index 0000000000..92fa010790 Binary files /dev/null and b/docs/screenshots/mobile-chat.webp differ diff --git a/docs/screenshots/mobile-diff.webp b/docs/screenshots/mobile-diff.webp new file mode 100644 index 0000000000..0e033f3910 Binary files /dev/null and b/docs/screenshots/mobile-diff.webp differ
Kanban
Kanban