From 740d3cbd420516616fed1d9ddbe667bb49e21abb Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 15:52:45 +0000 Subject: [PATCH 1/5] fix(handlers/channels_test): use RowError() to trigger rows.Err() in List test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous approach of adding a second row with matching columns does not trigger rows.Err() in sqlmock v1.5.2. rows.Err() is only set when RowError(n, err) or SetError(err) is called explicitly. Use RowError(0, errors.New("connection lost")) instead — this causes Scan() to fail on row 0 and sets rows.Err() so the handler's new rows.Err() check is exercised by the test. Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/channels_test.go | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/workspace-server/internal/handlers/channels_test.go b/workspace-server/internal/handlers/channels_test.go index 7c3454c1..d1167a2f 100644 --- a/workspace-server/internal/handlers/channels_test.go +++ b/workspace-server/internal/handlers/channels_test.go @@ -7,6 +7,7 @@ import ( "crypto/rand" "encoding/hex" "encoding/json" + "errors" "io" "net/http" "net/http/httptest" @@ -1013,6 +1014,54 @@ func TestChannelHandler_Webhook_Discord_InvalidSig_Returns401(t *testing.T) { } } +// TestChannelHandler_List_RowsErr_LogsError verifies that when the row iterator +// returns an error after the last row (mid-stream DB error), rows.Err() is +// detected and logged, but the partial results are still returned as 200 OK. +// This is the fix for the missing rows.Err() check in List(). +func TestChannelHandler_List_RowsErr_LogsError(t *testing.T) { + mock := setupTestDB(t) + handler := NewChannelHandler(newTestChannelManager()) + + // Return one valid row, then mark row 0 as having a scan error. + // RowError(n, err) causes Scan() to fail on row n, and sets rows.Err() + // to the error. sqlmock docs: "you can register errors on specific row + // indexes so that they will be returned on scan." + rows := sqlmock.NewRows([]string{ + "id", "workspace_id", "channel_type", "channel_config", "enabled", + "allowed_users", "last_message_at", "message_count", "created_at", "updated_at", + }).AddRow( + "ch-row-err", "ws-1", "telegram", + []byte(`{"bot_token":"123:AAA","chat_id":"-100"}`), + true, []byte(`[]`), nil, 5, nil, nil, + ) + rows = rows.RowError(0, errors.New("connection lost")) + + mock.ExpectQuery("SELECT .* FROM workspace_channels WHERE workspace_id"). + WithArgs("ws-1"). + WillReturnRows(rows) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request, _ = http.NewRequest("GET", "/workspaces/ws-1/channels", nil) + c.Params = gin.Params{{Key: "id", Value: "ws-1"}} + + handler.List(c) + + // Partial results still returned — the bug was silent 200 with partial data. + if w.Code != 200 { + t.Errorf("expected 200 (partial results on rows.Err), got %d: %s", w.Code, w.Body.String()) + } + // The rows.Err() is logged, not surfaced to the client (non-fatal). + var result []map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &result) + if len(result) == 0 { + t.Error("expected at least partial results despite rows.Err") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations not met: %v", err) + } +} + // TestChannelHandler_Webhook_Discord_ValidSig_PingAccepted verifies that a // correctly signed Discord PING (type=1) passes the signature gate and the // handler returns 200 (PING returns nil msg → "ignored" status). -- 2.52.0 From 6c313fe7a2be825279f24ad4ada9f925ebf4ce0c Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 17:22:55 +0000 Subject: [PATCH 2/5] fix(channels): remove duplicate EncryptSensitiveFields call (CWE-312) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit channels.go Create() was calling EncryptSensitiveFields twice in a row (lines 146–153 and 155–162). Both encrypt the same config; the second call is a no-op that wastes CPU. The duplicate was introduced in commit 989912da as part of PR #1193 and never removed. Also removes a stale CI re-trigger comment. CWE-312: Cleartext Storage of Sensitive Information. Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/handlers/channels.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/workspace-server/internal/handlers/channels.go b/workspace-server/internal/handlers/channels.go index 6d9008bf..e27a93be 100644 --- a/workspace-server/internal/handlers/channels.go +++ b/workspace-server/internal/handlers/channels.go @@ -149,15 +149,6 @@ func (h *ChannelHandler) Create(c *gin.Context) { return } - // #319: encrypt sensitive fields (bot_token, webhook_secret) before - // persisting so a DB read/backup leak can't recover the credentials. - // Validation above ran against plaintext; storage is ciphertext. - if err := channels.EncryptSensitiveFields(body.Config); err != nil { - log.Printf("Channels: encrypt config failed for workspace %s: %v", workspaceID, err) - c.JSON(http.StatusInternalServerError, gin.H{"error": "encrypt failed"}) - return - } - configJSON, _ := json.Marshal(body.Config) allowedJSON, _ := json.Marshal(body.AllowedUsers) enabled := true -- 2.52.0 From 7533265b99facff1f0a614b7f63867fc45171967 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 17:51:54 +0000 Subject: [PATCH 3/5] fix(handlers): add rows.Err() checks in channels.go List() and Webhook() Two handlers iterated DB rows without checking rows.Err() after the rows.Next() loop. If the DB errored mid-stream, partial results were silently returned as 200 OK with no error logged. Fixes: - List(): added rows.Err() check after the channel scan loop. Logs workspaceID + error on failure but still returns partial results. - Webhook(): same fix for the channel-lookup rows.Next() loop. Cherry-picked from fix/channels-rows-err onto fresh staging base. Also includes CWE-312 fix (duplicate EncryptSensitiveFields removal). Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/handlers/channels.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/workspace-server/internal/handlers/channels.go b/workspace-server/internal/handlers/channels.go index e27a93be..f223f22f 100644 --- a/workspace-server/internal/handlers/channels.go +++ b/workspace-server/internal/handlers/channels.go @@ -104,6 +104,9 @@ func (h *ChannelHandler) List(c *gin.Context) { } result = append(result, entry) } + if err := rows.Err(); err != nil { + log.Printf("Channels list rows.Err workspace=%s: %v", workspaceID, err) + } c.JSON(http.StatusOK, result) } @@ -505,6 +508,9 @@ func (h *ChannelHandler) Webhook(c *gin.Context) { candidates = append(candidates, row) } } + if err := rows.Err(); err != nil { + log.Printf("Channels webhook rows.Err channel_type=%s: %v", channelType, err) + } if targetSlug != "" { // [slug] routing — match against config username (lowercased) -- 2.52.0 From d6fbc3726b640efa816ff1e45abe38354cebd1a2 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 23:08:16 +0000 Subject: [PATCH 4/5] =?UTF-8?q?fix(handlers):=20Phase=201=20stub=20?= =?UTF-8?q?=E2=80=94=20/agent-home=20root=20key=20+=20501=20dispatch=20(in?= =?UTF-8?q?ternal#425=20RFC)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 stub for the Files API /agent-home root (internal#425 RFC). Changes: - allowedRoots gains "/agent-home": true (templates.go) - isAgentHomeStubRequest helper short-circuits all four verbs (ListFiles/ReadFile/WriteFile/DeleteFile) with 501 + canonical JSON body BEFORE the DB workspace lookup — prevents "workspace not found" log noise from canvas probe requests - Error messages updated to list /agent-home as a known root key - New test file: template_files_agent_home_stub_test.go — 5 tests: body-is-canonical, ListFiles 501, ReadFile 501, WriteFile 501, DeleteFile 501, allowedRoots pinned. sqlmock used so DB is never consulted for stub requests (verifies pre-DB short-circuit). Phase 2b (docker-exec backend) implements the actual file operations. Closes #1247. --- .../template_files_agent_home_stub_test.go | 187 ++++++++++++++++++ .../internal/handlers/templates.go | 52 ++++- 2 files changed, 231 insertions(+), 8 deletions(-) create mode 100644 workspace-server/internal/handlers/template_files_agent_home_stub_test.go diff --git a/workspace-server/internal/handlers/template_files_agent_home_stub_test.go b/workspace-server/internal/handlers/template_files_agent_home_stub_test.go new file mode 100644 index 00000000..f2f54664 --- /dev/null +++ b/workspace-server/internal/handlers/template_files_agent_home_stub_test.go @@ -0,0 +1,187 @@ +package handlers + +// template_files_agent_home_stub_test.go — Phase 1 stub tests for issue #1247 +// (internal#425 RFC). +// +// Phase 1 stub contract: +// • allowedRoots["/agent-home"] = true (templates.go) +// • GET/POST/PUT/DELETE with ?root=/agent-home short-circuits with +// 501 + the canonical JSON error body BEFORE the DB workspace lookup. +// This prevents "workspace not found" log noise from probe requests. +// • Error message for an unknown root now mentions /agent-home as a known key. +// +// Phase 2b (not here) implements the docker-exec backend to fulfil those requests. + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/gin-gonic/gin" +) + +// canonicalAgentHomeBody is the exact 501 response body the Phase 1 stub must emit. +const canonicalAgentHomeBody = `{"error":"/agent-home not implemented yet (internal#425 RFC Phase 2b — docker-exec backend pending)"}` + +// setupAgentHomeTest wires a minimal TemplatesHandler and sqlmock for +// agent-home stub tests. The DB is never consulted — the 501 fires before +// the workspace lookup — but the mock must exist so the router doesn't panic. +func setupAgentHomeTest(t *testing.T) (*TemplatesHandler, sqlmock.Sqlmock, *httptest.ResponseRecorder, *gin.Context) { + t.Helper() + mockDB, mock, err := sqlmock.New() + if err != nil { + t.Fatalf("sqlmock.New: %v", err) + } + db.DB = mockDB + t.Cleanup(func() { db.DB = nil; mockDB.Close() }) + + handler := NewTemplatesHandler(t.TempDir(), nil, nil) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + return handler, mock, w, c +} + +// stubNotImplementedBody pinned to the exact canonical string so the test +// verifies byte-for-byte equivalence with the handler constant. +func TestAgentHomeStub_NotImplementedBodyIsCanonical(t *testing.T) { + if stubNotImplementedBody != canonicalAgentHomeBody { + t.Fatalf("stubNotImplementedBody = %q; want %q", stubNotImplementedBody, canonicalAgentHomeBody) + } +} + +// TestAgentHomeStub_ListFiles_501 verifies that GET /workspaces/:id/files?root=/agent-home +// returns 501 with the canonical body and does NOT query the database. +func TestAgentHomeStub_ListFiles_501(t *testing.T) { + h, mock, w, c := setupAgentHomeTest(t) + c.Request = httptest.NewRequest("GET", "/workspaces/ws-123/files?root=/agent-home", nil) + c.Params = []gin.Param{{Key: "id", Value: "ws-123"}} + // DB must not be called — 501 fires before workspace lookup. + // We do NOT set up any mock rows; if the handler queries the DB the test + // will fail with "there is a remaining expectations which was not matched". + h.ListFiles(c) + + if w.Code != http.StatusNotImplemented { + t.Errorf("ListFiles?root=/agent-home: got status %d, want 501", w.Code) + } + if ct := w.Header().Get("Content-Type"); !strings.Contains(ct, "application/json") { + t.Errorf("Content-Type = %q, want application/json", ct) + } + if body := w.Body.String(); body != canonicalAgentHomeBody { + t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// TestAgentHomeStub_ReadFile_501 verifies that GET /workspaces/:id/files/path?root=/agent-home +// returns 501 and does NOT query the database. +func TestAgentHomeStub_ReadFile_501(t *testing.T) { + h, mock, w, c := setupAgentHomeTest(t) + c.Request = httptest.NewRequest("GET", "/workspaces/ws-123/files/config.yaml?root=/agent-home", nil) + c.Params = []gin.Param{{Key: "id", Value: "ws-123"}} + c.Params = append(c.Params, gin.Param{Key: "path", Value: "config.yaml"}) + h.ReadFile(c) + + if w.Code != http.StatusNotImplemented { + t.Errorf("ReadFile?root=/agent-home: got status %d, want 501", w.Code) + } + if body := w.Body.String(); body != canonicalAgentHomeBody { + t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// TestAgentHomeStub_WriteFile_501 verifies that PUT /workspaces/:id/files/path?root=/agent-home +// returns 501 and does NOT query the database. +func TestAgentHomeStub_WriteFile_501(t *testing.T) { + h, mock, w, c := setupAgentHomeTest(t) + c.Request = httptest.NewRequest("PUT", "/workspaces/ws-123/files/config.yaml?root=/agent-home", nil) + c.Params = []gin.Param{{Key: "id", Value: "ws-123"}} + c.Params = append(c.Params, gin.Param{Key: "path", Value: "config.yaml"}) + h.WriteFile(c) + + if w.Code != http.StatusNotImplemented { + t.Errorf("WriteFile?root=/agent-home: got status %d, want 501", w.Code) + } + if body := w.Body.String(); body != canonicalAgentHomeBody { + t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// TestAgentHomeStub_DeleteFile_501 verifies that DELETE /workspaces/:id/files/path?root=/agent-home +// returns 501 and does NOT query the database. +func TestAgentHomeStub_DeleteFile_501(t *testing.T) { + h, mock, w, c := setupAgentHomeTest(t) + c.Request = httptest.NewRequest("DELETE", "/workspaces/ws-123/files/config.yaml?root=/agent-home", nil) + c.Params = []gin.Param{{Key: "id", Value: "ws-123"}} + c.Params = append(c.Params, gin.Param{Key: "path", Value: "config.yaml"}) + h.DeleteFile(c) + + if w.Code != http.StatusNotImplemented { + t.Errorf("DeleteFile?root=/agent-home: got status %d, want 501", w.Code) + } + if body := w.Body.String(); body != canonicalAgentHomeBody { + t.Errorf("body = %q; want %q", body, canonicalAgentHomeBody) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// TestAgentHomeStub_AllowedRootsPinned verifies that /agent-home is in +// allowedRoots so that the stub short-circuit is reachable. +func TestAgentHomeStub_AllowedRootsPinned(t *testing.T) { + if !allowedRoots["/agent-home"] { + t.Error("/agent-home must be in allowedRoots") + } + // Sanity-check the other roots are still present (regression guard). + for _, root := range []string{"/configs", "/workspace", "/home", "/plugins"} { + if !allowedRoots[root] { + t.Errorf("%s must still be in allowedRoots", root) + } + } +} + +// TestAgentHomeStub_ListFiles_ExistingRootsUnaffected verifies that the four +// original roots still route through to the DB path (we don't wire sqlmock +// expectations here — we just confirm they return 400, not 501, for an +// invalid workspace ID). +func TestAgentHomeStub_OtherRoots_Return400OnBadWorkspace(t *testing.T) { + h, mock, w, c := setupAgentHomeTest(t) + + for _, root := range []string{"/configs", "/workspace", "/home", "/plugins"} { + // The DB returns ErrNoRows → 404, not 400 or 501. + mock.ExpectQuery(`SELECT name, COALESCE`).WithArgs("bad-ws").WillReturnError(sql.ErrNoRows) + + c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root="+root, nil) + c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}} + // Reset the response recorder between calls. + w = httptest.NewRecorder() + c.Reset() + + // We can't use c.Reset() cleanly across different request/response pairs + // in a loop without re-creating the context. Just run one representative case. + if root == "/configs" { + c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root="+root, nil) + c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}} + w = httptest.NewRecorder() + c, _ = gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root="+root, nil) + c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}} + h.ListFiles(c) + // 404 → workspace not found (DB returned ErrNoRows), not 501. + if w.Code == http.StatusNotImplemented { + t.Error("/configs must NOT return 501 — only /agent-home short-circuits") + } + } + } + _ = h // suppress unused +} diff --git a/workspace-server/internal/handlers/templates.go b/workspace-server/internal/handlers/templates.go index d51c19cc..f40cf2cc 100644 --- a/workspace-server/internal/handlers/templates.go +++ b/workspace-server/internal/handlers/templates.go @@ -19,12 +19,22 @@ import ( // allowedRoots are the container paths that the Files API can browse. var allowedRoots = map[string]bool{ - "/configs": true, - "/workspace": true, - "/home": true, - "/plugins": true, + "/configs": true, + "/workspace": true, + "/home": true, + "/plugins": true, + "/agent-home": true, // Phase 1 stub — docker-exec backend in Phase 2b (internal#425 RFC) } +// isAgentHomeStubRequest returns true when rootPath targets the /agent-home +// root. Phase 1 returns 501; Phase 2b implements the docker-exec backend. +func isAgentHomeStubRequest(rootPath string) bool { + return rootPath == "/agent-home" +} + +// stubNotImplementedBody is the canonical 501 response body for Phase 1 stub requests. +const stubNotImplementedBody = `{"error":"/agent-home not implemented yet (internal#425 RFC Phase 2b — docker-exec backend pending)"}` + // maxUploadFiles limits the number of files in a single import/replace. const maxUploadFiles = 200 @@ -219,7 +229,15 @@ func (h *TemplatesHandler) ListFiles(c *gin.Context) { // ?depth= — max depth to recurse (default: 1, max: 5) rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + // Phase 1 stub — fires BEFORE the DB lookup so probe requests don't + // generate "workspace not found" log noise. Phase 2b implements the + // docker-exec backend (internal#425 RFC). + if isAgentHomeStubRequest(rootPath) { + c.Header("Content-Type", "application/json") + c.String(http.StatusNotImplemented, stubNotImplementedBody) return } subPath := c.DefaultQuery("path", "") @@ -383,7 +401,13 @@ func (h *TemplatesHandler) ReadFile(c *gin.Context) { ctx := c.Request.Context() rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + // Phase 1 stub — fires BEFORE the DB lookup (internal#425 RFC Phase 2b). + if isAgentHomeStubRequest(rootPath) { + c.Header("Content-Type", "application/json") + c.String(http.StatusNotImplemented, stubNotImplementedBody) return } @@ -496,7 +520,13 @@ func (h *TemplatesHandler) WriteFile(c *gin.Context) { ctx := c.Request.Context() rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + // Phase 1 stub — fires BEFORE the DB lookup (internal#425 RFC Phase 2b). + if isAgentHomeStubRequest(rootPath) { + c.Header("Content-Type", "application/json") + c.String(http.StatusNotImplemented, stubNotImplementedBody) return } var wsName, instanceID, runtime string @@ -573,7 +603,13 @@ func (h *TemplatesHandler) DeleteFile(c *gin.Context) { ctx := c.Request.Context() rootPath := c.DefaultQuery("root", "/configs") if !allowedRoots[rootPath] { - c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins"}) + c.JSON(http.StatusBadRequest, gin.H{"error": "root must be one of: /configs, /workspace, /home, /plugins, /agent-home"}) + return + } + // Phase 1 stub — fires BEFORE the DB lookup (internal#425 RFC Phase 2b). + if isAgentHomeStubRequest(rootPath) { + c.Header("Content-Type", "application/json") + c.String(http.StatusNotImplemented, stubNotImplementedBody) return } var wsName, instanceID, runtime string -- 2.52.0 From e29a128bb66b061ea7ac76f78388ef3c990bdcf7 Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Fri, 15 May 2026 23:29:06 +0000 Subject: [PATCH 5/5] fix(handlers): test fixes for agent-home stub + rows.Err test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: - template_files_agent_home_stub_test.go: add missing imports (database/sql, internal/db); fix WriteFile test body to send JSON; simplify OtherRoots regression test loop. - channels_test.go: fix TestChannelHandler_List_RowsErr_LogsError mock setup — RowError(0, err) on a single-row result makes the only row fail to scan (empty result), breaking the "partial results" assertion. Fix: add two rows, RowError(1, err) on the second so the first scans successfully and the error is still captured via rows.Err(). Co-Authored-By: Claude Opus 4.7 --- .../internal/handlers/channels_test.go | 19 ++++--- .../template_files_agent_home_stub_test.go | 52 ++++++++----------- 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/workspace-server/internal/handlers/channels_test.go b/workspace-server/internal/handlers/channels_test.go index d1167a2f..338eda14 100644 --- a/workspace-server/internal/handlers/channels_test.go +++ b/workspace-server/internal/handlers/channels_test.go @@ -1022,19 +1022,26 @@ func TestChannelHandler_List_RowsErr_LogsError(t *testing.T) { mock := setupTestDB(t) handler := NewChannelHandler(newTestChannelManager()) - // Return one valid row, then mark row 0 as having a scan error. - // RowError(n, err) causes Scan() to fail on row n, and sets rows.Err() - // to the error. sqlmock docs: "you can register errors on specific row - // indexes so that they will be returned on scan." + // Return one valid row (row 0), then a second row (row 1) that triggers a + // scan error. RowError is indexed from 0, so RowError(1, ...) affects the + // second AddRow. This way row 0 scans successfully and is appended to results; + // row 1 fails to scan, rows.Err() captures the error, and the handler + // returns 200 with the partial result (one row). rows := sqlmock.NewRows([]string{ "id", "workspace_id", "channel_type", "channel_config", "enabled", "allowed_users", "last_message_at", "message_count", "created_at", "updated_at", }).AddRow( - "ch-row-err", "ws-1", "telegram", + // Row 0 — valid; appended to results. + "ch-valid-0", "ws-1", "telegram", []byte(`{"bot_token":"123:AAA","chat_id":"-100"}`), true, []byte(`[]`), nil, 5, nil, nil, + ).AddRow( + // Row 1 — will fail to scan due to RowError(1, ...); skipped. + "ch-err-1", "ws-1", "telegram", + []byte(`{"bot_token":"456:BBB"}`), + true, []byte(`[]`), nil, 10, nil, nil, ) - rows = rows.RowError(0, errors.New("connection lost")) + rows = rows.RowError(1, errors.New("connection lost")) mock.ExpectQuery("SELECT .* FROM workspace_channels WHERE workspace_id"). WithArgs("ws-1"). diff --git a/workspace-server/internal/handlers/template_files_agent_home_stub_test.go b/workspace-server/internal/handlers/template_files_agent_home_stub_test.go index f2f54664..ebce8419 100644 --- a/workspace-server/internal/handlers/template_files_agent_home_stub_test.go +++ b/workspace-server/internal/handlers/template_files_agent_home_stub_test.go @@ -13,12 +13,14 @@ package handlers // Phase 2b (not here) implements the docker-exec backend to fulfil those requests. import ( + "database/sql" "net/http" "net/http/httptest" "strings" "testing" "github.com/DATA-DOG/go-sqlmock" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" "github.com/gin-gonic/gin" ) @@ -100,7 +102,9 @@ func TestAgentHomeStub_ReadFile_501(t *testing.T) { // returns 501 and does NOT query the database. func TestAgentHomeStub_WriteFile_501(t *testing.T) { h, mock, w, c := setupAgentHomeTest(t) - c.Request = httptest.NewRequest("PUT", "/workspaces/ws-123/files/config.yaml?root=/agent-home", nil) + body := `{"content":"hello"}` + c.Request = httptest.NewRequest("PUT", "/workspaces/ws-123/files/config.yaml?root=/agent-home", strings.NewReader(body)) + c.Request.Header.Set("Content-Type", "application/json") c.Params = []gin.Param{{Key: "id", Value: "ws-123"}} c.Params = append(c.Params, gin.Param{Key: "path", Value: "config.yaml"}) h.WriteFile(c) @@ -150,38 +154,28 @@ func TestAgentHomeStub_AllowedRootsPinned(t *testing.T) { } } -// TestAgentHomeStub_ListFiles_ExistingRootsUnaffected verifies that the four -// original roots still route through to the DB path (we don't wire sqlmock -// expectations here — we just confirm they return 400, not 501, for an -// invalid workspace ID). +// TestAgentHomeStub_OtherRoots_Return400OnBadWorkspace verifies that the four +// original roots still route through to the DB path — they return 404 (or 400) +// for a bad workspace ID, NOT 501. Only /agent-home short-circuits with 501. func TestAgentHomeStub_OtherRoots_Return400OnBadWorkspace(t *testing.T) { - h, mock, w, c := setupAgentHomeTest(t) + h, mock, _, _ := setupAgentHomeTest(t) - for _, root := range []string{"/configs", "/workspace", "/home", "/plugins"} { - // The DB returns ErrNoRows → 404, not 400 or 501. - mock.ExpectQuery(`SELECT name, COALESCE`).WithArgs("bad-ws").WillReturnError(sql.ErrNoRows) + // Representative case: /configs with a bad workspace ID. + // DB returns ErrNoRows → handler returns 404, not 501. + mock.ExpectQuery(`SELECT name, COALESCE`).WithArgs("bad-ws").WillReturnError(sql.ErrNoRows) - c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root="+root, nil) - c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}} - // Reset the response recorder between calls. - w = httptest.NewRecorder() - c.Reset() + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root=/configs", nil) + c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}} + h.ListFiles(c) - // We can't use c.Reset() cleanly across different request/response pairs - // in a loop without re-creating the context. Just run one representative case. - if root == "/configs" { - c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root="+root, nil) - c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}} - w = httptest.NewRecorder() - c, _ = gin.CreateTestContext(w) - c.Request = httptest.NewRequest("GET", "/workspaces/bad-ws/files?root="+root, nil) - c.Params = []gin.Param{{Key: "id", Value: "bad-ws"}} - h.ListFiles(c) - // 404 → workspace not found (DB returned ErrNoRows), not 501. - if w.Code == http.StatusNotImplemented { - t.Error("/configs must NOT return 501 — only /agent-home short-circuits") - } - } + // 404 → workspace not found (DB returned ErrNoRows), NOT 501. + if w.Code == http.StatusNotImplemented { + t.Error("/configs must NOT return 501 — only /agent-home short-circuits") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) } _ = h // suppress unused } -- 2.52.0