diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bfa03df..37f99d4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,6 +22,11 @@ permissions: jobs: docker: runs-on: ubuntu-latest + # A stuck build (e.g. a target-platform stage running under QEMU + # emulation instead of natively — see the Dockerfile's --platform= + # $BUILDPLATFORM comment) would otherwise silently run for GitHub's + # 360-minute hard cap before getting cancelled. Fail fast instead. + timeout-minutes: 30 permissions: contents: read packages: write # to push to ghcr.io diff --git a/Dockerfile b/Dockerfile index 559641a..8b86306 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,13 +1,23 @@ # syntax=docker/dockerfile:1 -FROM node:22-alpine AS web-build +## --platform=$BUILDPLATFORM pins these two stages to the runner's own +# architecture (amd64) instead of the target platform buildx is building for. +# Neither stage needs to run target-arch code — the web build only produces +# static JS/CSS, and Go cross-compiles (GOOS/GOARCH below) without ever +# executing arm64 instructions. Without this pin, buildx runs BOTH stages +# under QEMU user-mode emulation for a linux/arm64 build: npm/Node under +# QEMU is known to hang outright rather than just run slow, which is what +# turned a normal few-minute image build into a 360-minute (GitHub's hard +# cap) stuck job. Only the final base image below stays platform-matched — +# that's just filesystem layers, nothing to execute. +FROM --platform=$BUILDPLATFORM node:22-alpine AS web-build WORKDIR /web COPY web/package.json web/package-lock.json* ./ RUN npm ci COPY web/ ./ RUN npm run build -FROM golang:1.26-alpine AS go-build +FROM --platform=$BUILDPLATFORM golang:1.26-alpine AS go-build WORKDIR /src COPY go.mod go.sum ./ RUN go mod download diff --git a/cmd/ferrum/main.go b/cmd/ferrum/main.go index 75ab9b6..d8dae53 100644 --- a/cmd/ferrum/main.go +++ b/cmd/ferrum/main.go @@ -14,6 +14,7 @@ import ( "os" "os/signal" "strings" + "sync" "syscall" "time" @@ -187,16 +188,29 @@ func runServer(ctx context.Context, cfg config.Config) { // stops — rather than polling through — the graceful-shutdown window. pollerCtx, stopPoller := context.WithCancel(ctx) defer stopPoller() - go evaluator.Run(pollerCtx, srv.AlertPollInterval(ctx)) - go webhookDispatcher.Run(pollerCtx, eventBus) + // wg tracks these four background loops so shutdown can wait for them to + // actually return before the deferred db.Close()/srv.Close() run — + // canceling pollerCtx only asks them to stop; without this wait, a poll + // tick still in flight when shutdown proceeds keeps issuing queries + // against a database (and server resources) that are already closing. + var wg sync.WaitGroup + runLoop := func(fn func(context.Context)) { + wg.Add(1) + go func() { + defer wg.Done() + fn(pollerCtx) + }() + } + runLoop(func(ctx context.Context) { evaluator.Run(ctx, srv.AlertPollInterval(ctx)) }) + runLoop(func(ctx context.Context) { webhookDispatcher.Run(ctx, eventBus) }) // Snapshot retention sweep + orphaned-disk check — read-heavy and slower // moving than the metric alert evaluator, so it runs on its own longer // interval rather than sharing AlertPollInterval. lifecycleEvaluator := poller.NewLifecycleEvaluator(db, connections.New(db, secretBox)) - go lifecycleEvaluator.Run(pollerCtx, lifecycleSweepInterval) + runLoop(func(ctx context.Context) { lifecycleEvaluator.Run(ctx, lifecycleSweepInterval) }) - go digestScheduler.Run(pollerCtx) + runLoop(digestScheduler.Run) distFS, err := web.DistFS() if err != nil { @@ -237,6 +251,26 @@ func runServer(ctx context.Context, cfg config.Config) { if err := httpServer.Shutdown(shutdownCtx); err != nil { slog.Error("graceful shutdown failed", "error", err) } + + // Stop the background loops and wait for their current iteration to + // actually return before this function's own defers (db.Close, + // srv.Close) run — see the wg comment above for why. + stopPoller() + // 5s on top of httpServer.Shutdown's own 10s budget above — kept under + // 15s total so this always finishes within the Windows service wrapper's + // own 15s stop deadline (service_windows.go). + waitCtx, cancelWait := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelWait() + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + case <-waitCtx.Done(): + slog.Warn("background loops did not stop within the shutdown deadline") + } } // newLogger builds the default logger and returns a cleanup func that closes diff --git a/internal/api/ai_chat.go b/internal/api/ai_chat.go index eb64ddf..591992f 100644 --- a/internal/api/ai_chat.go +++ b/internal/api/ai_chat.go @@ -131,6 +131,11 @@ type toolResultEnvelope struct { ToolResult *toolActivity `json:"ferrum_tool_result,omitempty"` } type toolActivity struct { + // ID is the provider's own tool_call_id — present so the client can + // correlate a result back to its call even when the same tool name is + // invoked twice in the same round (parallel tool calls), instead of + // guessing via "most recent call with this name still running". + ID string `json:"id,omitempty"` Name string `json:"name"` Args any `json:"args,omitempty"` OK bool `json:"ok,omitempty"` @@ -561,10 +566,10 @@ func (s *Server) aiChat(w http.ResponseWriter, r *http.Request) { _ = json.Unmarshal([]byte(argsStr), &argsParsed) // The live envelope gets the same redaction recordToolCall applies // to the persisted copy — args often carry credentials. - writeSSEJSON(w, flusher, toolCallEnvelope{ToolCall: &toolActivity{Name: name, Args: redactSensitive(argsParsed)}}) + writeSSEJSON(w, flusher, toolCallEnvelope{ToolCall: &toolActivity{ID: id, Name: name, Args: redactSensitive(argsParsed)}}) resultText, isErr := s.mcp.CallTool(ctx, user, "chat", name, json.RawMessage(argsStr)) - writeSSEJSON(w, flusher, toolResultEnvelope{ToolResult: &toolActivity{Name: name, OK: !isErr, Result: truncateForDisplay(resultText)}}) + writeSSEJSON(w, flusher, toolResultEnvelope{ToolResult: &toolActivity{ID: id, Name: name, OK: !isErr, Result: truncateForDisplay(resultText)}}) lastToolRound = append(lastToolRound, toolRoundResult{name: name, result: resultText, isErr: isErr}) messages = append(messages, map[string]any{"role": "tool", "tool_call_id": id, "content": resultText}) diff --git a/internal/api/console.go b/internal/api/console.go index ccf4b2f..ae346dc 100644 --- a/internal/api/console.go +++ b/internal/api/console.go @@ -188,6 +188,27 @@ func newConsoleSession(connID, guestType, node string, vmid int, port, ticket st return sessionID } +// openSessionsSem caps concurrently open console/shell WebSocket sessions — +// shared by consoleWebSocket (VNC/termproxy) and sshWebSocket (direct SSH, +// in ssh_console.go) since both live in this package and mint one goroutine +// pair plus an upstream dial per session. Without this, a user (or a script) +// opening many sessions at once accumulates unbounded goroutines/dials. +var openSessionsSem = make(chan struct{}, 50) + +// acquireSessionSlot tries to claim a slot in openSessionsSem without +// blocking. On success the caller must release it (e.g. via defer) once the +// session ends; on failure it has already written a 503 response and the +// caller must not upgrade the connection. +func acquireSessionSlot(w http.ResponseWriter) (release func(), ok bool) { + select { + case openSessionsSem <- struct{}{}: + return func() { <-openSessionsSem }, true + default: + http.Error(w, "too many open console sessions, try again shortly", http.StatusServiceUnavailable) + return nil, false + } +} + var upgrader = websocket.Upgrader{ ReadBufferSize: 8192, WriteBufferSize: 8192, @@ -227,6 +248,12 @@ func (s *Server) consoleWebSocket(w http.ResponseWriter, r *http.Request) { return } + release, ok := acquireSessionSlot(w) + if !ok { + return + } + defer release() + host, port, verifyTLS, err := s.connectionHost(r.Context(), sess.connectionID) if err != nil { http.Error(w, "connection lookup failed", http.StatusBadGateway) diff --git a/internal/api/ssh_console.go b/internal/api/ssh_console.go index cfa56f6..884acdb 100644 --- a/internal/api/ssh_console.go +++ b/internal/api/ssh_console.go @@ -2,9 +2,7 @@ package api import ( "context" - "database/sql" "encoding/json" - "errors" "fmt" "log/slog" "net" @@ -64,19 +62,33 @@ const sshSessionTTL = 60 * time.Second // verifySSHHostKey implements trust-on-first-use host-key pinning against // the ssh_known_hosts table: the first key seen for a host:port is stored // and accepted, and every later connection must present that exact key. -func (s *Server) verifySSHHostKey(ctx context.Context, host string, port int) ssh.HostKeyCallback { +func (s *Server) verifySSHHostKey(host string, port int) ssh.HostKeyCallback { return func(_ string, _ net.Addr, key ssh.PublicKey) error { got := ssh.FingerprintSHA256(key) - var pinned string - err := s.db.QueryRowContext(ctx, `SELECT fingerprint FROM ssh_known_hosts WHERE host = ? AND port = ?`, host, port).Scan(&pinned) - if errors.Is(err, sql.ErrNoRows) { - if _, insErr := s.db.ExecContext(ctx, `INSERT INTO ssh_known_hosts (host, port, fingerprint, created_at) VALUES (?, ?, ?, ?)`, - host, port, got, time.Now().UTC().Format(time.RFC3339)); insErr != nil { - slog.Warn("ssh host key pin failed", "host", host, "port", port, "error", insErr) - } - return nil + // A background context, not the dial's request context: this pin + // check/write is a durability concern independent of the request + // lifecycle — a browser tab closing mid-handshake must not turn + // into a bogus "checking pinned host key" failure via a context + // that was canceled for an unrelated reason. + ctx := context.Background() + + // Atomic claim-then-read: INSERT ... ON CONFLICT DO NOTHING lets at + // most one of two concurrent first-connections to the same + // host:port actually write the pin, and the SELECT right after + // always reads back whichever fingerprint won — so both goroutines + // compare against the same authoritative row instead of each one + // silently trusting whatever key it happened to see first. + if _, err := s.db.ExecContext(ctx, ` + INSERT INTO ssh_known_hosts (host, port, fingerprint, created_at) VALUES (?, ?, ?, ?) + ON CONFLICT (host, port) DO NOTHING`, + host, port, got, time.Now().UTC().Format(time.RFC3339)); err != nil { + // Fail closed: a security pin we couldn't durably record must + // not silently degrade back to "accept anything" — that defeats + // the whole point of this check. + return fmt.Errorf("pinning host key: %w", err) } - if err != nil { + var pinned string + if err := s.db.QueryRowContext(ctx, `SELECT fingerprint FROM ssh_known_hosts WHERE host = ? AND port = ?`, host, port).Scan(&pinned); err != nil { return fmt.Errorf("checking pinned host key: %w", err) } if pinned != got { @@ -210,6 +222,12 @@ func (s *Server) sshWebSocket(w http.ResponseWriter, r *http.Request) { return } + release, ok := acquireSessionSlot(w) + if !ok { + return + } + defer release() + clientConn, err := upgrader.Upgrade(w, r, nil) if err != nil { return @@ -233,7 +251,7 @@ func (s *Server) sshWebSocket(w http.ResponseWriter, r *http.Request) { // rejected instead of silently trusted. The first connection itself // is unverifiable without an out-of-band fingerprint — same as any // classic known_hosts flow the first time you connect to a host. - HostKeyCallback: s.verifySSHHostKey(r.Context(), sess.host, sess.port), + HostKeyCallback: s.verifySSHHostKey(sess.host, sess.port), } sshConn, err := ssh.Dial("tcp", addr, config) if err != nil { diff --git a/internal/digest/digest.go b/internal/digest/digest.go index 922c35f..0c0c9f4 100644 --- a/internal/digest/digest.go +++ b/internal/digest/digest.go @@ -128,7 +128,7 @@ func Build(ctx context.Context, db *store.DB, conns *connections.Resolver) (Flee summary.RunningGuests += cs.RunningGuests summary.BackupTotal += cs.BackupTotal summary.BackupOK += cs.BackupOK - if cs.Nodes > 0 { + if cs.OnlineNodes > 0 { cpuSum += cs.CPUPct memSum += cs.MemPct cpuN++ diff --git a/internal/notify/webhooks.go b/internal/notify/webhooks.go index 4141560..487b91d 100644 --- a/internal/notify/webhooks.go +++ b/internal/notify/webhooks.go @@ -146,8 +146,16 @@ func (d *WebhookDispatcher) Run(ctx context.Context, bus *events.Bus) { } } +// deliverToAll is called from Run's receive loop with that loop's own +// (cancellable-on-shutdown) ctx, but only passes it to dispatchOutboxRow's +// actual network delivery — a shutdown racing an incoming event must still +// be able to cut short a slow/retrying HTTP call. Loading subscriptions and +// enqueueing the outbox row use context.Background() instead: those are the +// durability-critical steps this dispatcher's whole design depends on (see +// Run's doc comment) — canceling either mid-shutdown would drop the event +// before the sweep ever gets a chance to pick it back up on restart. func (d *WebhookDispatcher) deliverToAll(ctx context.Context, evt events.Event) { - subs, err := d.matchingSubscriptions(ctx, evt.Type) + subs, err := d.matchingSubscriptions(context.Background(), evt.Type) if err != nil { slog.Error("webhook dispatcher: loading subscriptions failed", "error", err) return @@ -161,7 +169,7 @@ func (d *WebhookDispatcher) deliverToAll(ctx context.Context, evt events.Event) // Durable queue first: until this row exists the event isn't // promised to the subscription, and once it does a crash (or an // exhausted receiver) can't lose it — the sweep keeps trying. - if err := d.enqueueOutbox(ctx, sub.ID, evt, body); err != nil { + if err := d.enqueueOutbox(context.Background(), sub.ID, evt, body); err != nil { slog.Error("webhook dispatcher: queueing event failed", "subscriptionId", sub.ID, "eventId", evt.ID, "error", err) continue } diff --git a/internal/pbs/client.go b/internal/pbs/client.go index 6642ca4..729482a 100644 --- a/internal/pbs/client.go +++ b/internal/pbs/client.go @@ -29,6 +29,21 @@ const maxResponseBytes = 32 << 20 // 32 MiB // errors.Is(err, ErrUnauthorized). var ErrUnauthorized = errors.New("pbs: unauthorized") +// PBSResponseTooLargeError reports an upstream response body that exceeded +// maxResponseBytes. The body is deliberately truncated at the limit instead +// of exhausting memory, but surfaced as this explicit error so callers never +// see the confusing "unexpected end of JSON input" that json.Unmarshal +// produces on a half-read body. Same rationale as pve.ResponseTooLargeError. +type PBSResponseTooLargeError struct { + Method string + Path string + Limit int +} + +func (e *PBSResponseTooLargeError) Error() string { + return fmt.Sprintf("pbs %s %s response exceeded %d MiB limit", e.Method, e.Path, e.Limit>>20) +} + // StatusError is a non-2xx upstream PBS response. type StatusError struct { Method string @@ -352,10 +367,13 @@ func (c *Client) doOn(ctx context.Context, hc *http.Client, method, path string, } defer resp.Body.Close() - raw, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBytes)) + raw, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBytes+1)) if err != nil { return err } + if len(raw) > maxResponseBytes { + return &PBSResponseTooLargeError{Method: method, Path: path, Limit: maxResponseBytes} + } if resp.StatusCode >= 300 { err := &StatusError{Method: method, Path: path, StatusCode: resp.StatusCode, Body: string(raw)} if resp.StatusCode == http.StatusUnauthorized { diff --git a/internal/store/db.go b/internal/store/db.go index 79c0849..40617fb 100644 --- a/internal/store/db.go +++ b/internal/store/db.go @@ -24,16 +24,34 @@ func (d *DB) rebind(query string) string { if d.driver != "postgres" { return query } + return rebindPostgres(query) +} + +// rebindPostgres rewrites "?" placeholders to Postgres's "$1, $2, ..." style, +// skipping any "?" inside a single-quoted SQL string literal (tracking '' +// as an escaped quote, not a close) so a query with a literal "?" in a LIKE +// pattern or a Postgres JSONB "?"/"?|"/"?&" operator isn't corrupted into a +// bogus positional parameter — every query in this codebase currently only +// uses "?" as a placeholder, but this makes that an enforced invariant +// instead of a silent assumption the moment one doesn't. +func rebindPostgres(query string) string { var b strings.Builder n := 0 - for _, r := range query { - if r == '?' { + inString := false + runes := []rune(query) + for i := 0; i < len(runes); i++ { + r := runes[i] + switch { + case r == '\'': + inString = !inString + b.WriteRune(r) + case r == '?' && !inString: n++ b.WriteByte('$') b.WriteString(strconv.Itoa(n)) - continue + default: + b.WriteRune(r) } - b.WriteRune(r) } return b.String() } @@ -80,18 +98,7 @@ func (t *Tx) rebind(query string) string { if t.driver != "postgres" { return query } - var b strings.Builder - n := 0 - for _, r := range query { - if r == '?' { - n++ - b.WriteByte('$') - b.WriteString(strconv.Itoa(n)) - continue - } - b.WriteRune(r) - } - return b.String() + return rebindPostgres(query) } func (t *Tx) Exec(query string, args ...any) (sql.Result, error) { diff --git a/internal/store/db_test.go b/internal/store/db_test.go index e5fd2fc..c9f36bf 100644 --- a/internal/store/db_test.go +++ b/internal/store/db_test.go @@ -35,3 +35,15 @@ func TestTxRebindMatchesDBRebind(t *testing.T) { t.Fatalf("Tx.rebind(postgres) = %q, want %q", got, want) } } + +// A literal "?" inside a quoted string (a LIKE pattern, or Postgres's JSONB +// "?"/"?|"/"?&" operators) must not be treated as a placeholder — only the +// real one outside the string should be renumbered. +func TestRebindIgnoresPlaceholdersInsideStringLiterals(t *testing.T) { + db := &DB{driver: "postgres"} + got := db.rebind(`SELECT * FROM t WHERE name LIKE '%?%' AND id = ?`) + want := `SELECT * FROM t WHERE name LIKE '%?%' AND id = $1` + if got != want { + t.Fatalf("rebind(postgres) = %q, want %q", got, want) + } +} diff --git a/web/src/components/ai/Markdown.tsx b/web/src/components/ai/Markdown.tsx index eba2186..20b74b7 100644 --- a/web/src/components/ai/Markdown.tsx +++ b/web/src/components/ai/Markdown.tsx @@ -4,8 +4,9 @@ import ReactMarkdown from "react-markdown" import rehypeKatex from "rehype-katex" import remarkGfm from "remark-gfm" import remarkMath from "remark-math" -import { memo, useState } from "react" +import { memo } from "react" import { toast } from "sonner" +import { useCopiedFlag } from "@/lib/useCopiedFlag" import { cn } from "@/lib/utils" /** @@ -77,12 +78,11 @@ export const Markdown = memo(function Markdown({ text }: { text: string }) { }) function CodeBlock({ lang, code }: { lang: string; code: string }) { - const [copied, setCopied] = useState(false) + const [copied, flashCopied] = useCopiedFlag() function copy() { navigator.clipboard.writeText(code).then(() => { - setCopied(true) + flashCopied() toast.success("Copied to clipboard") - setTimeout(() => setCopied(false), 1500) }).catch(() => toast.error("Could not copy to clipboard")) } return ( diff --git a/web/src/components/charts/GaugeChart.tsx b/web/src/components/charts/GaugeChart.tsx index ca2b5fa..d699b25 100644 --- a/web/src/components/charts/GaugeChart.tsx +++ b/web/src/components/charts/GaugeChart.tsx @@ -1,3 +1,4 @@ +import { useId } from "react" import { cn } from "@/lib/utils" interface GaugeChartProps { @@ -23,7 +24,11 @@ function gaugeColor(value: number): string { export function GaugeChart({ value, label, size = 110 }: GaugeChartProps) { const clamped = Math.max(0, Math.min(100, value)) const color = gaugeColor(clamped) - const gradId = `gauge-${label.replace(/[^a-z0-9]/gi, "")}` + // useId, not just the label — two gauges sharing a label (e.g. two guests + // both showing "CPU") would otherwise emit duplicate s, + // and the browser renders both using whichever came first. + const autoId = useId() + const gradId = `gauge-${label.replace(/[^a-z0-9]/gi, "")}${autoId.replace(/[^a-z0-9]/gi, "")}` const stroke = 7 const r = (size - stroke) / 2 diff --git a/web/src/components/charts/Sparkline.tsx b/web/src/components/charts/Sparkline.tsx index 183b6e5..d2a606c 100644 --- a/web/src/components/charts/Sparkline.tsx +++ b/web/src/components/charts/Sparkline.tsx @@ -1,3 +1,4 @@ +import { useId } from "react" import { Area, AreaChart, Line, LineChart, ResponsiveContainer } from "recharts" interface SparklineProps { @@ -12,10 +13,15 @@ interface SparklineProps { /** Tiny inline trend chart without axes — gives KPI cards an at-a-glance * history without competing with the main charts for space. */ export function Sparkline({ data, color = "var(--chart-1)", height = 30, variant = "area" }: SparklineProps) { + // useId, not just the color — two sparklines sharing a color would + // otherwise emit duplicate s, and the browser renders + // both using whichever came first. Same fix as GaugeChart. Called + // unconditionally, before the early return below, per the Rules of Hooks. + const autoId = useId() const rows = data.map((v, i) => ({ i, v: typeof v === "number" && Number.isFinite(v) ? v : undefined })) if (rows.length < 2) return
const Chart = variant === "line" ? LineChart : AreaChart - const gradId = `spark-${color.replace(/[^a-z0-9]/gi, "")}` + const gradId = `spark-${color.replace(/[^a-z0-9]/gi, "")}${autoId.replace(/[^a-z0-9]/gi, "")}` return ( // Purely decorative trend cue — the KpiCard it lives in already states the // number and label in text, so screen readers should skip this entirely diff --git a/web/src/components/dashboard/widgets/ClusterActivityWidget.tsx b/web/src/components/dashboard/widgets/ClusterActivityWidget.tsx index 3ddd4e0..6397689 100644 --- a/web/src/components/dashboard/widgets/ClusterActivityWidget.tsx +++ b/web/src/components/dashboard/widgets/ClusterActivityWidget.tsx @@ -1,7 +1,7 @@ import { useQueries } from "@tanstack/react-query" import { StatusDot } from "@/components/ui/status-dot" import { Timestamp } from "@/components/ui/timestamp" -import { api, type ClusterLogEntry } from "@/lib/api" +import { api, ApiError, type ClusterLogEntry } from "@/lib/api" import type { WidgetSettings } from "@/lib/dashboardTypes" import { scopedConnection, useConnections } from "@/lib/fleet" import { WidgetError } from "@/components/dashboard/WidgetChrome" @@ -30,7 +30,15 @@ export function ClusterActivityWidget({ settings }: { settings: WidgetSettings } }) if (connError) return - if (targets.length > 0 && logQueries.every((q) => q.isError)) return + if (targets.length > 0 && logQueries.every((q) => q.isError)) { + // The generic "check your connection" copy is actively misleading here: + // every one of these connections is reachable (Cluster Comparison shows + // them online) — this endpoint specifically fails when the API + // token/user lacks Sys.Audit on "/", a distinct, fixable cause worth + // surfacing instead of hiding behind a vague network-sounding message. + const firstErr = logQueries.map((q) => q.error).find((e) => e instanceof ApiError) as ApiError | undefined + return + } const entries: FleetLogEntry[] = targets .flatMap((c, i) => (logQueries[i].data ?? []).map((e) => ({ ...e, connName: c.name }))) diff --git a/web/src/components/inventory/GuestDetailDialog.tsx b/web/src/components/inventory/GuestDetailDialog.tsx index 57f54f9..6d34fba 100644 --- a/web/src/components/inventory/GuestDetailDialog.tsx +++ b/web/src/components/inventory/GuestDetailDialog.tsx @@ -1,6 +1,6 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { Camera, Copy, HardDrive, Loader2, Lock, Network, Pencil, Snowflake, SquareTerminal, Sun, Terminal, Trash2, Workflow, X } from "lucide-react" -import { useEffect, useMemo, useState } from "react" +import { useEffect, useMemo, useRef, useState } from "react" import { toast } from "sonner" import { GaugeChart } from "@/components/charts/GaugeChart" import { ResourceAreaChart } from "@/components/charts/ResourceAreaChart" @@ -186,9 +186,18 @@ export function GuestDetailDialog({ connId, guest, onOpenChange }: GuestDetailDi enabled: open && execPid !== null, refetchInterval: (query) => (query.state.data?.exited ? false : 1000), }) + // Always the guest currently shown, read (not closed over) inside + // onSuccess below — an exec started for guest A that's still in flight + // when the dialog switches to guest B must not let its result land in + // execPid after the switch, or the dialog starts polling B's agent for a + // pid that belongs to A's process table. + const currentGuestIdRef = useRef(guest?.id) + currentGuestIdRef.current = guest?.id const execMutation = useMutation({ - mutationFn: () => api.post(`${base}/agent/exec`, { command: ["/bin/sh", "-c", execCommand] }), - onSuccess: (res) => setExecPid(res.pid), + mutationFn: (forGuestId: string | undefined) => api.post(`${base}/agent/exec`, { command: ["/bin/sh", "-c", execCommand] }).then((res) => ({ res, forGuestId })), + onSuccess: ({ res, forGuestId }) => { + if (forGuestId === currentGuestIdRef.current) setExecPid(res.pid) + }, onError: (err) => toast.error(err instanceof ApiError ? err.message : "Exec failed — is the guest agent running?"), }) @@ -1024,7 +1033,7 @@ export function GuestDetailDialog({ connId, guest, onOpenChange }: GuestDetailDi onChange={(e) => setExecCommand(e.target.value)} className="font-mono text-xs" /> -
diff --git a/web/src/components/profile/ApiKeysCard.tsx b/web/src/components/profile/ApiKeysCard.tsx index 3565ea9..45148cb 100644 --- a/web/src/components/profile/ApiKeysCard.tsx +++ b/web/src/components/profile/ApiKeysCard.tsx @@ -14,6 +14,7 @@ import { Label } from "@/components/ui/label" import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select" import { Skeleton } from "@/components/ui/skeleton" import { api, ApiError, type ApiKey, type CreatedApiKey } from "@/lib/api" +import { useCopiedFlag } from "@/lib/useCopiedFlag" import { formatRelativeTime } from "@/lib/utils" const EXPIRY_OPTIONS = [ @@ -39,7 +40,7 @@ export function ApiKeysCard() { const [scope, setScope] = useState<"api" | "mcp">("api") const [expiresInDays, setExpiresInDays] = useState("0") const [created, setCreated] = useState(null) - const [copied, setCopied] = useState(false) + const [copied, flashCopied] = useCopiedFlag(2000) const confirm = useConfirm() const query = useQuery({ @@ -90,9 +91,8 @@ export function ApiKeysCard() { function copyKey() { if (!created) return navigator.clipboard.writeText(created.key).then(() => { - setCopied(true) + flashCopied() toast.success("Copied to clipboard") - setTimeout(() => setCopied(false), 2000) }).catch(() => toast.error("Could not copy to clipboard")) } diff --git a/web/src/components/system/NodeSystemPanel.tsx b/web/src/components/system/NodeSystemPanel.tsx index 14791da..17beeed 100644 --- a/web/src/components/system/NodeSystemPanel.tsx +++ b/web/src/components/system/NodeSystemPanel.tsx @@ -1,6 +1,6 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { Play, RotateCw, ShieldCheck, Square, Trash2, Upload } from "lucide-react" -import { useEffect, useState } from "react" +import { useEffect, useRef, useState } from "react" import { toast } from "sonner" import { Badge } from "@/components/ui/badge" import { Button } from "@/components/ui/button" @@ -29,13 +29,25 @@ export function NodeSystemPanel({ connId, node }: NodeSystemPanelProps) { const base = `/connections/${connId}/nodes/${node}` const queryClient = useQueryClient() const confirm = useConfirm() + // Every query below polls/refetches-on-focus by default (main.tsx's + // global QueryClient config), and syncing form state from query data on + // every render of that data would silently overwrite whatever an admin is + // actively typing into these fields. Each form syncs from the server only + // once per node (tracked by this key) — an initial load, or switching to + // a different node — never again just because a background refetch + // returned a new object reference for the same node. + const nodeKey = `${connId}/${node}` // --- DNS --- const dnsQuery = useQuery({ queryKey: ["node-dns", connId, node], queryFn: () => api.get(`${base}/dns`) }) const [dnsForm, setDnsForm] = useState({}) + const dnsSyncedFor = useRef(null) useEffect(() => { - if (dnsQuery.data) setDnsForm(dnsQuery.data) - }, [dnsQuery.data]) + if (dnsQuery.data && dnsSyncedFor.current !== nodeKey) { + setDnsForm(dnsQuery.data) + dnsSyncedFor.current = nodeKey + } + }, [dnsQuery.data, nodeKey]) const saveDns = useMutation({ mutationFn: () => api.put(`${base}/dns`, dnsForm), onSuccess: () => { @@ -48,9 +60,13 @@ export function NodeSystemPanel({ connId, node }: NodeSystemPanelProps) { // --- Time --- const timeQuery = useQuery({ queryKey: ["node-time", connId, node], queryFn: () => api.get(`${base}/time`) }) const [timezone, setTimezone] = useState("") + const timeSyncedFor = useRef(null) useEffect(() => { - if (timeQuery.data) setTimezone(timeQuery.data.timezone) - }, [timeQuery.data]) + if (timeQuery.data && timeSyncedFor.current !== nodeKey) { + setTimezone(timeQuery.data.timezone) + timeSyncedFor.current = nodeKey + } + }, [timeQuery.data, nodeKey]) const saveTimezone = useMutation({ mutationFn: () => api.put(`${base}/time`, { timezone }), onSuccess: () => { @@ -63,9 +79,13 @@ export function NodeSystemPanel({ connId, node }: NodeSystemPanelProps) { // --- Hosts --- const hostsQuery = useQuery({ queryKey: ["node-hosts", connId, node], queryFn: () => api.get(`${base}/hosts`) }) const [hostsData, setHostsData] = useState("") + const hostsSyncedFor = useRef(null) useEffect(() => { - if (hostsQuery.data) setHostsData(hostsQuery.data.data) - }, [hostsQuery.data]) + if (hostsQuery.data && hostsSyncedFor.current !== nodeKey) { + setHostsData(hostsQuery.data.data) + hostsSyncedFor.current = nodeKey + } + }, [hostsQuery.data, nodeKey]) const saveHosts = useMutation({ mutationFn: () => api.put(`${base}/hosts`, { data: hostsData, digest: hostsQuery.data?.digest }), onSuccess: () => { diff --git a/web/src/components/ui/meter.tsx b/web/src/components/ui/meter.tsx index eef005f..8923a26 100644 --- a/web/src/components/ui/meter.tsx +++ b/web/src/components/ui/meter.tsx @@ -54,7 +54,11 @@ export function Meter({ return (
void] { + const [copied, setCopied] = useState(false) + const timeoutRef = useRef | null>(null) + + useEffect(() => () => { + if (timeoutRef.current !== null) clearTimeout(timeoutRef.current) + }, []) + + function flash() { + if (timeoutRef.current !== null) clearTimeout(timeoutRef.current) + setCopied(true) + timeoutRef.current = setTimeout(() => setCopied(false), ms) + } + + return [copied, flash] +} diff --git a/web/src/pages/AIAssistantPage.tsx b/web/src/pages/AIAssistantPage.tsx index 35c9e31..abc98f7 100644 --- a/web/src/pages/AIAssistantPage.tsx +++ b/web/src/pages/AIAssistantPage.tsx @@ -42,6 +42,7 @@ import { Skeleton } from "@/components/ui/skeleton" import { api, notifyUnauthorized, type ToolCallRecord, type UsableAIProvider } from "@/lib/api" import { useAuth } from "@/lib/auth" import { type Conversation, type DisplayMessage, type MessageUsage, type ReasoningEffort, type ToolCallEntry, useAIConversations } from "@/lib/useAIConversations" +import { useCopiedFlag } from "@/lib/useCopiedFlag" import { cn, formatRelativeTime } from "@/lib/utils" function newId() { @@ -49,6 +50,10 @@ function newId() { } interface ToolActivity { + // The provider's tool_call_id when the SSE envelope carried one — undefined + // for a provider/runtime that doesn't echo it back, in which case matching + // falls back to name+status the way it always has. + id?: string name: string status: "running" | "ok" | "error" args?: unknown @@ -89,8 +94,8 @@ function formatJSON(raw: string): string { function parseSSELine(line: string): { delta?: string reasoning?: string - toolCall?: { name: string; args?: unknown } - toolResult?: { name: string; ok: boolean; result?: string } + toolCall?: { id?: string; name: string; args?: unknown } + toolResult?: { id?: string; name: string; ok: boolean; result?: string } usage?: MessageUsage error?: string done?: boolean @@ -102,9 +107,9 @@ function parseSSELine(line: string): { const parsed = JSON.parse(payload) if (parsed.ferrum_error) return { error: parsed.ferrum_error } if (typeof parsed.ferrum_reasoning === "string") return { reasoning: parsed.ferrum_reasoning } - if (parsed.ferrum_tool_call) return { toolCall: { name: parsed.ferrum_tool_call.name, args: parsed.ferrum_tool_call.args } } + if (parsed.ferrum_tool_call) return { toolCall: { id: parsed.ferrum_tool_call.id, name: parsed.ferrum_tool_call.name, args: parsed.ferrum_tool_call.args } } if (parsed.ferrum_tool_result) - return { toolResult: { name: parsed.ferrum_tool_result.name, ok: !!parsed.ferrum_tool_result.ok, result: parsed.ferrum_tool_result.result } } + return { toolResult: { id: parsed.ferrum_tool_result.id, name: parsed.ferrum_tool_result.name, ok: !!parsed.ferrum_tool_result.ok, result: parsed.ferrum_tool_result.result } } if (parsed.ferrum_usage) return { usage: parsed.ferrum_usage } return { delta: parsed.choices?.[0]?.delta?.content ?? undefined } } catch { @@ -206,6 +211,12 @@ export function AIAssistantPage() { const [streamingReasoning, setStreamingReasoning] = useState(null) const [toolActivity, setToolActivity] = useState([]) const [streaming, setStreaming] = useState(false) + // Which conversation the in-flight completion belongs to — only one + // completion can run at a time app-wide (see send/regenerate's `streaming` + // guard), but switching conversations mid-stream must not let the live + // bubble (or its tokens, once they land) render under whatever + // conversation happens to be active when they arrive. + const [streamingConvId, setStreamingConvId] = useState(null) const [slashIndex, setSlashIndex] = useState(0) const [selectMode, setSelectMode] = useState(false) const [selectedIds, setSelectedIds] = useState>(new Set()) @@ -262,7 +273,11 @@ export function AIAssistantPage() { const reasoningEffort: ReasoningEffort = active?.reasoningEffort ?? pendingReasoningEffort const selectedModel = flatModels.find((m) => m.modelRowId === modelId) const committedMessages = active?.messages ?? [] - const displayMessages: (DisplayMessage & { streaming?: boolean })[] = streaming + // Gated on streamingConvId matching activeId, not just `streaming` — the + // completion itself is global (only one runs at a time), but its live + // bubble must only appear in the transcript it actually belongs to. + const isStreamingHere = streaming && streamingConvId === activeId + const displayMessages: (DisplayMessage & { streaming?: boolean })[] = isStreamingHere ? [...committedMessages, { id: "__streaming__", role: "assistant", content: streamingText ?? "", reasoning: streamingReasoning ?? undefined, streaming: true }] : committedMessages @@ -337,6 +352,7 @@ export function AIAssistantPage() { async function runCompletion(convId: string, history: DisplayMessage[], useModelId: string, useReasoningEffort: ReasoningEffort) { setStreaming(true) + setStreamingConvId(convId) setStreamingText("") setStreamingReasoning(null) setToolActivity([]) @@ -402,23 +418,32 @@ export function AIAssistantPage() { reasoningText += evt.reasoning scheduleFlush() } else if (evt.toolCall) { - const { name, args } = evt.toolCall - setToolActivity((prev) => [...prev, { name, args, status: "running" }]) - toolLog.push({ name, args, ok: true }) + const { id, name, args } = evt.toolCall + setToolActivity((prev) => [...prev, { id, name, args, status: "running" }]) + toolLog.push({ id, name, args, ok: true }) } else if (evt.toolResult) { - const { name, ok, result } = evt.toolResult + const { id, name, ok, result } = evt.toolResult + // Prefer matching by the provider's own tool_call_id — falling + // back to "most recent running call with this name" only when + // the provider didn't echo one back. Name-only matching can + // attach a result to the wrong pill when the model calls the + // same tool twice in parallel before either result arrives. setToolActivity((prev) => { - const idx = [...prev].reverse().findIndex((t) => t.name === name && t.status === "running") + const idx = id + ? prev.findIndex((t) => t.id === id) + : [...prev].reverse().findIndex((t) => t.name === name && t.status === "running") if (idx === -1) return prev - const realIdx = prev.length - 1 - idx const next = [...prev] - next[realIdx] = { ...next[realIdx], status: ok ? "ok" : "error", result } + next[idx] = { ...next[idx], status: ok ? "ok" : "error", result } return next }) - const logIdx = [...toolLog].reverse().findIndex((t) => t.name === name && t.result === undefined) + let logIdx = id ? toolLog.findIndex((t) => t.id === id) : -1 + if (logIdx === -1 && !id) { + const fromEnd = [...toolLog].reverse().findIndex((t) => t.name === name && t.result === undefined) + logIdx = fromEnd === -1 ? -1 : toolLog.length - 1 - fromEnd + } if (logIdx !== -1) { - const realIdx = toolLog.length - 1 - logIdx - toolLog[realIdx] = { ...toolLog[realIdx], ok, result } + toolLog[logIdx] = { ...toolLog[logIdx], ok, result } } } else if (evt.usage) { usage = evt.usage @@ -445,6 +470,7 @@ export function AIAssistantPage() { flushRafRef.current = null } setStreaming(false) + setStreamingConvId(null) setStreamingText(null) setStreamingReasoning(null) setToolActivity([]) @@ -1215,7 +1241,7 @@ function ThinkingDots() { } function CopyMessageButton({ content, onCopy }: { content: string; onCopy: (content: string) => void }) { - const [copied, setCopied] = useState(false) + const [copied, flashCopied] = useCopiedFlag() return (
@@ -334,7 +339,9 @@ export function BackupsPage() { value={scheduleForm.bwlimit} onChange={(e) => setScheduleForm({ ...scheduleForm, bwlimit: e.target.value })} placeholder="unlimited" + aria-invalid={bwlimitInvalid} /> + {bwlimitInvalid &&

Must be a number

}
@@ -344,7 +351,9 @@ export function BackupsPage() { value={scheduleForm.pigz} onChange={(e) => setScheduleForm({ ...scheduleForm, pigz: e.target.value })} placeholder="off" + aria-invalid={pigzInvalid} /> + {pigzInvalid &&

Must be a number

}
)} @@ -353,7 +362,7 @@ export function BackupsPage() { className="mt-3" size="sm" loading={createJob.isPending} - disabled={!scheduleConnId || !scheduleForm.storage || scheduleLooksInvalid(scheduleForm.schedule)} + disabled={!scheduleConnId || !scheduleForm.storage || scheduleLooksInvalid(scheduleForm.schedule) || pruneInvalid || bwlimitInvalid || pigzInvalid} onClick={() => createJob.mutate()} > {!createJob.isPending && } Schedule job diff --git a/web/src/pages/ClusterPage.tsx b/web/src/pages/ClusterPage.tsx index e5fbabb..6969115 100644 --- a/web/src/pages/ClusterPage.tsx +++ b/web/src/pages/ClusterPage.tsx @@ -58,7 +58,11 @@ export function ClusterPage() { }) const connections = inventory ?? [] const [connId, setConnId] = useState("") - const activeConnId = connId || connections[0]?.connectionId || "" + // Fall back to the first connection when connId points at one that's no + // longer in the list (deleted elsewhere, dropped from inventory) — same + // fallback as the no-selection case, so panels below don't keep querying + // a connection that no longer exists. + const activeConnId = (connId && connections.some((c) => c.connectionId === connId) ? connId : connections[0]?.connectionId) || "" const base = activeConnId ? `/connections/${activeConnId}` : "" return ( @@ -157,6 +161,7 @@ function SDNPanel({ base, connId }: { base: string; connId: string }) { }) const [vnetForm, setVnetForm] = useState({ vnet: "", zone: "", tag: "" }) + const vnetTagInvalid = vnetForm.tag !== "" && !Number.isFinite(Number(vnetForm.tag)) const createVnet = useMutation({ mutationFn: () => api.post(`${base}/cluster/sdn/vnets`, { vnet: vnetForm.vnet, zone: vnetForm.zone, tag: vnetForm.tag ? Number(vnetForm.tag) : undefined }), onSuccess: () => { @@ -168,8 +173,12 @@ function SDNPanel({ base, connId }: { base: string; connId: string }) { }) const deleteVnet = useMutation({ mutationFn: (vnet: string) => api.delete(`${base}/cluster/sdn/vnets/${encodeURIComponent(vnet)}`), - onSuccess: () => { + onSuccess: (_data, vnet) => { toast.success("Vnet deleted") + // The deleted vnet may have been the one selected for the Subnets + // card below — clear it so that card closes instead of continuing to + // query subnets for a vnet that no longer exists. + if (vnet === selectedVnet) setSelectedVnet(null) invalidate() }, onError: (err) => toast.error(err instanceof ApiError ? err.message : "Failed to delete vnet"), @@ -397,9 +406,10 @@ function SDNPanel({ base, connId }: { base: string; connId: string }) {
- setVnetForm((f) => ({ ...f, tag: e.target.value }))} className="w-24" /> + setVnetForm((f) => ({ ...f, tag: e.target.value }))} className="w-24" aria-invalid={vnetTagInvalid} /> + {vnetTagInvalid &&

Must be a number

}
- diff --git a/web/src/pages/DashboardPage.tsx b/web/src/pages/DashboardPage.tsx index 8b922dc..8031ac5 100644 --- a/web/src/pages/DashboardPage.tsx +++ b/web/src/pages/DashboardPage.tsx @@ -188,10 +188,17 @@ export function DashboardPage() { if (!targetId) return dirty.current = false api.put(`/dashboards/${targetId}`, { version: LAYOUT_VERSION, widgets: next }).catch((err: unknown) => { - // Save failed — the edit is still only local, so put the dirty flag - // back or it's lost for good on next unload/switch with no retry. - dirty.current = true - toast.error(err instanceof ApiError ? err.message : "Failed to save dashboard layout — your changes are not saved yet.") + // Only re-arm the dirty flag if targetId is still the dashboard on + // screen — switchDashboard's flush-on-leave calls this for the + // dashboard being LEFT, and by the time a failure lands here + // activeId may already point at a freshly-loaded, genuinely-clean + // dashboard; marking that one dirty would trigger a false "unsaved + // changes" warning while doing nothing for the edit that actually + // failed (which has no retry path once its own widgets are gone + // from state — the toast is the only signal left for that case). + if (targetId === activeId) dirty.current = true + const msg = err instanceof ApiError ? err.message : "Failed to save dashboard layout" + toast.error(targetId === activeId ? `${msg} — your changes are not saved yet.` : `${msg} — changes to the dashboard you left were not saved.`) }) } @@ -406,7 +413,15 @@ export function DashboardPage() { {/* Menu scrolls instead of overflowing the viewport when the widget list is taller than the screen. */} - Available widgets + {/* DropdownMenuContent has its own p-1 padding, so a plain + "sticky top-0" label sits inset from the real top edge — + scrolled items show through that inset strip. Pull the + label out to the container's true edges with negative + margins and re-add the padding itself, plus a border to + seal the boundary against the list scrolling under it. */} + + Available widgets + {addableTypes.map((a) => a.fleetFree ? ( addWidget(a.type)}> diff --git a/web/src/pages/PBSPage.tsx b/web/src/pages/PBSPage.tsx index c46addc..f1a5f4c 100644 --- a/web/src/pages/PBSPage.tsx +++ b/web/src/pages/PBSPage.tsx @@ -150,6 +150,13 @@ function DatastoreCard({ connId, ds, isAdmin }: { connId: string; ds: PBSDatasto const used = ds.used ?? 0 const totals = countTotals(ds.counts) + // Lifted out of MaintenanceTab: Radix TabsContent unmounts inactive panels + // by default, so state local to that component would lose a running GC + // job's UPID whenever the user switched away from Maintenance and back. + // DatastoreCard stays mounted for as long as the tabs it hosts do. + const [gcUpid, setGcUpid] = useState(null) + const [logUpid, setLogUpid] = useState(null) + return ( @@ -192,7 +199,16 @@ function DatastoreCard({ connId, ds, isAdmin }: { connId: string; ds: PBSDatasto - + @@ -377,12 +393,29 @@ function GCStat({ label, value }: { label: string; value: string }) { ) } -function MaintenanceTab({ connId, store, gcStatus, isAdmin }: { connId: string; store: string; gcStatus?: PBSGCStatus; isAdmin: boolean }) { - const queryClient = useQueryClient() +function MaintenanceTab({ + connId, + store, + gcStatus, + isAdmin, + gcUpid, + setGcUpid, + logUpid, + setLogUpid, +}: { + connId: string + store: string + gcStatus?: PBSGCStatus + isAdmin: boolean // The UPID returned by the GC start call — while set, its task status is // polled every 3s, but only for as long as the task reports "running". - const [gcUpid, setGcUpid] = useState(null) - const [logUpid, setLogUpid] = useState(null) + // Lifted into DatastoreCard so it survives this tab unmounting/remounting. + gcUpid: string | null + setGcUpid: (upid: string | null) => void + logUpid: string | null + setLogUpid: (upid: string | null) => void +}) { + const queryClient = useQueryClient() const taskQuery = useQuery({ queryKey: ["pbs-task", connId, gcUpid],