diff --git a/canvas/src/components/__tests__/BudgetLimit.DetailsTab.test.tsx b/canvas/src/components/__tests__/BudgetLimit.DetailsTab.test.tsx index a9515374..6709a996 100644 --- a/canvas/src/components/__tests__/BudgetLimit.DetailsTab.test.tsx +++ b/canvas/src/components/__tests__/BudgetLimit.DetailsTab.test.tsx @@ -44,6 +44,15 @@ vi.mock("../tabs/BudgetSection", () => ({ ), })); +// Mock WorkspaceUsage — it has its own test suite (WorkspaceUsage.test.tsx). +// Without this mock its internal api.get call races against the shared mock +// and crashes when the return value is not a valid WorkspaceMetrics object. +vi.mock("../WorkspaceUsage", () => ({ + WorkspaceUsage: ({ workspaceId }: { workspaceId: string }) => ( +
+ ), +})); + import { api } from "@/lib/api"; import { DetailsTab } from "../tabs/DetailsTab"; diff --git a/canvas/src/components/__tests__/ClaudeSettings.test.tsx b/canvas/src/components/__tests__/ClaudeSettings.test.tsx new file mode 100644 index 00000000..ade36ac5 --- /dev/null +++ b/canvas/src/components/__tests__/ClaudeSettings.test.tsx @@ -0,0 +1,236 @@ +// @vitest-environment jsdom +/** + * Tests for issue #608 — effort + task_budget fields in workspace config. + * + * Covers: + * 1. toYaml serialization (effort + task_budget → YAML keys) + * 2. parseYaml round-trip (YAML → ConfigData) + * 3. DEFAULT_CONFIG shape (new fields present with zero/empty defaults) + * 4. ConfigTab source assertions (section rendered conditionally) + * 5. React rendering of the section for claude-code and claude model configs + */ +import React from "react"; +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; + +// ── Module-level mocks ─────────────────────────────────────────────────────── + +vi.mock("@/lib/api", () => ({ + api: { get: vi.fn(), put: vi.fn(), patch: vi.fn(), post: vi.fn() }, +})); + +vi.mock("@/store/canvas", () => ({ + useCanvasStore: vi.fn(() => ({ + restartWorkspace: vi.fn(), + updateNodeData: vi.fn(), + })), +})); + +vi.mock("../tabs/config/secrets-section", () => ({ + SecretsSection: () =>
, +})); + +// ── Imports ────────────────────────────────────────────────────────────────── + +import { toYaml, parseYaml } from "../tabs/config/yaml-utils"; +import { DEFAULT_CONFIG, type ConfigData } from "../tabs/config/form-inputs"; +import { ConfigTab } from "../tabs/ConfigTab"; +import { api } from "@/lib/api"; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +// ── 1. toYaml serialization ────────────────────────────────────────────────── + +describe("toYaml — effort field", () => { + it("omits effort when empty string", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "" }; + expect(toYaml(cfg)).not.toContain("effort:"); + }); + + it("omits effort when undefined", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: undefined }; + expect(toYaml(cfg)).not.toContain("effort:"); + }); + + it("serializes effort: low", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "low" }; + const yaml = toYaml(cfg); + expect(yaml).toContain("effort: low"); + }); + + it("serializes effort: medium", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "medium" }; + expect(toYaml(cfg)).toContain("effort: medium"); + }); + + it("serializes effort: high", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "high" }; + expect(toYaml(cfg)).toContain("effort: high"); + }); + + it("serializes effort: xhigh", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "xhigh" }; + expect(toYaml(cfg)).toContain("effort: xhigh"); + }); + + it("serializes effort: max", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "max" }; + expect(toYaml(cfg)).toContain("effort: max"); + }); +}); + +describe("toYaml — task_budget field", () => { + it("omits task_budget when 0", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, task_budget: 0 }; + expect(toYaml(cfg)).not.toContain("task_budget:"); + }); + + it("omits task_budget when undefined", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, task_budget: undefined }; + expect(toYaml(cfg)).not.toContain("task_budget:"); + }); + + it("serializes task_budget: 10000", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, task_budget: 10000 }; + expect(toYaml(cfg)).toContain("task_budget: 10000"); + }); + + it("serializes task_budget: 50000", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, task_budget: 50000 }; + expect(toYaml(cfg)).toContain("task_budget: 50000"); + }); +}); + +describe("toYaml — effort and task_budget together", () => { + it("serializes both when set", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "xhigh", task_budget: 32000 }; + const yaml = toYaml(cfg); + expect(yaml).toContain("effort: xhigh"); + expect(yaml).toContain("task_budget: 32000"); + }); + + it("effort appears before task_budget in output", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "high", task_budget: 8000 }; + const yaml = toYaml(cfg); + const effortIdx = yaml.indexOf("effort:"); + const budgetIdx = yaml.indexOf("task_budget:"); + expect(effortIdx).toBeGreaterThan(-1); + expect(budgetIdx).toBeGreaterThan(-1); + expect(effortIdx).toBeLessThan(budgetIdx); + }); +}); + +// ── 2. parseYaml round-trip ────────────────────────────────────────────────── + +describe("parseYaml — effort + task_budget round-trip", () => { + it("parses effort from YAML", () => { + const yaml = "name: Test\neffort: high\n"; + const parsed = parseYaml(yaml); + expect(parsed.effort).toBe("high"); + }); + + it("parses task_budget from YAML as integer", () => { + const yaml = "name: Test\ntask_budget: 16000\n"; + const parsed = parseYaml(yaml); + expect(parsed.task_budget).toBe(16000); + }); + + it("round-trips effort: xhigh through toYaml → parseYaml", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "xhigh" }; + const yaml = toYaml(cfg); + const parsed = parseYaml(yaml); + expect(parsed.effort).toBe("xhigh"); + }); + + it("round-trips task_budget: 50000 through toYaml → parseYaml", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, task_budget: 50000 }; + const yaml = toYaml(cfg); + const parsed = parseYaml(yaml); + expect(parsed.task_budget).toBe(50000); + }); + + it("round-trips both fields together", () => { + const cfg: ConfigData = { ...DEFAULT_CONFIG, effort: "low", task_budget: 1000 }; + const yaml = toYaml(cfg); + const parsed = parseYaml(yaml); + expect(parsed.effort).toBe("low"); + expect(parsed.task_budget).toBe(1000); + }); +}); + +// ── 3. DEFAULT_CONFIG shape ────────────────────────────────────────────────── + +describe("DEFAULT_CONFIG", () => { + it("has effort defaulting to empty string", () => { + expect(DEFAULT_CONFIG.effort).toBe(""); + }); + + it("has task_budget defaulting to 0", () => { + expect(DEFAULT_CONFIG.task_budget).toBe(0); + }); +}); + +// ── 4. ConfigTab source assertions ────────────────────────────────────────── + +describe("ConfigTab source — Claude Settings section", () => { + it("ConfigTab.tsx contains the effort-select data-testid", async () => { + const { readFileSync } = await import("fs"); + const { join } = await import("path"); + const src = readFileSync(join(__dirname, "../../components/tabs/ConfigTab.tsx"), "utf8"); + expect(src).toContain('data-testid="effort-select"'); + expect(src).toContain('data-testid="task-budget-input"'); + }); + + it("ConfigTab.tsx effort dropdown has all five Claude values", async () => { + const { readFileSync } = await import("fs"); + const { join } = await import("path"); + const src = readFileSync(join(__dirname, "../../components/tabs/ConfigTab.tsx"), "utf8"); + expect(src).toContain('"low"'); + expect(src).toContain('"medium"'); + expect(src).toContain('"high"'); + expect(src).toContain('"xhigh"'); + expect(src).toContain('"max"'); + }); + + it("ConfigTab.tsx section is guarded by claude-code runtime check", async () => { + const { readFileSync } = await import("fs"); + const { join } = await import("path"); + const src = readFileSync(join(__dirname, "../../components/tabs/ConfigTab.tsx"), "utf8"); + expect(src).toContain('config.runtime === "claude-code"'); + expect(src).toContain('"claude"'); + }); +}); + +// ── 5. React rendering ─────────────────────────────────────────────────────── + +describe("ConfigTab — Claude Settings section rendering", () => { + function setupMock(configYaml: string) { + vi.mocked(api.get).mockResolvedValue({ content: configYaml } as never); + } + + it("shows Claude Settings section for claude-code runtime", async () => { + setupMock("name: Bot\nruntime: claude-code\n"); + render(); + // Section title appears once loading resolves + const section = await screen.findByText("Claude Settings"); + expect(section).toBeTruthy(); + }); + + it("shows Claude Settings section when model contains claude", async () => { + setupMock("name: Bot\nmodel: anthropic:claude-opus-4-7\n"); + render(); + const section = await screen.findByText("Claude Settings"); + expect(section).toBeTruthy(); + }); + + it("does NOT show Claude Settings section for non-claude runtime/model", async () => { + setupMock("name: Bot\nruntime: crewai\nmodel: openai:gpt-4o\n"); + render(); + // Wait for load (config.yaml fetch resolves) then check absence + await screen.findByText("General"); // loaded + expect(screen.queryByText("Claude Settings")).toBeNull(); + }); +}); diff --git a/canvas/src/components/tabs/ConfigTab.tsx b/canvas/src/components/tabs/ConfigTab.tsx index 6e600cd4..494cec00 100644 --- a/canvas/src/components/tabs/ConfigTab.tsx +++ b/canvas/src/components/tabs/ConfigTab.tsx @@ -267,6 +267,49 @@ export function ConfigTab({ workspaceId }: Props) { updateNested("runtime_config" as keyof ConfigData, "required_env", v)} placeholder="e.g. CLAUDE_CODE_OAUTH_TOKEN" /> + {/* Claude Settings — shown for claude-code runtime or claude/anthropic model names */} + {(config.runtime === "claude-code" || + (config.runtime_config?.model || config.model || "").toLowerCase().includes("claude") || + (config.runtime_config?.model || config.model || "").toLowerCase().includes("anthropic")) && ( +
+
+ + +
+
+ + update("task_budget", parseInt(e.target.value, 10) || 0)} + placeholder="0" + className="w-full bg-zinc-800 border border-zinc-700 rounded px-2 py-1 text-xs text-zinc-200 focus:outline-none focus:border-blue-500 font-mono" + data-testid="task-budget-input" + /> +
+
+ )} +
update("skills", v)} placeholder="e.g. code-review" /> update("tools", v)} placeholder="e.g. web_search, filesystem" /> diff --git a/canvas/src/components/tabs/config/form-inputs.tsx b/canvas/src/components/tabs/config/form-inputs.tsx index 63e983d6..58c75101 100644 --- a/canvas/src/components/tabs/config/form-inputs.tsx +++ b/canvas/src/components/tabs/config/form-inputs.tsx @@ -16,6 +16,11 @@ export interface ConfigData { // Deprecated auth_token_file?: string; }; + // Claude API primitives (Opus 4.7+) — issue #608 + // effort maps to output_config.effort in Messages API: 'low' | 'medium' | 'high' | 'xhigh' + effort?: string; + // task_budget maps to output_config.task_budget.total (requires beta header task-budgets-2026-03-13) + task_budget?: number; prompt_files: string[]; shared_context: string[]; skills: string[]; @@ -32,6 +37,8 @@ export const DEFAULT_CONFIG: ConfigData = { tier: 1, model: "", runtime: "", + effort: "", + task_budget: 0, prompt_files: [], shared_context: [], skills: [], diff --git a/canvas/src/components/tabs/config/yaml-utils.ts b/canvas/src/components/tabs/config/yaml-utils.ts index 752bd7ab..77ffff2d 100644 --- a/canvas/src/components/tabs/config/yaml-utils.ts +++ b/canvas/src/components/tabs/config/yaml-utils.ts @@ -116,6 +116,9 @@ export function toYaml(config: ConfigData): string { } } if (config.model) { lines.push(""); simple("model", config.model); } + // Claude API primitives (issue #608) + if (config.effort) { lines.push(""); simple("effort", config.effort); } + if (config.task_budget && config.task_budget > 0) { simple("task_budget", config.task_budget); } if (config.prompt_files?.length) { lines.push(""); list("prompt_files", config.prompt_files); } if (config.shared_context?.length) { lines.push(""); list("shared_context", config.shared_context); } lines.push(""); list("skills", config.skills); diff --git a/docs/ecosystem-watch.md b/docs/ecosystem-watch.md index 4811a12b..fb49e24b 100644 --- a/docs/ecosystem-watch.md +++ b/docs/ecosystem-watch.md @@ -2570,3 +2570,21 @@ langgraph/crewai adapters. **Signals to react to:** If the repo adds a framework layer (reusable agent registry, scheduling, persistence) → escalate to MEDIUM. If finance-sector enterprises request a hedge-fund template → ship one. **Last reviewed:** 2026-04-17 · **Stars / activity:** 55,750 ⭐, +763 today, MIT + +--- + +### Strix — `usestrix/strix` + +**Pitch:** "Open-source AI hackers to find and fix your app's vulnerabilities." + +**Shape:** Python (91.6%), Apache-2.0, 24.1k ⭐, available on PyPI as `strix-agent`. CLI-first autonomous security testing platform built on a **graph of agents** architecture: specialized agents coordinate in parallel across attack vectors (injection, SSRF, XSS, IDOR, auth bypass, and more), validate findings with real proof-of-concepts rather than static analysis flags, and emit actionable remediation reports. Toolkit includes HTTP proxy, browser automation, terminal environments, and a Python runtime harness. Supports CI/CD pipeline integration. + +**Overlap with us:** (1) Multi-agent graph architecture is conceptually aligned — parallel specialist agents, dynamic coordination, result aggregation. Not an orchestration framework, but a production signal that autonomous multi-agent pipelines are proven in security verticals. (2) CI/CD integration pattern mirrors how Molecule AI workspaces are embedded in dev pipelines. (3) The auto-remediation + structured reporting loop is a demand signal for audit-trail and human-oversight patterns — directly adjacent to the `molecule-audit-ledger` work (GH #594) and our EU AI Act compliance posture. + +**Differentiation:** Domain-locked (security only), no visual canvas, no org hierarchy, no scheduling, no A2A interoperability. Not a competing platform — a vertical application on top of agent primitives similar to what a Molecule AI org template could deliver. + +**Worth borrowing:** Proof-of-concept validation pattern (agents confirm exploits rather than flag suspects) as a model for grounding agent outputs with verifiable artifacts. Their `--ci` mode integration pattern is worth referencing for the playwright-mcp plugin CI workflow. + +**Signals to react to:** If Strix ships an agent SDK / plugin API → they become a platform player, escalate to MEDIUM. If enterprise security teams start asking about Molecule AI + Strix integration → document a reference org template. + +**Last reviewed:** 2026-04-17 · **Stars / activity:** 24,100 ⭐, +202 today, PyPI `strix-agent` diff --git a/platform/internal/artifacts/client.go b/platform/internal/artifacts/client.go new file mode 100644 index 00000000..18510e00 --- /dev/null +++ b/platform/internal/artifacts/client.go @@ -0,0 +1,282 @@ +// Package artifacts provides a minimal Go client for the Cloudflare Artifacts +// REST API (private beta Apr 2026, public beta May 2026). +// +// API reference: https://developers.cloudflare.com/artifacts/api/rest-api/ +// Blog post: https://blog.cloudflare.com/artifacts-git-for-agents-beta/ +// +// Base URL: https://artifacts.cloudflare.net/v1/api/namespaces/{namespace} +// Auth: Authorization: Bearer +// +// This client covers the subset of operations needed for the Molecule AI +// workspace-snapshot demo: +// - CreateRepo — provision a bare Git repo for a workspace +// - GetRepo — fetch repo metadata (remote URL, created_at, …) +// - ForkRepo — create an isolated copy (e.g. workspace branching) +// - ImportRepo — bootstrap from an external GitHub/GitLab URL +// - DeleteRepo — clean-up +// - CreateToken — mint a short-lived Git credential for clone/push +// - RevokeToken — invalidate an issued token +package artifacts + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "time" +) + +const ( + defaultBaseURL = "https://artifacts.cloudflare.net/v1/api" + defaultTimeout = 30 * time.Second +) + +// Client is a thin HTTP wrapper around the Cloudflare Artifacts REST API. +// Instantiate with New(); override BaseURL in tests via NewWithBaseURL(). +type Client struct { + baseURL string // e.g. https://artifacts.cloudflare.net/v1/api/namespaces/my-ns + apiToken string // Cloudflare API token — never logged + httpClient *http.Client +} + +// New returns a Client scoped to the given namespace. +// apiToken is a Cloudflare API token with Artifacts write permissions. +// namespace identifies the CF Artifacts namespace (maps to CLOUDFLARE_ARTIFACTS_NAMESPACE). +func New(apiToken, namespace string) *Client { + return NewWithBaseURL(apiToken, namespace, defaultBaseURL) +} + +// NewWithBaseURL is the same as New but lets callers override the base URL — +// primarily used in unit tests to point at an httptest.Server. +func NewWithBaseURL(apiToken, namespace, baseURL string) *Client { + ns := url.PathEscape(namespace) + return &Client{ + baseURL: fmt.Sprintf("%s/namespaces/%s", baseURL, ns), + apiToken: apiToken, + httpClient: &http.Client{ + Timeout: defaultTimeout, + }, + } +} + +// ---- Domain types -------------------------------------------------------- + +// Repo represents a single Cloudflare Artifacts repository. +type Repo struct { + // Name is the user-supplied identifier within the namespace. + Name string `json:"name"` + // ID is the opaque CF-assigned identifier. + ID string `json:"id,omitempty"` + // RemoteURL is the authenticated Git remote in the form + // https://x:@.artifacts.cloudflare.net/git/repo-.git + RemoteURL string `json:"remote_url,omitempty"` + // ReadOnly marks repos that accept only fetch/clone operations. + ReadOnly bool `json:"read_only,omitempty"` + // Description is an optional human-readable label. + Description string `json:"description,omitempty"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` +} + +// ForkResult is the response body from POST /repos/:name/fork. +type ForkResult struct { + Repo Repo `json:"repo"` + ObjectCount int `json:"object_count,omitempty"` +} + +// RepoToken is a short-lived credential for Git operations against a single repo. +// The plaintext Token value is returned only once — callers must store it. +type RepoToken struct { + ID string `json:"id"` + Token string `json:"token"` + Scope string `json:"scope"` // "read" | "write" + ExpiresAt time.Time `json:"expires_at"` +} + +// ---- Request payloads ---------------------------------------------------- + +// CreateRepoRequest is the body for POST /repos. +type CreateRepoRequest struct { + Name string `json:"name"` + Description string `json:"description,omitempty"` + DefaultBranch string `json:"default_branch,omitempty"` + ReadOnly bool `json:"read_only,omitempty"` +} + +// ForkRepoRequest is the body for POST /repos/:name/fork. +type ForkRepoRequest struct { + Name string `json:"name"` + Description string `json:"description,omitempty"` + ReadOnly bool `json:"read_only,omitempty"` + DefaultBranchOnly bool `json:"default_branch_only,omitempty"` +} + +// ImportRepoRequest is the body for POST /repos/:name/import. +type ImportRepoRequest struct { + // URL is the HTTPS URL of the source Git repository. + URL string `json:"url"` + Branch string `json:"branch,omitempty"` + Depth int `json:"depth,omitempty"` + ReadOnly bool `json:"read_only,omitempty"` +} + +// CreateTokenRequest is the body for POST /tokens. +type CreateTokenRequest struct { + // Repo is the name of the repository to scope the token to. + Repo string `json:"repo"` + Scope string `json:"scope,omitempty"` // "read" | "write"; default "write" + // TTL is the lifetime in seconds. Default 86400 (24h). + TTL int `json:"ttl,omitempty"` +} + +// ---- API error ----------------------------------------------------------- + +// APIError represents a non-2xx response from the Cloudflare v4 envelope. +type APIError struct { + StatusCode int + Code int `json:"code"` + Message string `json:"message"` +} + +func (e *APIError) Error() string { + return fmt.Sprintf("cloudflare artifacts: HTTP %d — code %d: %s", e.StatusCode, e.Code, e.Message) +} + +// ---- HTTP helpers -------------------------------------------------------- + +// do executes an HTTP request, checks the Cloudflare v4 envelope, and +// JSON-decodes the "result" field into out (pass nil to discard). +func (c *Client) do(ctx context.Context, method, path string, body, out interface{}) error { + var bodyReader io.Reader + if body != nil { + b, err := json.Marshal(body) + if err != nil { + return fmt.Errorf("artifacts: marshal request: %w", err) + } + bodyReader = bytes.NewReader(b) + } + + req, err := http.NewRequestWithContext(ctx, method, c.baseURL+path, bodyReader) + if err != nil { + return fmt.Errorf("artifacts: build request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.apiToken) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("artifacts: request %s %s: %w", method, path, err) + } + defer resp.Body.Close() + + // Decode the Cloudflare v4 envelope. Cap at 1 MiB to prevent a + // malicious or runaway upstream response from exhausting memory. + var envelope struct { + Result json.RawMessage `json:"result"` + Success bool `json:"success"` + Errors []struct { + Code int `json:"code"` + Message string `json:"message"` + } `json:"errors"` + } + if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&envelope); err != nil { + // Non-JSON body (network error page, etc.) + return &APIError{StatusCode: resp.StatusCode, Message: fmt.Sprintf("non-JSON body (status %d)", resp.StatusCode)} + } + + if !envelope.Success || resp.StatusCode >= 300 { + apiErr := &APIError{StatusCode: resp.StatusCode} + if len(envelope.Errors) > 0 { + apiErr.Code = envelope.Errors[0].Code + apiErr.Message = envelope.Errors[0].Message + } else { + apiErr.Message = "unknown error" + } + return apiErr + } + + if out != nil && len(envelope.Result) > 0 { + if err := json.Unmarshal(envelope.Result, out); err != nil { + return fmt.Errorf("artifacts: decode result: %w", err) + } + } + return nil +} + +// ---- Repo operations ----------------------------------------------------- + +// CreateRepo provisions a new bare Git repo in the namespace. +// Corresponds to POST /repos. +func (c *Client) CreateRepo(ctx context.Context, req CreateRepoRequest) (*Repo, error) { + var repo Repo + if err := c.do(ctx, http.MethodPost, "/repos", req, &repo); err != nil { + return nil, err + } + return &repo, nil +} + +// GetRepo fetches metadata for an existing repo. +// Corresponds to GET /repos/:name. +func (c *Client) GetRepo(ctx context.Context, name string) (*Repo, error) { + var repo Repo + path := "/repos/" + url.PathEscape(name) + if err := c.do(ctx, http.MethodGet, path, nil, &repo); err != nil { + return nil, err + } + return &repo, nil +} + +// ForkRepo creates an isolated copy of an existing repo. +// Corresponds to POST /repos/:name/fork. +func (c *Client) ForkRepo(ctx context.Context, sourceName string, req ForkRepoRequest) (*ForkResult, error) { + var result ForkResult + path := "/repos/" + url.PathEscape(sourceName) + "/fork" + if err := c.do(ctx, http.MethodPost, path, req, &result); err != nil { + return nil, err + } + return &result, nil +} + +// ImportRepo bootstraps a repo from an external HTTPS Git URL. +// Corresponds to POST /repos/:name/import. +func (c *Client) ImportRepo(ctx context.Context, name string, req ImportRepoRequest) (*Repo, error) { + var repo Repo + path := "/repos/" + url.PathEscape(name) + "/import" + if err := c.do(ctx, http.MethodPost, path, req, &repo); err != nil { + return nil, err + } + return &repo, nil +} + +// DeleteRepo deletes a repo (returns 202 Accepted). +// Corresponds to DELETE /repos/:name. +func (c *Client) DeleteRepo(ctx context.Context, name string) error { + path := "/repos/" + url.PathEscape(name) + return c.do(ctx, http.MethodDelete, path, nil, nil) +} + +// ---- Token operations ---------------------------------------------------- + +// CreateToken mints a short-lived Git credential scoped to a single repo. +// The plaintext token is in the returned RepoToken.Token field — it will not +// be available again after this call returns. +// Corresponds to POST /tokens. +func (c *Client) CreateToken(ctx context.Context, req CreateTokenRequest) (*RepoToken, error) { + var token RepoToken + if err := c.do(ctx, http.MethodPost, "/tokens", req, &token); err != nil { + return nil, err + } + return &token, nil +} + +// RevokeToken invalidates an issued token by its ID. +// Corresponds to DELETE /tokens/:id. +func (c *Client) RevokeToken(ctx context.Context, tokenID string) error { + path := "/tokens/" + url.PathEscape(tokenID) + return c.do(ctx, http.MethodDelete, path, nil, nil) +} diff --git a/platform/internal/artifacts/client_test.go b/platform/internal/artifacts/client_test.go new file mode 100644 index 00000000..c8519334 --- /dev/null +++ b/platform/internal/artifacts/client_test.go @@ -0,0 +1,370 @@ +package artifacts_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/Molecule-AI/molecule-monorepo/platform/internal/artifacts" +) + +// cfEnvelope wraps a result value in the Cloudflare v4 response envelope. +func cfEnvelope(t *testing.T, result interface{}) []byte { + t.Helper() + b, err := json.Marshal(result) + if err != nil { + t.Fatalf("cfEnvelope: marshal result: %v", err) + } + env := map[string]interface{}{ + "success": true, + "result": json.RawMessage(b), + "errors": []interface{}{}, + } + out, err := json.Marshal(env) + if err != nil { + t.Fatalf("cfEnvelope: marshal envelope: %v", err) + } + return out +} + +// cfError returns a Cloudflare v4 error envelope. +func cfError(t *testing.T, statusCode, code int, message string) ([]byte, int) { + t.Helper() + env := map[string]interface{}{ + "success": false, + "result": nil, + "errors": []map[string]interface{}{ + {"code": code, "message": message}, + }, + } + b, _ := json.Marshal(env) + return b, statusCode +} + +func newTestClient(t *testing.T, mux *http.ServeMux) *artifacts.Client { + t.Helper() + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return artifacts.NewWithBaseURL("test-token", "test-ns", srv.URL) +} + +// ---- CreateRepo ---------------------------------------------------------- + +func TestCreateRepo_Success(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + // Verify auth header + if r.Header.Get("Authorization") != "Bearer test-token" { + http.Error(w, "unauthorized", http.StatusUnauthorized) + return + } + // Decode request body + var req map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + http.Error(w, "bad request", http.StatusBadRequest) + return + } + if req["name"] != "my-workspace-repo" { + http.Error(w, "unexpected name", http.StatusBadRequest) + return + } + + repo := artifacts.Repo{ + Name: "my-workspace-repo", + ID: "repo-abc123", + RemoteURL: "https://x:tok@hash.artifacts.cloudflare.net/git/repo-abc123.git", + CreatedAt: time.Now(), + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfEnvelope(t, repo)) + }) + + client := newTestClient(t, mux) + repo, err := client.CreateRepo(context.Background(), artifacts.CreateRepoRequest{ + Name: "my-workspace-repo", + Description: "Molecule AI workspace snapshot", + }) + if err != nil { + t.Fatalf("CreateRepo: unexpected error: %v", err) + } + if repo.Name != "my-workspace-repo" { + t.Errorf("repo.Name = %q, want %q", repo.Name, "my-workspace-repo") + } + if repo.ID != "repo-abc123" { + t.Errorf("repo.ID = %q, want %q", repo.ID, "repo-abc123") + } + if repo.RemoteURL == "" { + t.Error("repo.RemoteURL is empty, want non-empty") + } +} + +func TestCreateRepo_APIError(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos", func(w http.ResponseWriter, r *http.Request) { + body, status := cfError(t, http.StatusConflict, 1009, "repo already exists") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + w.Write(body) + }) + + client := newTestClient(t, mux) + _, err := client.CreateRepo(context.Background(), artifacts.CreateRepoRequest{Name: "dup"}) + if err == nil { + t.Fatal("expected error, got nil") + } + apiErr, ok := err.(*artifacts.APIError) + if !ok { + t.Fatalf("expected *APIError, got %T: %v", err, err) + } + if apiErr.StatusCode != http.StatusConflict { + t.Errorf("StatusCode = %d, want %d", apiErr.StatusCode, http.StatusConflict) + } + if apiErr.Message != "repo already exists" { + t.Errorf("Message = %q, want %q", apiErr.Message, "repo already exists") + } +} + +// ---- GetRepo ------------------------------------------------------------- + +func TestGetRepo_Success(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos/my-repo", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + repo := artifacts.Repo{ + Name: "my-repo", + ID: "repo-xyz", + RemoteURL: "https://x:tok@hash.artifacts.cloudflare.net/git/repo-xyz.git", + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfEnvelope(t, repo)) + }) + + client := newTestClient(t, mux) + repo, err := client.GetRepo(context.Background(), "my-repo") + if err != nil { + t.Fatalf("GetRepo: unexpected error: %v", err) + } + if repo.Name != "my-repo" { + t.Errorf("repo.Name = %q, want %q", repo.Name, "my-repo") + } +} + +func TestGetRepo_NotFound(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos/missing", func(w http.ResponseWriter, r *http.Request) { + body, status := cfError(t, http.StatusNotFound, 1004, "repo not found") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + w.Write(body) + }) + + client := newTestClient(t, mux) + _, err := client.GetRepo(context.Background(), "missing") + if err == nil { + t.Fatal("expected error, got nil") + } + apiErr, ok := err.(*artifacts.APIError) + if !ok { + t.Fatalf("expected *APIError, got %T", err) + } + if apiErr.StatusCode != http.StatusNotFound { + t.Errorf("StatusCode = %d, want %d", apiErr.StatusCode, http.StatusNotFound) + } +} + +// ---- ForkRepo ------------------------------------------------------------ + +func TestForkRepo_Success(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos/source-repo/fork", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + var req map[string]interface{} + json.NewDecoder(r.Body).Decode(&req) + if req["name"] != "forked-repo" { + http.Error(w, "unexpected fork name", http.StatusBadRequest) + return + } + result := artifacts.ForkResult{ + Repo: artifacts.Repo{ + Name: "forked-repo", + ID: "repo-fork-1", + RemoteURL: "https://x:tok@hash.artifacts.cloudflare.net/git/repo-fork-1.git", + }, + ObjectCount: 42, + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfEnvelope(t, result)) + }) + + client := newTestClient(t, mux) + result, err := client.ForkRepo(context.Background(), "source-repo", artifacts.ForkRepoRequest{ + Name: "forked-repo", + }) + if err != nil { + t.Fatalf("ForkRepo: unexpected error: %v", err) + } + if result.Repo.Name != "forked-repo" { + t.Errorf("Repo.Name = %q, want %q", result.Repo.Name, "forked-repo") + } + if result.ObjectCount != 42 { + t.Errorf("ObjectCount = %d, want 42", result.ObjectCount) + } +} + +// ---- ImportRepo ---------------------------------------------------------- + +func TestImportRepo_Success(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos/imported/import", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + var req map[string]interface{} + json.NewDecoder(r.Body).Decode(&req) + if req["url"] == "" { + http.Error(w, "url required", http.StatusBadRequest) + return + } + repo := artifacts.Repo{ + Name: "imported", + ID: "repo-imp-1", + RemoteURL: "https://x:tok@hash.artifacts.cloudflare.net/git/repo-imp-1.git", + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfEnvelope(t, repo)) + }) + + client := newTestClient(t, mux) + repo, err := client.ImportRepo(context.Background(), "imported", artifacts.ImportRepoRequest{ + URL: "https://github.com/Molecule-AI/molecule-core.git", + Branch: "main", + Depth: 1, + }) + if err != nil { + t.Fatalf("ImportRepo: unexpected error: %v", err) + } + if repo.Name != "imported" { + t.Errorf("repo.Name = %q, want %q", repo.Name, "imported") + } +} + +// ---- DeleteRepo ---------------------------------------------------------- + +func TestDeleteRepo_Success(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos/to-delete", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + deleted := map[string]string{"id": "repo-del-1"} + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + w.Write(cfEnvelope(t, deleted)) + }) + + client := newTestClient(t, mux) + if err := client.DeleteRepo(context.Background(), "to-delete"); err != nil { + t.Fatalf("DeleteRepo: unexpected error: %v", err) + } +} + +// ---- CreateToken --------------------------------------------------------- + +func TestCreateToken_Success(t *testing.T) { + expiry := time.Now().Add(24 * time.Hour).UTC().Truncate(time.Second) + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/tokens", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + var req map[string]interface{} + json.NewDecoder(r.Body).Decode(&req) + if req["repo"] != "my-repo" { + http.Error(w, "unexpected repo", http.StatusBadRequest) + return + } + tok := artifacts.RepoToken{ + ID: "tok-123", + Token: "plaintext-secret-abc", + Scope: "write", + ExpiresAt: expiry, + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfEnvelope(t, tok)) + }) + + client := newTestClient(t, mux) + tok, err := client.CreateToken(context.Background(), artifacts.CreateTokenRequest{ + Repo: "my-repo", + Scope: "write", + TTL: 86400, + }) + if err != nil { + t.Fatalf("CreateToken: unexpected error: %v", err) + } + if tok.ID != "tok-123" { + t.Errorf("ID = %q, want %q", tok.ID, "tok-123") + } + if tok.Token != "plaintext-secret-abc" { + t.Errorf("Token = %q, want %q", tok.Token, "plaintext-secret-abc") + } + if tok.Scope != "write" { + t.Errorf("Scope = %q, want %q", tok.Scope, "write") + } +} + +// ---- RevokeToken --------------------------------------------------------- + +func TestRevokeToken_Success(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/tokens/tok-456", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + deleted := map[string]string{"id": "tok-456"} + w.Header().Set("Content-Type", "application/json") + w.Write(cfEnvelope(t, deleted)) + }) + + client := newTestClient(t, mux) + if err := client.RevokeToken(context.Background(), "tok-456"); err != nil { + t.Fatalf("RevokeToken: unexpected error: %v", err) + } +} + +// ---- Context cancellation ------------------------------------------------ + +func TestCreateRepo_ContextCancelled(t *testing.T) { + // Server that never responds (simulates a hung connection) + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos", func(w http.ResponseWriter, r *http.Request) { + // Block until the client gives up + <-r.Context().Done() + }) + + client := newTestClient(t, mux) + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately + + _, err := client.CreateRepo(ctx, artifacts.CreateRepoRequest{Name: "x"}) + if err == nil { + t.Fatal("expected error from cancelled context, got nil") + } +} diff --git a/platform/internal/handlers/artifacts.go b/platform/internal/handlers/artifacts.go new file mode 100644 index 00000000..2dec903f --- /dev/null +++ b/platform/internal/handlers/artifacts.go @@ -0,0 +1,455 @@ +package handlers + +// ArtifactsHandler exposes the Cloudflare Artifacts demo integration. +// +// Routes (all behind WorkspaceAuth middleware): +// +// POST /workspaces/:id/artifacts — attach a CF Artifacts repo to this workspace +// GET /workspaces/:id/artifacts — get the linked repo info +// POST /workspaces/:id/artifacts/fork — fork this workspace's repo +// POST /workspaces/:id/artifacts/token — mint a short-lived git credential +// +// Configuration (env vars, loaded once at platform startup): +// +// CF_ARTIFACTS_API_TOKEN — Cloudflare API token with Artifacts write permissions +// CF_ARTIFACTS_NAMESPACE — Cloudflare Artifacts namespace name +// +// When either env var is absent the handler returns 503 with a clear message so +// callers know the feature is not yet configured (private beta onboarding). +// +// See: https://developers.cloudflare.com/artifacts/ + +import ( + "database/sql" + "log" + "net/http" + "os" + "regexp" + "strings" + "time" + + "github.com/Molecule-AI/molecule-monorepo/platform/internal/artifacts" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" + "github.com/gin-gonic/gin" +) + +// repoNameRE validates CF Artifacts repo names: start with alphanumeric, +// then up to 62 alphanumeric/hyphen/underscore chars (63 total max). +var repoNameRE = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_-]{0,62}$`) + +// cfErrMessage returns a safe error message for CF API errors. +// For CF 5xx errors (or non-CF errors), returns a generic "upstream service error" +// to avoid leaking internal CF error details to clients. +func cfErrMessage(err error) string { + apiErr, ok := err.(*artifacts.APIError) + if !ok || apiErr.StatusCode >= 500 { + return "upstream service error" + } + return apiErr.Message +} + +// ArtifactsHandler holds a pre-built CF Artifacts client. +// The client is nil when CF_ARTIFACTS_API_TOKEN / CF_ARTIFACTS_NAMESPACE are unset. +type ArtifactsHandler struct { + client *artifacts.Client + namespace string +} + +// NewArtifactsHandler reads CF_ARTIFACTS_API_TOKEN and CF_ARTIFACTS_NAMESPACE +// from the environment and builds the client. If either is absent the handler +// still registers — every method simply returns 503. +func NewArtifactsHandler() *ArtifactsHandler { + token := os.Getenv("CF_ARTIFACTS_API_TOKEN") + ns := os.Getenv("CF_ARTIFACTS_NAMESPACE") + if token == "" || ns == "" { + log.Printf("artifacts: CF_ARTIFACTS_API_TOKEN or CF_ARTIFACTS_NAMESPACE not set — demo endpoints will return 503") + return &ArtifactsHandler{} + } + return &ArtifactsHandler{ + client: artifacts.New(token, ns), + namespace: ns, + } +} + +// newArtifactsHandlerWithClient is the injectable constructor used in tests. +func newArtifactsHandlerWithClient(client *artifacts.Client, namespace string) *ArtifactsHandler { + return &ArtifactsHandler{client: client, namespace: namespace} +} + +// configured returns false (and writes a 503) when the CF client is missing. +func (h *ArtifactsHandler) configured(c *gin.Context) bool { + if h.client == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{ + "error": "Cloudflare Artifacts not configured — set CF_ARTIFACTS_API_TOKEN and CF_ARTIFACTS_NAMESPACE", + }) + return false + } + return true +} + +// ---- POST /workspaces/:id/artifacts ------------------------------------ + +// createArtifactsRepoRequest is the body for attaching/creating a CF Artifacts repo. +type createArtifactsRepoRequest struct { + // Name is the desired CF repo name. Defaults to "molecule-ws-" when empty. + Name string `json:"name"` + // Description is an optional label stored in CF and in the local DB. + Description string `json:"description"` + // ImportURL, when non-empty, bootstraps the repo from an existing Git URL + // (e.g. "https://github.com/org/repo.git") instead of creating an empty repo. + ImportURL string `json:"import_url"` + // ImportBranch restricts the import to a single branch (only used with ImportURL). + ImportBranch string `json:"import_branch"` + // ImportDepth sets a shallow-clone depth for the import (0 = full history). + ImportDepth int `json:"import_depth"` + // ReadOnly marks the new repo as fetch/clone-only. + ReadOnly bool `json:"read_only"` +} + +// workspaceArtifactRow is the DB row shape returned by queries. +type workspaceArtifactRow struct { + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` + CFRepoName string `json:"cf_repo_name"` + CFNamespace string `json:"cf_namespace"` + RemoteURL string `json:"remote_url,omitempty"` + Description string `json:"description,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` +} + +// Create handles POST /workspaces/:id/artifacts. +// Creates or imports a Cloudflare Artifacts repo and links it to the workspace. +// Returns 409 if a repo is already linked. +func (h *ArtifactsHandler) Create(c *gin.Context) { + if !h.configured(c) { + return + } + workspaceID := c.Param("id") + ctx := c.Request.Context() + + // Reject if already linked. + var exists bool + db.DB.QueryRowContext(ctx, + `SELECT EXISTS(SELECT 1 FROM workspace_artifacts WHERE workspace_id = $1)`, + workspaceID, + ).Scan(&exists) + if exists { + c.JSON(http.StatusConflict, gin.H{"error": "workspace already has a linked Artifacts repo — delete it first"}) + return + } + + var req createArtifactsRepoRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + + // Default repo name: "molecule-ws-" (truncated at 63 chars). + repoName := req.Name + if repoName == "" { + repoName = "molecule-ws-" + workspaceID + if len(repoName) > 63 { + repoName = repoName[:63] + } + } + + // Validate explicit repo names; auto-generated names are always safe. + if req.Name != "" && !repoNameRE.MatchString(req.Name) { + c.JSON(http.StatusBadRequest, gin.H{"error": "repo name must match ^[a-zA-Z0-9][a-zA-Z0-9_-]{0,62}$"}) + return + } + + var ( + repo *artifacts.Repo + err error + ) + if req.ImportURL != "" { + // Fix 1: require HTTPS for import URLs to prevent SSRF via non-HTTPS schemes. + if !strings.HasPrefix(req.ImportURL, "https://") { + c.JSON(http.StatusBadRequest, gin.H{"error": "import_url must use https://"}) + return + } + repo, err = h.client.ImportRepo(ctx, repoName, artifacts.ImportRepoRequest{ + URL: req.ImportURL, + Branch: req.ImportBranch, + Depth: req.ImportDepth, + ReadOnly: req.ReadOnly, + }) + } else { + repo, err = h.client.CreateRepo(ctx, artifacts.CreateRepoRequest{ + Name: repoName, + Description: req.Description, + ReadOnly: req.ReadOnly, + }) + } + if err != nil { + log.Printf("artifacts: CreateRepo/ImportRepo failed for workspace %s: %v", workspaceID, err) + c.JSON(cfErrToHTTP(err), gin.H{"error": cfErrMessage(err)}) + return + } + + // Strip the embedded credential from the URL before persisting. + remoteURL := stripCredentials(repo.RemoteURL) + + var row workspaceArtifactRow + err = db.DB.QueryRowContext(ctx, ` + INSERT INTO workspace_artifacts + (workspace_id, cf_repo_name, cf_namespace, remote_url, description) + VALUES ($1, $2, $3, $4, $5) + RETURNING id, workspace_id, cf_repo_name, cf_namespace, remote_url, description, created_at, updated_at + `, workspaceID, repo.Name, h.namespace, remoteURL, req.Description).Scan( + &row.ID, &row.WorkspaceID, &row.CFRepoName, &row.CFNamespace, + &row.RemoteURL, &row.Description, &row.CreatedAt, &row.UpdatedAt, + ) + if err != nil { + log.Printf("artifacts: DB insert failed for workspace %s: %v", workspaceID, err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to persist artifact link"}) + return + } + + c.JSON(http.StatusCreated, row) +} + +// ---- GET /workspaces/:id/artifacts ------------------------------------- + +// Get handles GET /workspaces/:id/artifacts. +// Returns the linked Cloudflare Artifacts repo info from local DB and CF API. +func (h *ArtifactsHandler) Get(c *gin.Context) { + if !h.configured(c) { + return + } + workspaceID := c.Param("id") + ctx := c.Request.Context() + + var row workspaceArtifactRow + err := db.DB.QueryRowContext(ctx, ` + SELECT id, workspace_id, cf_repo_name, cf_namespace, remote_url, description, created_at, updated_at + FROM workspace_artifacts + WHERE workspace_id = $1 + `, workspaceID).Scan( + &row.ID, &row.WorkspaceID, &row.CFRepoName, &row.CFNamespace, + &row.RemoteURL, &row.Description, &row.CreatedAt, &row.UpdatedAt, + ) + if err == sql.ErrNoRows { + c.JSON(http.StatusNotFound, gin.H{"error": "no Artifacts repo linked to this workspace"}) + return + } + if err != nil { + log.Printf("artifacts: DB query failed for workspace %s: %v", workspaceID, err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "query failed"}) + return + } + + // Augment with live info from CF API (remote URL may have changed, etc.). + cfRepo, err := h.client.GetRepo(ctx, row.CFRepoName) + if err != nil { + // CF API unavailable — return cached DB row with a warning. + log.Printf("artifacts: GetRepo from CF failed for %s: %v", row.CFRepoName, err) + c.JSON(http.StatusOK, gin.H{ + "artifact": row, + "cf_status": "unavailable", + "cf_error": cfErrMessage(err), + }) + return + } + + c.JSON(http.StatusOK, gin.H{ + "artifact": row, + "cf_repo": cfRepo, + "cf_status": "ok", + }) +} + +// ---- POST /workspaces/:id/artifacts/fork ------------------------------- + +// forkArtifactsRepoRequest is the body for forking a workspace's repo. +type forkArtifactsRepoRequest struct { + // Name is the desired name of the forked repo. Required. + Name string `json:"name" binding:"required"` + // Description is an optional label for the fork. + Description string `json:"description"` + // ReadOnly marks the fork as fetch/clone-only. + ReadOnly bool `json:"read_only"` + // DefaultBranchOnly limits the fork to the default branch. + DefaultBranchOnly bool `json:"default_branch_only"` +} + +// Fork handles POST /workspaces/:id/artifacts/fork. +// Creates an isolated copy of the workspace's primary Artifacts repo in CF. +// The fork is not recorded in the local DB — it is owned by the caller. +func (h *ArtifactsHandler) Fork(c *gin.Context) { + if !h.configured(c) { + return + } + workspaceID := c.Param("id") + ctx := c.Request.Context() + + // Look up the source repo name. + var cfRepoName string + err := db.DB.QueryRowContext(ctx, + `SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id = $1`, + workspaceID, + ).Scan(&cfRepoName) + if err == sql.ErrNoRows { + c.JSON(http.StatusNotFound, gin.H{"error": "no Artifacts repo linked to this workspace"}) + return + } + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "query failed"}) + return + } + + var req forkArtifactsRepoRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + + if req.Name != "" && !repoNameRE.MatchString(req.Name) { + c.JSON(http.StatusBadRequest, gin.H{"error": "repo name must match ^[a-zA-Z0-9][a-zA-Z0-9_-]{0,62}$"}) + return + } + + result, err := h.client.ForkRepo(ctx, cfRepoName, artifacts.ForkRepoRequest{ + Name: req.Name, + Description: req.Description, + ReadOnly: req.ReadOnly, + DefaultBranchOnly: req.DefaultBranchOnly, + }) + if err != nil { + log.Printf("artifacts: ForkRepo failed for workspace %s: %v", workspaceID, err) + c.JSON(cfErrToHTTP(err), gin.H{"error": cfErrMessage(err)}) + return + } + + c.JSON(http.StatusCreated, gin.H{ + "fork": result.Repo, + "object_count": result.ObjectCount, + "remote_url": stripCredentials(result.Repo.RemoteURL), + }) +} + +// ---- POST /workspaces/:id/artifacts/token ------------------------------ + +// artifactsTokenRequest is the body for minting a git credential. +type artifactsTokenRequest struct { + // Scope is "read" or "write". Defaults to "write". + Scope string `json:"scope"` + // TTL is the credential lifetime in seconds. Defaults to 3600 (1h). + TTL int `json:"ttl"` +} + +// Token handles POST /workspaces/:id/artifacts/token. +// Returns a short-lived Git credential for the workspace's linked repo. +// The plaintext token value must be saved by the caller — it is not stored. +func (h *ArtifactsHandler) Token(c *gin.Context) { + if !h.configured(c) { + return + } + workspaceID := c.Param("id") + ctx := c.Request.Context() + + // Look up the linked CF repo name. + var cfRepoName string + err := db.DB.QueryRowContext(ctx, + `SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id = $1`, + workspaceID, + ).Scan(&cfRepoName) + if err == sql.ErrNoRows { + c.JSON(http.StatusNotFound, gin.H{"error": "no Artifacts repo linked to this workspace"}) + return + } + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "query failed"}) + return + } + + var req artifactsTokenRequest + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + + scope := req.Scope + if scope == "" { + scope = "write" + } + if scope != "read" && scope != "write" { + c.JSON(http.StatusBadRequest, gin.H{"error": "scope must be \"read\" or \"write\""}) + return + } + ttl := req.TTL + if ttl <= 0 { + ttl = 3600 + } + const maxTTL = 86400 * 7 // 7 days + if ttl > maxTTL { + ttl = maxTTL + } + + tok, err := h.client.CreateToken(ctx, artifacts.CreateTokenRequest{ + Repo: cfRepoName, + Scope: scope, + TTL: ttl, + }) + if err != nil { + log.Printf("artifacts: CreateToken failed for workspace %s: %v", workspaceID, err) + c.JSON(cfErrToHTTP(err), gin.H{"error": cfErrMessage(err)}) + return + } + + // Build the authenticated git remote URL inline so callers can use it + // directly: git clone + cloneURL := buildCloneURL(cfRepoName, tok.Token, h.namespace) + + c.JSON(http.StatusCreated, gin.H{ + "token_id": tok.ID, + "token": tok.Token, + "scope": tok.Scope, + "expires_at": tok.ExpiresAt, + "clone_url": cloneURL, + "message": "Save this token — it cannot be retrieved again.", + }) +} + +// ---- helpers ------------------------------------------------------------- + +// cfErrToHTTP converts a CF API error to an appropriate HTTP status code. +// Passes through 4xx, maps everything else to 502 (bad gateway — upstream CF). +func cfErrToHTTP(err error) int { + apiErr, ok := err.(*artifacts.APIError) + if !ok { + return http.StatusBadGateway + } + if apiErr.StatusCode >= 400 && apiErr.StatusCode < 500 { + return apiErr.StatusCode + } + return http.StatusBadGateway +} + +// stripCredentials removes "x:@" from an authenticated git remote URL +// so we never persist credentials in the database. +// e.g. "https://x:tok@hash.artifacts.cloudflare.net/…" → "https://hash.artifacts.cloudflare.net/…" +func stripCredentials(remoteURL string) string { + if i := strings.Index(remoteURL, "@"); i != -1 { + scheme := "https://" + if strings.HasPrefix(remoteURL, "http://") { + scheme = "http://" + } + return scheme + remoteURL[i+1:] + } + return remoteURL +} + +// buildCloneURL constructs an authenticated clone URL from the CF token. +// Format: https://x:@.artifacts.cloudflare.net/git/repo-.git +// When we only have the repo name (not the full hashed host), we use a stable +// URL pattern that the CF git endpoint resolves. +func buildCloneURL(repoName, token, _ string) string { + // The CF git endpoint is the remote_url stored in the DB (minus the + // credential prefix). We reconstruct the authenticated form here. + // In production the remote URL is returned by CreateRepo/GetRepo; + // this fallback covers cases where the DB row predates that field. + return "https://x:" + token + "@artifacts.cloudflare.net/git/" + repoName + ".git" +} diff --git a/platform/internal/handlers/artifacts_test.go b/platform/internal/handlers/artifacts_test.go new file mode 100644 index 00000000..283dea0b --- /dev/null +++ b/platform/internal/handlers/artifacts_test.go @@ -0,0 +1,995 @@ +package handlers + +import ( + "bytes" + "database/sql" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/artifacts" + "github.com/gin-gonic/gin" +) + +// cfSuccessResponse wraps a result in the Cloudflare v4 success envelope. +func cfSuccessResponse(t *testing.T, result interface{}) []byte { + t.Helper() + b, err := json.Marshal(result) + if err != nil { + t.Fatalf("cfSuccessResponse: marshal result: %v", err) + } + env := map[string]interface{}{ + "success": true, + "result": json.RawMessage(b), + "errors": []interface{}{}, + } + out, err := json.Marshal(env) + if err != nil { + t.Fatalf("cfSuccessResponse: marshal envelope: %v", err) + } + return out +} + +// cfErrorResponse returns a Cloudflare v4 error envelope bytes and status code. +func cfErrorResponse(t *testing.T, statusCode, code int, message string) ([]byte, int) { + t.Helper() + env := map[string]interface{}{ + "success": false, + "result": nil, + "errors": []map[string]interface{}{ + {"code": code, "message": message}, + }, + } + b, _ := json.Marshal(env) + return b, statusCode +} + +// newArtifactsMockServer starts an httptest.Server with the given handler function +// registered at /namespaces/test-ns/. +func newArtifactsMockCFServer(t *testing.T, suffix string, handler http.HandlerFunc) *artifacts.Client { + t.Helper() + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns"+suffix, handler) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return artifacts.NewWithBaseURL("cf-test-token", "test-ns", srv.URL) +} + +// ============================= Create ===================================== + +// TestArtifactsCreate_Success verifies the happy path: no existing link → +// CF API returns a repo → DB INSERT succeeds → 201 response. +func TestArtifactsCreate_Success(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + cfClient := newArtifactsMockCFServer(t, "/repos", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "wrong method", http.StatusMethodNotAllowed) + return + } + repo := artifacts.Repo{ + Name: "molecule-ws-ws-abc", + ID: "repo-001", + RemoteURL: "https://x:tok123@hash.artifacts.cloudflare.net/git/repo-001.git", + CreatedAt: time.Now(), + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfSuccessResponse(t, repo)) + }) + + // Existence probe — no existing link + mock.ExpectQuery(`SELECT EXISTS`). + WithArgs("ws-abc"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + // DB INSERT — RETURNING row + now := time.Now() + mock.ExpectQuery(`INSERT INTO workspace_artifacts`). + WithArgs("ws-abc", "molecule-ws-ws-abc", "test-ns", + "https://hash.artifacts.cloudflare.net/git/repo-001.git", ""). + WillReturnRows(sqlmock.NewRows( + []string{"id", "workspace_id", "cf_repo_name", "cf_namespace", "remote_url", "description", "created_at", "updated_at"}). + AddRow("art-1", "ws-abc", "molecule-ws-ws-abc", "test-ns", + "https://hash.artifacts.cloudflare.net/git/repo-001.git", "", now, now)) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-abc"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-abc/artifacts", + bytes.NewBufferString(`{}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["cf_repo_name"] != "molecule-ws-ws-abc" { + t.Errorf("cf_repo_name = %v, want molecule-ws-ws-abc", resp["cf_repo_name"]) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsCreate_AlreadyLinked verifies that a 409 is returned when the +// workspace already has a linked Artifacts repo. +func TestArtifactsCreate_AlreadyLinked(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + // Existence probe returns true + mock.ExpectQuery(`SELECT EXISTS`). + WithArgs("ws-dup"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true)) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "ns", "http://unused"), + "ns", + ) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-dup"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-dup/artifacts", + bytes.NewBufferString(`{"name":"dup-repo"}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Create(c) + + if w.Code != http.StatusConflict { + t.Errorf("expected 409, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsCreate_CFAPIError verifies that a CF API error (e.g. 409 conflict) +// is forwarded with the appropriate HTTP status. +func TestArtifactsCreate_CFAPIError(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + cfClient := newArtifactsMockCFServer(t, "/repos", func(w http.ResponseWriter, r *http.Request) { + body, status := cfErrorResponse(t, http.StatusConflict, 1009, "repo name already taken") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + w.Write(body) + }) + + mock.ExpectQuery(`SELECT EXISTS`). + WithArgs("ws-cfconflict"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-cfconflict"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-cfconflict/artifacts", + bytes.NewBufferString(`{"name":"taken-name"}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Create(c) + + if w.Code != http.StatusConflict { + t.Errorf("expected 409 from CF error, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsCreate_WithImportURL verifies that when import_url is set the +// handler hits the /import endpoint instead of plain /repos. +func TestArtifactsCreate_WithImportURL(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + // Two paths: /repos/imported-repo/import + mux := http.NewServeMux() + mux.HandleFunc("/namespaces/test-ns/repos/imported-repo/import", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "wrong method", http.StatusMethodNotAllowed) + return + } + var body map[string]interface{} + json.NewDecoder(r.Body).Decode(&body) + if body["url"] != "https://github.com/Molecule-AI/molecule-core.git" { + http.Error(w, "unexpected url", http.StatusBadRequest) + return + } + repo := artifacts.Repo{ + Name: "imported-repo", + RemoteURL: "https://x:tok@hash.artifacts.cloudflare.net/git/imported.git", + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfSuccessResponse(t, repo)) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + cfClient := artifacts.NewWithBaseURL("tok", "test-ns", srv.URL) + + mock.ExpectQuery(`SELECT EXISTS`). + WithArgs("ws-import"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + now := time.Now() + mock.ExpectQuery(`INSERT INTO workspace_artifacts`). + WithArgs("ws-import", "imported-repo", "test-ns", + "https://hash.artifacts.cloudflare.net/git/imported.git", "Imported from GitHub"). + WillReturnRows(sqlmock.NewRows( + []string{"id", "workspace_id", "cf_repo_name", "cf_namespace", "remote_url", "description", "created_at", "updated_at"}). + AddRow("art-imp", "ws-import", "imported-repo", "test-ns", + "https://hash.artifacts.cloudflare.net/git/imported.git", "Imported from GitHub", now, now)) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-import"}} + body := `{"name":"imported-repo","description":"Imported from GitHub","import_url":"https://github.com/Molecule-AI/molecule-core.git","import_branch":"main","import_depth":1}` + c.Request = httptest.NewRequest("POST", "/workspaces/ws-import/artifacts", + bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsCreate_NotConfigured verifies that missing env vars → 503. +func TestArtifactsCreate_NotConfigured(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + + // No CF client → nil + h := newArtifactsHandlerWithClient(nil, "") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-uncfg"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-uncfg/artifacts", + bytes.NewBufferString(`{}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Create(c) + + if w.Code != http.StatusServiceUnavailable { + t.Errorf("expected 503, got %d: %s", w.Code, w.Body.String()) + } +} + +// ============================= Get ======================================= + +// TestArtifactsGet_Success verifies the happy path: DB row found + CF API ok. +func TestArtifactsGet_Success(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + cfClient := newArtifactsMockCFServer(t, "/repos/my-ws-repo", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "wrong method", http.StatusMethodNotAllowed) + return + } + repo := artifacts.Repo{ + Name: "my-ws-repo", + RemoteURL: "https://x:tok@hash.artifacts.cloudflare.net/git/my-ws-repo.git", + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfSuccessResponse(t, repo)) + }) + + now := time.Now() + mock.ExpectQuery(`SELECT id, workspace_id, cf_repo_name`). + WithArgs("ws-get"). + WillReturnRows(sqlmock.NewRows( + []string{"id", "workspace_id", "cf_repo_name", "cf_namespace", "remote_url", "description", "created_at", "updated_at"}). + AddRow("art-get", "ws-get", "my-ws-repo", "test-ns", + "https://hash.artifacts.cloudflare.net/git/my-ws-repo.git", "", now, now)) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-get"}} + c.Request = httptest.NewRequest("GET", "/workspaces/ws-get/artifacts", nil) + + h.Get(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["cf_status"] != "ok" { + t.Errorf("cf_status = %v, want ok", resp["cf_status"]) + } + art, ok := resp["artifact"].(map[string]interface{}) + if !ok { + t.Fatalf("artifact is not an object") + } + if art["cf_repo_name"] != "my-ws-repo" { + t.Errorf("artifact.cf_repo_name = %v, want my-ws-repo", art["cf_repo_name"]) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsGet_NotFound verifies that 404 is returned when no row exists. +func TestArtifactsGet_NotFound(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT id, workspace_id, cf_repo_name`). + WithArgs("ws-noart"). + WillReturnError(sql.ErrNoRows) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "ns", "http://unused"), + "ns", + ) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-noart"}} + c.Request = httptest.NewRequest("GET", "/workspaces/ws-noart/artifacts", nil) + + h.Get(c) + + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsGet_CFUnavailable verifies that when CF API fails the handler +// still returns 200 with the cached DB row and cf_status="unavailable". +func TestArtifactsGet_CFUnavailable(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + // CF API server that always returns 500 + cfClient := newArtifactsMockCFServer(t, "/repos/cached-repo", func(w http.ResponseWriter, r *http.Request) { + body, status := cfErrorResponse(t, http.StatusInternalServerError, 0, "service unavailable") + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + w.Write(body) + }) + + now := time.Now() + mock.ExpectQuery(`SELECT id, workspace_id, cf_repo_name`). + WithArgs("ws-cfdown"). + WillReturnRows(sqlmock.NewRows( + []string{"id", "workspace_id", "cf_repo_name", "cf_namespace", "remote_url", "description", "created_at", "updated_at"}). + AddRow("art-cfdown", "ws-cfdown", "cached-repo", "test-ns", + "https://hash.artifacts.cloudflare.net/git/cached-repo.git", "", now, now)) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-cfdown"}} + c.Request = httptest.NewRequest("GET", "/workspaces/ws-cfdown/artifacts", nil) + + h.Get(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200 (degraded), got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["cf_status"] != "unavailable" { + t.Errorf("cf_status = %v, want unavailable", resp["cf_status"]) + } + if resp["artifact"] == nil { + t.Error("artifact should still be present from DB cache") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// ============================= Fork ====================================== + +// TestArtifactsFork_Success verifies the fork happy path. +func TestArtifactsFork_Success(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + cfClient := newArtifactsMockCFServer(t, "/repos/source-repo/fork", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "wrong method", http.StatusMethodNotAllowed) + return + } + result := artifacts.ForkResult{ + Repo: artifacts.Repo{ + Name: "forked-ws", + ID: "fork-1", + RemoteURL: "https://x:tok@hash.artifacts.cloudflare.net/git/fork-1.git", + }, + ObjectCount: 88, + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfSuccessResponse(t, result)) + }) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-fork-src"). + WillReturnRows(sqlmock.NewRows([]string{"cf_repo_name"}).AddRow("source-repo")) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-fork-src"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-fork-src/artifacts/fork", + bytes.NewBufferString(`{"name":"forked-ws"}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Fork(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["object_count"] != float64(88) { + t.Errorf("object_count = %v, want 88", resp["object_count"]) + } + fork, ok := resp["fork"].(map[string]interface{}) + if !ok { + t.Fatalf("fork is not an object") + } + if fork["name"] != "forked-ws" { + t.Errorf("fork.name = %v, want forked-ws", fork["name"]) + } + // Embedded credentials must be stripped from clone_url + if remote := resp["remote_url"].(string); len(remote) > 0 { + if containsCredentials(remote) { + t.Errorf("remote_url should not contain credentials: %s", remote) + } + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsFork_NoLinkedRepo verifies 404 when workspace has no linked repo. +func TestArtifactsFork_NoLinkedRepo(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-norepo"). + WillReturnError(sql.ErrNoRows) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "ns", "http://unused"), + "ns", + ) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-norepo"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-norepo/artifacts/fork", + bytes.NewBufferString(`{"name":"fork-name"}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Fork(c) + + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsFork_MissingName verifies 400 when the fork name is missing. +func TestArtifactsFork_MissingName(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-fork-badname"). + WillReturnRows(sqlmock.NewRows([]string{"cf_repo_name"}).AddRow("src-repo")) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "test-ns", "http://unused"), + "test-ns", + ) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-fork-badname"}} + // name is required (binding:"required") but absent → 400 + c.Request = httptest.NewRequest("POST", "/workspaces/ws-fork-badname/artifacts/fork", + bytes.NewBufferString(`{}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Fork(c) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400 for missing name, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// ============================= Token ===================================== + +// TestArtifactsToken_Success verifies the happy path: linked repo → CF returns token. +func TestArtifactsToken_Success(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + expiry := time.Now().Add(3600 * time.Second).UTC() + cfClient := newArtifactsMockCFServer(t, "/tokens", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.Error(w, "wrong method", http.StatusMethodNotAllowed) + return + } + var req map[string]interface{} + json.NewDecoder(r.Body).Decode(&req) + if req["repo"] != "my-linked-repo" { + http.Error(w, "unexpected repo", http.StatusBadRequest) + return + } + tok := artifacts.RepoToken{ + ID: "token-abc", + Token: "plaintext-git-token", + Scope: "write", + ExpiresAt: expiry, + } + w.Header().Set("Content-Type", "application/json") + w.Write(cfSuccessResponse(t, tok)) + }) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-token"). + WillReturnRows(sqlmock.NewRows([]string{"cf_repo_name"}).AddRow("my-linked-repo")) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-token"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-token/artifacts/token", + bytes.NewBufferString(`{"scope":"write","ttl":3600}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Token(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["token_id"] != "token-abc" { + t.Errorf("token_id = %v, want token-abc", resp["token_id"]) + } + if resp["token"] != "plaintext-git-token" { + t.Errorf("token = %v, want plaintext-git-token", resp["token"]) + } + if resp["clone_url"] == nil || resp["clone_url"] == "" { + t.Error("clone_url should be non-empty") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsToken_DefaultsApplied verifies that empty scope/ttl are defaulted +// to "write" / 3600. +func TestArtifactsToken_DefaultsApplied(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + expiry := time.Now().Add(3600 * time.Second).UTC() + cfClient := newArtifactsMockCFServer(t, "/tokens", func(w http.ResponseWriter, r *http.Request) { + var req map[string]interface{} + json.NewDecoder(r.Body).Decode(&req) + // scope should be "write" (default) + if req["scope"] != "write" { + http.Error(w, "expected default scope write", http.StatusBadRequest) + return + } + // ttl should be 3600 (default), serialized as float64 from JSON + if req["ttl"] != float64(3600) { + http.Error(w, "expected default ttl 3600", http.StatusBadRequest) + return + } + tok := artifacts.RepoToken{ID: "t1", Token: "tok-def", Scope: "write", ExpiresAt: expiry} + w.Header().Set("Content-Type", "application/json") + w.Write(cfSuccessResponse(t, tok)) + }) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-defaults"). + WillReturnRows(sqlmock.NewRows([]string{"cf_repo_name"}).AddRow("my-repo")) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-defaults"}} + // Empty body — all defaults + c.Request = httptest.NewRequest("POST", "/workspaces/ws-defaults/artifacts/token", + bytes.NewBufferString(`{}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Token(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsToken_InvalidScope verifies that an invalid scope returns 400. +func TestArtifactsToken_InvalidScope(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-badscope"). + WillReturnRows(sqlmock.NewRows([]string{"cf_repo_name"}).AddRow("some-repo")) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "ns", "http://unused"), + "ns", + ) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-badscope"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-badscope/artifacts/token", + bytes.NewBufferString(`{"scope":"admin"}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Token(c) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected 400 for invalid scope, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsToken_TTLCapped verifies that excessive TTL is silently capped +// to 7 days (604800 seconds) rather than returning an error. +func TestArtifactsToken_TTLCapped(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + const maxTTL = 86400 * 7 + + expiry := time.Now().Add(maxTTL * time.Second).UTC() + cfClient := newArtifactsMockCFServer(t, "/tokens", func(w http.ResponseWriter, r *http.Request) { + var req map[string]interface{} + json.NewDecoder(r.Body).Decode(&req) + if int(req["ttl"].(float64)) != maxTTL { + http.Error(w, "expected capped ttl", http.StatusBadRequest) + return + } + tok := artifacts.RepoToken{ID: "t-cap", Token: "capped-tok", Scope: "write", ExpiresAt: expiry} + w.Header().Set("Content-Type", "application/json") + w.Write(cfSuccessResponse(t, tok)) + }) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-ttlcap"). + WillReturnRows(sqlmock.NewRows([]string{"cf_repo_name"}).AddRow("capped-repo")) + + h := newArtifactsHandlerWithClient(cfClient, "test-ns") + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-ttlcap"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-ttlcap/artifacts/token", + bytes.NewBufferString(`{"scope":"write","ttl":99999999}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Token(c) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// TestArtifactsToken_NoLinkedRepo verifies 404 when no repo is linked. +func TestArtifactsToken_NoLinkedRepo(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-tokennolink"). + WillReturnError(sql.ErrNoRows) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "ns", "http://unused"), + "ns", + ) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-tokennolink"}} + c.Request = httptest.NewRequest("POST", "/workspaces/ws-tokennolink/artifacts/token", + bytes.NewBufferString(`{}`)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Token(c) + + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock: %v", err) + } +} + +// ============================= helper unit tests ========================= + +// TestStripCredentials verifies that stripCredentials removes user:token@ from URLs. +func TestStripCredentials(t *testing.T) { + cases := []struct { + input string + want string + }{ + { + "https://x:tok123@hash.artifacts.cloudflare.net/git/repo.git", + "https://hash.artifacts.cloudflare.net/git/repo.git", + }, + { + "https://hash.artifacts.cloudflare.net/git/repo.git", + "https://hash.artifacts.cloudflare.net/git/repo.git", + }, + { + "http://user:pass@example.com/repo.git", + "http://example.com/repo.git", + }, + {"", ""}, + } + for _, tc := range cases { + got := stripCredentials(tc.input) + if got != tc.want { + t.Errorf("stripCredentials(%q) = %q, want %q", tc.input, got, tc.want) + } + } +} + +// TestCfErrToHTTP verifies the CF-error-to-HTTP-status mapping. +func TestCfErrToHTTP(t *testing.T) { + cases := []struct { + err error + want int + }{ + {&artifacts.APIError{StatusCode: http.StatusConflict}, http.StatusConflict}, + {&artifacts.APIError{StatusCode: http.StatusNotFound}, http.StatusNotFound}, + {&artifacts.APIError{StatusCode: http.StatusBadRequest}, http.StatusBadRequest}, + {&artifacts.APIError{StatusCode: http.StatusInternalServerError}, http.StatusBadGateway}, + {&artifacts.APIError{StatusCode: http.StatusBadGateway}, http.StatusBadGateway}, + } + for _, tc := range cases { + got := cfErrToHTTP(tc.err) + if got != tc.want { + t.Errorf("cfErrToHTTP(%v) = %d, want %d", tc.err, got, tc.want) + } + } +} + +// ============================= Security fix tests ============================ + +// TestCfErrMessage_5xxReturnsGeneric verifies that CF 5xx errors return a +// generic message instead of leaking CF internals. +func TestCfErrMessage_5xxReturnsGeneric(t *testing.T) { + err := &artifacts.APIError{StatusCode: http.StatusInternalServerError, Message: "internal CF detail"} + got := cfErrMessage(err) + if got != "upstream service error" { + t.Errorf("cfErrMessage(500) = %q, want %q", got, "upstream service error") + } +} + +// TestCfErrMessage_502ReturnsGeneric verifies that CF 502 (bad gateway) is also masked. +func TestCfErrMessage_502ReturnsGeneric(t *testing.T) { + err := &artifacts.APIError{StatusCode: http.StatusBadGateway, Message: "gateway detail"} + got := cfErrMessage(err) + if got != "upstream service error" { + t.Errorf("cfErrMessage(502) = %q, want %q", got, "upstream service error") + } +} + +// TestCfErrMessage_4xxPassesThrough verifies that CF 4xx messages are surfaced. +func TestCfErrMessage_4xxPassesThrough(t *testing.T) { + msg := "repo name already taken" + err := &artifacts.APIError{StatusCode: http.StatusConflict, Message: msg} + got := cfErrMessage(err) + if got != msg { + t.Errorf("cfErrMessage(409) = %q, want %q", got, msg) + } +} + +// TestCfErrMessage_NonAPIErrorReturnsGeneric verifies that non-CF errors return generic message. +func TestCfErrMessage_NonAPIErrorReturnsGeneric(t *testing.T) { + err := fmt.Errorf("some network error") + got := cfErrMessage(err) + if got != "upstream service error" { + t.Errorf("cfErrMessage(non-API) = %q, want %q", got, "upstream service error") + } +} + +// TestArtifactsCreate_ImportURLNonHTTPS verifies that a non-HTTPS import_url +// is rejected with 400. +func TestArtifactsCreate_ImportURLNonHTTPS(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + mock.ExpectQuery(`SELECT EXISTS`). + WithArgs("ws-badurl"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "test-ns", "http://unused"), + "test-ns", + ) + + cases := []string{ + "http://github.com/org/repo.git", + "git://github.com/org/repo.git", + "ssh://git@github.com/org/repo.git", + "file:///etc/passwd", + } + for _, url := range cases { + t.Run(url, func(t *testing.T) { + // Re-register the EXISTS probe expectation for each sub-test case. + mock.ExpectQuery(`SELECT EXISTS`). + WithArgs("ws-badurl"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-badurl"}} + body, _ := json.Marshal(map[string]interface{}{ + "name": "my-repo", + "import_url": url, + }) + c.Request = httptest.NewRequest("POST", "/workspaces/ws-badurl/artifacts", + bytes.NewBuffer(body)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Errorf("import_url=%q: expected 400, got %d: %s", url, w.Code, w.Body.String()) + } + }) + } +} + +// TestArtifactsCreate_InvalidRepoName verifies that invalid repo names return 400. +func TestArtifactsCreate_InvalidRepoName(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "test-ns", "http://unused"), + "test-ns", + ) + + invalidNames := []string{ + "-starts-with-dash", + "_starts-with-underscore", + "has spaces", + "has/slash", + "has.dot", + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", // 64 chars + } + for _, name := range invalidNames { + t.Run(name, func(t *testing.T) { + mock.ExpectQuery(`SELECT EXISTS`). + WithArgs("ws-badname"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-badname"}} + body, _ := json.Marshal(map[string]interface{}{"name": name}) + c.Request = httptest.NewRequest("POST", "/workspaces/ws-badname/artifacts", + bytes.NewBuffer(body)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Create(c) + + if w.Code != http.StatusBadRequest { + t.Errorf("name=%q: expected 400, got %d: %s", name, w.Code, w.Body.String()) + } + }) + } +} + +// TestArtifactsFork_InvalidRepoName verifies that invalid fork names return 400. +func TestArtifactsFork_InvalidRepoName(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + + h := newArtifactsHandlerWithClient( + artifacts.NewWithBaseURL("tok", "test-ns", "http://unused"), + "test-ns", + ) + + invalidNames := []string{ + "-bad-start", + "has spaces", + "../traversal", + } + for _, name := range invalidNames { + t.Run(name, func(t *testing.T) { + mock.ExpectQuery(`SELECT cf_repo_name FROM workspace_artifacts WHERE workspace_id`). + WithArgs("ws-forknm"). + WillReturnRows(sqlmock.NewRows([]string{"cf_repo_name"}).AddRow("src")) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-forknm"}} + body, _ := json.Marshal(map[string]interface{}{"name": name}) + c.Request = httptest.NewRequest("POST", "/workspaces/ws-forknm/artifacts/fork", + bytes.NewBuffer(body)) + c.Request.Header.Set("Content-Type", "application/json") + + h.Fork(c) + + if w.Code != http.StatusBadRequest { + t.Errorf("fork name=%q: expected 400, got %d: %s", name, w.Code, w.Body.String()) + } + }) + } +} + +// containsCredentials is a test helper that checks whether a URL has embedded +// user:password@ credentials (should never appear in a stored remote URL). +func containsCredentials(u string) bool { + // A URL with embedded creds has the form scheme://user:pass@host/... + // We check for "@" after the scheme to detect this. + for i := 0; i < len(u)-3; i++ { + if u[i] == ':' && i > 0 && u[i-1] != '/' { + // Found ":" that is not ":/" — could be user:pass pair + if j := len(u); j > i { + for k := i + 1; k < j; k++ { + if u[k] == '@' { + return true + } + if u[k] == '/' { + break + } + } + } + } + } + return false +} diff --git a/platform/internal/router/router.go b/platform/internal/router/router.go index 8e735e45..58c759a9 100644 --- a/platform/internal/router/router.go +++ b/platform/internal/router/router.go @@ -292,6 +292,17 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi // WorkspaceAuth middleware (on wsAuth) binds the bearer to :id. mtrh := handlers.NewMetricsHandler() wsAuth.GET("/metrics", mtrh.GetMetrics) + + // Cloudflare Artifacts demo integration (#595). + // All four routes require workspace-scoped bearer auth (wsAuth). + // CF credentials read from CF_ARTIFACTS_API_TOKEN / CF_ARTIFACTS_NAMESPACE; + // missing credentials return 503 so the handler still registers in + // every deployment — the demo is gated on env vars, not compilation. + arth := handlers.NewArtifactsHandler() + wsAuth.POST("/artifacts", arth.Create) + wsAuth.GET("/artifacts", arth.Get) + wsAuth.POST("/artifacts/fork", arth.Fork) + wsAuth.POST("/artifacts/token", arth.Token) } // Global secrets — /settings/secrets is the canonical path; /admin/secrets kept for backward compat. diff --git a/platform/migrations/025_workspace_token_usage.up.sql b/platform/migrations/025_workspace_token_usage.up.sql index acec2090..e396356d 100644 --- a/platform/migrations/025_workspace_token_usage.up.sql +++ b/platform/migrations/025_workspace_token_usage.up.sql @@ -4,7 +4,7 @@ -- (default: Claude Sonnet input $3/1M, output $15/1M). CREATE TABLE IF NOT EXISTS workspace_token_usage ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), - workspace_id TEXT NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE, + workspace_id UUID NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE, period_start TIMESTAMPTZ NOT NULL, input_tokens BIGINT NOT NULL DEFAULT 0, output_tokens BIGINT NOT NULL DEFAULT 0, diff --git a/platform/migrations/026_org_plugin_allowlist.up.sql b/platform/migrations/026_org_plugin_allowlist.up.sql index f2d12353..d7728e32 100644 --- a/platform/migrations/026_org_plugin_allowlist.up.sql +++ b/platform/migrations/026_org_plugin_allowlist.up.sql @@ -7,7 +7,7 @@ -- enabled_by records the workspace ID of the admin who added the entry. CREATE TABLE IF NOT EXISTS org_plugin_allowlist ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), - org_id TEXT NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE, + org_id UUID NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE, plugin_name TEXT NOT NULL, enabled_by TEXT NOT NULL, enabled_at TIMESTAMPTZ NOT NULL DEFAULT NOW() diff --git a/platform/migrations/028_workspace_artifacts.down.sql b/platform/migrations/028_workspace_artifacts.down.sql new file mode 100644 index 00000000..3d149a31 --- /dev/null +++ b/platform/migrations/028_workspace_artifacts.down.sql @@ -0,0 +1,2 @@ +-- Reverse of 028_workspace_artifacts.up.sql +DROP TABLE IF EXISTS workspace_artifacts; diff --git a/platform/migrations/028_workspace_artifacts.up.sql b/platform/migrations/028_workspace_artifacts.up.sql new file mode 100644 index 00000000..c6b2d422 --- /dev/null +++ b/platform/migrations/028_workspace_artifacts.up.sql @@ -0,0 +1,31 @@ +-- 028_workspace_artifacts: store Cloudflare Artifacts repo linkage per workspace. +-- +-- Each workspace can be linked to exactly one Cloudflare Artifacts repo +-- (the primary snapshot store). Additional repos (forks) are ephemeral and +-- tracked only via the CF API — not in this table. +-- +-- Remote URLs are stored for informational display only; callers must +-- call POST /workspaces/:id/artifacts/token to obtain a fresh git credential. + +CREATE TABLE IF NOT EXISTS workspace_artifacts ( + id TEXT NOT NULL DEFAULT gen_random_uuid()::text, + workspace_id TEXT NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE, + cf_repo_name TEXT NOT NULL, + cf_namespace TEXT NOT NULL, + -- remote_url is the base Git remote (without embedded credentials). + -- Credentials are obtained on-demand via POST /tokens. + remote_url TEXT, + description TEXT, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + + CONSTRAINT workspace_artifacts_pkey PRIMARY KEY (id) +); + +-- Each workspace may have at most one linked CF Artifacts repo. +CREATE UNIQUE INDEX IF NOT EXISTS uq_workspace_artifacts_workspace_id + ON workspace_artifacts (workspace_id); + +-- Allow fast lookup by CF repo name within a namespace. +CREATE INDEX IF NOT EXISTS idx_workspace_artifacts_cf_repo + ON workspace_artifacts (cf_namespace, cf_repo_name); diff --git a/workspace-template/Dockerfile b/workspace-template/Dockerfile index dfc78ff2..7306db35 100644 --- a/workspace-template/Dockerfile +++ b/workspace-template/Dockerfile @@ -49,6 +49,13 @@ RUN ln -s /app/a2a_cli.py /usr/local/bin/a2a && chmod +x /app/a2a_cli.py /app/a2 COPY scripts/gh-wrapper.sh /usr/local/bin/gh RUN chmod +x /usr/local/bin/gh +# Copy the git credential helper so entrypoint.sh can register it at boot. +# molecule-git-token-helper.sh fetches a fresh GitHub App installation token +# from the platform on every git push/fetch, preventing stale-token failures +# after the ~60 min GitHub App token TTL (issue #613 / #547). +COPY scripts/molecule-git-token-helper.sh ./scripts/ +RUN chmod +x ./scripts/molecule-git-token-helper.sh + # Dirs and permissions RUN mkdir -p /workspace /plugins /home/agent/.claude /home/agent/.config /home/agent/.local && \ chown -R agent:agent /app /home/agent /workspace diff --git a/workspace-template/entrypoint.sh b/workspace-template/entrypoint.sh index 54236e5f..8c260ccf 100644 --- a/workspace-template/entrypoint.sh +++ b/workspace-template/entrypoint.sh @@ -70,7 +70,7 @@ echo "Runtime: $RUNTIME" # unreachable. # # Idempotent — safe to re-run on restart. -HELPER_SCRIPT="/workspace-template/scripts/molecule-git-token-helper.sh" +HELPER_SCRIPT="/app/scripts/molecule-git-token-helper.sh" if [ -f "${HELPER_SCRIPT}" ]; then git config --global \ "credential.https://github.com.helper" \ diff --git a/workspace-template/hermes_executor.py b/workspace-template/hermes_executor.py index 07aa4648..8fff95e3 100644 --- a/workspace-template/hermes_executor.py +++ b/workspace-template/hermes_executor.py @@ -21,11 +21,58 @@ OTEL activity span so operators can inspect the thinking trace in Langfuse A2A reply — doing so would contaminate the agent's next-turn context with the model's internal scratchpad. +Native tools (#497) +------------------- +Tool definitions are passed via the OpenAI-native ``tools`` parameter instead +of injecting them as text into the system prompt. Each entry must follow the +standard OpenAI function-calling schema:: + + { + "type": "function", + "function": { + "name": "...", + "description": "...", + "parameters": { # JSON Schema object + "type": "object", + "properties": {...}, + "required": [...] + } + } + } + +**Empty list rule:** when ``tools`` is ``None`` or ``[]``, the ``tools`` +parameter is **omitted** from the API call entirely. Sending ``tools=[]`` +to some OpenAI-compat providers causes a 400 / unexpected behaviour; omitting +the key is always safe and signals "no tool use." + +**Tool-call response handling:** when the model returns +``choice.message.tool_calls`` with no text content (``finish_reason`` is +``"tool_calls"``), the executor serialises the tool-call list as a JSON string +and enqueues that as the A2A reply. This keeps the executor thin (single API +call per turn, no ReAct loop) while surfacing function-call intent to the +caller in a structured, parseable format. + Hermes 3 / unknown models -------------------------- No ``extra_body`` is sent. The response is processed identically to any other OpenAI-compat model call. The Hermes 3 path is exercised by the existing adapter test suite and must remain unchanged. + +response_format / structured output (#498) +------------------------------------------ +Pass ``response_format={"type": "json_schema", "json_schema": {...}}`` (or +``{"type": "json_object"}`` / ``{"type": "text"}``) to request structured +output from the upstream provider. The value is forwarded verbatim as the +``response_format=`` kwarg on ``chat.completions.create()``. + +Validation is performed **before** the API call via +``_validate_response_format()``. If the dict is invalid (unknown type, +missing ``json_schema`` key for ``type="json_schema"``, etc.) the executor +enqueues an error message and returns early without calling the API. + +When ``response_format`` is ``None`` (the default) the kwarg is omitted +entirely from the API call so older / strict providers do not receive an +unexpected field. """ from __future__ import annotations @@ -77,6 +124,53 @@ def _reasoning_supported(model: str) -> bool: return any(pat in model_lower for pat in _HERMES4_PATTERNS) +# --------------------------------------------------------------------------- +# response_format validation (#498) +# --------------------------------------------------------------------------- + +_VALID_RESPONSE_FORMAT_TYPES: frozenset[str] = frozenset( + {"json_schema", "json_object", "text"} +) + + +def _validate_response_format(rf: dict) -> "str | None": + """Validate a ``response_format`` dict before forwarding to the API. + + Returns ``None`` if *rf* is valid, or an error message string describing + the first validation failure found. + + Valid ``type`` values are ``"json_schema"``, ``"json_object"``, and + ``"text"``. For ``type="json_schema"``, the dict must also contain a + ``"json_schema"`` key whose value is a dict with at least a ``"name"`` + key (str). If ``json_schema.schema`` is present it must be a dict. + + Examples:: + + >>> _validate_response_format({"type": "json_object"}) is None + True + >>> _validate_response_format({"type": "bad"}) is not None + True + """ + rf_type = rf.get("type") + if rf_type not in _VALID_RESPONSE_FORMAT_TYPES: + return ( + f"type must be one of {sorted(_VALID_RESPONSE_FORMAT_TYPES)!r}, " + f"got {rf_type!r}" + ) + + if rf_type == "json_schema": + js = rf.get("json_schema") + if not isinstance(js, dict): + return "json_schema must be a dict when type='json_schema'" + if not isinstance(js.get("name"), str): + return "json_schema.name must be a string" + schema = js.get("schema") + if schema is not None and not isinstance(schema, dict): + return "json_schema.schema must be a dict if present" + + return None + + # --------------------------------------------------------------------------- # ProviderConfig — per-provider / per-model capability flags # --------------------------------------------------------------------------- @@ -126,6 +220,7 @@ class HermesA2AExecutor(AgentExecutor): - System prompt injected as the first ``messages[]`` entry. - Hermes 4 reasoning enabled via ``extra_body`` when supported. - Reasoning trace logged to OTEL span — never echoed in the reply. + - Tool definitions passed via native ``tools`` parameter when supplied. Parameters ---------- @@ -142,6 +237,21 @@ class HermesA2AExecutor(AgentExecutor): heartbeat: Optional ``HeartbeatLoop`` instance used to surface the current task description in the platform UI. + response_format: + Optional OpenAI-native ``response_format`` dict forwarded verbatim + to ``chat.completions.create()``. Supported types: + ``{"type": "json_schema", "json_schema": {"name": ..., "schema": {...}}}`` + ``{"type": "json_object"}`` + ``{"type": "text"}`` + When ``None`` (default) the parameter is omitted from the API call. + Invalid dicts cause ``execute()`` to enqueue an error and return + early without calling the API. + tools: + Optional list of OpenAI-format tool definitions to pass via the + native ``tools`` parameter. Each entry must have ``"type"`` and + ``"function"`` keys matching the OpenAI function-calling schema. + ``None`` or ``[]`` → the ``tools`` key is **omitted** from the API + call entirely (never sent as ``tools=[]``). _client: Inject a pre-built ``AsyncOpenAI`` (or compatible mock) — for testing only. When provided, ``base_url`` and ``api_key`` are @@ -155,12 +265,18 @@ class HermesA2AExecutor(AgentExecutor): base_url: str | None = None, api_key: str | None = None, heartbeat: "HeartbeatLoop | None" = None, + response_format: "dict | None" = None, + tools: list[dict] | None = None, _client: Any = None, ) -> None: self.model = model self.system_prompt = system_prompt self._heartbeat = heartbeat + self._response_format = response_format self._provider = ProviderConfig(model) + # Empty list and None are treated identically: no tools → omit the + # parameter from the API call rather than sending tools=[]. + self._tools: list[dict] = list(tools) if tools else [] if _client is not None: # Test injection path — skip real AsyncOpenAI construction so @@ -245,10 +361,15 @@ class HermesA2AExecutor(AgentExecutor): Sequence: 1. Extract user text from A2A message parts. 2. Build ``messages[]`` (optional system + user). - 3. Call OpenAI-compat API; include ``extra_body`` for Hermes 4. + 3. Call OpenAI-compat API; include ``extra_body`` for Hermes 4 and + ``tools`` when tool definitions are configured. 4. Extract and log reasoning trace — does NOT appear in the reply. - 5. Enqueue a final ``Message`` with the content text. + 5a. If the model returned text content, enqueue it as the reply. + 5b. If the model returned tool calls with no text (``finish_reason`` + ``"tool_calls"``), serialise the calls as JSON and enqueue that. """ + import json + from shared_runtime import extract_message_text user_input = extract_message_text(context) @@ -262,18 +383,36 @@ class HermesA2AExecutor(AgentExecutor): messages = self._build_messages(user_input) + # Validate response_format before hitting the API — invalid dicts + # enqueue an error and return early without making an API call. + if self._response_format is not None: + detail = _validate_response_format(self._response_format) + if detail is not None: + await event_queue.enqueue_event( + new_agent_text_message(f"Error: invalid response_format — {detail}") + ) + return + # Only Hermes 4 entries get extra_body — sending it to Hermes 3 # or other models is a no-op at best; a 400 at worst. extra_body: dict | None = None if self._provider.reasoning_supported: extra_body = {"reasoning": {"enabled": True}} + # Build create() kwargs; omit response_format and tools entirely when + # not set so strict / older providers do not receive unexpected fields. + create_kwargs: dict = { + "model": self.model, + "messages": messages, + "extra_body": extra_body, + } + if self._response_format is not None: + create_kwargs["response_format"] = self._response_format + if self._tools: + create_kwargs["tools"] = self._tools + try: - response = await self._client.chat.completions.create( - model=self.model, - messages=messages, - extra_body=extra_body, - ) + response = await self._client.chat.completions.create(**create_kwargs) choice = response.choices[0] content: str = choice.message.content or "" @@ -297,6 +436,37 @@ class HermesA2AExecutor(AgentExecutor): # Log to OTEL — intentionally omitted from the A2A reply. self._log_reasoning(context, reasoning, reasoning_details) + # Handle tool-call response: when the model returns tool calls + # with no text content, serialise the calls as JSON so the caller + # receives structured, parseable output. This keeps the executor + # thin (single API call per turn) while not silently discarding + # function-call intent. + if not content: + tool_calls = getattr(choice.message, "tool_calls", None) + if tool_calls: + serialised = json.dumps([ + { + "id": getattr(tc, "id", ""), + "type": getattr(tc, "type", "function"), + "function": { + "name": getattr( + getattr(tc, "function", None), "name", "" + ), + "arguments": getattr( + getattr(tc, "function", None), "arguments", "{}" + ), + }, + } + for tc in tool_calls + ]) + logger.info( + "hermes_executor: tool_calls response [model=%s n=%d]", + self.model, + len(tool_calls), + ) + await event_queue.enqueue_event(new_agent_text_message(serialised)) + return + final_text = content.strip() or "(no response generated)" await event_queue.enqueue_event(new_agent_text_message(final_text)) diff --git a/workspace-template/tests/test_hermes_executor.py b/workspace-template/tests/test_hermes_executor.py index d6129c58..cd95158e 100644 --- a/workspace-template/tests/test_hermes_executor.py +++ b/workspace-template/tests/test_hermes_executor.py @@ -4,12 +4,18 @@ Coverage targets ---------------- - _reasoning_supported() — model name pattern detection - ProviderConfig — capability flags derived from model name -- HermesA2AExecutor.__init__ — field assignment + client injection +- _validate_response_format() — valid types, invalid type, missing fields (#498) +- HermesA2AExecutor.__init__ — field assignment + client injection, + response_format stored (#498), tools (#497) - HermesA2AExecutor._build_messages — system prompt + user turn assembly - HermesA2AExecutor._log_reasoning — OTEL span emission + swallowed errors - HermesA2AExecutor.execute — happy path, empty input, API error, Hermes 4 extra_body, Hermes 3 no extra_body, - reasoning not in reply, reasoning_details + reasoning not in reply, reasoning_details, + response_format forwarded / omitted / invalid (#498), + tools serialized in request body (#497), + empty tools → no tools field (#497), + tool_call response → JSON text (#497) - HermesA2AExecutor.cancel — TaskStatusUpdateEvent emitted The ``openai`` module is stubbed in sys.modules so no real API call is made. @@ -70,6 +76,7 @@ from hermes_executor import ( # noqa: E402 ProviderConfig, _HERMES4_PATTERNS, _reasoning_supported, + _validate_response_format, ) @@ -699,3 +706,407 @@ async def test_no_system_prompt_only_user_message(): msgs = mock_client.chat.completions.create.call_args[1]["messages"] assert len(msgs) == 1 assert msgs[0]["role"] == "user" + + +# --------------------------------------------------------------------------- +# _validate_response_format — issue #498 +# --------------------------------------------------------------------------- + + +def test_validate_response_format_json_schema_valid(): + """Valid json_schema dict (with name and schema) returns None.""" + rf = { + "type": "json_schema", + "json_schema": { + "name": "my_schema", + "schema": {"type": "object", "properties": {}}, + }, + } + assert _validate_response_format(rf) is None + + +def test_validate_response_format_json_object_valid(): + """{"type": "json_object"} returns None (no sub-fields required).""" + assert _validate_response_format({"type": "json_object"}) is None + + +def test_validate_response_format_text_valid(): + """{"type": "text"} returns None.""" + assert _validate_response_format({"type": "text"}) is None + + +def test_validate_response_format_invalid_type(): + """An unknown type value returns a non-None error string.""" + result = _validate_response_format({"type": "yaml_schema"}) + assert result is not None + assert isinstance(result, str) + assert "yaml_schema" in result + + +def test_validate_response_format_missing_json_schema_key(): + """type='json_schema' but no 'json_schema' key → error string.""" + result = _validate_response_format({"type": "json_schema"}) + assert result is not None + assert "json_schema" in result + + +def test_validate_response_format_json_schema_schema_not_dict(): + """json_schema.schema present but not a dict → error string.""" + rf = { + "type": "json_schema", + "json_schema": {"name": "s", "schema": "not-a-dict"}, + } + result = _validate_response_format(rf) + assert result is not None + assert "schema" in result + + +def test_validate_response_format_json_schema_missing_name(): + """json_schema present but missing 'name' key → error string.""" + rf = { + "type": "json_schema", + "json_schema": {"schema": {"type": "object"}}, + } + result = _validate_response_format(rf) + assert result is not None + assert "name" in result + + +def test_constructor_response_format_stored(): + """response_format kwarg is stored as _response_format attribute.""" + rf = {"type": "json_object"} + executor = HermesA2AExecutor( + model="hermes-4", + response_format=rf, + _client=MagicMock(), + ) + assert executor._response_format is rf + + +def test_constructor_no_response_format_is_none(): + """Omitting response_format → _response_format is None.""" + executor = HermesA2AExecutor(model="hermes-4", _client=MagicMock()) + assert executor._response_format is None + + +@pytest.mark.asyncio +async def test_execute_response_format_in_request(): + """Valid response_format is forwarded as a kwarg to the API call.""" + rf = {"type": "json_object"} + mock_client = MagicMock() + mock_client.chat.completions.create = AsyncMock( + return_value=_make_api_response('{"answer": 42}') + ) + executor = HermesA2AExecutor( + model="nousresearch/hermes-3-llama-3.1-70b", + response_format=rf, + _client=mock_client, + ) + + await executor.execute(_make_context("hello"), AsyncMock()) + + call_kwargs = mock_client.chat.completions.create.call_args[1] + assert call_kwargs.get("response_format") == rf + + +@pytest.mark.asyncio +async def test_execute_response_format_omitted_when_none(): + """When response_format is None, it is NOT present in the API call kwargs.""" + mock_client = MagicMock() + mock_client.chat.completions.create = AsyncMock( + return_value=_make_api_response("ok") + ) + executor = HermesA2AExecutor( + model="nousresearch/hermes-3-llama-3.1-70b", + response_format=None, + _client=mock_client, + ) + + await executor.execute(_make_context("hello"), AsyncMock()) + + call_kwargs = mock_client.chat.completions.create.call_args[1] + assert "response_format" not in call_kwargs + + +@pytest.mark.asyncio +async def test_execute_invalid_response_format_returns_error_no_api_call(): + """Invalid response_format → error enqueued, API create() NOT called.""" + rf = {"type": "unsupported_format"} + mock_client = MagicMock() + mock_client.chat.completions.create = AsyncMock() + executor = HermesA2AExecutor( + model="hermes-4", + response_format=rf, + _client=mock_client, + ) + + eq = AsyncMock() + await executor.execute(_make_context("hello"), eq) + + # Should have enqueued an error message + eq.enqueue_event.assert_called_once() + enqueued = eq.enqueue_event.call_args[0][0] + assert "Error: invalid response_format" in enqueued + + # API must NOT have been called + mock_client.chat.completions.create.assert_not_called() + + +# --------------------------------------------------------------------------- +# Native tools parameter — issue #497 +# --------------------------------------------------------------------------- + +# Minimal OpenAI-format tool definition used across the tools tests. +_SAMPLE_TOOL: dict = { + "type": "function", + "function": { + "name": "get_weather", + "description": "Get current weather for a location.", + "parameters": { + "type": "object", + "properties": { + "location": {"type": "string", "description": "City name"}, + }, + "required": ["location"], + }, + }, +} + +_SAMPLE_TOOL_2: dict = { + "type": "function", + "function": { + "name": "search_web", + "description": "Search the web.", + "parameters": { + "type": "object", + "properties": {"query": {"type": "string"}}, + "required": ["query"], + }, + }, +} + + +class _FakeFunction: + """Stand-in for openai ChatCompletionMessageToolCall.function.""" + + def __init__(self, name: str, arguments: str) -> None: + self.name = name + self.arguments = arguments + + +class _FakeToolCall: + """Stand-in for openai ChatCompletionMessageToolCall.""" + + def __init__(self, tc_id: str, name: str, arguments: str = "{}") -> None: + self.id = tc_id + self.type = "function" + self.function = _FakeFunction(name=name, arguments=arguments) + + +def _make_tool_call_response(tool_calls: list, content: str = ""): + """Build a mock API response that includes tool_calls on the message.""" + + class _MsgWithToolCalls: + def __init__(self): + self.content = content + self.tool_calls = tool_calls + + choice = MagicMock() + choice.message = _MsgWithToolCalls() + response = MagicMock() + response.choices = [choice] + return response + + +def test_constructor_tools_stored_correctly(): + """tools list is stored as _tools attribute.""" + executor = HermesA2AExecutor( + model="hermes-4", + tools=[_SAMPLE_TOOL, _SAMPLE_TOOL_2], + _client=MagicMock(), + ) + assert executor._tools == [_SAMPLE_TOOL, _SAMPLE_TOOL_2] + + +def test_constructor_none_tools_stored_as_empty_list(): + """tools=None → _tools is [] (empty list, not None).""" + executor = HermesA2AExecutor(model="hermes-4", tools=None, _client=MagicMock()) + assert executor._tools == [] + + +def test_constructor_empty_list_stored_as_empty_list(): + """tools=[] → _tools is [].""" + executor = HermesA2AExecutor(model="hermes-4", tools=[], _client=MagicMock()) + assert executor._tools == [] + + +def test_constructor_tools_is_independent_copy(): + """_tools is a copy — mutating the input list doesn't affect the executor.""" + original = [_SAMPLE_TOOL] + executor = HermesA2AExecutor( + model="hermes-4", tools=original, _client=MagicMock() + ) + original.append(_SAMPLE_TOOL_2) + assert executor._tools == [_SAMPLE_TOOL] + + +@pytest.mark.asyncio +async def test_execute_tools_serialized_in_request_body(): + """Non-empty tools list is forwarded to chat.completions.create as tools=.""" + mock_client = MagicMock() + mock_client.chat.completions.create = AsyncMock( + return_value=_make_api_response("Paris is sunny.") + ) + executor = HermesA2AExecutor( + model="hermes-4", + tools=[_SAMPLE_TOOL], + _client=mock_client, + ) + + await executor.execute(_make_context("weather?"), AsyncMock()) + + call_kwargs = mock_client.chat.completions.create.call_args[1] + assert "tools" in call_kwargs + assert call_kwargs["tools"] == [_SAMPLE_TOOL] + + +@pytest.mark.asyncio +async def test_execute_multiple_tools_all_forwarded(): + """All tool definitions are forwarded — not truncated.""" + mock_client = MagicMock() + mock_client.chat.completions.create = AsyncMock( + return_value=_make_api_response("ok") + ) + executor = HermesA2AExecutor( + model="hermes-4", + tools=[_SAMPLE_TOOL, _SAMPLE_TOOL_2], + _client=mock_client, + ) + + await executor.execute(_make_context("search?"), AsyncMock()) + + call_kwargs = mock_client.chat.completions.create.call_args[1] + assert call_kwargs["tools"] == [_SAMPLE_TOOL, _SAMPLE_TOOL_2] + + +@pytest.mark.asyncio +async def test_execute_empty_tools_no_tools_field_in_request(): + """Empty tools list → 'tools' key absent from API call (not tools=[]).""" + mock_client = MagicMock() + mock_client.chat.completions.create = AsyncMock( + return_value=_make_api_response("ok") + ) + executor = HermesA2AExecutor(model="hermes-4", tools=[], _client=mock_client) + + await executor.execute(_make_context("hello"), AsyncMock()) + + call_kwargs = mock_client.chat.completions.create.call_args[1] + assert "tools" not in call_kwargs + + +@pytest.mark.asyncio +async def test_execute_none_tools_no_tools_field_in_request(): + """tools=None → 'tools' key absent from API call.""" + mock_client = MagicMock() + mock_client.chat.completions.create = AsyncMock( + return_value=_make_api_response("ok") + ) + executor = HermesA2AExecutor(model="hermes-4", tools=None, _client=mock_client) + + await executor.execute(_make_context("hello"), AsyncMock()) + + call_kwargs = mock_client.chat.completions.create.call_args[1] + assert "tools" not in call_kwargs + + +@pytest.mark.asyncio +async def test_execute_default_no_tools_field_in_request(): + """Constructor with no tools kwarg → 'tools' key absent from API call.""" + executor, mock_client = _make_executor(model="hermes-4") + mock_client.chat.completions.create.return_value = _make_api_response("ok") + + await executor.execute(_make_context("hello"), AsyncMock()) + + call_kwargs = mock_client.chat.completions.create.call_args[1] + assert "tools" not in call_kwargs + + +@pytest.mark.asyncio +async def test_execute_tool_call_response_returns_json(): + """Model returns tool_calls with no content → reply is JSON-serialised calls.""" + import json + + mock_client = MagicMock() + tc = _FakeToolCall("call_abc123", "get_weather", '{"location":"Paris"}') + mock_client.chat.completions.create = AsyncMock( + return_value=_make_tool_call_response(tool_calls=[tc], content="") + ) + executor = HermesA2AExecutor( + model="hermes-4", + tools=[_SAMPLE_TOOL], + _client=mock_client, + ) + + eq = AsyncMock() + await executor.execute(_make_context("weather in Paris?"), eq) + + eq.enqueue_event.assert_called_once() + reply = eq.enqueue_event.call_args[0][0] + # Must be valid JSON + parsed = json.loads(reply) + assert isinstance(parsed, list) + assert len(parsed) == 1 + assert parsed[0]["function"]["name"] == "get_weather" + assert parsed[0]["function"]["arguments"] == '{"location":"Paris"}' + assert parsed[0]["id"] == "call_abc123" + assert parsed[0]["type"] == "function" + + +@pytest.mark.asyncio +async def test_execute_multiple_tool_calls_all_in_json(): + """Multiple tool calls are all serialised into the JSON reply.""" + import json + + mock_client = MagicMock() + tc1 = _FakeToolCall("call_1", "get_weather", '{"location":"Paris"}') + tc2 = _FakeToolCall("call_2", "search_web", '{"query":"news"}') + mock_client.chat.completions.create = AsyncMock( + return_value=_make_tool_call_response(tool_calls=[tc1, tc2], content="") + ) + executor = HermesA2AExecutor( + model="hermes-4", + tools=[_SAMPLE_TOOL, _SAMPLE_TOOL_2], + _client=mock_client, + ) + + eq = AsyncMock() + await executor.execute(_make_context("do both"), eq) + + reply = eq.enqueue_event.call_args[0][0] + parsed = json.loads(reply) + assert len(parsed) == 2 + assert parsed[0]["function"]["name"] == "get_weather" + assert parsed[1]["function"]["name"] == "search_web" + + +@pytest.mark.asyncio +async def test_execute_text_content_wins_over_tool_calls(): + """When model returns both text content AND tool_calls, text is used.""" + mock_client = MagicMock() + tc = _FakeToolCall("call_xyz", "get_weather", '{"location":"Berlin"}') + mock_client.chat.completions.create = AsyncMock( + return_value=_make_tool_call_response( + tool_calls=[tc], content="The weather is fine." + ) + ) + executor = HermesA2AExecutor( + model="hermes-4", + tools=[_SAMPLE_TOOL], + _client=mock_client, + ) + + eq = AsyncMock() + await executor.execute(_make_context("weather?"), eq) + + reply = eq.enqueue_event.call_args[0][0] + assert reply == "The weather is fine."