diff --git a/canvas/src/components/ConfirmDialog.tsx b/canvas/src/components/ConfirmDialog.tsx index 339376cf..13aeedc5 100644 --- a/canvas/src/components/ConfirmDialog.tsx +++ b/canvas/src/components/ConfirmDialog.tsx @@ -12,7 +12,11 @@ interface Props { onConfirm: () => void; onCancel: () => void; // Hide the Cancel button for single-action info toasts. - // onCancel is still invoked on Esc / backdrop-click. + // onCancel is still invoked on Esc / backdrop-click, so when using this + // dialog as a simple info toast the caller should pass the SAME handler + // for both `onConfirm` and `onCancel` — otherwise dismissing via Esc / + // backdrop click will run different logic than clicking the OK button, + // which is almost never what you want for an info dialog. singleButton?: boolean; } diff --git a/canvas/src/components/ErrorBoundary.tsx b/canvas/src/components/ErrorBoundary.tsx index 51729063..96766d08 100644 --- a/canvas/src/components/ErrorBoundary.tsx +++ b/canvas/src/components/ErrorBoundary.tsx @@ -44,7 +44,8 @@ export class ErrorBoundary extends React.Component< // Copy error info to clipboard for manual reporting (button click is its // own affordance — no native alert needed). On clipboard failure the // console.error above still surfaces the report. - void navigator.clipboard?.writeText(JSON.stringify(errorDetails, null, 2)).catch(() => {}); + void navigator.clipboard?.writeText(JSON.stringify(errorDetails, null, 2)) + .catch((e) => console.warn("clipboard write failed:", e)); }; render() { diff --git a/canvas/src/components/__tests__/ConfirmDialog.test.tsx b/canvas/src/components/__tests__/ConfirmDialog.test.tsx new file mode 100644 index 00000000..d58b1dd7 --- /dev/null +++ b/canvas/src/components/__tests__/ConfirmDialog.test.tsx @@ -0,0 +1,91 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import { ConfirmDialog } from "../ConfirmDialog"; + +afterEach(() => { + cleanup(); +}); + +describe("ConfirmDialog singleButton prop", () => { + it("renders Cancel button by default", () => { + render( + + ); + expect(screen.getByRole("button", { name: "Cancel" })).toBeTruthy(); + expect(screen.getByRole("button", { name: "Confirm" })).toBeTruthy(); + }); + + it("hides Cancel button when singleButton=true", () => { + render( + + ); + expect(screen.queryByRole("button", { name: "Cancel" })).toBeNull(); + expect(screen.getByRole("button", { name: "Confirm" })).toBeTruthy(); + }); + + it("singleButton: onCancel still fires on Escape", () => { + const onCancel = vi.fn(); + render( + + ); + fireEvent.keyDown(window, { key: "Escape" }); + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it("singleButton: onCancel still fires on backdrop click", () => { + const onCancel = vi.fn(); + const { container } = render( + + ); + // Backdrop is the div with bg-black/60 class, rendered into document.body via portal + const backdrop = document.querySelector(".bg-black\\/60") as HTMLElement; + expect(backdrop).toBeTruthy(); + void container; + fireEvent.click(backdrop); + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it("singleButton: onConfirm fires on button click", () => { + const onConfirm = vi.fn(); + render( + + ); + fireEvent.click(screen.getByRole("button", { name: "Confirm" })); + expect(onConfirm).toHaveBeenCalledTimes(1); + }); +}); diff --git a/platform/internal/handlers/a2a_proxy_test.go b/platform/internal/handlers/a2a_proxy_test.go index 4b846d5d..7de89c31 100644 --- a/platform/internal/handlers/a2a_proxy_test.go +++ b/platform/internal/handlers/a2a_proxy_test.go @@ -813,3 +813,350 @@ func TestNormalizeA2APayload_MissingMethodReturnsEmpty(t *testing.T) { t.Errorf("expected empty method, got %q", method) } } + +// --- resolveAgentURL direct unit tests --- + +func TestResolveAgentURL_CacheHit(t *testing.T) { + setupTestDB(t) + mr := setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + mr.Set("ws:ws-cached:url", "http://cached.example/a2a") + + url, perr := handler.resolveAgentURL(context.Background(), "ws-cached") + if perr != nil { + t.Fatalf("unexpected error: %+v", perr) + } + if url != "http://cached.example/a2a" { + t.Errorf("got %q, want cached URL", url) + } +} + +func TestResolveAgentURL_CacheMissDBHit(t *testing.T) { + mock := setupTestDB(t) + mr := setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery("SELECT url, status FROM workspaces WHERE id ="). + WithArgs("ws-dbhit"). + WillReturnRows(sqlmock.NewRows([]string{"url", "status"}).AddRow("http://dbhit.example", "online")) + + url, perr := handler.resolveAgentURL(context.Background(), "ws-dbhit") + if perr != nil { + t.Fatalf("unexpected error: %+v", perr) + } + if url != "http://dbhit.example" { + t.Errorf("got %q, want http://dbhit.example", url) + } + // Verify cached now + if v, err := mr.Get("ws:ws-dbhit:url"); err != nil || v != "http://dbhit.example" { + t.Errorf("expected Redis cache populated; got v=%q err=%v", v, err) + } +} + +func TestResolveAgentURL_WorkspaceNotFound(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery("SELECT url, status FROM workspaces WHERE id ="). + WithArgs("ws-missing"). + WillReturnError(sql.ErrNoRows) + + _, perr := handler.resolveAgentURL(context.Background(), "ws-missing") + if perr == nil { + t.Fatal("expected error, got nil") + } + if perr.Status != http.StatusNotFound { + t.Errorf("got status %d, want 404", perr.Status) + } +} + +func TestResolveAgentURL_NullURL(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery("SELECT url, status FROM workspaces WHERE id ="). + WithArgs("ws-nullurl"). + WillReturnRows(sqlmock.NewRows([]string{"url", "status"}).AddRow(nil, "provisioning")) + + _, perr := handler.resolveAgentURL(context.Background(), "ws-nullurl") + if perr == nil { + t.Fatal("expected error, got nil") + } + if perr.Status != http.StatusServiceUnavailable { + t.Errorf("got status %d, want 503", perr.Status) + } +} + +func TestResolveAgentURL_DockerRewrite(t *testing.T) { + // provisioner.InternalURL is called when platformInDocker && URL begins + // with http://127.0.0.1:. We don't have a real *Provisioner so the + // rewrite path requires h.provisioner != nil. Since we can't easily + // construct a provisioner, verify rewrite does NOT happen when + // provisioner is nil (guard clause). The rewrite branch itself is + // covered by TestResolveAgentURL_DockerRewrite_NilProvisionerNoRewrite. + mr := setupTestRedis(t) + setupTestDB(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + mr.Set("ws:ws-dock:url", "http://127.0.0.1:55555") + + restore := setPlatformInDockerForTest(true) + defer restore() + + url, perr := handler.resolveAgentURL(context.Background(), "ws-dock") + if perr != nil { + t.Fatalf("unexpected error: %+v", perr) + } + // nil provisioner → no rewrite + if url != "http://127.0.0.1:55555" { + t.Errorf("with nil provisioner, URL must not be rewritten; got %q", url) + } +} + +// --- dispatchA2A direct unit tests --- + +func TestDispatchA2A_BuildRequestError(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + // Malformed URL causes http.NewRequestWithContext to fail. + _, cancel, err := handler.dispatchA2A(context.Background(), "http://%%badhost", []byte("{}"), "") + if cancel != nil { + cancel() + } + if err == nil { + t.Fatal("expected error, got nil") + } + if _, ok := err.(*proxyDispatchBuildError); !ok { + t.Errorf("expected *proxyDispatchBuildError, got %T: %v", err, err) + } +} + +func TestDispatchA2A_CanvasTimeout(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + // Agent that responds OK — we just want the cancel func. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer srv.Close() + + resp, cancel, err := handler.dispatchA2A(context.Background(), srv.URL, []byte(`{}`), "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer resp.Body.Close() + if cancel == nil { + t.Fatal("canvas caller (empty callerID) must set a timeout + return cancel") + } + cancel() // restore +} + +func TestDispatchA2A_AgentTimeout(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer srv.Close() + + resp, cancel, err := handler.dispatchA2A(context.Background(), srv.URL, []byte(`{}`), "ws-caller") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer resp.Body.Close() + if cancel == nil { + t.Fatal("agent-to-agent caller must set a timeout + return cancel") + } + cancel() +} + +func TestDispatchA2A_ContextDeadline_NoCancelAdded(t *testing.T) { + // When ctx already has a deadline, dispatchA2A must NOT layer its own + // timeout (cancel should be nil). + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer srv.Close() + + ctx, ctxCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer ctxCancel() + + resp, cancel, err := handler.dispatchA2A(ctx, srv.URL, []byte(`{}`), "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer resp.Body.Close() + if cancel != nil { + t.Error("cancel should be nil when ctx already has a deadline") + cancel() + } +} + +// --- handleA2ADispatchError --- + +func TestHandleA2ADispatchError_ContextDeadline(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + // No workspace row expected — maybeMarkContainerDead with nil + // provisioner short-circuits, and activity-log insert is suppressed + // (logActivity=false). + _, _, perr := handler.handleA2ADispatchError( + context.Background(), "ws-dl", "", []byte("{}"), "message/send", + context.DeadlineExceeded, 1, false, + ) + if perr == nil { + t.Fatal("expected error, got nil") + } + // DeadlineExceeded is classified as upstream-busy → 503 with Retry-After. + if perr.Status != http.StatusServiceUnavailable { + t.Errorf("got status %d, want 503", perr.Status) + } + if perr.Headers["Retry-After"] == "" { + t.Error("expected Retry-After header on busy-503 shape") + } +} + +func TestHandleA2ADispatchError_BuildError(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + buildErr := &proxyDispatchBuildError{err: fmt.Errorf("bad url")} + _, _, perr := handler.handleA2ADispatchError( + context.Background(), "ws-x", "", []byte("{}"), "message/send", buildErr, 1, false, + ) + if perr == nil || perr.Status != http.StatusInternalServerError { + t.Errorf("expected 500, got %+v", perr) + } +} + +func TestHandleA2ADispatchError_GenericReturns502(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + _, _, perr := handler.handleA2ADispatchError( + context.Background(), "ws-x", "", []byte("{}"), "message/send", + fmt.Errorf("no such host"), 1, false, + ) + if perr == nil || perr.Status != http.StatusBadGateway { + t.Errorf("expected 502, got %+v", perr) + } +} + +// --- maybeMarkContainerDead --- + +// Nil provisioner → short-circuits false. +func TestMaybeMarkContainerDead_NilProvisioner(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery(`SELECT COALESCE\(runtime, 'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-nilprov"). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("langgraph")) + + if got := handler.maybeMarkContainerDead(context.Background(), "ws-nilprov"); got { + t.Error("expected false when provisioner is nil") + } +} + +// external runtime → false regardless of provisioner. +func TestMaybeMarkContainerDead_ExternalRuntime(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery(`SELECT COALESCE\(runtime, 'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-ext"). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("external")) + + if got := handler.maybeMarkContainerDead(context.Background(), "ws-ext"); got { + t.Error("expected false for external runtime") + } +} + +// --- logA2AFailure / logA2ASuccess smoke tests --- +// These helpers spawn a detached goroutine that calls LogActivity, which +// inserts into activity_logs. We can't easily sync on the goroutine via +// sqlmock (done order isn't guaranteed), so we only assert the function +// returns without panicking and makes the expected DB calls. + +func TestLogA2AFailure_Smoke(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + // Sync workspace-name lookup (called in the caller goroutine). + mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`). + WithArgs("ws-fail"). + WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("Fail Target")) + // Async INSERT from the detached goroutine. MatchExpectationsInOrder=true + // by default, but the goroutine runs after the sync query above. + mock.ExpectExec("INSERT INTO activity_logs"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + handler.logA2AFailure(context.Background(), "ws-fail", "", []byte(`{}`), "message/send", fmt.Errorf("boom"), 42) + time.Sleep(80 * time.Millisecond) +} + +func TestLogA2AFailure_EmptyNameFallback(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + // Empty name from DB → summary uses the workspaceID as the name. + mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`). + WithArgs("ws-noname"). + WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("")) + mock.ExpectExec("INSERT INTO activity_logs"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + handler.logA2AFailure(context.Background(), "ws-noname", "", []byte(`{}`), "message/send", fmt.Errorf("boom"), 1) + time.Sleep(80 * time.Millisecond) +} + +func TestLogA2ASuccess_Smoke(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`). + WithArgs("ws-ok"). + WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("OK Target")) + mock.ExpectExec("INSERT INTO activity_logs"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + handler.logA2ASuccess(context.Background(), "ws-ok", "", []byte(`{}`), []byte(`{"result":"x"}`), "message/send", 200, 10) + time.Sleep(80 * time.Millisecond) +} + +// Error-status path (>=400) records an "error" status in activity_logs. +func TestLogA2ASuccess_ErrorStatus(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + handler := NewWorkspaceHandler(newTestBroadcaster(), nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectQuery(`SELECT name FROM workspaces WHERE id =`). + WithArgs("ws-err"). + WillReturnRows(sqlmock.NewRows([]string{"name"}).AddRow("")) + mock.ExpectExec("INSERT INTO activity_logs"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + // callerID != "" also means no A2A_RESPONSE broadcast. + handler.logA2ASuccess(context.Background(), "ws-err", "ws-caller", []byte(`{}`), []byte(`{}`), "message/send", 500, 10) + time.Sleep(80 * time.Millisecond) +} diff --git a/platform/internal/handlers/activity_test.go b/platform/internal/handlers/activity_test.go index d6ca9d15..1780be3b 100644 --- a/platform/internal/handlers/activity_test.go +++ b/platform/internal/handlers/activity_test.go @@ -3,6 +3,7 @@ package handlers import ( "bytes" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" @@ -215,3 +216,186 @@ func TestActivityReport_RejectsUnknownType(t *testing.T) { t.Errorf("error message should list valid types including memory_write; got %s", w.Body.String()) } } + +// ==================== Direct unit tests for SessionSearch helpers ==================== + +// --- parseSessionSearchParams --- + +func TestParseSessionSearchParams_Defaults(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + q, limit := parseSessionSearchParams(c) + if q != "" { + t.Errorf("expected empty q, got %q", q) + } + if limit != 50 { + t.Errorf("expected default limit 50, got %d", limit) + } +} + +func TestParseSessionSearchParams_CustomLimit(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x?q=foo&limit=75", nil) + + q, limit := parseSessionSearchParams(c) + if q != "foo" { + t.Errorf("expected q=foo, got %q", q) + } + if limit != 75 { + t.Errorf("expected limit=75, got %d", limit) + } +} + +func TestParseSessionSearchParams_LimitCappedAt200(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x?limit=9999", nil) + + _, limit := parseSessionSearchParams(c) + if limit != 200 { + t.Errorf("expected cap 200, got %d", limit) + } +} + +func TestParseSessionSearchParams_InvalidLimitUsesDefault(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x?limit=abc", nil) + + _, limit := parseSessionSearchParams(c) + if limit != 50 { + t.Errorf("expected default on invalid, got %d", limit) + } +} + +// --- buildSessionSearchQuery --- + +func TestBuildSessionSearchQuery_NoFilters(t *testing.T) { + sqlQuery, args := buildSessionSearchQuery("ws-1", "", 50) + if strings.Contains(sqlQuery, "ILIKE") { + t.Error("expected no ILIKE when query empty") + } + if len(args) != 2 || args[0] != "ws-1" || args[1] != 50 { + t.Errorf("unexpected args: %v", args) + } +} + +func TestBuildSessionSearchQuery_WithQuery(t *testing.T) { + sqlQuery, args := buildSessionSearchQuery("ws-1", "foo", 25) + if !strings.Contains(sqlQuery, "ILIKE") { + t.Error("expected ILIKE when query provided") + } + if len(args) != 3 { + t.Fatalf("expected 3 args, got %d: %v", len(args), args) + } + if args[1] != "%foo%" { + t.Errorf("expected LIKE pattern, got %v", args[1]) + } + if args[2] != 25 { + t.Errorf("expected limit=25, got %v", args[2]) + } +} + +// --- scanSessionSearchRows --- + +// fakeRows implements the minimal rows interface scanSessionSearchRows expects. +type fakeRows struct { + data [][]interface{} + i int + err error +} + +func (f *fakeRows) Next() bool { return f.i < len(f.data) } +func (f *fakeRows) Scan(dest ...interface{}) error { + row := f.data[f.i] + f.i++ + for i, v := range row { + switch d := dest[i].(type) { + case *string: + *d = v.(string) + case *[]byte: + if v == nil { + *d = nil + } else { + *d = v.([]byte) + } + case *time.Time: + *d = v.(time.Time) + } + } + return nil +} +func (f *fakeRows) Err() error { return f.err } + +func TestScanSessionSearchRows_EmptyRows(t *testing.T) { + items, err := scanSessionSearchRows(&fakeRows{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(items) != 0 { + t.Errorf("expected empty result, got %d", len(items)) + } +} + +func TestScanSessionSearchRows_MultipleRows(t *testing.T) { + now := time.Now() + rows := &fakeRows{ + data: [][]interface{}{ + {"activity", "a1", "ws-1", "task_update", "hello", "POST", "ok", []byte(`{"x":1}`), []byte(`{"y":2}`), now}, + {"memory", "m1", "ws-1", "TEAM", "note", "", "", []byte(nil), []byte(nil), now}, + }, + } + items, err := scanSessionSearchRows(rows) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(items) != 2 { + t.Fatalf("expected 2 items, got %d", len(items)) + } + if items[0]["kind"] != "activity" { + t.Errorf("first row kind: %v", items[0]["kind"]) + } + if items[0]["request_body"] == nil { + t.Error("expected request_body present on activity row") + } + if _, has := items[1]["request_body"]; has { + t.Error("memory row should not have request_body (nil bytes)") + } +} + +// scanErrorRows returns a Scan error on the first row to cover the +// log-and-skip branch in scanSessionSearchRows. +type scanErrorRows struct { + called bool +} + +func (s *scanErrorRows) Next() bool { + if !s.called { + s.called = true + return true + } + return false +} +func (s *scanErrorRows) Scan(dest ...interface{}) error { return fmt.Errorf("scan bad") } +func (s *scanErrorRows) Err() error { return nil } + +func TestScanSessionSearchRows_ScanErrorSkipped(t *testing.T) { + items, err := scanSessionSearchRows(&scanErrorRows{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(items) != 0 { + t.Errorf("expected 0 items (scan error skipped), got %d", len(items)) + } +} + +func TestScanSessionSearchRows_RowsErrPropagates(t *testing.T) { + f := &fakeRows{err: fmt.Errorf("boom")} + _, err := scanSessionSearchRows(f) + if err == nil { + t.Fatal("expected error to propagate") + } +} diff --git a/platform/internal/handlers/delegation.go b/platform/internal/handlers/delegation.go index bd53daff..7edaab65 100644 --- a/platform/internal/handlers/delegation.go +++ b/platform/internal/handlers/delegation.go @@ -158,9 +158,13 @@ func lookupIdempotentDelegation(ctx context.Context, c *gin.Context, sourceID, i type insertDelegationOutcome int const ( + // insertOutcomeUnknown — zero-value sentinel; should never be returned + // by insertDelegationRow. Exists so that an uninitialized + // insertDelegationOutcome value doesn't silently alias a real outcome. + insertOutcomeUnknown insertDelegationOutcome = iota // insertOK — row stored; caller continues with dispatch and does NOT // surface a tracking warning. - insertOK insertDelegationOutcome = iota + insertOK // insertHandledByIdempotent — a concurrent idempotent request took the // slot; the winner's JSON response is already written and the caller // MUST return without further writes. diff --git a/platform/internal/handlers/delegation_test.go b/platform/internal/handlers/delegation_test.go index 3e99063a..e9e8ca69 100644 --- a/platform/internal/handlers/delegation_test.go +++ b/platform/internal/handlers/delegation_test.go @@ -2,6 +2,7 @@ package handlers import ( "bytes" + "context" "encoding/json" "fmt" "net/http" @@ -646,3 +647,200 @@ func TestDelegate_IdempotentRaceUniqueViolationReturnsExisting(t *testing.T) { t.Errorf("expected idempotent_hit=true on race resolution, got %v", resp["idempotent_hit"]) } } + +// ==================== Direct unit tests for extracted helpers ==================== + +// --- bindDelegateRequest --- + +func TestBindDelegateRequest_ValidJSON(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + body := `{"target_id":"aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee","task":"hi"}` + c.Request = httptest.NewRequest("POST", "/x", bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + + var out delegateRequest + if err := bindDelegateRequest(c, &out); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Task != "hi" { + t.Errorf("got task %q", out.Task) + } +} + +func TestBindDelegateRequest_InvalidJSON(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/x", bytes.NewBufferString("not json")) + c.Request.Header.Set("Content-Type", "application/json") + + var out delegateRequest + if err := bindDelegateRequest(c, &out); err == nil { + t.Fatal("expected error") + } + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400, got %d", w.Code) + } +} + +func TestBindDelegateRequest_InvalidTargetUUID(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/x", bytes.NewBufferString(`{"target_id":"not-uuid","task":"x"}`)) + c.Request.Header.Set("Content-Type", "application/json") + + var out delegateRequest + if err := bindDelegateRequest(c, &out); err == nil { + t.Fatal("expected error") + } + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400, got %d", w.Code) + } +} + +// --- lookupIdempotentDelegation --- + +func TestLookupIdempotentDelegation_NoKey(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + if hit := lookupIdempotentDelegation(context.Background(), c, "ws-x", ""); hit { + t.Error("empty key should never hit") + } +} + +func TestLookupIdempotentDelegation_NoMatch(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + mock.ExpectQuery("SELECT request_body->>'delegation_id', status, target_id"). + WithArgs("ws-x", "some-key"). + WillReturnError(fmt.Errorf("sql: no rows")) + + if hit := lookupIdempotentDelegation(context.Background(), c, "ws-x", "some-key"); hit { + t.Error("expected false when no row found") + } +} + +func TestLookupIdempotentDelegation_FailedRowDeleted(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + mock.ExpectQuery("SELECT request_body->>'delegation_id', status, target_id"). + WithArgs("ws-x", "k"). + WillReturnRows(sqlmock.NewRows([]string{"delegation_id", "status", "target_id"}). + AddRow("old", "failed", "ws-target")) + mock.ExpectExec("DELETE FROM activity_logs"). + WithArgs("ws-x", "k"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + if hit := lookupIdempotentDelegation(context.Background(), c, "ws-x", "k"); hit { + t.Error("failed row should be released, returning false") + } +} + +func TestLookupIdempotentDelegation_ExistingPending(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + mock.ExpectQuery("SELECT request_body->>'delegation_id', status, target_id"). + WithArgs("ws-x", "k"). + WillReturnRows(sqlmock.NewRows([]string{"delegation_id", "status", "target_id"}). + AddRow("del-123", "pending", "ws-target")) + + if hit := lookupIdempotentDelegation(context.Background(), c, "ws-x", "k"); !hit { + t.Fatal("expected hit=true") + } + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["delegation_id"] != "del-123" || resp["idempotent_hit"] != true { + t.Errorf("unexpected response: %v", resp) + } +} + +// --- insertDelegationRow --- + +func TestInsertDelegationRow_Success(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + mock.ExpectExec("INSERT INTO activity_logs"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + out := insertDelegationRow(context.Background(), c, + "ws-src", + delegateRequest{TargetID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", Task: "hi"}, + "del-1") + if out != insertOK { + t.Errorf("got %v, want insertOK", out) + } +} + +func TestInsertDelegationRow_IdempotentConflict(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + mock.ExpectExec("INSERT INTO activity_logs"). + WillReturnError(fmt.Errorf("pq: duplicate key value violates unique constraint")) + mock.ExpectQuery("SELECT request_body->>'delegation_id', status"). + WithArgs("ws-src", "k1"). + WillReturnRows(sqlmock.NewRows([]string{"delegation_id", "status"}). + AddRow("winner-del", "pending")) + + out := insertDelegationRow(context.Background(), c, + "ws-src", + delegateRequest{TargetID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", Task: "hi", IdempotencyKey: "k1"}, + "loser-del") + if out != insertHandledByIdempotent { + t.Errorf("got %v, want insertHandledByIdempotent", out) + } + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestInsertDelegationRow_OtherDBError(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + + // Without IdempotencyKey, the follow-up SELECT is skipped — any insert + // error falls straight to insertTrackingUnavailable. + mock.ExpectExec("INSERT INTO activity_logs"). + WillReturnError(fmt.Errorf("connection refused")) + + out := insertDelegationRow(context.Background(), c, + "ws-src", + delegateRequest{TargetID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", Task: "hi"}, + "del-x") + if out != insertTrackingUnavailable { + t.Errorf("got %v, want insertTrackingUnavailable", out) + } +} + +// Verify the enum zero-value sentinel is defined and distinct from real outcomes. +func TestInsertDelegationOutcome_ZeroValueIsUnknown(t *testing.T) { + var zero insertDelegationOutcome + if zero != insertOutcomeUnknown { + t.Errorf("zero-value insertDelegationOutcome should equal insertOutcomeUnknown") + } + if insertOutcomeUnknown == insertOK { + t.Errorf("insertOutcomeUnknown must not collide with insertOK") + } +} diff --git a/platform/internal/handlers/discovery_test.go b/platform/internal/handlers/discovery_test.go index ae6bf8c3..5b16738a 100644 --- a/platform/internal/handlers/discovery_test.go +++ b/platform/internal/handlers/discovery_test.go @@ -2,6 +2,7 @@ package handlers import ( "bytes" + "context" "database/sql" "encoding/json" "net/http" @@ -324,3 +325,296 @@ func TestCheckAccess_SameWorkspace(t *testing.T) { t.Errorf("expected allowed=true for same workspace, got %v", resp["allowed"]) } } + +// ==================== Direct unit tests for extracted helpers ==================== + +// --- discoverWorkspacePeer --- + +func TestDiscoverWorkspacePeer_Online(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + // name/runtime lookup → non-external + mock.ExpectQuery(`SELECT COALESCE\(name,''\), COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-online"). + WillReturnRows(sqlmock.NewRows([]string{"name", "runtime"}).AddRow("Target", "langgraph")) + // No cached internal URL → DB status lookup → online + mock.ExpectQuery(`SELECT status FROM workspaces WHERE id =`). + WithArgs("ws-online"). + WillReturnRows(sqlmock.NewRows([]string{"status"}).AddRow("online")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverWorkspacePeer(context.Background(), c, "ws-caller", "ws-online") + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["id"] != "ws-online" || resp["url"] == "" { + t.Errorf("unexpected body: %v", resp) + } +} + +func TestDiscoverWorkspacePeer_NotFound(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT COALESCE\(name,''\), COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-missing"). + WillReturnRows(sqlmock.NewRows([]string{"name", "runtime"}).AddRow("", "langgraph")) + mock.ExpectQuery(`SELECT status FROM workspaces WHERE id =`). + WithArgs("ws-missing"). + WillReturnError(sql.ErrNoRows) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverWorkspacePeer(context.Background(), c, "ws-caller", "ws-missing") + + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestDiscoverWorkspacePeer_ExternalRuntime_HandledByExternalURL(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT COALESCE\(name,''\), COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-ext"). + WillReturnRows(sqlmock.NewRows([]string{"name", "runtime"}).AddRow("Ext", "external")) + // writeExternalWorkspaceURL's two queries + mock.ExpectQuery(`SELECT COALESCE\(url,''\) FROM workspaces WHERE id =`). + WithArgs("ws-ext"). + WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("http://external.example")) + mock.ExpectQuery(`SELECT COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-caller"). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("external")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverWorkspacePeer(context.Background(), c, "ws-caller", "ws-ext") + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestDiscoverWorkspacePeer_CachedInternalURLHit(t *testing.T) { + mock := setupTestDB(t) + mr := setupTestRedis(t) + + mock.ExpectQuery(`SELECT COALESCE\(name,''\), COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-cached"). + WillReturnRows(sqlmock.NewRows([]string{"name", "runtime"}).AddRow("Cached", "langgraph")) + mr.Set("ws:ws-cached:internal_url", "http://ws-cached:8000") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverWorkspacePeer(context.Background(), c, "ws-caller", "ws-cached") + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["url"] != "http://ws-cached:8000" { + t.Errorf("expected cached internal URL, got %v", resp["url"]) + } +} + +func TestDiscoverWorkspacePeer_NotReachable(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT COALESCE\(name,''\), COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-paused"). + WillReturnRows(sqlmock.NewRows([]string{"name", "runtime"}).AddRow("Paused", "langgraph")) + mock.ExpectQuery(`SELECT status FROM workspaces WHERE id =`). + WithArgs("ws-paused"). + WillReturnRows(sqlmock.NewRows([]string{"status"}).AddRow("paused")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverWorkspacePeer(context.Background(), c, "ws-caller", "ws-paused") + + if w.Code != http.StatusServiceUnavailable { + t.Errorf("expected 503, got %d: %s", w.Code, w.Body.String()) + } +} + +// --- writeExternalWorkspaceURL --- + +func TestWriteExternalWorkspaceURL_Success(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT COALESCE\(url,''\) FROM workspaces WHERE id =`). + WithArgs("ws-ext"). + WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("http://external.example/a2a")) + mock.ExpectQuery(`SELECT COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-caller"). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("langgraph")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + handled := writeExternalWorkspaceURL(context.Background(), c, "ws-caller", "ws-ext", "External WS") + if !handled { + t.Error("expected handled=true when URL present") + } + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["url"] != "http://external.example/a2a" { + t.Errorf("got url %v", resp["url"]) + } + if resp["name"] != "External WS" { + t.Errorf("got name %v", resp["name"]) + } +} + +func TestWriteExternalWorkspaceURL_NoURL_FallsThrough(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT COALESCE\(url,''\) FROM workspaces WHERE id =`). + WithArgs("ws-ext"). + WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + if handled := writeExternalWorkspaceURL(context.Background(), c, "ws-caller", "ws-ext", ""); handled { + t.Error("expected handled=false when URL empty") + } +} + +func TestWriteExternalWorkspaceURL_RewritesLocalhostForDockerCaller(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT COALESCE\(url,''\) FROM workspaces WHERE id =`). + WithArgs("ws-ext"). + WillReturnRows(sqlmock.NewRows([]string{"url"}).AddRow("http://127.0.0.1:8000/a2a")) + // non-external caller runtime → rewrite enabled + mock.ExpectQuery(`SELECT COALESCE\(runtime,'langgraph'\) FROM workspaces WHERE id =`). + WithArgs("ws-caller"). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("langgraph")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + writeExternalWorkspaceURL(context.Background(), c, "ws-caller", "ws-ext", "") + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["url"] != "http://host.docker.internal:8000/a2a" { + t.Errorf("expected 127.0.0.1 → host.docker.internal rewrite, got %v", resp["url"]) + } +} + +// --- discoverHostPeer smoke (currently unreachable via Discover) --- + +func TestDiscoverHostPeer_Smoke_CacheHit(t *testing.T) { + setupTestDB(t) + mr := setupTestRedis(t) + mr.Set("ws:ws-host:url", "http://hostcache.example") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverHostPeer(context.Background(), c, "ws-host") + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestDiscoverHostPeer_Smoke_NotFound(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT url, status, forwarded_to FROM workspaces WHERE id =`). + WithArgs("ws-none"). + WillReturnError(sql.ErrNoRows) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverHostPeer(context.Background(), c, "ws-none") + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d", w.Code) + } +} + +func TestDiscoverHostPeer_Smoke_DBError(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT url, status, forwarded_to FROM workspaces WHERE id =`). + WithArgs("ws-err"). + WillReturnError(sql.ErrConnDone) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverHostPeer(context.Background(), c, "ws-err") + if w.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d", w.Code) + } +} + +func TestDiscoverHostPeer_Smoke_ForwardedChainAndNullURL(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT url, status, forwarded_to FROM workspaces WHERE id =`). + WithArgs("ws-a"). + WillReturnRows(sqlmock.NewRows([]string{"url", "status", "forwarded_to"}).AddRow(nil, "online", "ws-b")) + mock.ExpectQuery(`SELECT url, status, forwarded_to FROM workspaces WHERE id =`). + WithArgs("ws-b"). + WillReturnRows(sqlmock.NewRows([]string{"url", "status", "forwarded_to"}).AddRow(nil, "offline", nil)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverHostPeer(context.Background(), c, "ws-a") + if w.Code != http.StatusServiceUnavailable { + t.Errorf("expected 503 for null URL after chain, got %d", w.Code) + } +} + +func TestDiscoverHostPeer_Smoke_Success(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT url, status, forwarded_to FROM workspaces WHERE id =`). + WithArgs("ws-ok"). + WillReturnRows(sqlmock.NewRows([]string{"url", "status", "forwarded_to"}).AddRow("http://ok.example", "online", nil)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/x", nil) + + discoverHostPeer(context.Background(), c, "ws-ok") + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +}