Merge pull request #2309 from Molecule-AI/auto/issue-2308-external-upload-422

fix(chat_files): return 422 (not misleading 503) for external workspace upload (#2308)

workspace-server/internal/handlers/chat_files.go

@@ -13,6 +13,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/rand"
+	"database/sql"
 	"encoding/hex"
 	"fmt"
 	"io"
@@ -24,6 +25,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
 	"github.com/docker/docker/api/types/container"
 	"github.com/gin-gonic/gin"
 )
@@ -177,6 +179,29 @@ func (h *ChatFilesHandler) Upload(c *gin.Context) {
 	}
 
 	ctx := c.Request.Context()
+
+	// External workspaces have no container — the container-find-then-tar-exec
+	// flow below doesn't apply. Surface a clear 4xx instead of the misleading
+	// "container not running" 503 (#2308). File ingest for external workspaces
+	// is on the v0.2 roadmap (likely via the artifacts table or the channel-
+	// plugin's inbox/ pattern); flagging unsupported is the v0.1 contract.
+	var wsRuntime string
+	if err := db.DB.QueryRowContext(ctx,
+		`SELECT COALESCE(runtime, '') FROM workspaces WHERE id = $1`, workspaceID,
+	).Scan(&wsRuntime); err != nil && err != sql.ErrNoRows {
+		log.Printf("chat_files Upload: runtime lookup failed for %s: %v", workspaceID, err)
+	}
+	if wsRuntime == "external" {
+		c.JSON(http.StatusUnprocessableEntity, gin.H{
+			"error": "file upload not supported for external workspaces",
+			"detail": "External workspaces have no container to receive uploads. " +
+				"Use POST /admin/secrets to share data, or attach via your external agent's own input channel. " +
+				"Native external-workspace upload tracked at https://github.com/Molecule-AI/molecule-core/issues/2308.",
+			"runtime": "external",
+		})
+		return
+	}
+
 	containerName := h.templates.findContainer(ctx, workspaceID)
 	if containerName == "" {
 		c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace container not running"})

workspace-server/internal/handlers/chat_files_external_test.go

@@ -0,0 +1,99 @@
+package handlers
+
+import (
+	"bytes"
+	"encoding/json"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/DATA-DOG/go-sqlmock"
+	"github.com/gin-gonic/gin"
+)
+
+// TestChatFilesUpload_ExternalRuntime_Returns422 pins the contract that
+// upload to a runtime="external" workspace returns a clear 422 with the
+// "external workspaces don't support file upload" message instead of the
+// misleading 503 "container not running" the v0.1 surface returned (#2308).
+//
+// Without this branch, an operator pasting a screenshot into the canvas chat
+// for an external CEO workspace got a `503 {"error":"workspace container
+// not running"}` — accurate from the upload handler's POV (no container
+// exists for external workspaces) but misleading because it implies the
+// container has crashed. The 422 with structured detail tells the operator
+// what's actually happening + points at the v0.2 follow-up issue.
+func TestChatFilesUpload_ExternalRuntime_Returns422(t *testing.T) {
+	mock := setupTestDB(t)
+
+	const wsID = "00000000-0000-0000-0000-000000000001"
+
+	// Runtime lookup returns "external" — should trigger the early return.
+	mock.ExpectQuery("SELECT COALESCE\\(runtime, ''\\) FROM workspaces").
+		WithArgs(wsID).
+		WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("external"))
+
+	// Construct a multipart upload body — handler must reject BEFORE
+	// touching docker, so a nil templates field is intentional. If the
+	// runtime check is removed in a future change, this test crashes on
+	// nil deref of h.templates instead of silently passing.
+	body, contentType := buildMultipartUpload(t, "screenshot.png", []byte("fake-png-bytes"))
+
+	// Use a nil-templates handler — proves runtime check happens BEFORE
+	// any docker plumbing. The handler is constructed in production via
+	// NewChatFilesHandler(templates) but the runtime branch should never
+	// reach into templates for external workspaces.
+	h := &ChatFilesHandler{templates: nil}
+
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	c.Params = gin.Params{{Key: "id", Value: wsID}}
+	c.Request = httptest.NewRequest("POST", "/workspaces/"+wsID+"/chat/files", bytes.NewReader(body))
+	c.Request.Header.Set("Content-Type", contentType)
+
+	h.Upload(c)
+
+	if w.Code != http.StatusUnprocessableEntity {
+		t.Fatalf("expected 422, got %d: %s", w.Code, w.Body.String())
+	}
+
+	var resp map[string]string
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("failed to parse response: %v", err)
+	}
+	if !strings.Contains(resp["error"], "external workspaces") {
+		t.Errorf("expected error to mention external workspaces, got: %q", resp["error"])
+	}
+	if resp["runtime"] != "external" {
+		t.Errorf("expected runtime=external in response, got %q", resp["runtime"])
+	}
+	// Spot-check that the error points at issue #2308 so operators reading
+	// it know where to track v0.2 file-ingest.
+	if !strings.Contains(resp["detail"], "2308") {
+		t.Errorf("expected detail to reference issue #2308, got: %q", resp["detail"])
+	}
+
+	if err := mock.ExpectationsWereMet(); err != nil {
+		t.Errorf("unmet sqlmock expectations: %v", err)
+	}
+}
+
+// buildMultipartUpload returns a body + content-type pair suitable for
+// posting a single file through the chat upload handler.
+func buildMultipartUpload(t *testing.T, filename string, contents []byte) ([]byte, string) {
+	t.Helper()
+	var buf bytes.Buffer
+	w := multipart.NewWriter(&buf)
+	part, err := w.CreateFormFile("files", filename)
+	if err != nil {
+		t.Fatalf("CreateFormFile: %v", err)
+	}
+	if _, err := part.Write(contents); err != nil {
+		t.Fatalf("part.Write: %v", err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatalf("multipart Close: %v", err)
+	}
+	return buf.Bytes(), w.FormDataContentType()
+}