diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 096a79a..793d74f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,10 @@ jobs: go-version-file: go.mod - name: Set up pnpm - uses: pnpm/action-setup@v4 + # Pinned to commit SHA per supply-chain hygiene — third-party + # actions can be rewritten under a moving tag. Bump by re-running + # `gh api repos/pnpm/action-setup/git/refs/tags/v4`. + uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4 with: version: 10.33.0 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f66400d..76c19b1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -41,7 +41,10 @@ jobs: go-version-file: go.mod - name: Set up pnpm - uses: pnpm/action-setup@v4 + # Pinned to commit SHA per supply-chain hygiene — third-party + # actions can be rewritten under a moving tag. Bump by re-running + # `gh api repos/pnpm/action-setup/git/refs/tags/v4`. + uses: pnpm/action-setup@b906affcce14559ad1aafd4ab0e942779e9f58b1 # v4 with: # Pinned to ui/package.json packageManager so a bumped lockfile # doesn't desync from the toolchain CI runs the build with. diff --git a/internal/serve/api/feed_history.go b/internal/serve/api/feed_history.go index 039d65a..d07e793 100644 --- a/internal/serve/api/feed_history.go +++ b/internal/serve/api/feed_history.go @@ -422,23 +422,8 @@ func summariseHistoryInput(raw map[string]any, tool string) string { if !ok { return "" } - get := func(k string) string { - if v, ok := in[k].(string); ok { - return v - } - return "" - } - switch tool { - case "Bash": - return truncateHistory(get("command")) - case "Edit", "Write", "Read", "MultiEdit", "NotebookEdit": - return truncateHistory(get("file_path")) - case "Glob", "Grep": - return truncateHistory(get("pattern")) - case "WebFetch": - return truncateHistory(get("url")) - case "Task": - return truncateHistory(get("description")) + if v, ok := truncateToolInputField(tool, in); ok { + return v } body, err := json.Marshal(in) if err != nil { diff --git a/internal/serve/api/handler_helpers.go b/internal/serve/api/handler_helpers.go new file mode 100644 index 0000000..b8316ce --- /dev/null +++ b/internal/serve/api/handler_helpers.go @@ -0,0 +1,65 @@ +package api + +import ( + "net/http" +) + +// requireSessionPreamble runs the boilerplate every /api/sessions/{name}/... +// JSON GET handler needs: enforce GET/HEAD only, set the standard +// Content-Type + Cache-Control headers, extract the {name} path param, and +// reject an empty name. Returns the session name on success; on failure it +// has already written the error response and the caller should return. +// +// Sonar previously flagged subagents.go, teams.go, etc. as duplicating this +// 16-line block — lifting it removes the copy-paste while keeping each +// handler's mode-specific logic local. +func requireSessionPreamble(w http.ResponseWriter, r *http.Request) (name string, ok bool) { + if r.Method != http.MethodGet && r.Method != http.MethodHead { + w.Header().Set("Allow", "GET, HEAD") + w.Header().Set("Cache-Control", "no-store") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return "", false + } + + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Cache-Control", "no-store") + + name = r.PathValue("name") + if name == "" { + writeJSON(w, http.StatusBadRequest, errorBody{Error: "session name required"}) + return "", false + } + return name, true +} + +// truncateToolInputField returns the well-known "primary input" string for +// the named tool — Bash → command, Edit/Write/etc. → file_path, Glob/Grep +// → pattern, WebFetch → url, Task → description. Returns ok=false for tools +// the switch doesn't know about so the caller can pick its own fallback +// (the feed-history summariser JSON-marshals the whole input map; the +// subagent label scans description/prompt/query). +// +// Both shortestSubagentInputLabel (subagents.go) and summariseHistoryInput +// (feed_history.go) used to inline the same get-closure + switch — Sonar +// reported the duplicate, this consolidates the per-tool routing. +func truncateToolInputField(tool string, in map[string]any) (string, bool) { + get := func(k string) string { + if v, ok := in[k].(string); ok { + return v + } + return "" + } + switch tool { + case "Bash": + return truncateHistory(get("command")), true + case "Edit", "Write", "Read", "MultiEdit", "NotebookEdit": + return truncateHistory(get("file_path")), true + case "Glob", "Grep": + return truncateHistory(get("pattern")), true + case "WebFetch": + return truncateHistory(get("url")), true + case "Task": + return truncateHistory(get("description")), true + } + return "", false +} diff --git a/internal/serve/api/subagents.go b/internal/serve/api/subagents.go index 6c1d66d..5be5121 100644 --- a/internal/serve/api/subagents.go +++ b/internal/serve/api/subagents.go @@ -92,19 +92,8 @@ type subagentsResponse struct { // the api package. func Subagents(logDir string, resolver UUIDNameResolver) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet && r.Method != http.MethodHead { - w.Header().Set("Allow", "GET, HEAD") - w.Header().Set("Cache-Control", "no-store") - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) - return - } - - w.Header().Set("Content-Type", "application/json") - w.Header().Set("Cache-Control", "no-store") - - name := r.PathValue("name") - if name == "" { - writeJSON(w, http.StatusBadRequest, errorBody{Error: "session name required"}) + name, ok := requireSessionPreamble(w, r) + if !ok { return } @@ -317,27 +306,12 @@ func parseSubagentLine(line []byte) (subagentLineMeta, bool) { // Mirrors the feed-row summariser's per-tool conventions so a // subagent expanded in the UI matches the feed rows shown below it. func shortestSubagentInputLabel(tool string, in map[string]any) string { - get := func(k string) string { - if v, ok := in[k].(string); ok { - return v - } - return "" - } - switch tool { - case "Bash": - return truncateHistory(get("command")) - case "Edit", "Write", "Read", "MultiEdit", "NotebookEdit": - return truncateHistory(get("file_path")) - case "Glob", "Grep": - return truncateHistory(get("pattern")) - case "WebFetch": - return truncateHistory(get("url")) - case "Task": - return truncateHistory(get("description")) + if v, ok := truncateToolInputField(tool, in); ok { + return v } // Fallback: any "description"-ish key. for _, k := range []string{"description", "prompt", "query"} { - if v := get(k); v != "" { + if v, ok := in[k].(string); ok && v != "" { return truncateHistory(v) } } diff --git a/internal/serve/api/teams.go b/internal/serve/api/teams.go index ea4e2ca..c9cf73c 100644 --- a/internal/serve/api/teams.go +++ b/internal/serve/api/teams.go @@ -86,19 +86,8 @@ type teamsResponse struct { // Teams returns GET /api/sessions/{name}/teams. func Teams(logDir string, resolver UUIDNameResolver) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet && r.Method != http.MethodHead { - w.Header().Set("Allow", "GET, HEAD") - w.Header().Set("Cache-Control", "no-store") - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) - return - } - - w.Header().Set("Content-Type", "application/json") - w.Header().Set("Cache-Control", "no-store") - - name := r.PathValue("name") - if name == "" { - writeJSON(w, http.StatusBadRequest, errorBody{Error: "session name required"}) + name, ok := requireSessionPreamble(w, r) + if !ok { return } diff --git a/ui/src/components/auth/AuthField.tsx b/ui/src/components/auth/AuthField.tsx new file mode 100644 index 0000000..d8f4a57 --- /dev/null +++ b/ui/src/components/auth/AuthField.tsx @@ -0,0 +1,29 @@ +// Shared input field for the LoginForm + SignupForm. Each form previously +// declared an identical local `Field` component — Sonar flagged the block +// as duplicated. Lifted here so both forms keep identical styling without +// drifting. + +interface Props { + label: string; + value: string; + onChange: (v: string) => void; + type?: string; + autoComplete?: string; +} + +export function AuthField({ label, value, onChange, type = "text", autoComplete }: Props) { + return ( + + ); +} diff --git a/ui/src/components/auth/serverMessage.test.ts b/ui/src/components/auth/serverMessage.test.ts new file mode 100644 index 0000000..5089b91 --- /dev/null +++ b/ui/src/components/auth/serverMessage.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from "vitest"; +import { ApiError } from "@/lib/api"; +import { serverMessage } from "./serverMessage"; + +describe("serverMessage", () => { + it("returns the body.message when ApiError carries a string message", () => { + const err = new ApiError(400, "bad request", { message: "username already taken" }); + expect(serverMessage(err)).toBe("username already taken"); + }); + + it("returns undefined when the error is not an ApiError", () => { + expect(serverMessage(new Error("network"))).toBeUndefined(); + expect(serverMessage("oops")).toBeUndefined(); + expect(serverMessage(null)).toBeUndefined(); + }); + + it("returns undefined when the body has no message field", () => { + const err = new ApiError(500, "internal", { code: 42 }); + expect(serverMessage(err)).toBeUndefined(); + }); + + it("returns undefined when message is non-string", () => { + const err = new ApiError(400, "bad", { message: 123 }); + expect(serverMessage(err)).toBeUndefined(); + }); + + it("returns undefined when body is null or non-object", () => { + expect(serverMessage(new ApiError(400, "bad", null))).toBeUndefined(); + expect(serverMessage(new ApiError(400, "bad", "string body"))).toBeUndefined(); + }); +}); diff --git a/ui/src/components/auth/serverMessage.ts b/ui/src/components/auth/serverMessage.ts new file mode 100644 index 0000000..6faaa4d --- /dev/null +++ b/ui/src/components/auth/serverMessage.ts @@ -0,0 +1,12 @@ +import { ApiError } from "@/lib/api"; + +// Pulls a `message` field out of an ApiError body when the server sent +// one. LoginForm and SignupForm both want this — `await mutateAsync` can +// surface a typed ApiError whose body shape is `{ message?: string }`. +export function serverMessage(e: unknown): string | undefined { + if (e instanceof ApiError && typeof e.body === "object" && e.body !== null) { + const m = (e.body as { message?: unknown }).message; + if (typeof m === "string") return m; + } + return undefined; +} diff --git a/ui/src/routes/LoginForm.tsx b/ui/src/routes/LoginForm.tsx index 190cf9b..291bb61 100644 --- a/ui/src/routes/LoginForm.tsx +++ b/ui/src/routes/LoginForm.tsx @@ -2,6 +2,8 @@ import { useState } from "react"; import { Button } from "@ossrandom/design-system"; import { ApiError } from "@/lib/api"; import { useLogin } from "@/hooks/useLogin"; +import { AuthField } from "@/components/auth/AuthField"; +import { serverMessage } from "@/components/auth/serverMessage"; interface Props { onSwitchToSignup?: () => void; @@ -38,8 +40,8 @@ export function LoginForm({ onSwitchToSignup }: Props) {