refactor: dedupe auth forms, yolo modes, session API preamble + pin actions (#9)

* refactor: dedupe auth forms, yolo modes, session API preamble

Targets the duplication blocks Sonar flagged on main. Behaviour
unchanged across the board — verified by 692 Go tests and 110 UI
tests passing.

UI:
- LoginForm/SignupForm shared an identical local <Field> input and
  serverMessage error parser. Lifted to ui/src/components/auth/.
- Cuts the 41-line duplicate from each form.

Go cmd/yolo.go:
- runYoloBang and runSafe shared the same 24-line preamble (serve up,
  config load, store/tmux clients, name + workdir resolution). Lifted
  to setupModeContext.
- runYolo and runSafe shared the same 24-line resume-or-recreate
  dance (banner, lifecycle hooks, preflight or kill+recreate). Lifted
  to runResumeOrRecreate with mode-specific knobs (banner color,
  hook/serve event names, store-delete log loudness).

Go internal/serve/api/:
- New handler_helpers.go houses two reused snippets:
  - requireSessionPreamble (method check + standard JSON headers +
    {name} extraction + empty-name reject) — Subagents and Teams both
    used this verbatim.
  - truncateToolInputField (per-tool primary-input field router for
    Bash/Edit/Glob/WebFetch/Task) — shortestSubagentInputLabel and
    summariseHistoryInput shared the get-closure + switch.

Workflow hygiene:
- pnpm/action-setup pinned to commit SHA in ci.yml + release.yml
  (the only third-party action). Same hardening already done in
  sonar.yml; closes the remaining 2 githubactions:S7637 hotspots.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(ui): cover serverMessage; defer cmd/yolo.go refactor

Two adjustments to satisfy the SonarCloud new-code coverage gate
(≥80% on PR-touched lines):

- Add ui/src/components/auth/serverMessage.test.ts — covers all five
  branches (string message hit, non-ApiError, missing message field,
  non-string message, null/string body). Bumps the file from 50% to
  100% new-code coverage.
- Revert cmd/yolo.go to main. The setupModeContext + runResumeOrRecreate
  refactor was correct (692 tests stayed green) but tightly coupled to
  proc.EnsureServeRunning / preflight / createAndAttach side effects,
  so the new lines landed uncovered and dragged the gate from 39.9%.
  This refactor is reattempted in PR-C alongside the cmd/ unit-test
  scaffolding that lets it be tested cleanly.

The api/ + ui/auth dedupes (the bulk of the dup-density wins) stay.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: aksOps

.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
 

.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.

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 {

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
+}

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)
 		}
 	}

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
 		}
 

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 (
+    <label className="block">
+      <span className="mb-1 block text-[11px] font-semibold uppercase tracking-[0.18em] text-fg-muted">
+        {label}
+      </span>
+      <input
+        type={type}
+        value={value}
+        onChange={(e) => onChange(e.target.value)}
+        autoComplete={autoComplete}
+        className="block w-full rounded border border-border bg-bg px-2 py-1.5 font-mono text-sm text-fg placeholder:text-fg-dim focus:outline-none focus:ring-1 focus:ring-accent-gold sm:text-xs"
+      />
+    </label>
+  );
+}

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();
+  });
+});

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;
+}

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) {
     <div className="mx-auto mt-16 w-full max-w-sm rounded-lg border border-border bg-surface p-6">
       <h1 className="mb-4 text-lg font-bold sm:text-xl">Log in to ctm</h1>
       <form onSubmit={onSubmit} className="space-y-3">
-        <Field label="Email" type="email" value={username} onChange={setUsername} autoComplete="email" />
-        <Field label="Password" type="password" value={password} onChange={setPassword} autoComplete="current-password" />
+        <AuthField label="Email" type="email" value={username} onChange={setUsername} autoComplete="email" />
+        <AuthField label="Password" type="password" value={password} onChange={setPassword} autoComplete="current-password" />
         {err && (
           <div role="alert" className="text-[11px] text-alert-ember">
             {err}
@@ -72,40 +74,3 @@ export function LoginForm({ onSwitchToSignup }: Props) {
     </div>
   );
 }
-
-function Field({
-  label,
-  value,
-  onChange,
-  type = "text",
-  autoComplete,
-}: {
-  label: string;
-  value: string;
-  onChange: (v: string) => void;
-  type?: string;
-  autoComplete?: string;
-}) {
-  return (
-    <label className="block">
-      <span className="mb-1 block text-[11px] font-semibold uppercase tracking-[0.18em] text-fg-muted">
-        {label}
-      </span>
-      <input
-        type={type}
-        value={value}
-        onChange={(e) => onChange(e.target.value)}
-        autoComplete={autoComplete}
-        className="block w-full rounded border border-border bg-bg px-2 py-1.5 font-mono text-sm text-fg placeholder:text-fg-dim focus:outline-none focus:ring-1 focus:ring-accent-gold sm:text-xs"
-      />
-    </label>
-  );
-}
-
-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;
-}