diff --git a/apps/backend/internal/agent/runtime/agentctl/client_attachments.go b/apps/backend/internal/agent/runtime/agentctl/client_attachments.go index b4c431dc79..d22e7cef92 100644 --- a/apps/backend/internal/agent/runtime/agentctl/client_attachments.go +++ b/apps/backend/internal/agent/runtime/agentctl/client_attachments.go @@ -35,25 +35,10 @@ func (c *Client) MaterializeAttachment( 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) + resp, err := c.doMultipartRequest(ctx, c.baseURL+"/api/v1/attachments/materialize", func(writer *multipart.Writer) error { + return writeMaterializedAttachmentMultipart(writer, sessionID, attachmentID, name, mimeType, sizeBytes, content) + }) 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() }() diff --git a/apps/backend/internal/agent/runtime/agentctl/client_multipart.go b/apps/backend/internal/agent/runtime/agentctl/client_multipart.go new file mode 100644 index 0000000000..33ad615782 --- /dev/null +++ b/apps/backend/internal/agent/runtime/agentctl/client_multipart.go @@ -0,0 +1,40 @@ +package client + +import ( + "context" + "fmt" + "io" + "mime/multipart" + "net/http" +) + +// doMultipartRequest owns the pipe and writer lifecycle for streaming agentctl +// requests. Callers only provide the endpoint and the fields/file writer. +func (c *Client) doMultipartRequest( + ctx context.Context, + url string, + write func(*multipart.Writer) error, +) (*http.Response, error) { + pipeReader, pipeWriter := io.Pipe() + multipartWriter := multipart.NewWriter(pipeWriter) + go func() { + if err := write(multipartWriter); err != nil { + _ = pipeWriter.CloseWithError(err) + return + } + _ = pipeWriter.Close() + }() + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, pipeReader) + if err != nil { + _ = pipeReader.Close() + return nil, fmt.Errorf("create multipart request: %w", err) + } + req.Header.Set("Content-Type", multipartWriter.FormDataContentType()) + resp, err := c.longRunningHTTPClient.Do(req) + if err != nil { + _ = pipeReader.Close() + return nil, err + } + return resp, nil +} diff --git a/apps/backend/internal/agent/runtime/agentctl/client_workspace_upload.go b/apps/backend/internal/agent/runtime/agentctl/client_workspace_upload.go new file mode 100644 index 0000000000..11a9a42737 --- /dev/null +++ b/apps/backend/internal/agent/runtime/agentctl/client_workspace_upload.go @@ -0,0 +1,158 @@ +package client + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "mime/multipart" + "net/http" + "strconv" + "strings" + + agentctltypes "github.com/kandev/kandev/internal/agentctl/types" + "github.com/kandev/kandev/internal/task/models" +) + +// The upload DTOs and the conflict sentinel live in internal/agentctl/types so +// higher-level callers can consume them without importing this runtime-tier +// package (ARCH-RUNTIME-IMPORT). +type ( + // UploadedWorkspaceFile is the path agentctl actually wrote. + UploadedWorkspaceFile = agentctltypes.UploadedWorkspaceFile + // WorkspaceUploadConflict is one destination that already exists. + WorkspaceUploadConflict = agentctltypes.WorkspaceUploadConflict +) + +// ErrWorkspaceUploadConflict reports that the destination already exists and the +// caller supplied no resolution. Callers surface this as a prompt rather than an +// error, so it is a sentinel instead of a status string. +var ErrWorkspaceUploadConflict = agentctltypes.ErrWorkspaceUploadConflict + +// WorkspaceUploadParams describes one file destined for the task workspace. +type WorkspaceUploadParams = agentctltypes.WorkspaceUploadParams + +// PreflightWorkspaceUpload reports which of the candidate destinations already +// exist, so the caller can resolve every conflict before sending any bytes. +func (c *Client) PreflightWorkspaceUpload( + ctx context.Context, + dir, repo string, + paths []string, +) ([]WorkspaceUploadConflict, error) { + payload, err := json.Marshal(map[string]any{"dir": dir, "repo": repo, "paths": paths}) + if err != nil { + return nil, fmt.Errorf("encode upload preflight: %w", err) + } + + req, err := http.NewRequestWithContext( + ctx, + http.MethodPost, + c.baseURL+"/api/v1/workspace/file/upload-preflight", + bytes.NewReader(payload), + ) + if err != nil { + return nil, fmt.Errorf("create upload preflight request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + + resp, err := c.longRunningHTTPClient.Do(req) + if err != nil { + return nil, fmt.Errorf("upload preflight: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if err := checkWorkspaceUploadStatus(resp); err != nil { + return nil, err + } + + var result struct { + Conflicts []WorkspaceUploadConflict `json:"conflicts"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("decode upload preflight: %w", err) + } + return result.Conflicts, nil +} + +// UploadWorkspaceFile streams one file into the task workspace. The body is +// multipart and piped, so no layer holds the whole file in memory and nothing +// is base64-encoded. +func (c *Client) UploadWorkspaceFile( + ctx context.Context, + upload WorkspaceUploadParams, + content io.Reader, +) (*UploadedWorkspaceFile, error) { + if strings.TrimSpace(upload.RelativePath) == "" || content == nil { + return nil, fmt.Errorf("upload path and content are required") + } + if upload.SizeBytes < 0 || upload.SizeBytes > models.MaxMessageAttachmentBytes { + return nil, fmt.Errorf("upload size exceeds maximum") + } + + resp, err := c.doMultipartRequest(ctx, c.baseURL+"/api/v1/workspace/file/upload", func(writer *multipart.Writer) error { + return writeWorkspaceUploadMultipart(writer, upload, content) + }) + if err != nil { + return nil, fmt.Errorf("upload workspace file: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + if err := checkWorkspaceUploadStatus(resp); err != nil { + return nil, err + } + + var result UploadedWorkspaceFile + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("decode workspace upload: %w", err) + } + if strings.TrimSpace(result.Path) == "" { + return nil, fmt.Errorf("workspace upload returned no path") + } + return &result, nil +} + +// checkWorkspaceUploadStatus converts a non-2xx response into an error, +// preserving the conflict case as a sentinel so callers can prompt instead of +// failing. +func checkWorkspaceUploadStatus(resp *http.Response) error { + if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices { + return nil + } + body, _ := io.ReadAll(io.LimitReader(resp.Body, 16*1024)) + message := strings.TrimSpace(string(body)) + if resp.StatusCode == http.StatusConflict { + return fmt.Errorf("%w: %s", ErrWorkspaceUploadConflict, message) + } + return fmt.Errorf("workspace upload failed with status %d: %s", resp.StatusCode, message) +} + +func writeWorkspaceUploadMultipart( + writer *multipart.Writer, + upload WorkspaceUploadParams, + content io.Reader, +) error { + fields := [...]struct { + key string + value string + }{ + {key: "dir", value: upload.Dir}, + {key: "repo", value: upload.Repo}, + {key: "relative_path", value: upload.RelativePath}, + {key: "resolution", value: upload.Resolution}, + {key: "size_bytes", value: strconv.FormatInt(upload.SizeBytes, 10)}, + } + for _, field := range fields { + if err := writer.WriteField(field.key, field.value); err != nil { + return fmt.Errorf("write upload field %s: %w", field.key, err) + } + } + part, err := writer.CreateFormFile("file", upload.RelativePath) + if err != nil { + return fmt.Errorf("create upload file part: %w", err) + } + if _, err := io.Copy(part, content); err != nil { + return fmt.Errorf("stream upload content: %w", err) + } + return writer.Close() +} diff --git a/apps/backend/internal/agent/runtime/agentctl/client_workspace_upload_test.go b/apps/backend/internal/agent/runtime/agentctl/client_workspace_upload_test.go new file mode 100644 index 0000000000..b4575b13e2 --- /dev/null +++ b/apps/backend/internal/agent/runtime/agentctl/client_workspace_upload_test.go @@ -0,0 +1,166 @@ +package client + +import ( + "context" + "errors" + "io" + "net/http" + "strings" + "testing" +) + +func TestUploadWorkspaceFile_StreamsMultipartVerbatim(t *testing.T) { + srv, got := captureServer(t, jsonResponder(http.StatusCreated, + `{"path":"fixtures/sample-1.json","size_bytes":7,"resolution_applied":"keep_both"}`)) + + content := "payload" + result, err := newHTTPOnlyClient(srv.URL).UploadWorkspaceFile(context.Background(), WorkspaceUploadParams{ + Dir: "fixtures", + Repo: "", + RelativePath: "sample.json", + Resolution: "keep_both", + SizeBytes: int64(len(content))}, strings.NewReader(content)) + if err != nil { + t.Fatalf("UploadWorkspaceFile: %v", err) + } + + if got.Method != http.MethodPost || got.Path != "/api/v1/workspace/file/upload" { + t.Errorf("request = %s %s, want POST /api/v1/workspace/file/upload", got.Method, got.Path) + } + + fields, fileName, fileBytes := parseMaterializeUpload(t, got) + if fields["dir"] != "fixtures" { + t.Errorf("dir = %q, want fixtures", fields["dir"]) + } + if fields["relative_path"] != "sample.json" { + t.Errorf("relative_path = %q, want sample.json", fields["relative_path"]) + } + if fields["resolution"] != "keep_both" { + t.Errorf("resolution = %q, want keep_both", fields["resolution"]) + } + if fields["size_bytes"] != "7" { + t.Errorf("size_bytes = %q, want 7", fields["size_bytes"]) + } + if fileName != "sample.json" { + t.Errorf("file part name = %q, want sample.json", fileName) + } + if string(fileBytes) != content { + t.Errorf("file bytes = %q, want %q streamed verbatim", fileBytes, content) + } + + // The server-chosen path is authoritative after a keep-both rename. + if result.Path != "fixtures/sample-1.json" { + t.Errorf("Path = %q, want the server-chosen fixtures/sample-1.json", result.Path) + } + if result.ResolutionApplied != "keep_both" { + t.Errorf("ResolutionApplied = %q, want keep_both", result.ResolutionApplied) + } +} + +func TestUploadWorkspaceFile_ConflictIsASentinel(t *testing.T) { + srv, _ := captureServer(t, jsonResponder(http.StatusConflict, + `{"error":"upload destination already exists: taken.txt"}`)) + + _, err := newHTTPOnlyClient(srv.URL).UploadWorkspaceFile(context.Background(), WorkspaceUploadParams{ + RelativePath: "taken.txt", + SizeBytes: 1}, strings.NewReader("x")) + + if !errors.Is(err, ErrWorkspaceUploadConflict) { + t.Fatalf("err = %v, want ErrWorkspaceUploadConflict", err) + } + if !strings.Contains(err.Error(), "taken.txt") { + t.Errorf("err = %v, want the server message preserved", err) + } +} + +func TestUploadWorkspaceFile_SurfacesServerError(t *testing.T) { + srv, _ := captureServer(t, jsonResponder(http.StatusBadRequest, + `{"error":"path traversal detected in relative_path"}`)) + + _, err := newHTTPOnlyClient(srv.URL).UploadWorkspaceFile(context.Background(), WorkspaceUploadParams{ + RelativePath: "../escaped.txt", + SizeBytes: 1}, strings.NewReader("x")) + + if err == nil { + t.Fatal("expected an error") + } + if errors.Is(err, ErrWorkspaceUploadConflict) { + t.Error("a 400 must not be reported as a conflict") + } + if !strings.Contains(err.Error(), "path traversal") { + t.Errorf("err = %v, want the server message surfaced", err) + } +} + +func TestUploadWorkspaceFile_RejectsInvalidArgumentsBeforeAnyRequest(t *testing.T) { + tests := []struct { + name string + upload WorkspaceUploadParams + content io.Reader + wantErr string + }{ + { + name: "blank path", + upload: WorkspaceUploadParams{RelativePath: " ", SizeBytes: 1}, content: strings.NewReader("x"), + wantErr: "path and content are required", + }, + { + name: "nil content", + upload: WorkspaceUploadParams{RelativePath: "a.txt", SizeBytes: 1}, + wantErr: "path and content are required", + }, + { + name: "negative size", + upload: WorkspaceUploadParams{RelativePath: "a.txt", SizeBytes: -1}, content: strings.NewReader("x"), + wantErr: "size exceeds maximum", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + srv, got := captureServer(t, jsonResponder(http.StatusCreated, `{}`)) + _, err := newHTTPOnlyClient(srv.URL).UploadWorkspaceFile(context.Background(), tc.upload, tc.content) + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("err = %v, want it to contain %q", err, tc.wantErr) + } + if got.Method != "" { + t.Error("an invalid argument must not reach the network") + } + }) + } +} + +func TestPreflightWorkspaceUpload(t *testing.T) { + srv, got := captureServer(t, jsonResponder(http.StatusOK, + `{"conflicts":[{"path":"fixtures/there.txt","is_dir":false}]}`)) + + conflicts, err := newHTTPOnlyClient(srv.URL).PreflightWorkspaceUpload( + context.Background(), "fixtures", "", []string{"there.txt", "missing.txt"}, + ) + if err != nil { + t.Fatalf("PreflightWorkspaceUpload: %v", err) + } + + if got.Method != http.MethodPost || got.Path != "/api/v1/workspace/file/upload-preflight" { + t.Errorf("request = %s %s, want POST /api/v1/workspace/file/upload-preflight", got.Method, got.Path) + } + body := string(got.Body) + for _, want := range []string{`"dir":"fixtures"`, `"there.txt"`, `"missing.txt"`} { + if !strings.Contains(body, want) { + t.Errorf("body %s missing %s", body, want) + } + } + if len(conflicts) != 1 || conflicts[0].Path != "fixtures/there.txt" { + t.Fatalf("conflicts = %+v, want the single reported conflict", conflicts) + } +} + +func TestPreflightWorkspaceUpload_SurfacesServerError(t *testing.T) { + srv, _ := captureServer(t, jsonResponder(http.StatusBadRequest, `{"error":"path outside workspace"}`)) + + _, err := newHTTPOnlyClient(srv.URL).PreflightWorkspaceUpload( + context.Background(), "", "", []string{"../escaped.txt"}, + ) + if err == nil || !strings.Contains(err.Error(), "path outside workspace") { + t.Fatalf("err = %v, want the server message surfaced", err) + } +} diff --git a/apps/backend/internal/agentctl/server/api/server.go b/apps/backend/internal/agentctl/server/api/server.go index d39cd35abc..39a037568b 100644 --- a/apps/backend/internal/agentctl/server/api/server.go +++ b/apps/backend/internal/agentctl/server/api/server.go @@ -140,6 +140,8 @@ func (s *Server) setupRoutes() { api.GET("/workspace/file/content-at-ref", s.handleFileContentAtRef) api.POST("/workspace/file/content", s.handleFileUpdate) api.POST("/workspace/file/create", s.handleFileCreate) + api.POST("/workspace/file/upload", s.handleFileUpload) + api.POST("/workspace/file/upload-preflight", s.handleUploadPreflight) api.POST("/workspace/file/rename", s.handleFileRename) api.DELETE("/workspace/file", s.handleFileDelete) api.GET("/workspace/search", s.handleFileSearch) diff --git a/apps/backend/internal/agentctl/server/api/workspace_upload.go b/apps/backend/internal/agentctl/server/api/workspace_upload.go new file mode 100644 index 0000000000..b2b629e2ea --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/workspace_upload.go @@ -0,0 +1,471 @@ +package api + +import ( + "errors" + "fmt" + "io" + "mime/multipart" + "net/http" + "os" + "path" + "strconv" + "strings" + + "github.com/gin-gonic/gin" + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/agentctl/server/process" + taskmodels "github.com/kandev/kandev/internal/task/models" +) + +// maxWorkspaceUploadBytes caps a single uploaded workspace file. It reuses the +// message-attachment limit so an upload and a chat attachment behave alike. +const maxWorkspaceUploadBytes int64 = taskmodels.MaxMessageAttachmentBytes + +// workspaceUploadRequestSlack allows for multipart framing on top of the file +// itself, matching the allowance the attachment endpoint already makes. +const workspaceUploadRequestSlack int64 = 4 * 1024 * 1024 + +const ( + workspaceUploadPreflightBodyLimit = 1 << 20 + workspaceUploadMaxPaths = 4096 + workspaceUploadMaxPathBytes = 4096 +) + +var errWorkspaceUploadTooLarge = errors.New("workspace upload exceeds maximum size") + +type uploadPreflightRequest struct { + Dir string `json:"dir"` + Repo string `json:"repo"` + Paths []string `json:"paths"` +} + +type uploadPreflightResponse struct { + Conflicts []process.UploadConflict `json:"conflicts"` +} + +type workspaceUploadResponse struct { + Path string `json:"path"` + SizeBytes int64 `json:"size_bytes"` + ResolutionApplied string `json:"resolution_applied,omitempty"` +} + +// handleUploadPreflight reports which of the requested destinations already +// exist, so the caller can resolve every conflict before sending any bytes. +func (s *Server) handleUploadPreflight(c *gin.Context) { + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, workspaceUploadPreflightBodyLimit) + var req uploadPreflightRequest + if err := c.ShouldBindJSON(&req); err != nil { + status := http.StatusBadRequest + var maxErr *http.MaxBytesError + if errors.As(err, &maxErr) { + status = http.StatusRequestEntityTooLarge + } + c.JSON(status, gin.H{"error": "invalid request: " + err.Error()}) + return + } + if len(req.Paths) > workspaceUploadMaxPaths { + c.JSON(http.StatusBadRequest, gin.H{"error": "too many upload paths"}) + return + } + if len(req.Dir) > workspaceUploadMaxPathBytes || len(req.Repo) > workspaceUploadMaxPathBytes { + c.JSON(http.StatusBadRequest, gin.H{"error": "upload directory is too long"}) + return + } + for _, uploadPath := range req.Paths { + if len(uploadPath) > workspaceUploadMaxPathBytes { + c.JSON(http.StatusBadRequest, gin.H{"error": "upload path is too long"}) + return + } + } + if len(req.Paths) == 0 { + c.JSON(http.StatusOK, uploadPreflightResponse{Conflicts: []process.UploadConflict{}}) + return + } + + scoped := make([]string, 0, len(req.Paths)) + for _, rel := range req.Paths { + joined, err := s.scopedUploadPath(req.Repo, req.Dir, rel) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + scoped = append(scoped, joined) + } + + conflicts, err := s.procMgr.GetWorkspaceTracker().CheckUploadConflicts(scoped) + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + c.JSON(http.StatusOK, uploadPreflightResponse{Conflicts: conflicts}) +} + +// handleFileUpload streams one multipart file part into the workspace. +func (s *Server) handleFileUpload(c *gin.Context) { + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, maxWorkspaceUploadBytes+workspaceUploadRequestSlack) + multipartReader, err := c.Request.MultipartReader() + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "multipart upload is required"}) + return + } + + parsed, parseErr := parseWorkspaceUploadMultipart(multipartReader) + if parsed != nil { + defer parsed.cleanup() + } + if parseErr != nil { + c.JSON(parseErr.status, gin.H{"error": parseErr.message}) + return + } + if parsed.staged != nil { + if _, err := parsed.staged.file.Seek(0, io.SeekStart); err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "workspace upload failed"}) + return + } + parsed.source = parsed.staged.file + } + if parsed.source == nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "file part is required"}) + return + } + response, err := s.writeWorkspaceUploadPart(c, parsed.fields, parsed.source) + if err != nil { + return + } + c.JSON(http.StatusCreated, response) +} + +type workspaceUploadParseError struct { + status int + message string +} + +func (e workspaceUploadParseError) Error() string { return e.message } + +type parsedWorkspaceUpload struct { + fields workspaceUploadFields + source io.Reader + sourceCloser io.Closer + staged *stagedWorkspaceUpload +} + +func (p *parsedWorkspaceUpload) cleanup() { + if p == nil { + return + } + if p.sourceCloser != nil { + _ = p.sourceCloser.Close() + } + if p.staged != nil { + p.staged.cleanup() + } +} + +func parseWorkspaceUploadMultipart(reader *multipart.Reader) (*parsedWorkspaceUpload, *workspaceUploadParseError) { + parsed := &parsedWorkspaceUpload{} + for { + part, nextErr := reader.NextPart() + if errors.Is(nextErr, io.EOF) { + return parsed, nil + } + if nextErr != nil { + if isMultipartSizeError(nextErr) { + return parsed, &workspaceUploadParseError{status: http.StatusRequestEntityTooLarge, message: "upload request is too large"} + } + return parsed, &workspaceUploadParseError{status: http.StatusBadRequest, message: "invalid multipart upload"} + } + if part.FileName() == "" { + if parseErr := parseWorkspaceUploadField(part, &parsed.fields); parseErr != nil { + return parsed, parseErr + } + continue + } + if part.FormName() != "file" || parsed.source != nil || parsed.staged != nil { + _ = part.Close() + return parsed, &workspaceUploadParseError{status: http.StatusBadRequest, message: "exactly one file part is required"} + } + if !parsed.fields.allFieldsSeen() { + staged, stageErr := stageWorkspaceUploadPart(part) + _ = part.Close() + if stageErr != nil { + if errors.Is(stageErr, errWorkspaceUploadTooLarge) || isMultipartSizeError(stageErr) { + return parsed, &workspaceUploadParseError{status: http.StatusRequestEntityTooLarge, message: "file exceeds the maximum upload size"} + } + return parsed, &workspaceUploadParseError{status: http.StatusInternalServerError, message: "workspace upload failed"} + } + parsed.staged = staged + continue + } + parsed.source = part + parsed.sourceCloser = part + return parsed, nil + } +} + +func parseWorkspaceUploadField(part *multipart.Part, fields *workspaceUploadFields) *workspaceUploadParseError { + value, readErr := readWorkspaceUploadField(part) + _ = part.Close() + if readErr != nil { + return &workspaceUploadParseError{status: http.StatusBadRequest, message: "invalid upload metadata"} + } + fields.set(part.FormName(), value) + return nil +} + +func isMultipartSizeError(err error) bool { + var maxErr *http.MaxBytesError + return errors.As(err, &maxErr) +} + +type workspaceUploadFields struct { + dir string + repo string + relativePath string + resolution string + declaredSize int64 + hasSize bool + seenDir bool + seenRepo bool + seenPath bool + seenResolution bool + seenSize bool +} + +func (f *workspaceUploadFields) set(name, value string) { + switch name { + case "dir": + f.dir = value + f.seenDir = true + case "repo": + f.repo = value + f.seenRepo = true + case "relative_path": + f.relativePath = value + f.seenPath = true + case "resolution": + f.resolution = value + f.seenResolution = true + case "size_bytes": + f.seenSize = true + if parsed, err := strconv.ParseInt(strings.TrimSpace(value), 10, 64); err == nil { + f.declaredSize = parsed + f.hasSize = true + } + } +} + +func (f workspaceUploadFields) allFieldsSeen() bool { + return f.seenDir && f.seenRepo && f.seenPath && f.seenResolution && f.seenSize +} + +func (f workspaceUploadFields) validate() (process.UploadResolution, error) { + if strings.TrimSpace(f.relativePath) == "" { + return process.UploadResolutionNone, errors.New("relative_path is required") + } + if len(f.relativePath) > workspaceUploadMaxPathBytes || len(f.dir) > workspaceUploadMaxPathBytes || len(f.repo) > workspaceUploadMaxPathBytes { + return process.UploadResolutionNone, errors.New("upload path is too long") + } + if !f.hasSize || f.declaredSize < 0 { + return process.UploadResolutionNone, errors.New("size_bytes is required") + } + if f.declaredSize > maxWorkspaceUploadBytes { + return process.UploadResolutionNone, errors.New("file exceeds the maximum upload size") + } + resolution, err := process.ParseUploadResolution(f.resolution) + if err != nil { + return process.UploadResolutionNone, err + } + return resolution, nil +} + +const workspaceUploadFieldLimit int64 = workspaceUploadMaxPathBytes + 128 + +func readWorkspaceUploadField(part *multipart.Part) (string, error) { + data, err := io.ReadAll(io.LimitReader(part, workspaceUploadFieldLimit+1)) + if err != nil { + return "", err + } + if int64(len(data)) > workspaceUploadFieldLimit { + return "", errors.New("upload metadata is too long") + } + return string(data), nil +} + +type boundedWorkspaceUploadReader struct { + part io.Reader + expected int64 + read int64 +} + +type stagedWorkspaceUpload struct { + file *os.File + path string +} + +func stageWorkspaceUploadPart(part io.Reader) (*stagedWorkspaceUpload, error) { + tmp, err := os.CreateTemp("", "kandev-workspace-upload-") + if err != nil { + return nil, fmt.Errorf("create upload staging file: %w", err) + } + cleanup := func() { + _ = tmp.Close() + _ = os.Remove(tmp.Name()) + } + written, err := io.Copy(tmp, io.LimitReader(part, maxWorkspaceUploadBytes+1)) + if err != nil { + cleanup() + return nil, fmt.Errorf("stage upload: %w", err) + } + if written > maxWorkspaceUploadBytes { + cleanup() + return nil, errWorkspaceUploadTooLarge + } + if _, err := tmp.Seek(0, io.SeekStart); err != nil { + cleanup() + return nil, fmt.Errorf("rewind upload staging file: %w", err) + } + return &stagedWorkspaceUpload{file: tmp, path: tmp.Name()}, nil +} + +func (s *Server) writeWorkspaceUploadPart(c *gin.Context, fields workspaceUploadFields, src io.Reader) (*workspaceUploadResponse, error) { + resolution, validateErr := fields.validate() + if validateErr != nil { + if fields.declaredSize > maxWorkspaceUploadBytes { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "file exceeds the maximum upload size"}) + } else { + c.JSON(http.StatusBadRequest, gin.H{"error": validateErr.Error()}) + } + return nil, validateErr + } + scopedPath, scopeErr := s.scopedUploadPath(fields.repo, fields.dir, fields.relativePath) + if scopeErr != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": scopeErr.Error()}) + return nil, scopeErr + } + writtenPath, written, writeErr := s.procMgr.GetWorkspaceTracker().WriteFileStream( + scopedPath, + resolution, + &boundedWorkspaceUploadReader{part: src, expected: fields.declaredSize}, + ) + if writeErr != nil { + s.respondUploadError(c, scopedPath, writeErr) + return nil, writeErr + } + return &workspaceUploadResponse{ + Path: writtenPath, + SizeBytes: written, + ResolutionApplied: string(resolution), + }, nil +} + +func (s *stagedWorkspaceUpload) cleanup() { + if s == nil || s.file == nil { + return + } + _ = s.file.Close() + _ = os.Remove(s.path) +} + +func (r *boundedWorkspaceUploadReader) Read(p []byte) (int, error) { + if len(p) == 0 { + return 0, nil + } + remaining := r.expected - r.read + if remaining < int64(len(p)) { + p = p[:remaining+1] + } + n, err := r.part.Read(p) + r.read += int64(n) + if r.read > r.expected { + return n, errors.New("size_bytes does not match uploaded content") + } + if errors.Is(err, io.EOF) && r.read != r.expected { + return n, errors.New("size_bytes does not match uploaded content") + } + return n, err +} + +// respondUploadError maps a write failure onto a status that tells the caller +// whether to resolve a conflict, fix the request, or retry. +func (s *Server) respondUploadError(c *gin.Context, scopedPath string, err error) { + if errors.Is(err, process.ErrUploadConflict) { + c.JSON(http.StatusConflict, gin.H{"error": err.Error()}) + return + } + if isUploadRequestError(err) { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + s.logger.Error("workspace file upload failed", zap.String("path", scopedPath), zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{"error": "workspace upload failed"}) +} + +// isUploadRequestError distinguishes a caller mistake, such as an escaping +// path, from a genuine server-side failure. +func isUploadRequestError(err error) bool { + msg := err.Error() + for _, marker := range []string{ + "path traversal", + "path outside workspace", + "outside the workspace", + "invalid path", + "is a directory", + "size_bytes does not match", + } { + if strings.Contains(msg, marker) { + return true + } + } + return false +} + +// scopedUploadPath joins the destination directory and the file's path beneath +// it, then applies repository scoping. +// +// An escaping segment is rejected rather than normalized away. Silently +// rewriting "../x" to "x" would land the file somewhere the caller did not ask +// for, which is a worse failure than refusing the request. +func (s *Server) scopedUploadPath(repo, dir, relativePath string) (string, error) { + cleanRel, err := sanitizeUploadSegment(relativePath, "relative_path") + if err != nil { + return "", err + } + + joined := cleanRel + if strings.TrimSpace(dir) != "" { + cleanDir, err := sanitizeUploadSegment(dir, "dir") + if err != nil { + return "", err + } + joined = cleanDir + "/" + cleanRel + } + + scoped, err := s.procMgr.JoinRepoPath(repo, joined) + if err != nil { + return "", err + } + return scoped, nil +} + +// sanitizeUploadSegment validates one caller-supplied path fragment, rejecting +// absolute paths and any traversal component. +func sanitizeUploadSegment(raw, field string) (string, error) { + normalized := strings.ReplaceAll(strings.TrimSpace(raw), "\\", "/") + if normalized == "" { + return "", fmt.Errorf("invalid path: %s is empty", field) + } + if strings.HasPrefix(normalized, "/") { + return "", fmt.Errorf("invalid path: %s must be relative", field) + } + for _, segment := range strings.Split(normalized, "/") { + if segment == ".." { + return "", fmt.Errorf("path traversal detected in %s", field) + } + } + cleaned := strings.Trim(path.Clean(normalized), "/") + if cleaned == "" || cleaned == "." { + return "", fmt.Errorf("invalid path: %s", raw) + } + return cleaned, nil +} diff --git a/apps/backend/internal/agentctl/server/api/workspace_upload_test.go b/apps/backend/internal/agentctl/server/api/workspace_upload_test.go new file mode 100644 index 0000000000..25d67ebaa5 --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/workspace_upload_test.go @@ -0,0 +1,306 @@ +package api + +import ( + "bytes" + "encoding/json" + "mime/multipart" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/kandev/kandev/internal/agentctl/server/config" + "github.com/kandev/kandev/internal/agentctl/server/process" +) + +func newUploadTestServer(t *testing.T) (*Server, string) { + t.Helper() + workDir := t.TempDir() + log := newTestLogger() + cfg := &config.InstanceConfig{Port: 0, WorkDir: workDir, AuthToken: "test-token"} + return NewServer(cfg, process.NewManager(cfg, log), nil, nil, log), workDir +} + +type uploadForm struct { + fields map[string]string + name string + content []byte +} + +func (f uploadForm) build(t *testing.T) (*bytes.Buffer, string) { + t.Helper() + var body bytes.Buffer + writer := multipart.NewWriter(&body) + for k, v := range f.fields { + if err := writer.WriteField(k, v); err != nil { + t.Fatal(err) + } + } + part, err := writer.CreateFormFile("file", f.name) + if err != nil { + t.Fatal(err) + } + if _, err := part.Write(f.content); err != nil { + t.Fatal(err) + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + return &body, writer.FormDataContentType() +} + +func postUpload(t *testing.T, server *Server, f uploadForm) *httptest.ResponseRecorder { + t.Helper() + body, contentType := f.build(t) + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspace/file/upload", body) + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("Content-Type", contentType) + rec := httptest.NewRecorder() + server.Router().ServeHTTP(rec, req) + return rec +} + +func TestHandleFileUpload_WritesFile(t *testing.T) { + server, workDir := newUploadTestServer(t) + content := []byte("uploaded bytes") + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "dir": "fixtures", + "relative_path": "sample.txt", + "size_bytes": strconv.Itoa(len(content)), + }, + name: "sample.txt", + content: content, + }) + + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var resp struct { + Path string `json:"path"` + SizeBytes int64 `json:"size_bytes"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if resp.Path != "fixtures/sample.txt" { + t.Errorf("path = %q, want fixtures/sample.txt", resp.Path) + } + if resp.SizeBytes != int64(len(content)) { + t.Errorf("size = %d, want %d", resp.SizeBytes, len(content)) + } + got, err := os.ReadFile(filepath.Join(workDir, "fixtures", "sample.txt")) + if err != nil { + t.Fatalf("read back: %v", err) + } + if string(got) != string(content) { + t.Errorf("content = %q, want %q", got, content) + } +} + +func TestHandleFileUpload_NestedRelativePath(t *testing.T) { + server, workDir := newUploadTestServer(t) + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "dir": "", + "relative_path": "tree/deep/leaf.txt", + "size_bytes": "4", + }, + name: "leaf.txt", + content: []byte("leaf"), + }) + + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + if _, err := os.Stat(filepath.Join(workDir, "tree", "deep", "leaf.txt")); err != nil { + t.Fatalf("nested file missing: %v", err) + } +} + +func TestHandleFileUpload_ConflictWithoutResolution(t *testing.T) { + server, workDir := newUploadTestServer(t) + if err := os.WriteFile(filepath.Join(workDir, "taken.txt"), []byte("original"), 0o644); err != nil { + t.Fatal(err) + } + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "relative_path": "taken.txt", + "size_bytes": "8", + }, + name: "taken.txt", + content: []byte("incoming"), + }) + + if rec.Code != http.StatusConflict { + t.Fatalf("status = %d, want 409, body = %s", rec.Code, rec.Body.String()) + } + got, _ := os.ReadFile(filepath.Join(workDir, "taken.txt")) + if string(got) != "original" { + t.Errorf("existing file changed: %q", got) + } +} + +func TestHandleFileUpload_KeepBothReportsRenamedPath(t *testing.T) { + server, workDir := newUploadTestServer(t) + if err := os.WriteFile(filepath.Join(workDir, "taken.txt"), []byte("original"), 0o644); err != nil { + t.Fatal(err) + } + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "relative_path": "taken.txt", + "resolution": "keep_both", + "size_bytes": "8", + }, + name: "taken.txt", + content: []byte("incoming"), + }) + + if rec.Code != http.StatusCreated { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var resp struct { + Path string `json:"path"` + ResolutionApplied string `json:"resolution_applied"` + } + _ = json.Unmarshal(rec.Body.Bytes(), &resp) + if resp.Path != "taken-1.txt" { + t.Errorf("path = %q, want taken-1.txt", resp.Path) + } + if resp.ResolutionApplied != "keep_both" { + t.Errorf("resolution_applied = %q, want keep_both", resp.ResolutionApplied) + } +} + +func TestHandleFileUpload_ContainmentRejected(t *testing.T) { + server, workDir := newUploadTestServer(t) + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "relative_path": "../escaped.txt", + "size_bytes": "3", + }, + name: "escaped.txt", + content: []byte("bad"), + }) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400, body = %s", rec.Code, rec.Body.String()) + } + if _, err := os.Stat(filepath.Join(filepath.Dir(workDir), "escaped.txt")); err == nil { + t.Fatal("wrote outside the workspace") + } +} + +func TestHandleFileUpload_SizeMismatchRejected(t *testing.T) { + server, workDir := newUploadTestServer(t) + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "relative_path": "mismatch.txt", + "size_bytes": "999", + }, + name: "mismatch.txt", + content: []byte("short"), + }) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400, body = %s", rec.Code, rec.Body.String()) + } + if _, err := os.Stat(filepath.Join(workDir, "mismatch.txt")); err == nil { + t.Error("a file was left behind after a size mismatch") + } +} + +func TestHandleFileUpload_OversizeRejected(t *testing.T) { + server, _ := newUploadTestServer(t) + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "relative_path": "big.bin", + "size_bytes": strconv.FormatInt(maxWorkspaceUploadBytes+1, 10), + }, + name: "big.bin", + content: []byte("small body, oversized declaration"), + }) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("status = %d, want 413, body = %s", rec.Code, rec.Body.String()) + } +} + +func TestHandleFileUpload_UnknownResolutionRejected(t *testing.T) { + server, _ := newUploadTestServer(t) + + rec := postUpload(t, server, uploadForm{ + fields: map[string]string{ + "relative_path": "x.txt", + "resolution": "obliterate", + "size_bytes": "1", + }, + name: "x.txt", + content: []byte("x"), + }) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400, body = %s", rec.Code, rec.Body.String()) + } +} + +func TestHandleUploadPreflight(t *testing.T) { + server, workDir := newUploadTestServer(t) + if err := os.MkdirAll(filepath.Join(workDir, "fixtures"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workDir, "fixtures", "there.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + body := `{"dir":"fixtures","paths":["there.txt","missing.txt"]}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspace/file/upload-preflight", strings.NewReader(body)) + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + server.Router().ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, body = %s", rec.Code, rec.Body.String()) + } + var resp struct { + Conflicts []struct { + Path string `json:"path"` + IsDir bool `json:"is_dir"` + } `json:"conflicts"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + if len(resp.Conflicts) != 1 { + t.Fatalf("conflicts = %+v, want exactly one", resp.Conflicts) + } + if resp.Conflicts[0].Path != "fixtures/there.txt" { + t.Errorf("conflict path = %q, want fixtures/there.txt", resp.Conflicts[0].Path) + } +} + +func TestHandleUploadPreflight_ContainmentIsAnError(t *testing.T) { + server, _ := newUploadTestServer(t) + + body := `{"dir":"","paths":["../escaped.txt"]}` + req := httptest.NewRequest(http.MethodPost, "/api/v1/workspace/file/upload-preflight", strings.NewReader(body)) + req.Header.Set("Authorization", "Bearer test-token") + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + server.Router().ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400, body = %s", rec.Code, rec.Body.String()) + } +} diff --git a/apps/backend/internal/agentctl/server/process/workspace_tracker.go b/apps/backend/internal/agentctl/server/process/workspace_tracker.go index 45261e5c0b..1551dccf93 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_tracker.go +++ b/apps/backend/internal/agentctl/server/process/workspace_tracker.go @@ -150,6 +150,10 @@ type WorkspaceTracker struct { // updateMu prevents concurrent updateGitStatus calls from the two polling loops. // Polling loops use TryLock (skip if busy); RefreshGitStatus uses Lock (always completes). updateMu sync.Mutex + // mutationMu serializes final upload target selection and placement. Upload + // bytes are staged before this lock, so slow readers do not block other + // workspace mutations while the target invariant remains atomic. + mutationMu sync.Mutex // gitStatusObserver is the expensive live repository observation. Keeping it // as a dependency makes the concurrency contract deterministic to test while diff --git a/apps/backend/internal/agentctl/server/process/workspace_upload.go b/apps/backend/internal/agentctl/server/process/workspace_upload.go new file mode 100644 index 0000000000..8dfeccbab8 --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/workspace_upload.go @@ -0,0 +1,236 @@ +package process + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/kandev/kandev/internal/agentctl/types" +) + +// UploadResolution says what to do when an upload's destination already exists. +// The zero value refuses to touch an existing file, so a caller that forgets to +// resolve a conflict cannot silently overwrite one. +type UploadResolution string + +const ( + // UploadResolutionNone writes only when the destination is free. + UploadResolutionNone UploadResolution = "" + // UploadResolutionReplace overwrites an existing destination. + UploadResolutionReplace UploadResolution = "replace" + // UploadResolutionKeepBoth writes beside an existing destination under the + // next free "name-.ext" variant. The hyphen form is deliberate over the + // Windows "name (n)" convention: these land in a code workspace, where a + // space and parentheses need quoting in every shell command and show up in + // git paths. It also matches the attachment installer. + UploadResolutionKeepBoth UploadResolution = "keep_both" +) + +// ErrUploadConflict reports that the destination exists and the caller supplied +// no resolution. It is deliberately distinct from a validation error so the HTTP +// layer can map it to 409 rather than 400. +var ErrUploadConflict = errors.New("upload destination already exists") + +// maxKeepBothAttempts bounds the "name-.ext" search. It matches the bound the +// attachment installer already uses. +const maxKeepBothAttempts = 10000 + +// uploadTempPrefix marks in-flight upload temporaries so a crash leaves an +// identifiable artifact rather than something that looks like workspace content. +const uploadTempPrefix = ".kandev-upload-" + +// UploadConflict is one destination path that already exists. +type UploadConflict struct { + Path string `json:"path"` + IsDir bool `json:"is_dir"` +} + +// ParseUploadResolution converts the wire value into an UploadResolution, +// rejecting anything unrecognized rather than falling back to a default. +func ParseUploadResolution(raw string) (UploadResolution, error) { + switch UploadResolution(strings.TrimSpace(raw)) { + case UploadResolutionNone: + return UploadResolutionNone, nil + case UploadResolutionReplace: + return UploadResolutionReplace, nil + case UploadResolutionKeepBoth: + return UploadResolutionKeepBoth, nil + default: + return UploadResolutionNone, fmt.Errorf("unknown upload resolution: %q", raw) + } +} + +// CheckUploadConflicts reports which of the candidate destinations already +// exist. Every candidate is resolved through the same containment rules as the +// write, so a path that could never be written is an error here rather than a +// conflict the user would be asked to resolve. +func (wt *WorkspaceTracker) CheckUploadConflicts(reqPaths []string) ([]UploadConflict, error) { + conflicts := make([]UploadConflict, 0, len(reqPaths)) + for _, reqPath := range reqPaths { + path, err := wt.resolveMutationPath(reqPath) + if err != nil { + return nil, err + } + info, statErr := path.root.Stat(path.rel) + _ = path.root.Close() + if statErr != nil { + if os.IsNotExist(statErr) { + continue + } + return nil, fmt.Errorf("failed to stat %s: %w", reqPath, statErr) + } + conflicts = append(conflicts, UploadConflict{Path: reqPath, IsDir: info.IsDir()}) + } + return conflicts, nil +} + +// WriteFileStream streams src into the workspace at reqPath and returns the +// path actually written, which differs from reqPath when the caller asked to +// keep both copies. +// +// The write is staged through a temporary file in the destination directory and +// renamed into place, so an agent reading the workspace never observes a +// partially written file, and a failed upload leaves the destination untouched. +func (wt *WorkspaceTracker) WriteFileStream( + reqPath string, + resolution UploadResolution, + src io.Reader, +) (string, int64, error) { + path, err := wt.resolveMutationPath(reqPath) + if err != nil { + return "", 0, err + } + defer func() { _ = path.root.Close() }() + runWorkspaceMutationBarrier() + + dir := filepath.Dir(path.rel) + if err := path.root.MkdirAll(dir, 0o755); err != nil { + return "", 0, fmt.Errorf("failed to create directories: %w", err) + } + + tmpRel, tmp, err := createUploadTemp(path.root, dir) + if err != nil { + return "", 0, err + } + cleanup := func() { + _ = tmp.Close() + _ = path.root.Remove(tmpRel) + } + + written, err := io.Copy(tmp, src) + if err != nil { + cleanup() + return "", 0, fmt.Errorf("failed to write upload: %w", err) + } + if err := tmp.Sync(); err != nil { + cleanup() + return "", 0, fmt.Errorf("failed to flush upload: %w", err) + } + if err := tmp.Close(); err != nil { + _ = path.root.Remove(tmpRel) + return "", 0, fmt.Errorf("failed to close upload: %w", err) + } + + // Re-check and place the staged file under a real tracker lock. This lock + // covers only metadata and rename, not the unbounded source read above. + wt.mutationMu.Lock() + targetRel, targetReq, err := resolveUploadTarget(path.root, path.rel, reqPath, resolution) + if err != nil { + wt.mutationMu.Unlock() + _ = path.root.Remove(tmpRel) + return "", 0, err + } + if err := path.root.Rename(tmpRel, targetRel); err != nil { + wt.mutationMu.Unlock() + _ = path.root.Remove(tmpRel) + return "", 0, fmt.Errorf("failed to place upload: %w", err) + } + wt.mutationMu.Unlock() + + notifyRel := wt.mutationNotificationPath(filepath.Join(filepath.Dir(path.safe), filepath.Base(targetRel))) + wt.notifyFileChange(notifyRel, types.FileOpCreate) + + return targetReq, written, nil +} + +// resolveUploadTarget applies the caller's resolution and returns the rooted +// relative path to write plus the request-shaped path to report back. +func resolveUploadTarget( + root *os.Root, + rel string, + reqPath string, + resolution UploadResolution, +) (string, string, error) { + info, err := root.Stat(rel) + switch { + case os.IsNotExist(err): + // Destination is free; the resolution is irrelevant. + return rel, reqPath, nil + case err != nil: + return "", "", fmt.Errorf("failed to stat target: %w", err) + } + + switch resolution { + case UploadResolutionReplace: + if info.IsDir() { + return "", "", fmt.Errorf("cannot replace destination: %s is a directory", reqPath) + } + return rel, reqPath, nil + case UploadResolutionKeepBoth: + freeRel, err := nextFreeUploadName(root, rel) + if err != nil { + return "", "", err + } + return freeRel, replaceLastSegment(reqPath, filepath.Base(freeRel)), nil + default: + return "", "", fmt.Errorf("%w: %s", ErrUploadConflict, reqPath) + } +} + +// nextFreeUploadName finds the first unused "name-.ext" beside rel. +func nextFreeUploadName(root *os.Root, rel string) (string, error) { + dir := filepath.Dir(rel) + base := filepath.Base(rel) + ext := filepath.Ext(base) + stem := strings.TrimSuffix(base, ext) + + for i := 1; i <= maxKeepBothAttempts; i++ { + candidate := filepath.Join(dir, fmt.Sprintf("%s-%d%s", stem, i, ext)) + if _, err := root.Stat(candidate); os.IsNotExist(err) { + return candidate, nil + } else if err != nil { + return "", fmt.Errorf("failed to stat candidate name: %w", err) + } + } + return "", fmt.Errorf("no available name for %s", rel) +} + +// replaceLastSegment swaps the final segment of a slash-separated request path. +func replaceLastSegment(reqPath, name string) string { + idx := strings.LastIndex(reqPath, "/") + if idx < 0 { + return name + } + return reqPath[:idx+1] + name +} + +// createUploadTemp opens a uniquely named temporary file inside dir through the +// rooted handle, so the staging file is subject to the same containment as the +// destination. +func createUploadTemp(root *os.Root, dir string) (string, *os.File, error) { + for attempt := 0; attempt < 1000; attempt++ { + name := fmt.Sprintf("%s%d-%d", uploadTempPrefix, os.Getpid(), attempt) + rel := filepath.Join(dir, name) + f, err := root.OpenFile(rel, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) + if err == nil { + return rel, f, nil + } + if !os.IsExist(err) { + return "", nil, fmt.Errorf("failed to stage upload: %w", err) + } + } + return "", nil, errors.New("failed to stage upload: no free temporary name") +} diff --git a/apps/backend/internal/agentctl/server/process/workspace_upload_test.go b/apps/backend/internal/agentctl/server/process/workspace_upload_test.go new file mode 100644 index 0000000000..f75cf684f3 --- /dev/null +++ b/apps/backend/internal/agentctl/server/process/workspace_upload_test.go @@ -0,0 +1,351 @@ +package process + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/kandev/kandev/internal/agentctl/types" +) + +// failingReader yields some bytes and then fails, to prove a mid-stream error +// leaves no file and no temporary artifact behind. +type failingReader struct { + prefix []byte + off int +} + +func (r *failingReader) Read(p []byte) (int, error) { + if r.off < len(r.prefix) { + n := copy(p, r.prefix[r.off:]) + r.off += n + return n, nil + } + return 0, errors.New("stream broke") +} + +func TestWriteFileStreamContainment(t *testing.T) { + cases := []struct { + name string + path string + absolutePath bool + }{ + {name: "parent traversal", path: "../escaped.txt"}, + {name: "deep traversal", path: "../../etc/passwd"}, + {name: "absolute path", absolutePath: true}, + {name: "interior traversal segment", path: "nested/../../escaped.txt"}, + {name: "traversal through existing dir", path: "sub/../../escaped.txt"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir, wt := setupTestDir(t) + if err := os.MkdirAll(filepath.Join(dir, "sub"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + parent := filepath.Dir(dir) + requestPath := tc.path + outsidePath := filepath.Join(parent, "escaped.txt") + if tc.absolutePath { + // Use a platform-native absolute path. Unix accepts /etc/passwd, + // but Windows treats that spelling as a relative path. + requestPath = filepath.Join(parent, "escaped-absolute.txt") + outsidePath = requestPath + } + + _, _, err := wt.WriteFileStream(requestPath, UploadResolutionNone, strings.NewReader("payload")) + if err == nil { + t.Fatalf("expected containment rejection for %q, got nil", requestPath) + } + if _, statErr := os.Stat(outsidePath); statErr == nil { + t.Fatalf("wrote outside the workspace for %q", requestPath) + } + }) + } +} + +func TestWriteFileStreamSymlinkedDirRejected(t *testing.T) { + dir, wt := setupTestDir(t) + outside := t.TempDir() + if err := os.Symlink(outside, filepath.Join(dir, "link")); err != nil { + t.Fatalf("create symlink: %v", err) + } + + _, _, err := wt.WriteFileStream("link/escaped.txt", UploadResolutionNone, strings.NewReader("payload")) + if err == nil { + t.Fatal("expected rejection when writing through a symlinked directory") + } + if _, statErr := os.Stat(filepath.Join(outside, "escaped.txt")); statErr == nil { + t.Fatal("wrote through a symlink out of the workspace") + } +} + +func TestWriteFileStreamWritesNestedFile(t *testing.T) { + dir, wt := setupTestDir(t) + + written, n, err := wt.WriteFileStream("nested/deep/file.txt", UploadResolutionNone, strings.NewReader("hello")) + if err != nil { + t.Fatalf("WriteFileStream: %v", err) + } + if written != "nested/deep/file.txt" { + t.Errorf("written path = %q, want nested/deep/file.txt", written) + } + if n != 5 { + t.Errorf("bytes = %d, want 5", n) + } + got, err := os.ReadFile(filepath.Join(dir, "nested", "deep", "file.txt")) + if err != nil { + t.Fatalf("read back: %v", err) + } + if string(got) != "hello" { + t.Errorf("content = %q, want hello", got) + } +} + +func TestWriteFileStreamResolutions(t *testing.T) { + t.Run("absent resolution refuses to overwrite", func(t *testing.T) { + dir, wt := setupTestDir(t) + writeFile(t, dir, "taken.txt", "original") + + _, _, err := wt.WriteFileStream("taken.txt", UploadResolutionNone, strings.NewReader("incoming")) + if !errors.Is(err, ErrUploadConflict) { + t.Fatalf("expected ErrUploadConflict, got %v", err) + } + got, _ := os.ReadFile(filepath.Join(dir, "taken.txt")) + if string(got) != "original" { + t.Errorf("existing file was modified: %q", got) + } + }) + + t.Run("replace overwrites", func(t *testing.T) { + dir, wt := setupTestDir(t) + writeFile(t, dir, "taken.txt", "original") + + written, _, err := wt.WriteFileStream("taken.txt", UploadResolutionReplace, strings.NewReader("incoming")) + if err != nil { + t.Fatalf("WriteFileStream: %v", err) + } + if written != "taken.txt" { + t.Errorf("written path = %q, want taken.txt", written) + } + got, _ := os.ReadFile(filepath.Join(dir, "taken.txt")) + if string(got) != "incoming" { + t.Errorf("content = %q, want incoming", got) + } + }) + + t.Run("replace refuses a directory", func(t *testing.T) { + dir, wt := setupTestDir(t) + if err := os.Mkdir(filepath.Join(dir, "taken.txt"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + + _, _, err := wt.WriteFileStream("taken.txt", UploadResolutionReplace, strings.NewReader("incoming")) + if err == nil || !strings.Contains(err.Error(), "is a directory") { + t.Fatalf("expected directory rejection, got %v", err) + } + }) + + t.Run("keep both renames and preserves the original", func(t *testing.T) { + dir, wt := setupTestDir(t) + writeFile(t, dir, "taken.txt", "original") + + written, _, err := wt.WriteFileStream("taken.txt", UploadResolutionKeepBoth, strings.NewReader("incoming")) + if err != nil { + t.Fatalf("WriteFileStream: %v", err) + } + if written != "taken-1.txt" { + t.Errorf("written path = %q, want taken-1.txt", written) + } + original, _ := os.ReadFile(filepath.Join(dir, "taken.txt")) + if string(original) != "original" { + t.Errorf("original was modified: %q", original) + } + renamed, err := os.ReadFile(filepath.Join(dir, "taken-1.txt")) + if err != nil { + t.Fatalf("renamed file missing: %v", err) + } + if string(renamed) != "incoming" { + t.Errorf("renamed content = %q, want incoming", renamed) + } + }) + + t.Run("keep both skips names already taken", func(t *testing.T) { + dir, wt := setupTestDir(t) + writeFile(t, dir, "taken.txt", "original") + writeFile(t, dir, "taken-1.txt", "first") + + written, _, err := wt.WriteFileStream("taken.txt", UploadResolutionKeepBoth, strings.NewReader("incoming")) + if err != nil { + t.Fatalf("WriteFileStream: %v", err) + } + if written != "taken-2.txt" { + t.Errorf("written path = %q, want taken-2.txt", written) + } + }) + + t.Run("keep both on a free name uses it unchanged", func(t *testing.T) { + _, wt := setupTestDir(t) + + written, _, err := wt.WriteFileStream("fresh.txt", UploadResolutionKeepBoth, strings.NewReader("incoming")) + if err != nil { + t.Fatalf("WriteFileStream: %v", err) + } + if written != "fresh.txt" { + t.Errorf("written path = %q, want fresh.txt", written) + } + }) +} + +func TestWriteFileStreamFailedStreamLeavesNothing(t *testing.T) { + dir, wt := setupTestDir(t) + + _, _, err := wt.WriteFileStream("partial.bin", UploadResolutionNone, &failingReader{prefix: []byte("abc")}) + if err == nil { + t.Fatal("expected an error from the failing reader") + } + if _, statErr := os.Stat(filepath.Join(dir, "partial.bin")); statErr == nil { + t.Error("a partial file was left at the destination") + } + entries, readErr := os.ReadDir(dir) + if readErr != nil { + t.Fatalf("readdir: %v", readErr) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), ".kandev-upload-") { + t.Errorf("temporary artifact left behind: %s", e.Name()) + } + } +} + +func TestWriteFileStreamDoesNotClobberExistingOnFailure(t *testing.T) { + dir, wt := setupTestDir(t) + writeFile(t, dir, "keep.txt", "original") + + _, _, err := wt.WriteFileStream("keep.txt", UploadResolutionReplace, &failingReader{prefix: []byte("abc")}) + if err == nil { + t.Fatal("expected an error from the failing reader") + } + got, _ := os.ReadFile(filepath.Join(dir, "keep.txt")) + if string(got) != "original" { + t.Errorf("existing file was clobbered by a failed replace: %q", got) + } +} + +func TestWriteFileStreamNotifiesChange(t *testing.T) { + _, wt := setupTestDir(t) + sub := make(types.WorkspaceStreamSubscriber, 8) + wt.workspaceSubMu.Lock() + if wt.workspaceStreamSubscribers == nil { + wt.workspaceStreamSubscribers = make(map[types.WorkspaceStreamSubscriber]struct{}) + } + wt.workspaceStreamSubscribers[sub] = struct{}{} + wt.workspaceSubMu.Unlock() + + if _, _, err := wt.WriteFileStream("noted.txt", UploadResolutionNone, strings.NewReader("x")); err != nil { + t.Fatalf("WriteFileStream: %v", err) + } + + select { + case msg := <-sub: + if msg.FileChange == nil { + t.Fatalf("expected a file-change notification, got %+v", msg) + } + if msg.FileChange.Path != "noted.txt" { + t.Errorf("notified path = %q, want noted.txt", msg.FileChange.Path) + } + default: + t.Fatal("expected a workspace change notification") + } +} + +func TestWriteFileStreamConcurrentKeepBothUsesUniqueNames(t *testing.T) { + dir, wt := setupTestDir(t) + writeFile(t, dir, "taken.txt", "original") + + const uploads = 8 + type uploadResult struct { + path string + err error + } + results := make(chan uploadResult, uploads) + for i := 0; i < uploads; i++ { + go func(i int) { + path, _, err := wt.WriteFileStream( + "taken.txt", + UploadResolutionKeepBoth, + strings.NewReader(fmt.Sprintf("incoming-%d", i)), + ) + results <- uploadResult{path: path, err: err} + }(i) + } + seen := map[string]bool{} + for i := 0; i < uploads; i++ { + result := <-results + if result.err != nil { + t.Fatalf("concurrent upload: %v", result.err) + } + if seen[result.path] { + t.Fatalf("duplicate final path %q", result.path) + } + seen[result.path] = true + } + if len(seen) != uploads { + t.Fatalf("got %d uploaded paths, want %d: %v", len(seen), uploads, seen) + } +} + +func TestCheckUploadConflicts(t *testing.T) { + t.Run("reports only existing paths", func(t *testing.T) { + dir, wt := setupTestDir(t) + writeFile(t, dir, "there.txt", "x") + if err := os.MkdirAll(filepath.Join(dir, "nested"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + writeFile(t, dir, filepath.Join("nested", "also.txt"), "x") + + conflicts, err := wt.CheckUploadConflicts([]string{"there.txt", "missing.txt", "nested/also.txt"}) + if err != nil { + t.Fatalf("CheckUploadConflicts: %v", err) + } + got := map[string]bool{} + for _, c := range conflicts { + got[c.Path] = true + } + if !got["there.txt"] || !got["nested/also.txt"] { + t.Errorf("missing expected conflicts, got %v", got) + } + if got["missing.txt"] { + t.Error("reported a conflict for a path that does not exist") + } + }) + + t.Run("marks directories", func(t *testing.T) { + dir, wt := setupTestDir(t) + if err := os.MkdirAll(filepath.Join(dir, "adir"), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + + conflicts, err := wt.CheckUploadConflicts([]string{"adir"}) + if err != nil { + t.Fatalf("CheckUploadConflicts: %v", err) + } + if len(conflicts) != 1 || !conflicts[0].IsDir { + t.Fatalf("expected one directory conflict, got %+v", conflicts) + } + }) + + t.Run("containment failure is an error, not a conflict", func(t *testing.T) { + _, wt := setupTestDir(t) + + if _, err := wt.CheckUploadConflicts([]string{"../escaped.txt"}); err == nil { + t.Fatal("expected an error for a path outside the workspace") + } + }) +} + +// io.Reader compile-time guard for the helper above. +var _ io.Reader = (*failingReader)(nil) diff --git a/apps/backend/internal/agentctl/types/workspace_upload.go b/apps/backend/internal/agentctl/types/workspace_upload.go new file mode 100644 index 0000000000..4f7d25468a --- /dev/null +++ b/apps/backend/internal/agentctl/types/workspace_upload.go @@ -0,0 +1,35 @@ +package types + +import "errors" + +// ErrWorkspaceUploadConflict reports that an upload destination already exists +// and the caller supplied no resolution. +// +// It lives here rather than beside the agentctl client so that higher-level +// callers can distinguish a conflict from a failure without importing the +// runtime-tier client package directly (ARCH-RUNTIME-IMPORT). +var ErrWorkspaceUploadConflict = errors.New("workspace upload destination already exists") + +// WorkspaceUploadConflict is one upload destination that already exists. +type WorkspaceUploadConflict struct { + Path string `json:"path"` + IsDir bool `json:"is_dir"` +} + +// UploadedWorkspaceFile is the path agentctl actually wrote. It differs from the +// requested path when the caller asked to keep both copies. +type UploadedWorkspaceFile struct { + Path string `json:"path"` + SizeBytes int64 `json:"size_bytes"` + ResolutionApplied string `json:"resolution_applied"` +} + +// WorkspaceUploadParams describes one file destined for a task workspace. +// Content is supplied separately so this stays a plain data type. +type WorkspaceUploadParams struct { + Dir string + Repo string + RelativePath string + Resolution string + SizeBytes int64 +} diff --git a/apps/backend/internal/backendapp/helpers.go b/apps/backend/internal/backendapp/helpers.go index 6d138892db..24003e6b6d 100644 --- a/apps/backend/internal/backendapp/helpers.go +++ b/apps/backend/internal/backendapp/helpers.go @@ -1278,7 +1278,8 @@ func registerTaskRoutes(p routeParams, planService *taskservice.PlanService, han p.router, p.gateway.Dispatcher, p.taskSvc, &orchestratorWrapper{svc: p.orchestratorSvc}, p.log, referenceValidators..., ) - taskhandlers.RegisterProcessRoutes(p.router, p.taskSvc, p.lifecycleMgr, p.log) + processHandlers := taskhandlers.RegisterProcessRoutes(p.router, p.taskSvc, p.lifecycleMgr, p.log) + taskhandlers.RegisterWorkspaceFileRoutes(p.router, processHandlers) analyticshandlers.RegisterStatsRoutes(p.router, p.analyticsRepo, p.taskSvc, p.log) agenthandlers.RegisterShellRoutes(p.router, p.lifecycleMgr, p.log) if p.services.Share != nil { diff --git a/apps/backend/internal/task/handlers/process_handlers.go b/apps/backend/internal/task/handlers/process_handlers.go index 91dde55a08..a6bf25de42 100644 --- a/apps/backend/internal/task/handlers/process_handlers.go +++ b/apps/backend/internal/task/handlers/process_handlers.go @@ -37,7 +37,7 @@ func RegisterProcessRoutes( svc *service.Service, lifecycleMgr *lifecycle.Manager, log *logger.Logger, -) { +) *ProcessHandlers { handlers := &ProcessHandlers{ service: svc, lifecycleMgr: lifecycleMgr, @@ -68,6 +68,7 @@ func RegisterProcessRoutes( session.POST("/set-model", handlers.httpSetSessionModel) session.POST("/set-config-option", handlers.httpSetSessionConfigOption) session.POST("/authenticate", handlers.httpAuthenticate) + return handlers } type httpStartProcessRequest struct { diff --git a/apps/backend/internal/task/handlers/workspace_file_http_handlers.go b/apps/backend/internal/task/handlers/workspace_file_http_handlers.go new file mode 100644 index 0000000000..f0604147fd --- /dev/null +++ b/apps/backend/internal/task/handlers/workspace_file_http_handlers.go @@ -0,0 +1,437 @@ +package handlers + +import ( + "context" + "errors" + "fmt" + "io" + "mime/multipart" + "net/http" + "os" + "strconv" + "strings" + + "github.com/gin-gonic/gin" + "go.uber.org/zap" + + agentctltypes "github.com/kandev/kandev/internal/agentctl/types" + "github.com/kandev/kandev/internal/task/models" +) + +// workspaceUploadClient is the slice of the agentctl client this handler needs. +// +// Declaring it here, structurally, keeps the handler off the runtime-tier +// package (ARCH-RUNTIME-IMPORT): the concrete client satisfies it without this +// file naming that package. +type workspaceUploadClient interface { + PreflightWorkspaceUpload( + ctx context.Context, dir, repo string, paths []string, + ) ([]agentctltypes.WorkspaceUploadConflict, error) + UploadWorkspaceFile( + ctx context.Context, params agentctltypes.WorkspaceUploadParams, content io.Reader, + ) (*agentctltypes.UploadedWorkspaceFile, error) +} + +// workspaceUploadRequestSlack allows for multipart framing on top of the file. +const workspaceUploadRequestSlack int64 = 4 * 1024 * 1024 + +// workspaceUploadPreflightBodyLimit bounds the JSON envelope before Gin +// decodes a caller-controlled path list. +const workspaceUploadPreflightBodyLimit int64 = 1 << 20 + +// These limits keep one preflight from consuming unbounded memory or work. +const ( + workspaceUploadMaxPaths = 4096 + workspaceUploadMaxPathBytes = 4096 +) + +var errWorkspaceUploadTooLarge = errors.New("workspace upload exceeds maximum size") + +// RegisterWorkspaceFileRoutes exposes the session-scoped workspace upload +// surface. Both routes reuse the same session authorization as the other +// task-session routes, so an unauthenticated or unknown session is refused +// before any byte is streamed. +func RegisterWorkspaceFileRoutes(router *gin.Engine, handlers *ProcessHandlers) { + files := router.Group("/api/v1/task-sessions/:id/workspace/files") + files.POST("", handlers.httpUploadWorkspaceFile) + files.POST("/preflight", handlers.httpPreflightWorkspaceUpload) +} + +type workspaceUploadPreflightRequest struct { + Dir string `json:"dir"` + Repo string `json:"repo"` + Paths []string `json:"paths"` +} + +// httpPreflightWorkspaceUpload reports which destinations already exist so the +// caller can resolve every conflict before uploading anything. +func (h *ProcessHandlers) httpPreflightWorkspaceUpload(c *gin.Context) { + sessionID := c.Param("id") + if sessionID == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "session_id is required"}) + return + } + if h.denySessionAccess(c, sessionID) { + return + } + + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, workspaceUploadPreflightBodyLimit) + var req workspaceUploadPreflightRequest + if err := c.ShouldBindJSON(&req); err != nil { + status := http.StatusBadRequest + var maxErr *http.MaxBytesError + if errors.As(err, &maxErr) { + status = http.StatusRequestEntityTooLarge + } + c.JSON(status, gin.H{"error": "invalid request: " + err.Error()}) + return + } + if len(req.Paths) > workspaceUploadMaxPaths { + c.JSON(http.StatusBadRequest, gin.H{"error": "too many upload paths"}) + return + } + for _, uploadPath := range req.Paths { + if len(uploadPath) > workspaceUploadMaxPathBytes { + c.JSON(http.StatusBadRequest, gin.H{"error": "upload path is too long"}) + return + } + } + if len(req.Dir) > workspaceUploadMaxPathBytes || len(req.Repo) > workspaceUploadMaxPathBytes { + c.JSON(http.StatusBadRequest, gin.H{"error": "upload directory is too long"}) + return + } + if len(req.Paths) == 0 { + c.JSON(http.StatusOK, gin.H{"conflicts": []agentctltypes.WorkspaceUploadConflict{}}) + return + } + + client, release, ok := h.workspaceUploadClient(c, sessionID) + if !ok { + return + } + defer release() + + conflicts, err := client.PreflightWorkspaceUpload(c.Request.Context(), req.Dir, req.Repo, req.Paths) + if err != nil { + h.respondWorkspaceUploadError(c, sessionID, err) + return + } + if conflicts == nil { + conflicts = []agentctltypes.WorkspaceUploadConflict{} + } + c.JSON(http.StatusOK, gin.H{"conflicts": conflicts}) +} + +// httpUploadWorkspaceFile streams one multipart file part through to agentctl. +// One request carries one file so that a rejection of one file in a selection +// does not fail the rest. +func (h *ProcessHandlers) httpUploadWorkspaceFile(c *gin.Context) { + sessionID := c.Param("id") + if sessionID == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "session_id is required"}) + return + } + if h.denySessionAccess(c, sessionID) { + return + } + + c.Request.Body = http.MaxBytesReader( + c.Writer, c.Request.Body, models.MaxMessageAttachmentBytes+workspaceUploadRequestSlack, + ) + // MultipartReader reads from Request.Body, so the body cap is applied before + // obtaining and consuming the reader. + multipartReader, err := c.Request.MultipartReader() + if err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "multipart upload is required"}) + return + } + + parsed, parseErr := parseTaskWorkspaceUploadMultipart(multipartReader) + if parsed != nil { + defer parsed.cleanup() + } + if parseErr != nil { + c.JSON(parseErr.status, gin.H{"error": parseErr.message}) + return + } + if parsed.staged == nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "file part is required"}) + return + } + if err := parsed.fields.validate(); err != nil { + if parsed.fields.declaredSize > models.MaxMessageAttachmentBytes { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "file exceeds the maximum upload size"}) + } else { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + } + return + } + if parsed.staged.size != parsed.fields.declaredSize { + c.JSON(http.StatusBadRequest, gin.H{"error": "size_bytes does not match uploaded content"}) + return + } + if _, err := parsed.staged.file.Seek(0, io.SeekStart); err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "workspace upload failed"}) + return + } + uploaded, err := h.forwardWorkspaceUpload(c, sessionID, parsed.fields, parsed.staged.file) + if err != nil { + return + } + c.JSON(http.StatusCreated, uploaded) +} + +type workspaceUploadParseError struct { + status int + message string +} + +type parsedTaskWorkspaceUpload struct { + fields workspaceUploadMultipartFields + staged *stagedWorkspaceUpload +} + +func (p *parsedTaskWorkspaceUpload) cleanup() { + if p == nil || p.staged == nil { + return + } + p.staged.cleanup() +} + +func parseTaskWorkspaceUploadMultipart(reader *multipart.Reader) (*parsedTaskWorkspaceUpload, *workspaceUploadParseError) { + parsed := &parsedTaskWorkspaceUpload{} + for { + part, nextErr := reader.NextPart() + if errors.Is(nextErr, io.EOF) { + return parsed, nil + } + if nextErr != nil { + if isMultipartSizeError(nextErr) { + return parsed, &workspaceUploadParseError{status: http.StatusRequestEntityTooLarge, message: "upload request is too large"} + } + return parsed, &workspaceUploadParseError{status: http.StatusBadRequest, message: "invalid multipart upload"} + } + if part.FileName() == "" { + if parseErr := parseTaskWorkspaceUploadField(part, &parsed.fields); parseErr != nil { + return parsed, parseErr + } + continue + } + if part.FormName() != "file" || parsed.staged != nil { + _ = part.Close() + return parsed, &workspaceUploadParseError{status: http.StatusBadRequest, message: "exactly one file part is required"} + } + staged, stageErr := stageWorkspaceUploadPart(part) + _ = part.Close() + if stageErr != nil { + if errors.Is(stageErr, errWorkspaceUploadTooLarge) || isMultipartSizeError(stageErr) { + return parsed, &workspaceUploadParseError{status: http.StatusRequestEntityTooLarge, message: "file exceeds the maximum upload size"} + } + return parsed, &workspaceUploadParseError{status: http.StatusInternalServerError, message: "workspace upload failed"} + } + parsed.staged = staged + } +} + +func parseTaskWorkspaceUploadField(part *multipart.Part, fields *workspaceUploadMultipartFields) *workspaceUploadParseError { + value, readErr := readWorkspaceUploadField(part) + _ = part.Close() + if readErr != nil { + return &workspaceUploadParseError{status: http.StatusBadRequest, message: "invalid upload metadata"} + } + fields.set(part.FormName(), value) + return nil +} + +func isMultipartSizeError(err error) bool { + var maxErr *http.MaxBytesError + return errors.As(err, &maxErr) +} + +type workspaceUploadMultipartFields struct { + dir string + repo string + relativePath string + resolution string + declaredSize int64 + hasSize bool +} + +func (f *workspaceUploadMultipartFields) set(name, value string) { + switch name { + case "dir": + f.dir = value + case "repo": + f.repo = value + case "relative_path": + f.relativePath = value + case "resolution": + f.resolution = value + case "size_bytes": + if parsed, err := strconv.ParseInt(strings.TrimSpace(value), 10, 64); err == nil && parsed >= 0 { + f.declaredSize = parsed + f.hasSize = true + } + } +} + +func (f workspaceUploadMultipartFields) validate() error { + if strings.TrimSpace(f.relativePath) == "" { + return errors.New("relative_path is required") + } + if len(f.relativePath) > workspaceUploadMaxPathBytes || len(f.dir) > workspaceUploadMaxPathBytes || len(f.repo) > workspaceUploadMaxPathBytes { + return errors.New("upload path is too long") + } + if !f.hasSize { + return errors.New("size_bytes is required") + } + if f.declaredSize > models.MaxMessageAttachmentBytes { + return errors.New("file exceeds the maximum upload size") + } + return nil +} + +const workspaceUploadFieldLimit int64 = workspaceUploadMaxPathBytes + 128 + +func readWorkspaceUploadField(part *multipart.Part) (string, error) { + data, err := io.ReadAll(io.LimitReader(part, workspaceUploadFieldLimit+1)) + if err != nil { + return "", err + } + if int64(len(data)) > workspaceUploadFieldLimit { + return "", errors.New("upload metadata is too long") + } + return string(data), nil +} + +// boundedWorkspaceUploadReader makes a size mismatch fail before the rooted +// writer renames its temporary file into the destination. +type boundedWorkspaceUploadReader struct { + part io.Reader + expected int64 + read int64 +} + +type stagedWorkspaceUpload struct { + file *os.File + path string + size int64 +} + +func stageWorkspaceUploadPart(part io.Reader) (*stagedWorkspaceUpload, error) { + tmp, err := os.CreateTemp("", "kandev-workspace-upload-") + if err != nil { + return nil, fmt.Errorf("create upload staging file: %w", err) + } + cleanup := func() { + _ = tmp.Close() + _ = os.Remove(tmp.Name()) + } + written, err := io.Copy(tmp, io.LimitReader(part, models.MaxMessageAttachmentBytes+1)) + if err != nil { + cleanup() + return nil, fmt.Errorf("stage upload: %w", err) + } + if written > models.MaxMessageAttachmentBytes { + cleanup() + return nil, errWorkspaceUploadTooLarge + } + if _, err := tmp.Seek(0, io.SeekStart); err != nil { + cleanup() + return nil, fmt.Errorf("rewind upload staging file: %w", err) + } + return &stagedWorkspaceUpload{file: tmp, path: tmp.Name(), size: written}, nil +} + +func (s *stagedWorkspaceUpload) cleanup() { + if s == nil || s.file == nil { + return + } + _ = s.file.Close() + _ = os.Remove(s.path) +} + +func (r *boundedWorkspaceUploadReader) Read(p []byte) (int, error) { + if len(p) == 0 { + return 0, nil + } + remaining := r.expected - r.read + if remaining < int64(len(p)) { + p = p[:remaining+1] + } + n, err := r.part.Read(p) + r.read += int64(n) + if r.read > r.expected { + return n, fmt.Errorf("size_bytes does not match uploaded content") + } + if errors.Is(err, io.EOF) && r.read != r.expected { + return n, fmt.Errorf("size_bytes does not match uploaded content") + } + return n, err +} + +func (h *ProcessHandlers) forwardWorkspaceUpload(c *gin.Context, sessionID string, fields workspaceUploadMultipartFields, src io.Reader) (*agentctltypes.UploadedWorkspaceFile, error) { + if err := fields.validate(); err != nil { + if fields.declaredSize > models.MaxMessageAttachmentBytes { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "file exceeds the maximum upload size"}) + } else { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + } + return nil, err + } + client, release, ok := h.workspaceUploadClient(c, sessionID) + if !ok { + return nil, errors.New("workspace not ready") + } + defer release() + uploaded, err := client.UploadWorkspaceFile(c.Request.Context(), agentctltypes.WorkspaceUploadParams{ + Dir: fields.dir, + Repo: fields.repo, + RelativePath: fields.relativePath, + Resolution: fields.resolution, + SizeBytes: fields.declaredSize, + }, &boundedWorkspaceUploadReader{part: src, expected: fields.declaredSize}) + if err != nil { + h.respondWorkspaceUploadError(c, sessionID, err) + return nil, err + } + return uploaded, nil +} + +// workspaceUploadClient resolves the session's agentctl client, responding with +// 503 when the session has no live execution to write into. +func (h *ProcessHandlers) workspaceUploadClient(c *gin.Context, sessionID string) (workspaceUploadClient, func(), bool) { + execution, err := h.lifecycleMgr.GetOrEnsureExecution(c.Request.Context(), sessionID) + if err != nil || execution == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace not ready"}) + return nil, func() {}, false + } + client, release := execution.AcquireAgentCtlClient() + if client == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace not ready"}) + return nil, func() {}, false + } + return client, release, true +} + +// respondWorkspaceUploadError preserves the conflict case so the caller can +// prompt for a resolution rather than treating it as a failure. +func (h *ProcessHandlers) respondWorkspaceUploadError(c *gin.Context, sessionID string, err error) { + if errors.Is(err, agentctltypes.ErrWorkspaceUploadConflict) { + c.JSON(http.StatusConflict, gin.H{"error": err.Error()}) + return + } + if strings.Contains(err.Error(), "status 413") { + c.JSON(http.StatusRequestEntityTooLarge, gin.H{"error": "file exceeds the maximum upload size"}) + return + } + if strings.Contains(err.Error(), "status 400") || strings.Contains(err.Error(), "size_bytes does not match") { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + h.logger.Warn("workspace file upload failed", + zap.String("session_id", sessionID), + zap.Error(err), + ) + c.JSON(http.StatusBadGateway, gin.H{"error": "workspace upload failed"}) +} diff --git a/apps/backend/internal/task/handlers/workspace_file_http_handlers_test.go b/apps/backend/internal/task/handlers/workspace_file_http_handlers_test.go new file mode 100644 index 0000000000..214ee76269 --- /dev/null +++ b/apps/backend/internal/task/handlers/workspace_file_http_handlers_test.go @@ -0,0 +1,160 @@ +package handlers + +import ( + "bytes" + "mime/multipart" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "sync/atomic" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/auth/authn" +) + +// agentctlUploadServer answers the two agentctl upload routes and records +// whether either was reached, so a denial can be proven to land first. +func agentctlUploadServer(t *testing.T) (*httptest.Server, *atomic.Bool) { + t.Helper() + reached := &atomic.Bool{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/workspace/file/upload": + reached.Store(true) + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"path":"fixtures/sample.txt","size_bytes":7,"resolution_applied":""}`)) + case "/api/v1/workspace/file/upload-preflight": + reached.Store(true) + _, _ = w.Write([]byte(`{"conflicts":[{"path":"fixtures/there.txt","is_dir":false}]}`)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + return server, reached +} + +// uploadRequestAs builds a multipart upload for sess-b as userID. +func uploadRequestAs(t *testing.T, userID string, fields map[string]string, content string) (*gin.Context, *httptest.ResponseRecorder) { + t.Helper() + gin.SetMode(gin.TestMode) + var body bytes.Buffer + writer := multipart.NewWriter(&body) + for k, v := range fields { + require.NoError(t, writer.WriteField(k, v)) + } + part, err := writer.CreateFormFile("file", "sample.txt") + require.NoError(t, err) + _, err = part.Write([]byte(content)) + require.NoError(t, err) + require.NoError(t, writer.Close()) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodPost, "/api/v1/task-sessions/sess-b/workspace/files", &body) + c.Request.Header.Set("Content-Type", writer.FormDataContentType()) + c.Request = c.Request.WithContext(authn.WithIdentity( + c.Request.Context(), authn.Identity{UserID: userID, Role: authn.RoleMember})) + c.Params = gin.Params{{Key: "id", Value: "sess-b"}} + return c, rec +} +func preflightRequestAs(t *testing.T, userID, body string) (*gin.Context, *httptest.ResponseRecorder) { + t.Helper() + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodPost, + "/api/v1/task-sessions/sess-b/workspace/files/preflight", strings.NewReader(body)) + c.Request.Header.Set("Content-Type", "application/json") + c.Request = c.Request.WithContext(authn.WithIdentity( + c.Request.Context(), authn.Identity{UserID: userID, Role: authn.RoleMember})) + c.Params = gin.Params{{Key: "id", Value: "sess-b"}} + return c, rec +} +func uploadFields(content string) map[string]string { + return map[string]string{ + "dir": "fixtures", + "relative_path": "sample.txt", + "size_bytes": strconv.Itoa(len(content)), + } +} + +// TestWorkspaceFileUploadDeniesForeignSession proves the scoping guard lands +// before any byte reaches agentctl. The owner request is the witness that the +// denial is scoping and not an unrelated failure. +func TestWorkspaceFileUploadDeniesForeignSession(t *testing.T) { + server, reached := agentctlUploadServer(t) + h := newForeignProcessHandlers(t, managerWithSessionExecution(t, server.URL, "sess-b")) + content := "payload" + c, rec := uploadRequestAs(t, "user-a", uploadFields(content), content) + h.httpUploadWorkspaceFile(c) + require.Equal(t, http.StatusNotFound, rec.Code, "body: %s", rec.Body.String()) + require.False(t, reached.Load(), "the denial must land before agentctl is contacted") + ownerCtx, ownerRec := uploadRequestAs(t, "user-b", uploadFields(content), content) + h.httpUploadWorkspaceFile(ownerCtx) + require.Equal(t, http.StatusCreated, ownerRec.Code, "body: %s", ownerRec.Body.String()) + require.Contains(t, ownerRec.Body.String(), "fixtures/sample.txt") + require.True(t, reached.Load()) +} +func TestWorkspaceFilePreflightDeniesForeignSession(t *testing.T) { + server, reached := agentctlUploadServer(t) + h := newForeignProcessHandlers(t, managerWithSessionExecution(t, server.URL, "sess-b")) + c, rec := preflightRequestAs(t, "user-a", `{"dir":"fixtures","paths":["there.txt"]}`) + h.httpPreflightWorkspaceUpload(c) + require.Equal(t, http.StatusNotFound, rec.Code) + require.False(t, reached.Load(), "the denial must land before agentctl is contacted") + ownerCtx, ownerRec := preflightRequestAs(t, "user-b", `{"dir":"fixtures","paths":["there.txt"]}`) + h.httpPreflightWorkspaceUpload(ownerCtx) + require.Equal(t, http.StatusOK, ownerRec.Code, "body: %s", ownerRec.Body.String()) + require.Contains(t, ownerRec.Body.String(), "fixtures/there.txt") + require.True(t, reached.Load()) +} +func TestWorkspaceFileUploadRequiresRelativePath(t *testing.T) { + server, reached := agentctlUploadServer(t) + h := newForeignProcessHandlers(t, managerWithSessionExecution(t, server.URL, "sess-b")) + fields := uploadFields("payload") + delete(fields, "relative_path") + c, rec := uploadRequestAs(t, "user-b", fields, "payload") + h.httpUploadWorkspaceFile(c) + require.Equal(t, http.StatusBadRequest, rec.Code) + require.False(t, reached.Load(), "a malformed request must not reach agentctl") +} +func TestWorkspaceFileUploadRejectsSizeMismatch(t *testing.T) { + server, reached := agentctlUploadServer(t) + h := newForeignProcessHandlers(t, managerWithSessionExecution(t, server.URL, "sess-b")) + fields := uploadFields("payload") + fields["size_bytes"] = "9999" + c, rec := uploadRequestAs(t, "user-b", fields, "payload") + h.httpUploadWorkspaceFile(c) + require.Equal(t, http.StatusBadRequest, rec.Code) + require.False(t, reached.Load(), "a size mismatch must not reach agentctl") +} + +// TestWorkspaceFileUploadWithoutExecution covers a session that is authorized +// but has no live workspace to write into. +func TestWorkspaceFileUploadWithoutExecution(t *testing.T) { + log := newTestLogger(t) + h := newForeignProcessHandlers(t, newLifecycleManager(t, log)) + content := "payload" + c, rec := uploadRequestAs(t, "user-b", uploadFields(content), content) + h.httpUploadWorkspaceFile(c) + require.Equal(t, http.StatusServiceUnavailable, rec.Code, "body: %s", rec.Body.String()) +} + +// TestRegisterWorkspaceFileRoutes pins the two route paths so a rename is a +// deliberate change rather than a silent 404 for every client. +func TestRegisterWorkspaceFileRoutes(t *testing.T) { + gin.SetMode(gin.TestMode) + router := gin.New() + log := newTestLogger(t) + RegisterWorkspaceFileRoutes(router, &ProcessHandlers{lifecycleMgr: newLifecycleManager(t, log), logger: log}) + paths := map[string]bool{} + for _, route := range router.Routes() { + paths[route.Method+" "+route.Path] = true + } + require.True(t, paths["POST /api/v1/task-sessions/:id/workspace/files"], "upload route missing: %v", paths) + require.True(t, paths["POST /api/v1/task-sessions/:id/workspace/files/preflight"], "preflight route missing: %v", paths) +} diff --git a/apps/web/components/editors/codemirror/codemirror-code-editor.download.test.tsx b/apps/web/components/editors/codemirror/codemirror-code-editor.download.test.tsx new file mode 100644 index 0000000000..4d4296c2d5 --- /dev/null +++ b/apps/web/components/editors/codemirror/codemirror-code-editor.download.test.tsx @@ -0,0 +1,68 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@uiw/react-codemirror", () => ({ + default: () =>
, +})); + +vi.mock("./use-codemirror-editor-state", () => ({ + useCodeMirrorEditorState: () => ({ + comments: [], + diffStats: null, + extensions: [], + floatingButtonPos: null, + textSelection: null, + commentView: null, + wrapEnabled: false, + setWrapEnabled: vi.fn(), + handleChange: vi.fn(), + }), +})); + +vi.mock("./use-codemirror-walkthrough-range", () => ({ + useCodeMirrorWalkthroughRange: () => null, +})); + +vi.mock("@/components/editors/external-vcs-file-link", () => ({ + ExternalVcsFileLink: () => null, + useExternalVcsFileStatus: () => null, +})); + +import { CodeMirrorCodeEditor } from "./codemirror-code-editor"; + +afterEach(cleanup); + +const baseProps = { + path: "assets/report.pdf", + content: "", + originalContent: "", + isDirty: false, + isSaving: false, + onChange: vi.fn(), + onSave: vi.fn(), +}; + +describe("CodeMirrorCodeEditor download action", () => { + it("invokes onDownload when the download control is activated", () => { + const onDownload = vi.fn(); + render( + + + , + ); + + screen.getByRole("button", { name: "Download file" }).click(); + expect(onDownload).toHaveBeenCalledTimes(1); + }); + + it("omits the download control when no handler is supplied", () => { + render( + + + , + ); + + expect(screen.queryByRole("button", { name: "Download file" })).toBeNull(); + }); +}); diff --git a/apps/web/components/editors/codemirror/codemirror-code-editor.tsx b/apps/web/components/editors/codemirror/codemirror-code-editor.tsx index 7a70853a9d..c6804e5dac 100644 --- a/apps/web/components/editors/codemirror/codemirror-code-editor.tsx +++ b/apps/web/components/editors/codemirror/codemirror-code-editor.tsx @@ -8,6 +8,7 @@ import { ScrollOnOverflow } from "@kandev/ui/scroll-on-overflow"; import { IconDeviceFloppy, IconLoader2, + IconDownload, IconTrash, IconTextWrap, IconTextWrapDisabled, @@ -59,6 +60,7 @@ type FileEditorContentProps = { onSave: () => void; onReloadFromAgent?: () => void; onDelete?: () => void; + onDownload?: () => void; }; function CodeMirrorCommentBadge({ @@ -161,6 +163,28 @@ function CodeMirrorDeleteButton({ onDelete }: { onDelete?: () => void }) { ); } +function CodeMirrorDownloadButton({ onDownload }: { onDownload?: () => void }) { + const { t } = useTranslation(); + if (!onDownload) return null; + + return ( + + + + + {t("editors:downloadFile")} + + ); +} + function CodeMirrorSaveButton({ isDirty, isSaving, @@ -234,6 +258,7 @@ function CodeMirrorToolbar({ onSave, onReloadFromAgent, onDelete, + onDownload, onToggleMarkdownPreview, }: { path: string; @@ -253,6 +278,7 @@ function CodeMirrorToolbar({ onSave: () => void; onReloadFromAgent?: () => void; onDelete?: () => void; + onDownload?: () => void; onToggleMarkdownPreview?: () => void; }) { const fileStatus = useExternalVcsFileStatus(path, sessionId, repositoryName); @@ -295,6 +321,7 @@ function CodeMirrorToolbar({ repositoryName={repositoryName} size="sm" /> +
@@ -408,6 +435,7 @@ export function CodeMirrorCodeEditor(props: FileEditorContentProps) { onSave, onReloadFromAgent, onDelete, + onDownload, } = props; const { wrapperRef, editorAreaRef, editorRef, state, walkthroughRange, handleCreateEditor } = useCodeMirrorCodeEditorSetup(props); @@ -432,6 +460,7 @@ export function CodeMirrorCodeEditor(props: FileEditorContentProps) { onSave={onSave} onReloadFromAgent={onReloadFromAgent} onDelete={onDelete} + onDownload={onDownload} onToggleMarkdownPreview={onToggleMarkdownPreview} />
diff --git a/apps/web/components/editors/monaco/monaco-code-editor.tsx b/apps/web/components/editors/monaco/monaco-code-editor.tsx index 743bcf0112..b1caa083c3 100644 --- a/apps/web/components/editors/monaco/monaco-code-editor.tsx +++ b/apps/web/components/editors/monaco/monaco-code-editor.tsx @@ -38,6 +38,7 @@ type MonacoCodeEditorProps = { onSave: () => void; onReloadFromAgent?: () => void; onDelete?: () => void; + onDownload?: () => void; }; function getMonacoTheme(resolvedTheme: string | undefined): string { @@ -241,6 +242,7 @@ export function MonacoCodeEditor(props: MonacoCodeEditorProps) { onSave={props.onSave} onReloadFromAgent={props.onReloadFromAgent} onDelete={props.onDelete} + onDownload={props.onDownload} onToggleMarkdownPreview={props.onToggleMarkdownPreview} />
diff --git a/apps/web/components/editors/monaco/monaco-editor-toolbar.test.tsx b/apps/web/components/editors/monaco/monaco-editor-toolbar.test.tsx index 47141585cd..a770b13aa7 100644 --- a/apps/web/components/editors/monaco/monaco-editor-toolbar.test.tsx +++ b/apps/web/components/editors/monaco/monaco-editor-toolbar.test.tsx @@ -99,3 +99,52 @@ describe("MonacoEditorToolbar external file action", () => { expect(screen.queryByTestId("lsp-status")).toBeNull(); }); }); + +describe("MonacoEditorToolbar download action", () => { + const baseProps = { + path: "assets/report.pdf", + isDirty: false, + isSaving: false, + diffStats: null, + wrapEnabled: false, + showDiffIndicators: false, + enableComments: false, + sessionId: "session-1", + commentCount: 0, + lspStatus: { state: "disabled" } as const, + lspProgress: { + initializingSince: null, + active: [], + completed: null, + hasReportedProgress: false, + }, + lspLanguage: null, + onToggleLsp: vi.fn(), + onToggleWrap: vi.fn(), + onToggleDiffIndicators: vi.fn(), + onSave: vi.fn(), + }; + + it("invokes onDownload when the download control is activated", async () => { + const onDownload = vi.fn(); + render( + + + , + ); + + const button = screen.getByRole("button", { name: "Download file" }); + button.click(); + expect(onDownload).toHaveBeenCalledTimes(1); + }); + + it("omits the download control when no handler is supplied", () => { + render( + + + , + ); + + expect(screen.queryByRole("button", { name: "Download file" })).toBeNull(); + }); +}); diff --git a/apps/web/components/editors/monaco/monaco-editor-toolbar.tsx b/apps/web/components/editors/monaco/monaco-editor-toolbar.tsx index 115c2ef5c4..bbc9a1db7f 100644 --- a/apps/web/components/editors/monaco/monaco-editor-toolbar.tsx +++ b/apps/web/components/editors/monaco/monaco-editor-toolbar.tsx @@ -6,6 +6,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { IconDeviceFloppy, IconLoader2, + IconDownload, IconTrash, IconTextWrap, IconTextWrapDisabled, @@ -209,6 +210,28 @@ function DeleteButton({ onDelete }: { onDelete?: () => void }) { ); } +function DownloadButton({ onDownload }: { onDownload?: () => void }) { + const { t } = useTranslation(); + if (!onDownload) return null; + + return ( + + + + + {t("editors:downloadFile")} + + ); +} + function MarkdownPreviewButton({ onTogglePreview }: { onTogglePreview: () => void }) { const { t } = useTranslation(); return ( @@ -253,6 +276,7 @@ interface MonacoEditorToolbarProps { onSave: () => void; onReloadFromAgent?: () => void; onDelete?: () => void; + onDownload?: () => void; onToggleMarkdownPreview?: () => void; } @@ -280,6 +304,7 @@ export function MonacoEditorToolbar({ onSave, onReloadFromAgent, onDelete, + onDownload, onToggleMarkdownPreview, }: MonacoEditorToolbarProps) { const fileStatus = useExternalVcsFileStatus(path, sessionId, repositoryName); @@ -331,6 +356,7 @@ export function MonacoEditorToolbar({ size="sm" /> +
diff --git a/apps/web/components/task/file-browser-apply-changes.test.ts b/apps/web/components/task/file-browser-apply-changes.test.ts index 9563143934..7edc2c3dfd 100644 --- a/apps/web/components/task/file-browser-apply-changes.test.ts +++ b/apps/web/components/task/file-browser-apply-changes.test.ts @@ -112,6 +112,20 @@ describe("applyFileChanges — refresh operation expands to all expanded folders expect(requestFileTreeMock.mock.calls.map((c) => c[2]).sort()).toEqual(["thm"]); }); + it("refreshes the nearest loaded ancestor for a new nested path", async () => { + mockEmptyTree(); + applyFileChanges({ + client: client(), + sessionId: SESSION_ID, + expandedPaths: new Set(), + changes: [{ path: "upload-bundle/nested/leaf.txt", operation: "create" }], + setTree: vi.fn(), + setLoadState: vi.fn(), + }); + await new Promise((r) => setTimeout(r, 0)); + expect(requestFileTreeMock.mock.calls.map((c) => c[2])).toEqual([""]); + }); + it("merges fresh children into the expanded subtree", async () => { requestFileTreeMock.mockImplementation((_c: unknown, _s: string, folder: string) => folder === "thm" diff --git a/apps/web/components/task/file-browser-header.tsx b/apps/web/components/task/file-browser-header.tsx index 2065fa07f3..4370b4010c 100644 --- a/apps/web/components/task/file-browser-header.tsx +++ b/apps/web/components/task/file-browser-header.tsx @@ -17,6 +17,7 @@ export type FileBrowserHeaderProps = { onOpenFolder: () => void; onCollapseAll: () => void; showCreateButton: boolean; + onUploadFiles?: (mode: "files" | "folder") => void; onAddSources?: (opener: HTMLButtonElement) => void; addSourcesButtonRef?: Ref; addSourcesDisabledReason?: string; @@ -34,6 +35,7 @@ export function FileBrowserHeader({ onOpenFolder, onCollapseAll, showCreateButton, + onUploadFiles, onAddSources, addSourcesButtonRef, addSourcesDisabledReason, @@ -62,6 +64,7 @@ export function FileBrowserHeader({ onStartSearch={() => search.setIsSearchActive(true)} onCollapseAll={onCollapseAll} showCreateButton={showCreateButton} + onUploadFiles={onUploadFiles} onAddSources={onAddSources} addSourcesButtonRef={addSourcesButtonRef} addSourcesDisabledReason={addSourcesDisabledReason} diff --git a/apps/web/components/task/file-browser-hooks.ts b/apps/web/components/task/file-browser-hooks.ts index a39a391589..30912b60bf 100644 --- a/apps/web/components/task/file-browser-hooks.ts +++ b/apps/web/components/task/file-browser-hooks.ts @@ -103,6 +103,16 @@ export function useFileBrowserSearch(sessionId: string) { }; } +function nearestExpandedFolder(parentPath: string, expandedPaths: ReadonlySet): string { + let candidate = parentPath; + while (candidate) { + if (expandedPaths.has(candidate)) return candidate; + const lastSlash = candidate.lastIndexOf("/"); + candidate = lastSlash === -1 ? "" : candidate.substring(0, lastSlash); + } + return ""; +} + /** Apply incoming file changes to the tree by refreshing affected folders. */ export function applyFileChanges(ctx: { client: ReturnType; @@ -128,7 +138,7 @@ export function applyFileChanges(ctx: { const p = change.path; const lastSlash = p.lastIndexOf("/"); const parent = lastSlash === -1 ? "" : p.substring(0, lastSlash); - if (parent === "" || expandedPaths.has(parent)) foldersToRefresh.add(parent); + foldersToRefresh.add(nearestExpandedFolder(parent, expandedPaths)); if (p === "" || expandedPaths.has(p)) foldersToRefresh.add(p); } if (foldersToRefresh.size === 0) { diff --git a/apps/web/components/task/file-browser-parts.tsx b/apps/web/components/task/file-browser-parts.tsx index f7ce9fb9ca..fe1eee167c 100644 --- a/apps/web/components/task/file-browser-parts.tsx +++ b/apps/web/components/task/file-browser-parts.tsx @@ -55,6 +55,7 @@ type TreeNodeRowProps = { onDeleteFile?: (path: string) => Promise; onRenameFile?: (oldPath: string, newPath: string) => Promise; onDownloadFile?: (path: string) => Promise; + onUploadFilesHere?: (path: string) => void; setTree: React.Dispatch>; isSelectedFn?: (path: string) => boolean; onSelect?: (path: string, e: React.MouseEvent) => boolean; @@ -236,6 +237,7 @@ export function TreeNodeItem(props: TreeNodeRowProps) { onDeleteFile, onRenameFile, onDownloadFile, + onUploadFilesHere, setTree, showTouchActions, onAddToChatContext, @@ -313,6 +315,7 @@ export function TreeNodeItem(props: TreeNodeRowProps) { onDeleteFile={onDeleteFile} onRenameFile={onRenameFile} onDownloadFile={onDownloadFile} + onUploadFilesHere={onUploadFilesHere} onStartRename={rename.handleStartRename} onAddToChatContext={onAddToChatContext} selectedCount={props.selectedCount} @@ -436,6 +439,7 @@ type FileBrowserContentAreaProps = { onDeleteFile?: (path: string) => Promise; onRenameFile?: (oldPath: string, newPath: string) => Promise; onDownloadFile?: (path: string) => Promise; + onUploadFilesHere?: (path: string) => void; onCreateFileSubmit: (parentPath: string, name: string) => void; onCancelCreate: () => void; onRetry: () => void; @@ -468,6 +472,7 @@ function rowToItemProps(props: FileBrowserContentAreaProps, row: FileBrowserRow) onDeleteFile: props.onDeleteFile, onRenameFile: props.onRenameFile, onDownloadFile: props.onDownloadFile, + onUploadFilesHere: props.onUploadFilesHere, setTree: props.setTree, isSelectedFn: props.isSelectedFn, onSelect: props.onSelect, diff --git a/apps/web/components/task/file-browser-toolbar.test.tsx b/apps/web/components/task/file-browser-toolbar.test.tsx index b92a18863c..6dc2ad1100 100644 --- a/apps/web/components/task/file-browser-toolbar.test.tsx +++ b/apps/web/components/task/file-browser-toolbar.test.tsx @@ -119,3 +119,99 @@ describe("FileBrowserToolbar workspace actions", () => { drawer.remove(); }); }); + +const CREATE_MENU_TESTID = "files-create-menu"; + +describe("FileBrowserToolbar create menu", () => { + const baseProps = { + displayPath: "workspace", + fullPath: "/workspace", + copied: false, + expandedPathsSize: 0, + onCopyPath: vi.fn(), + onOpenFolder: vi.fn(), + onStartSearch: vi.fn(), + onCollapseAll: vi.fn(), + }; + + it("keeps one-click New File when upload is unavailable", () => { + const onStartCreate = vi.fn(); + render( + + + , + ); + + fireEvent.click(screen.getByRole("button", { name: "New file" })); + expect(onStartCreate).toHaveBeenCalledTimes(1); + expect(screen.queryByTestId(CREATE_MENU_TESTID)).toBeNull(); + }); + + it("offers New File, Upload files, and Upload folder when upload is available", async () => { + const onStartCreate = vi.fn(); + const onUploadFiles = vi.fn(); + render( + + + , + ); + + const menuTrigger = screen.getByTestId(CREATE_MENU_TESTID); + fireEvent.pointerDown(menuTrigger, { button: 0, ctrlKey: false }); + fireEvent.click(menuTrigger); + await waitFor(() => expect(screen.getByRole("menuitem", { name: "New file" })).toBeTruthy()); + expect(screen.getByRole("menuitem", { name: "Upload files" })).toBeTruthy(); + expect(screen.getByRole("menuitem", { name: "Upload folder" })).toBeTruthy(); + }); + + it("still begins inline creation from the menu, unchanged", async () => { + const onStartCreate = vi.fn(); + render( + + + , + ); + + const menuTrigger = screen.getByTestId(CREATE_MENU_TESTID); + fireEvent.pointerDown(menuTrigger, { button: 0, ctrlKey: false }); + fireEvent.click(menuTrigger); + fireEvent.click(await screen.findByRole("menuitem", { name: "New file" })); + await waitFor(() => expect(onStartCreate).toHaveBeenCalledTimes(1)); + }); + + it("requests the right picker for each upload item", async () => { + const onUploadFiles = vi.fn(); + render( + + + , + ); + + const menuTrigger = screen.getByTestId(CREATE_MENU_TESTID); + fireEvent.pointerDown(menuTrigger, { button: 0, ctrlKey: false }); + fireEvent.click(menuTrigger); + fireEvent.click(await screen.findByRole("menuitem", { name: "Upload folder" })); + await waitFor(() => expect(onUploadFiles).toHaveBeenCalledWith("folder")); + + onUploadFiles.mockClear(); + fireEvent.pointerDown(menuTrigger, { button: 0, ctrlKey: false }); + fireEvent.click(menuTrigger); + fireEvent.click(await screen.findByRole("menuitem", { name: "Upload files" })); + await waitFor(() => expect(onUploadFiles).toHaveBeenCalledWith("files")); + }); +}); diff --git a/apps/web/components/task/file-browser-toolbar.tsx b/apps/web/components/task/file-browser-toolbar.tsx index 85f78c3d93..fe7e087133 100644 --- a/apps/web/components/task/file-browser-toolbar.tsx +++ b/apps/web/components/task/file-browser-toolbar.tsx @@ -7,7 +7,10 @@ import { IconFolderOpen, IconCopy, IconCheck, + IconFilePlus, + IconFolderUp, IconPlus, + IconUpload, IconDots, } from "@tabler/icons-react"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; @@ -59,6 +62,7 @@ type FileBrowserToolbarProps = { onStartSearch: () => void; onCollapseAll: () => void; showCreateButton: boolean; + onUploadFiles?: (mode: "files" | "folder") => void; onAddSources?: (opener: HTMLButtonElement) => void; addSourcesButtonRef?: Ref; addSourcesDisabledReason?: string; @@ -128,6 +132,71 @@ function useMobileDrawerFocusRestoration(triggerRef: RefObject void; + onUploadFiles?: (mode: "files" | "folder") => void; +}) { + const { t } = useTranslation(); + + // Without an upload handler there is only one action, so keep the original + // one-click button rather than burying New File behind a menu. + if (!onUploadFiles) { + return ( + } + /> + ); + } + + return ( + + + + + + + + {t("task:addToWorkspace")} + + + + + {t("task:newFile")} + + onUploadFiles("files")} + > + + {t("task:uploadFiles")} + + onUploadFiles("folder")} + > + + {t("task:uploadFolder")} + + + + ); +} + function WorkspaceActionsMenu({ onAddSources, onOpenFolder, @@ -225,6 +294,7 @@ export function FileBrowserToolbar({ onStartSearch, onCollapseAll, showCreateButton, + onUploadFiles, onAddSources, addSourcesButtonRef, addSourcesDisabledReason, @@ -258,11 +328,7 @@ export function FileBrowserToolbar({ right={ <> {showCreateButton && onStartCreate && ( - } - /> + )} Promise; onRenameFile?: (oldPath: string, newPath: string) => Promise; onDownloadFile?: (path: string) => Promise; + onUploadFilesHere?: (path: string) => void; activeFilePath?: string | null; onAddSources?: (opener: HTMLButtonElement) => void; addSourcesButtonRef?: Ref; @@ -462,6 +465,7 @@ function FileBrowserTreeContent({ onDeleteFile, onRenameFile, onDownloadFile, + onUploadFilesHere, showTouchActions, }: Omit & { scrollAreaRef: React.RefObject; @@ -494,6 +498,7 @@ function FileBrowserTreeContent({ onDeleteFile={onDeleteFile} onRenameFile={onRenameFile} onDownloadFile={onDownloadFile} + onUploadFilesHere={onUploadFilesHere} onAddToChatContext={handlers.handleAddToChatContext} showTouchActions={showTouchActions} onCreateFileSubmit={handlers.handleCreateFileSubmit} @@ -544,6 +549,15 @@ export function FileBrowser({ containerRef, }); const { openFolder, copied, copyPath, search, treeState, fullPath, displayPath } = data; + const upload = useFileUploadEntryPoints(sessionId); + const handleToolbarUpload = useCallback( + (mode: "files" | "folder") => upload.openPicker(mode, handlers.activeFolderPath ?? ""), + [upload, handlers.activeFolderPath], + ); + const handleUploadHere = useCallback( + (path: string) => upload.openPicker("files", path), + [upload], + ); return (
+ + {upload.elements}
); diff --git a/apps/web/components/task/file-context-menu-items.tsx b/apps/web/components/task/file-context-menu-items.tsx new file mode 100644 index 0000000000..63adbf5cf3 --- /dev/null +++ b/apps/web/components/task/file-context-menu-items.tsx @@ -0,0 +1,81 @@ +"use client"; + +import { IconDownload, IconPencil, IconTrash, IconUpload } from "@tabler/icons-react"; +import { ContextMenuItem, ContextMenuSeparator } from "@kandev/ui/context-menu"; +import { useTranslation } from "react-i18next"; +import type { FileTreeNode } from "@/lib/types/backend"; + +export type FileContextMenuItemsProps = { + node: FileTreeNode; + isBulk: boolean; + selectedCount: number; + onDeleteFile?: (path: string) => Promise; + onRenameFile?: (oldPath: string, newPath: string) => Promise; + onDownloadFile?: (path: string) => Promise; + onUploadFilesHere?: (path: string) => void; + onStartRename: () => void; + onDelete: (event: Event) => void; +}; + +/** + * Which node-scoped actions apply, resolved once so the render stays flat. + * + * Download and upload are exact inverses: a file can be downloaded, a folder can + * receive an upload, and a multi-selection supports neither because it has no + * single subject or destination. + */ +function resolveActions({ + node, + isBulk, + onRenameFile, + onDownloadFile, + onUploadFilesHere, +}: FileContextMenuItemsProps) { + return { + showRename: !!onRenameFile && !isBulk, + download: !node.is_dir && !isBulk ? onDownloadFile : undefined, + upload: node.is_dir && !isBulk ? onUploadFilesHere : undefined, + }; +} + +export function FileContextMenuItems(props: FileContextMenuItemsProps) { + const { node, isBulk, selectedCount, onDeleteFile, onStartRename, onDelete } = props; + const { t } = useTranslation(); + const { showRename, download, upload } = resolveActions(props); + const deleteLabel = isBulk + ? t("task:deleteItemsLabel", { count: selectedCount }) + : t("task:delete"); + const hasLeadingItem = showRename || !!onDeleteFile; + + return ( + <> + {onDeleteFile && ( + + + {deleteLabel} + + )} + {showRename && onDeleteFile && } + {showRename && ( + + + {t("task:rename")} + + )} + {download && hasLeadingItem && } + {download && ( + void download(node.path)}> + + {t("task:download")} + + )} + {upload && hasLeadingItem && } + {upload && ( + upload(node.path)}> + + {t("task:uploadFilesHere")} + + )} + + ); +} diff --git a/apps/web/components/task/file-context-menu.test.tsx b/apps/web/components/task/file-context-menu.test.tsx index f61914b661..0c50e4db0e 100644 --- a/apps/web/components/task/file-context-menu.test.tsx +++ b/apps/web/components/task/file-context-menu.test.tsx @@ -436,3 +436,50 @@ describe("FileContextMenu touch actions", () => { await waitFor(() => expect(onDeleteFile).toHaveBeenCalledWith(FILE_NODE.path)); }); }); + +describe("FileContextMenu upload guard", () => { + const UPLOAD_LABEL = "Upload files here"; + const ROW = "upload-guard-row"; + + function renderFor(node: FileTreeNode, selectedCount = 1) { + const onUploadFilesHere = vi.fn(); + render( + 1 ? new Set(["a.txt", "b.txt"]) : undefined} + > +
row
+ , + ); + openMenu(ROW); + return { onUploadFilesHere }; + } + + it("offers upload on a folder and targets that folder", async () => { + const { onUploadFilesHere } = renderFor(DIR_NODE); + + const item = await screen.findByRole("menuitem", { name: UPLOAD_LABEL }); + fireEvent.click(item); + await waitFor(() => expect(onUploadFilesHere).toHaveBeenCalledWith("src")); + }); + + it("is absent for a file, the exact inverse of the download guard", async () => { + renderFor(FILE_NODE); + + await screen.findByRole("menuitem", { name: "Delete" }); + expect(screen.queryByRole("menuitem", { name: UPLOAD_LABEL })).toBeNull(); + }); + + it("is absent for a multi-selection, which has no single destination", async () => { + renderFor(DIR_NODE, 2); + + await waitFor(() => expect(screen.queryAllByRole("menuitem").length).toBeGreaterThan(0)); + expect(screen.queryByRole("menuitem", { name: UPLOAD_LABEL })).toBeNull(); + }); +}); diff --git a/apps/web/components/task/file-context-menu.tsx b/apps/web/components/task/file-context-menu.tsx index 16ba575a99..212b7fa775 100644 --- a/apps/web/components/task/file-context-menu.tsx +++ b/apps/web/components/task/file-context-menu.tsx @@ -2,7 +2,7 @@ import React, { useState, useCallback, useRef, useEffect } from "react"; import { Input } from "@kandev/ui/input"; -import { IconDownload, IconMessageDots, IconPencil, IconTrash } from "@tabler/icons-react"; +import { IconMessageDots } from "@tabler/icons-react"; import { AlertDialog } from "@kandev/ui/alert-dialog"; import { ContextMenu, @@ -17,6 +17,7 @@ import { ActionConfirmPopover } from "@/components/confirmation/action-confirm-p import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint"; import { useTranslation } from "react-i18next"; import type { FileTreeNode } from "@/lib/types/backend"; +import { FileContextMenuItems } from "./file-context-menu-items"; import type { FileInfo } from "@/lib/state/store"; import { removeNodeFromTree, @@ -85,59 +86,6 @@ function removeSuccessfullyDeletedPaths( return nextTree; } -type FileContextMenuItemsProps = { - node: FileTreeNode; - isBulk: boolean; - selectedCount: number; - onDeleteFile?: (path: string) => Promise; - onRenameFile?: (oldPath: string, newPath: string) => Promise; - onDownloadFile?: (path: string) => Promise; - onStartRename: () => void; - onDelete: (event: Event) => void; -}; - -function FileContextMenuItems({ - node, - isBulk, - selectedCount, - onDeleteFile, - onRenameFile, - onDownloadFile, - onStartRename, - onDelete, -}: FileContextMenuItemsProps) { - const { t } = useTranslation(); - const deleteLabel = isBulk - ? t("task:deleteItemsLabel", { count: selectedCount }) - : t("task:delete"); - const showRename = !!onRenameFile && !isBulk; - const download = !node.is_dir && !isBulk ? onDownloadFile : undefined; - return ( - <> - {onDeleteFile && ( - - - {deleteLabel} - - )} - {showRename && onDeleteFile && } - {showRename && ( - - - {t("task:rename")} - - )} - {download && (showRename || onDeleteFile) && } - {download && ( - void download(node.path)}> - - {t("task:download")} - - )} - - ); -} - function ChatContextMenuItem({ node, onAddToChatContext, @@ -256,6 +204,7 @@ type FileContextMenuSurfaceProps = { onDeleteFile?: (path: string) => Promise; onRenameFile?: (oldPath: string, newPath: string) => Promise; onDownloadFile?: (path: string) => Promise; + onUploadFilesHere?: (path: string) => void; onStartRename: () => void; onAddToChatContext?: (node: FileTreeNode) => void; selectedCount: number; @@ -279,6 +228,7 @@ function FileContextMenuSurface({ onDeleteFile, onRenameFile, onDownloadFile, + onUploadFilesHere, onStartRename, onAddToChatContext, selectedCount, @@ -329,6 +279,7 @@ function FileContextMenuSurface({ onDeleteFile={onDeleteFile} onRenameFile={onRenameFile} onDownloadFile={onDownloadFile} + onUploadFilesHere={onUploadFilesHere} onStartRename={handleStartRename} onDelete={onDelete} /> @@ -361,6 +312,51 @@ function FileContextMenuSurface({ } /** Context menu for file nodes with Download, Rename, and Delete options */ +/** + * Assemble the confirm-delete descriptor. Extracted so `FileContextMenu` stays + * within the component size and complexity limits. + */ +function useDeleteAction({ + node, + isBulk, + selectedCount, + enabled, + confirming, + handleDelete, + handleConfirmDelete, + setDeleteConfirmationOpen, +}: { + node: FileTreeNode; + isBulk: boolean; + selectedCount: number; + enabled: boolean; + confirming: boolean; + handleDelete: () => void; + handleConfirmDelete: () => void; + setDeleteConfirmationOpen: (open: boolean) => void; +}) { + const { t } = useTranslation(); + if (!enabled) return null; + return { + isBulk, + selectedCount, + confirming, + title: isBulk + ? t("task:deleteItemsTitle", { count: selectedCount }) + : t("task:delete2", { name: node.name }), + label: isBulk ? t("task:deleteItemsLabel", { count: selectedCount }) : t("task:delete"), + cancelLabel: t("common:cancel"), + description: node.is_dir ? ( + + ) : ( + + ), + onDelete: handleDelete, + onCancel: () => setDeleteConfirmationOpen(false), + onConfirm: handleConfirmDelete, + }; +} + export function FileContextMenu({ children, node, @@ -369,6 +365,7 @@ export function FileContextMenu({ onDeleteFile, onRenameFile, onDownloadFile, + onUploadFilesHere, onStartRename, onAddToChatContext, selectedCount = 0, @@ -382,6 +379,7 @@ export function FileContextMenu({ onDeleteFile?: (path: string) => Promise; onRenameFile?: (oldPath: string, newPath: string) => Promise; onDownloadFile?: (path: string) => Promise; + onUploadFilesHere?: (path: string) => void; onStartRename: () => void; onAddToChatContext?: (node: FileTreeNode) => void; selectedCount?: number; @@ -390,11 +388,11 @@ export function FileContextMenu({ }) { const editorActions = useFileTreeEditorActions(); const { isFinePointer } = useResponsiveBreakpoint(); - const { t } = useTranslation(); const isBulk = selectedCount > 1; // A bulk selection would make a single-node "Open in " ambiguous. const showOpenInEditor = !isBulk && canOpenNodeInEditor(editorActions, node); - const hasFileActions = !!onDeleteFile || !!onRenameFile || !!onDownloadFile; + const hasFileActions = + !!onDeleteFile || !!onRenameFile || !!onDownloadFile || !!onUploadFilesHere; const showAddToChatContext = !isBulk && !!onAddToChatContext; const fallbackAnchorRef = useRef(null); const anchorRef = providedAnchorRef ?? fallbackAnchorRef; @@ -415,26 +413,16 @@ export function FileContextMenu({ contextMenuRef, }); - const deleteAction = onDeleteFile - ? { - isBulk, - selectedCount, - confirming: deleteConfirmationOpen, - title: isBulk - ? t("task:deleteItemsTitle", { count: selectedCount }) - : t("task:delete2", { name: node.name }), - label: isBulk ? t("task:deleteItemsLabel", { count: selectedCount }) : t("task:delete"), - cancelLabel: t("common:cancel"), - description: node.is_dir ? ( - - ) : ( - - ), - onDelete: handleDelete, - onCancel: () => setDeleteConfirmationOpen(false), - onConfirm: handleConfirmDelete, - } - : null; + const deleteAction = useDeleteAction({ + node, + isBulk, + selectedCount, + enabled: !!onDeleteFile, + confirming: deleteConfirmationOpen, + handleDelete, + handleConfirmDelete, + setDeleteConfirmationOpen, + }); if (!hasFileActions && !showOpenInEditor && !showAddToChatContext) return <>{children}; @@ -444,6 +432,7 @@ export function FileContextMenu({ onDeleteFile={onDeleteFile} onRenameFile={onRenameFile} onDownloadFile={onDownloadFile} + onUploadFilesHere={onUploadFilesHere} onStartRename={onStartRename} onAddToChatContext={onAddToChatContext} selectedCount={selectedCount} diff --git a/apps/web/components/task/file-editor-content.tsx b/apps/web/components/task/file-editor-content.tsx index ef86035916..a9c7427574 100644 --- a/apps/web/components/task/file-editor-content.tsx +++ b/apps/web/components/task/file-editor-content.tsx @@ -26,6 +26,7 @@ export type FileEditorContentProps = { onSave: () => void; onReloadFromAgent?: () => void; onDelete?: () => void; + onDownload?: () => void; }; export const FileEditorContent = memo(function FileEditorContent(props: FileEditorContentProps) { @@ -42,6 +43,7 @@ export const FileEditorContent = memo(function FileEditorContent(props: FileEdit repositoryId={props.repositoryId} repositoryName={props.repo} enableComments={props.enableComments} + onDownload={props.onDownload} onTogglePreview={props.onToggleMarkdownPreview} /> ); diff --git a/apps/web/components/task/file-editor-panel.download.test.tsx b/apps/web/components/task/file-editor-panel.download.test.tsx new file mode 100644 index 0000000000..74e04d423f --- /dev/null +++ b/apps/web/components/task/file-editor-panel.download.test.tsx @@ -0,0 +1,102 @@ +import { cleanup, render, screen } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +const triggerFileDownload = vi.fn(); + +vi.mock("@/lib/utils/file-download", () => ({ + triggerFileDownload: (...args: unknown[]) => triggerFileDownload(...args), + fileBasename: (p: string) => p.split("/").pop() ?? p, +})); + +vi.mock("@/components/editors/external-vcs-file-link", () => ({ + ExternalVcsFileLink: () => , + useExternalVcsFileStatus: () => null, +})); + +import { FileViewerDownloadButton } from "./file-viewer-header"; +import { FileBinaryViewer } from "./file-binary-viewer"; +import { FileImageViewer } from "./file-image-viewer"; + +const DOWNLOAD_LABEL = "Download file"; + +afterEach(() => { + cleanup(); + triggerFileDownload.mockReset(); +}); + +describe("FileViewerDownloadButton", () => { + it("renders nothing without a handler", () => { + render( + + + , + ); + expect(screen.queryByRole("button", { name: DOWNLOAD_LABEL })).toBeNull(); + }); + + it("invokes the handler when activated", () => { + const onDownload = vi.fn(); + render( + + + , + ); + screen.getByRole("button", { name: DOWNLOAD_LABEL }).click(); + expect(onDownload).toHaveBeenCalledTimes(1); + }); +}); + +describe("viewer headerActions contract", () => { + it("surfaces the download control on the unpreviewable-file screen", () => { + const onDownload = vi.fn(); + render( + + } + /> + , + ); + + screen.getByRole("button", { name: DOWNLOAD_LABEL }).click(); + expect(onDownload).toHaveBeenCalledTimes(1); + }); + + it("surfaces the download control on the image viewer", () => { + const onDownload = vi.fn(); + render( + + } + /> + , + ); + + screen.getByRole("button", { name: DOWNLOAD_LABEL }).click(); + expect(onDownload).toHaveBeenCalledTimes(1); + }); +}); + +describe("binary download fidelity", () => { + it("passes base64 content through as binary so the bytes survive", () => { + const content = "iVBORw0KGgo="; + const onDownload = () => + triggerFileDownload({ fileName: "assets/logo.png", content, isBinary: true }); + + render( + + + , + ); + screen.getByRole("button", { name: DOWNLOAD_LABEL }).click(); + + expect(triggerFileDownload).toHaveBeenCalledWith({ + fileName: "assets/logo.png", + content, + isBinary: true, + }); + }); +}); diff --git a/apps/web/components/task/file-editor-panel.image.test.tsx b/apps/web/components/task/file-editor-panel.image.test.tsx index 7f03b94fd7..fe39badacd 100644 --- a/apps/web/components/task/file-editor-panel.image.test.tsx +++ b/apps/web/components/task/file-editor-panel.image.test.tsx @@ -1,4 +1,5 @@ import { act, cleanup, render, screen } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { useDockviewStore, type FileEditorState } from "@/lib/state/dockview-store"; import { buildRepoScopedItemId } from "@/lib/state/dockview-panel-actions"; @@ -98,7 +99,9 @@ describe("FileEditorPanel image preview", () => { it("updates displayed image content when a reused preview tab switches files", async () => { act(() => seedImage("docs/first.png", "first-image")); const { rerender } = render( - , + + + , ); expect(screen.getByTestId(IMAGE_VIEWER_TEST_ID).textContent).toBe("first-image"); @@ -107,7 +110,11 @@ describe("FileEditorPanel image preview", () => { }); act(() => seedImage("docs/second.png", "second-image")); - rerender(); + rerender( + + + , + ); expect(screen.getByTestId(IMAGE_VIEWER_TEST_ID).textContent).toBe("second-image"); }); @@ -118,19 +125,23 @@ describe("FileEditorPanel image preview", () => { seedImage(SHARED_IMAGE_PATH, REPO_B_CONTENT, "repo-b"); }); const { rerender } = render( - , + + + , ); expect(screen.getByTestId(IMAGE_VIEWER_TEST_ID).textContent).toBe(REPO_A_CONTENT); rerender( - , + + + , ); expect(screen.getByTestId(IMAGE_VIEWER_TEST_ID).textContent).toBe(REPO_B_CONTENT); @@ -140,10 +151,12 @@ describe("FileEditorPanel image preview", () => { act(() => seedImage(SHARED_IMAGE_PATH, REPO_A_CONTENT, "repo-a")); render( - , + + + , ); const props = JSON.parse( @@ -163,10 +176,12 @@ describe("FileEditorPanel image preview", () => { act(() => seedImage(SHARED_IMAGE_PATH, REPO_A_CONTENT, "repo-a")); render( - , + + + , ); expect(screen.getByTestId(IMAGE_VIEWER_TEST_ID).getAttribute("data-worktree-path")).toBe( diff --git a/apps/web/components/task/file-editor-panel.tsx b/apps/web/components/task/file-editor-panel.tsx index 9fe2ceaf35..82b49bc7b0 100644 --- a/apps/web/components/task/file-editor-panel.tsx +++ b/apps/web/components/task/file-editor-panel.tsx @@ -1,8 +1,8 @@ "use client"; -import { memo, useCallback, useEffect, useRef } from "react"; +import { memo, useCallback, useEffect, useMemo, useRef } from "react"; import { PanelRoot, PanelBody } from "./panel-primitives"; -import { FileEditorContent } from "./file-editor-content"; +import { FileEditorContent, type FileEditorContentProps } from "./file-editor-content"; import { FileImageViewer } from "./file-image-viewer"; import { FileBinaryViewer } from "./file-binary-viewer"; import { useAppStore } from "@/components/state-provider"; @@ -16,7 +16,8 @@ import { calculateHash } from "@/lib/utils/file-diff"; import { panelPortalManager } from "@/lib/layout/panel-portal-manager"; import { syncOpenFileFromWorkspace } from "@/hooks/file-editors-sync"; import { buildRepoScopedItemId } from "@/lib/state/dockview-panel-actions"; -import { FileViewerExternalLink } from "./file-viewer-header"; +import { FileViewerDownloadButton, FileViewerExternalLink } from "./file-viewer-header"; +import { triggerFileDownload } from "@/lib/utils/file-download"; import { getSessionWorkspacePath } from "@/lib/session-workspace-path"; import { useTranslation } from "react-i18next"; @@ -64,6 +65,78 @@ type StaticFilePanelProps = { repositoryName?: string; }; +/** + * Download the open file from the copy already in the dockview store. + * + * The viewer only renders after `useFileLoader` resolves, so the bytes are + * always in hand here and no refetch is needed. Binary content is base64, + * matching the `workspace.file.get` contract `triggerFileDownload` expects. + */ +function useOpenFileDownload(fileKey: string, path: string): (() => void) | undefined { + const hasFile = useDockviewStore((s) => s.openFiles.has(fileKey)); + const content = useDockviewStore((s) => s.openFiles.get(fileKey)?.content ?? ""); + const isBinary = useDockviewStore((s) => s.openFiles.get(fileKey)?.isBinary ?? false); + + return useMemo(() => { + if (!hasFile) return undefined; + return () => triggerFileDownload({ fileName: path, content, isBinary }); + }, [hasFile, path, content, isBinary]); +} + +function useLoadedFileDownload( + hasFile: boolean, + originalHash: string, + path: string, + content: string, + isBinary: boolean, +): (() => void) | undefined { + return useMemo( + () => + hasFile && originalHash + ? () => triggerFileDownload({ fileName: path, content, isBinary }) + : undefined, + [hasFile, originalHash, path, content, isBinary], + ); +} + +type FileEditorPanelActions = Pick< + ReturnType, + "handleFileChange" | "saveFile" | "deleteFile" | "applyRemoteUpdate" +>; + +function useFileEditorPanelActions({ + path, + repo, + fileKey, + markdownPreview, + updateFileState, + actions, +}: { + path: string; + repo: string | undefined; + fileKey: string; + markdownPreview: boolean; + updateFileState: (path: string, updates: Partial) => void; + actions: FileEditorPanelActions; +}) { + const { handleFileChange, saveFile, deleteFile, applyRemoteUpdate } = actions; + const onChange = useCallback( + (newContent: string) => handleFileChange(path, newContent, repo), + [handleFileChange, path, repo], + ); + const onSave = useCallback(() => saveFile(path, repo), [saveFile, path, repo]); + const onReloadFromAgent = useCallback( + () => applyRemoteUpdate(path, repo), + [applyRemoteUpdate, path, repo], + ); + const onDelete = useCallback(() => deleteFile(path, repo), [deleteFile, path, repo]); + const onToggleMarkdownPreview = useCallback( + () => updateFileState(fileKey, { markdownPreview: !markdownPreview }), + [updateFileState, fileKey, markdownPreview], + ); + return { onChange, onSave, onReloadFromAgent, onDelete, onToggleMarkdownPreview }; +} + function StaticFilePanel({ category, fileKey, @@ -74,14 +147,18 @@ function StaticFilePanel({ repositoryId, repositoryName, }: StaticFilePanelProps) { + const onDownload = useOpenFileDownload(fileKey, path); const headerActions = ( - + <> + + + ); if (category === "image") { return ( @@ -249,6 +326,7 @@ function useFileEditorBuffer(fileKey: string) { ); const isBinary = useDockviewStore((s) => s.openFiles.get(fileKey)?.isBinary ?? false); const originalContent = useDockviewStore((s) => s.openFiles.get(fileKey)?.originalContent ?? ""); + const originalHash = useDockviewStore((s) => s.openFiles.get(fileKey)?.originalHash ?? ""); const markdownPreview = useDockviewStore( (s) => s.openFiles.get(fileKey)?.markdownPreview ?? false, ); @@ -259,6 +337,7 @@ function useFileEditorBuffer(fileKey: string) { hasRemoteUpdate, isBinary, originalContent, + originalHash, markdownPreview, }; } @@ -278,6 +357,52 @@ function LoadingFilePanel() { ); } +type LoadedFilePanelProps = { + category: FileCategory; + fileKey: string; + path: string; + worktreePath: string | undefined; + sessionId: string | null; + taskId: string | null; + repositoryId: string | undefined; + repositoryName: string | undefined; + editorProps: FileEditorContentProps; +}; + +function LoadedFilePanel({ + category, + fileKey, + path, + worktreePath, + sessionId, + taskId, + repositoryId, + repositoryName, + editorProps, +}: LoadedFilePanelProps) { + if (category !== "text") { + return ( + + ); + } + return ( + + + + + + ); +} + export const FileEditorPanel = memo(function FileEditorPanel({ panelId, params, @@ -286,8 +411,16 @@ export const FileEditorPanel = memo(function FileEditorPanel({ const repo = params.repo as string | undefined; const fileKey = buildRepoScopedItemId(path, repo); - const { hasFile, content, isDirty, hasRemoteUpdate, isBinary, originalContent, markdownPreview } = - useFileEditorBuffer(fileKey); + const { + hasFile, + content, + isDirty, + hasRemoteUpdate, + isBinary, + originalContent, + originalHash, + markdownPreview, + } = useFileEditorBuffer(fileKey); const setFileState = useDockviewStore((s) => s.setFileState); const updateFileState = useDockviewStore((s) => s.updateFileState); @@ -311,70 +444,63 @@ export const FileEditorPanel = memo(function FileEditorPanel({ updateFileState, }); - const onChange = useCallback( - (newContent: string) => handleFileChange(path, newContent, repo), - [handleFileChange, path, repo], - ); - const onSave = useCallback(() => saveFile(path, repo), [saveFile, path, repo]); - const onReloadFromAgent = useCallback( - () => applyRemoteUpdate(path, repo), - [applyRemoteUpdate, path, repo], - ); - const onDelete = useCallback(() => deleteFile(path, repo), [deleteFile, path, repo]); - const onToggleMarkdownPreview = useCallback( - () => updateFileState(fileKey, { markdownPreview: !markdownPreview }), - [updateFileState, fileKey, markdownPreview], - ); + const { onChange, onSave, onReloadFromAgent, onDelete, onToggleMarkdownPreview } = + useFileEditorPanelActions({ + path, + repo, + fileKey, + markdownPreview, + updateFileState, + actions: { + handleFileChange, + saveFile, + deleteFile, + applyRemoteUpdate, + }, + }); + const onDownload = useLoadedFileDownload(hasFile, originalHash, path, content, isBinary); - if (!hasFile) { + if (!hasFile || !originalHash) { return ; } const worktreePath = getSessionWorkspacePath(activeSession); const repositoryId = activeSession?.repository_id ?? undefined; const category = resolveFileCategory(isBinary, path); - if (category !== "text") { - return ( - - ); - } - const isMarkdown = isMarkdownFile(path); return ( - - - - - + ); }); diff --git a/apps/web/components/task/file-upload-conflict-dialog.test.tsx b/apps/web/components/task/file-upload-conflict-dialog.test.tsx new file mode 100644 index 0000000000..3e165bc917 --- /dev/null +++ b/apps/web/components/task/file-upload-conflict-dialog.test.tsx @@ -0,0 +1,127 @@ +import { cleanup, fireEvent, render, screen, within } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { FileUploadConflictDialog } from "./file-upload-conflict-dialog"; +import type { ConflictChoice, PendingConflicts } from "@/hooks/use-file-upload"; + +afterEach(cleanup); + +const KEEP_BOTH_LABEL = "Keep both"; +const REPLACE_LABEL = "Replace"; +const SKIP_LABEL = "Skip"; +const UPLOAD_LABEL = "Upload"; +const APPLY_TO_ALL = "Apply to all"; +const NOT_PRESSED = "false"; +const ARIA_PRESSED = "aria-pressed"; +const A_TXT = "fixtures/a.txt"; +const B_TXT = "fixtures/b.txt"; + +function pendingFor(paths: string[]): PendingConflicts { + return { + conflicts: paths.map((path) => ({ path, is_dir: false })), + byDestination: new Map(paths.map((path) => [path, path])), + }; +} + +function renderDialog(paths: string[]) { + const onResolve = vi.fn(); + const onCancel = vi.fn(); + render( + , + ); + return { onResolve, onCancel }; +} + +function choicesFrom(onResolve: ReturnType): Map { + return onResolve.mock.calls[0][0] as Map; +} + +function clickIn(groupLabel: string, buttonLabel: string) { + const group = screen.getByRole("group", { name: groupLabel }); + fireEvent.click(within(group).getByRole("button", { name: buttonLabel })); +} + +describe("FileUploadConflictDialog", () => { + it("renders nothing when there is no parked batch", () => { + render(); + expect(screen.queryByRole("dialog")).toBeNull(); + }); + + it("lists every conflicting path", () => { + renderDialog([A_TXT, "fixtures/deep/b.json"]); + + expect(screen.getByText(A_TXT)).toBeTruthy(); + expect(screen.getByText("fixtures/deep/b.json")).toBeTruthy(); + }); + + it("defaults to the non-destructive choice", () => { + const { onResolve } = renderDialog([A_TXT]); + + fireEvent.click(screen.getByRole("button", { name: UPLOAD_LABEL })); + + expect(onResolve).toHaveBeenCalledTimes(1); + expect(choicesFrom(onResolve).get(A_TXT)).toBe("keep_both"); + }); + + it("applies one choice to every remaining conflict", () => { + const { onResolve } = renderDialog([A_TXT, B_TXT, "fixtures/c.txt"]); + + clickIn(APPLY_TO_ALL, REPLACE_LABEL); + fireEvent.click(screen.getByRole("button", { name: UPLOAD_LABEL })); + + expect([...choicesFrom(onResolve).values()]).toEqual(["replace", "replace", "replace"]); + }); + + it("keeps a per-file override after an apply-to-all", () => { + const { onResolve } = renderDialog([A_TXT, B_TXT]); + + clickIn(APPLY_TO_ALL, SKIP_LABEL); + clickIn(A_TXT, KEEP_BOTH_LABEL); + fireEvent.click(screen.getByRole("button", { name: UPLOAD_LABEL })); + + const choices = choicesFrom(onResolve); + expect(choices.get(A_TXT)).toBe("keep_both"); + expect(choices.get(B_TXT)).toBe("skip"); + }); + + it("does not show an apply-to-all choice when per-file choices differ", () => { + renderDialog([A_TXT, B_TXT]); + + clickIn(A_TXT, REPLACE_LABEL); + + const applyGroup = screen.getByRole("group", { name: APPLY_TO_ALL }); + expect( + within(applyGroup).getByRole("button", { name: KEEP_BOTH_LABEL }).getAttribute(ARIA_PRESSED), + ).toBe(NOT_PRESSED); + expect( + within(applyGroup).getByRole("button", { name: REPLACE_LABEL }).getAttribute(ARIA_PRESSED), + ).toBe(NOT_PRESSED); + }); + + it("marks the active choice for assistive technology", () => { + renderDialog([A_TXT]); + + const group = screen.getByRole("group", { name: A_TXT }); + expect( + within(group).getByRole("button", { name: KEEP_BOTH_LABEL }).getAttribute(ARIA_PRESSED), + ).toBe("true"); + + clickIn(A_TXT, SKIP_LABEL); + expect(within(group).getByRole("button", { name: SKIP_LABEL }).getAttribute(ARIA_PRESSED)).toBe( + "true", + ); + }); + + it("reports cancellation without resolving anything", () => { + const { onResolve, onCancel } = renderDialog([A_TXT]); + + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + + expect(onCancel).toHaveBeenCalledTimes(1); + expect(onResolve).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/components/task/file-upload-conflict-dialog.tsx b/apps/web/components/task/file-upload-conflict-dialog.tsx new file mode 100644 index 0000000000..c0d9d3544e --- /dev/null +++ b/apps/web/components/task/file-upload-conflict-dialog.tsx @@ -0,0 +1,170 @@ +"use client"; + +import { useCallback, useMemo, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; +import { Button } from "@kandev/ui/button"; +import { cn } from "@/lib/utils"; +import type { ConflictChoice, PendingConflicts } from "@/hooks/use-file-upload"; + +// Persisted decisions that cross the wire, never displayed. +const KEEP_BOTH: ConflictChoice = "keep_both"; // i18n-exempt: wire value +const REPLACE: ConflictChoice = "replace"; // i18n-exempt: wire value +const SKIP: ConflictChoice = "skip"; // i18n-exempt: wire value + +// Keep both leads because it is the only non-destructive option. +const CHOICES: ConflictChoice[] = [KEEP_BOTH, REPLACE, SKIP]; + +type FileUploadConflictDialogProps = { + pending: PendingConflicts | null; + onResolve: (choices: Map) => void; + onCancel: () => void; +}; + +function ChoiceGroup({ + labels, + value, + ariaLabel, + onChange, +}: { + labels: Record; + value?: ConflictChoice; + ariaLabel: string; + onChange: (choice: ConflictChoice) => void; +}) { + return ( +
+ {CHOICES.map((choice) => ( + + ))} +
+ ); +} + +/** + * Collects a per-file decision for every conflicting destination before any + * byte is uploaded. + * + * Keep both is the default because it is the only non-destructive option; + * Replace is irreversible and there is no version history behind it. + */ +export function FileUploadConflictDialog({ + pending, + onResolve, + onCancel, +}: FileUploadConflictDialogProps) { + const { t } = useTranslation(); + const [choices, setChoices] = useState>(new Map()); + + const labels = useMemo>( + () => ({ + keep_both: t("task:uploadConflictKeepBoth"), + replace: t("task:uploadConflictReplace"), + skip: t("task:uploadConflictSkip"), + }), + [t], + ); + + const paths = useMemo( + () => (pending ? pending.conflicts.map((conflict) => conflict.path) : []), + [pending], + ); + + const choiceFor = useCallback( + (path: string): ConflictChoice => choices.get(path) ?? KEEP_BOTH, + [choices], + ); + + const applyToAllChoice = useMemo(() => { + if (paths.length === 0) return KEEP_BOTH; + const first = choiceFor(paths[0]); + return paths.every((path) => choiceFor(path) === first) ? first : undefined; + }, [paths, choiceFor]); + + const handleConfirm = useCallback(() => { + onResolve(new Map(paths.map((path) => [path, choices.get(path) ?? KEEP_BOTH]))); + setChoices(new Map()); + }, [onResolve, paths, choices]); + + const handleCancel = useCallback(() => { + setChoices(new Map()); + onCancel(); + }, [onCancel]); + + if (!pending) return null; + + return ( + !open && handleCancel()}> + + + {t("task:uploadConflictTitle", { count: paths.length })} + {t("task:uploadConflictBody")} + + +
+ + {t("task:uploadConflictApplyToAll")} + + setChoices(new Map(paths.map((path) => [path, choice])))} + /> +
+ +
    + {paths.map((path) => ( +
  • + + {path} + + setChoices((prev) => new Map(prev).set(path, choice))} + /> +
    • + ))} +
+ + + + + + + + ); +} diff --git a/apps/web/components/task/file-upload-status-list.tsx b/apps/web/components/task/file-upload-status-list.tsx new file mode 100644 index 0000000000..361d47c5c4 --- /dev/null +++ b/apps/web/components/task/file-upload-status-list.tsx @@ -0,0 +1,50 @@ +"use client"; + +import { useTranslation } from "react-i18next"; +import type { TFunction } from "i18next"; +import type { UploadItem, UploadStatus } from "@/hooks/use-file-upload"; + +function uploadStatusLabel(status: UploadStatus, t: TFunction): string { + switch (status) { + case "pending": + return t("common:pending"); + case "blocked": + return t("task:blocked"); + case "uploading": + return t("task:loading"); + case "ready": + return t("task:done"); + case "failed": + return t("task:failed"); + } +} + +export function FileUploadStatusList({ uploads }: { uploads: UploadItem[] }) { + const { t } = useTranslation(); + if (uploads.length === 0) return null; + return ( +
+
+ {uploads.map((upload) => ( +
+ + {upload.destinationPath} + + + {uploadStatusLabel(upload.status, t)} + + {upload.error && ( + + {upload.error} + + )} +
+ ))} +
+
+ ); +} diff --git a/apps/web/components/task/file-viewer-header.tsx b/apps/web/components/task/file-viewer-header.tsx index e268633774..7685120495 100644 --- a/apps/web/components/task/file-viewer-header.tsx +++ b/apps/web/components/task/file-viewer-header.tsx @@ -1,6 +1,10 @@ "use client"; import type { ReactNode } from "react"; +import { useTranslation } from "react-i18next"; +import { IconDownload } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { toRelativePath } from "@/lib/utils"; import { ExternalVcsFileLink, @@ -63,3 +67,34 @@ export function FileViewerExternalLink({ /> ); } + +type FileViewerDownloadButtonProps = { + onDownload?: () => void; +}; + +/** + * Download control for the viewer header. Shared by the binary and image + * viewers so both screens, and the mobile viewer that renders the same + * `headerActions`, gain the action from one place. + */ +export function FileViewerDownloadButton({ onDownload }: FileViewerDownloadButtonProps) { + const { t } = useTranslation(); + if (!onDownload) return null; + + return ( + + + + + {t("editors:downloadFile")} + + ); +} diff --git a/apps/web/components/task/markdown-preview-content.tsx b/apps/web/components/task/markdown-preview-content.tsx index 489ee52765..57fc1866c0 100644 --- a/apps/web/components/task/markdown-preview-content.tsx +++ b/apps/web/components/task/markdown-preview-content.tsx @@ -28,6 +28,7 @@ import { import { ResizableMarkdownTable } from "@/components/shared/resizable-markdown-table"; import { cn, toRelativePath } from "@/lib/utils"; import { PanelHeaderBarSplit } from "@/components/task/panel-primitives"; +import { FileViewerDownloadButton } from "@/components/task/file-viewer-header"; import { EditorCommentPopover } from "@/components/task/editor-comment-popover"; import { CommentViewPopover } from "@/components/task/comment-view-popover"; import { useMarkdownPreviewComments } from "@/hooks/domains/comments/use-markdown-preview-comments"; @@ -54,6 +55,7 @@ interface MarkdownPreviewToolbarProps { repositoryId?: string | null; repositoryName?: string; showExternalVcsLink: boolean; + onDownload?: () => void; onTogglePreview: () => void; } @@ -67,6 +69,7 @@ function MarkdownPreviewToolbar({ repositoryId, repositoryName, showExternalVcsLink, + onDownload, onTogglePreview, }: MarkdownPreviewToolbarProps) { const { t } = useTranslation(); @@ -93,6 +96,7 @@ function MarkdownPreviewToolbar({ size="sm" /> )} + {commentsEnabled && commentCount > 0 && (
@@ -105,7 +109,7 @@ function MarkdownPreviewToolbar({ size="sm" variant="ghost" onClick={onTogglePreview} - className="h-8 w-8 p-0 cursor-pointer text-foreground" + className="h-11 w-11 p-0 cursor-pointer text-foreground sm:h-8 sm:w-8" data-testid="markdown-preview-toggle" > @@ -129,6 +133,7 @@ interface MarkdownPreviewContentProps { repositoryName?: string; enableComments?: boolean; showExternalVcsLink?: boolean; + onDownload?: () => void; onTogglePreview: () => void; } @@ -380,6 +385,7 @@ export const MarkdownPreviewContent = memo(function MarkdownPreviewContent({ repositoryName, enableComments = false, showExternalVcsLink = true, + onDownload, onTogglePreview, }: MarkdownPreviewContentProps) { const rootRef = useRef(null); @@ -432,6 +438,7 @@ export const MarkdownPreviewContent = memo(function MarkdownPreviewContent({ repositoryId={repositoryId} repositoryName={repositoryName} showExternalVcsLink={showExternalVcsLink} + onDownload={onDownload} onTogglePreview={onTogglePreview} />
diff --git a/apps/web/components/task/mobile/mobile-file-viewer-panel.test.tsx b/apps/web/components/task/mobile/mobile-file-viewer-panel.test.tsx index 23735bac12..060f479b6f 100644 --- a/apps/web/components/task/mobile/mobile-file-viewer-panel.test.tsx +++ b/apps/web/components/task/mobile/mobile-file-viewer-panel.test.tsx @@ -1,4 +1,5 @@ import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { TooltipProvider } from "@kandev/ui/tooltip"; import { afterEach, describe, expect, it, vi } from "vitest"; const state = { @@ -47,19 +48,21 @@ afterEach(cleanup); describe("MobileFileViewerPanel workspace path", () => { it("uses the effective workspace path for binary file viewers", () => { render( - , + + + , ); expect(screen.getByTestId("binary-viewer").getAttribute("data-worktree-path")).toBe( @@ -71,19 +74,21 @@ describe("MobileFileViewerPanel workspace path", () => { describe("MobileFileViewerPanel external file action", () => { it("renders a touch-sized action scoped to the open file's repository", () => { render( - , + + + , ); const props = JSON.parse( @@ -101,19 +106,21 @@ describe("MobileFileViewerPanel external file action", () => { it("opens a Markdown file directly in preview mode when requested", () => { render( - , + + + , ); expect(screen.getByTestId("markdown-preview")).toBeTruthy(); @@ -122,38 +129,42 @@ describe("MobileFileViewerPanel external file action", () => { it("resets preview mode when the same path is opened from another repository", () => { const { rerender } = render( - , + + + , ); fireEvent.click(screen.getByTestId("markdown-preview-toggle")); expect(screen.getByTestId("markdown-preview")).toBeTruthy(); rerender( - , + + + , ); expect(screen.getByTestId("file-content")).toBeTruthy(); diff --git a/apps/web/components/task/mobile/mobile-file-viewer-panel.tsx b/apps/web/components/task/mobile/mobile-file-viewer-panel.tsx index 1a7bedea4d..562817cead 100644 --- a/apps/web/components/task/mobile/mobile-file-viewer-panel.tsx +++ b/apps/web/components/task/mobile/mobile-file-viewer-panel.tsx @@ -8,7 +8,9 @@ import { FileViewerContent } from "../file-viewer-content"; import { MarkdownPreviewContent } from "../markdown-preview-content"; import { FileImageViewer } from "../file-image-viewer"; import { FileBinaryViewer } from "../file-binary-viewer"; +import { FileViewerDownloadButton } from "../file-viewer-header"; import { getFileCategory, isMarkdownFile } from "@/lib/utils/file-types"; +import { triggerFileDownload } from "@/lib/utils/file-download"; import { useAppStore } from "@/components/state-provider"; import type { OpenFileTab } from "@/lib/types/backend"; import { getSessionWorkspacePath } from "@/lib/session-workspace-path"; @@ -99,6 +101,15 @@ export function MobileFileViewerPanel({ const fileStatus = useExternalVcsFileStatus(file.path, sessionId, file.repo); const viewerKind = useMemo(() => resolveViewerKind(file), [file]); const markdownFile = isMarkdownFile(file.path); + const onDownload = useMemo( + () => () => + triggerFileDownload({ + fileName: file.path, + content: file.content, + isBinary: !!file.isBinary, + }), + [file.path, file.content, file.isBinary], + ); const [markdownPreview, setMarkdownPreview] = useState(initialMarkdownPreview); const fileIdentity = `${file.repo ?? ""}\u0000${file.path}`; @@ -129,6 +140,7 @@ export function MobileFileViewerPanel({ repositoryName={file.repo} size="touch" /> + {markdownFile && !markdownPreview && (