From 48a621a5669c7524eb309cf61b11544f13ae9d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Sun, 7 Jun 2026 12:38:28 +0100 Subject: [PATCH] fix(utility): surface inference-agent probe status + add refresh endpoint (#1287) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(utility): surface probe status in inference-agents endpoint GET /api/v1/utility/inference-agents previously filtered out agents whose host-utility probe didn't reach StatusOK, so a Claude entry with auth or install issues vanished from the response. The frontend then defaulted agent_id to "claude-acp", failed to match anything in the list, and silently rendered an empty Model picker with no explanation. Include every registered ACP-capable agent in the response with a new `status` field (and sanitized `status_message`) so the UI can render an inline note + Refresh button. Add a companion POST /api/v1/utility/inference-agents/:id/refresh endpoint backed by the existing hostutility.Manager.Refresh so users can re-probe after fixing auth or installing the CLI without restarting kandev. Credential-looking substrings ("api_key=...", "token=...") in probe errors are stripped before reaching the wire. Refs #1269, #1263. * fix(web): show probe status note + Refresh on utility-agent model picker Previously the Model picker rendered empty and disabled whenever the selected inference agent was missing from /inference-agents (probe hadn't reached StatusOK) or advertised no models, with no way for the user to understand or recover. Wire the new `status` field through the API client and render an inline note ("Sign in to Claude to load models.", "Claude CLI is not installed on this machine.", "Setting up Claude…") with a Refresh button that calls the new POST /inference-agents/:id/refresh endpoint and merges the updated agent back into local state — no full refetch, so a slow probe on a different agent doesn't block the UI. Both call sites — the create/edit dialog and the Default Model row in the settings page — render the note. Component-level unit tests cover the status-to-copy mapping, fallback ("no longer available") copy, and the in-flight Refresh button disabled state. The e2e test asserts the auth_required → click Refresh → status:ok state machine end to end. Refs #1269. * test(utility): add HTTP integration tests for inference-agents endpoints End-to-end coverage of the new /api/v1/utility/inference-agents and /api/v1/utility/inference-agents/:id/refresh routes against a real gin router with a stateful in-memory host stub. - ProbeStateMachine: GET (auth_required) → POST refresh → GET (ok+models) - RefreshUnknownAgent: 404 without invoking the host probe - RefreshPathFuzz: traversal, null byte, 512-char id — no 5xx, no probe - ConcurrentRefresh: 50 parallel POSTs under -race, no panic - StatusMessageSanitizationOnTheWire: 110KB payload + unicode probe errors — no secret material reaches the wire, unicode prose preserved Caught the gap unit tests had: the multi-request GET→POST→GET sequence on a shared cache, and the wire format (Content-Type, JSON shape) through the real router. * fix(utility): tighten credential regex + drop Go 1.22 loop-var shadow Address two inline review comments from claude bot on PR #1287: - Drop `ia := ia` shadow in httpRefreshInferenceAgent — pre-Go 1.22 pointer-capture workaround, dead weight under Go 1.24+ (per apps/backend/AGENTS.md). - Split the credential sanitizer into two patterns so prose without a real `=`/`:` separator is no longer mangled. The single regex previously turned "access token was revoked" into "access token= revoked" by consuming the next word as the value. New shape: - credentialKVPattern — requires `[:=]` (api_key, token, secret, password) - credentialBearerPattern — allows bare whitespace, anchored to "bearer" only Existing redaction cases still pass; two new prose cases guard the regression. * fix(utility): address cubic + greptile review findings on PR #1287 Round-2 review feedback: - P0 (cubic): postEmpty called `t.Fatalf` from `go func()` in TestIntegration_ConcurrentRefresh. Go panics on t.Fatalf from non-test goroutines. Split into `postEmptyE` (error-returning, goroutine-safe) and `postEmpty` (main-goroutine wrapper). Concurrent test now forwards the error through the existing `errs` channel. - P1 (cubic + greptile): tightened the credential sanitizer regex. * `api[ _-]?key` now also matches a literal space ("invalid api key=…") * separators are restricted to `[^\S\n]` so multi-line stderr like "invalid token\ncaused by network" no longer eats words across lines Test coverage added for both regressions (and for tab as a valid horizontal-whitespace separator). - P2 (cubic): refresh-failure log now feeds `err.Error()` through sanitizeStatusMessage before zapping it — same wire-side guard, but applied to server logs too. Closes the "credentials in journalctl" hole. - P2 (cubic): InferenceAgent.status is now optional in the TS type and the status note treats an undefined status with `models.length > 0` as healthy, so older API responses don't flash a spurious warning. Deferred (acknowledged in PR replies, not changed here): - cubic P2 useInferenceAgents drift — the hook is currently called in both the dialog and the parent section. Drift is bounded by the dialog's own `onSuccess` refetch and only visible inside an open dialog. Lifting state to the parent and prop-drilling is the right fix but touches multiple files; out of scope for this review round. * docs(utility): fix stale comments flagged by claude review on PR #1287 Two doc-only items, no code change: - handlers_integration_test.go: TestIntegration_ConcurrentRefresh comment said "we don't assert on call count" but the test DOES assert `host.refreshCalls != N` two lines later. Updated to match reality (singleflight lives in the real Manager, the stub has none → every call must reach the stub). - utility-agents.spec.ts: the ACP-probe E2E said "backend filters out non-StatusOK agents", which is the exact behavior this PR removes. Updated to explain the real reason only Mock appears (KANDEV_MOCK_AGENT=only registers only `mock-agent`). --------- Co-authored-by: Kandev Agent --- apps/backend/internal/utility/dto/dto.go | 15 +- .../internal/utility/handlers/handlers.go | 135 +++++-- .../handlers/handlers_integration_test.go | 335 ++++++++++++++++++ .../utility/handlers/handlers_test.go | 332 ++++++++++++++--- .../settings/inference-agent-status.test.tsx | 124 +++++++ .../settings/inference-agent-status.tsx | 110 ++++++ .../settings/use-inference-agents.ts | 40 +++ .../settings/utility-agent-dialog.tsx | 80 +++-- .../settings/utility-agents-section.tsx | 6 +- .../components/settings/utility-sections.tsx | 10 + .../e2e/tests/settings/utility-agents.spec.ts | 89 ++++- apps/web/lib/api/domains/utility-api.ts | 36 ++ 12 files changed, 1197 insertions(+), 115 deletions(-) create mode 100644 apps/backend/internal/utility/handlers/handlers_integration_test.go create mode 100644 apps/web/components/settings/inference-agent-status.test.tsx create mode 100644 apps/web/components/settings/inference-agent-status.tsx create mode 100644 apps/web/components/settings/use-inference-agents.ts diff --git a/apps/backend/internal/utility/dto/dto.go b/apps/backend/internal/utility/dto/dto.go index 427bc29ee3..bd501e5c16 100644 --- a/apps/backend/internal/utility/dto/dto.go +++ b/apps/backend/internal/utility/dto/dto.go @@ -170,11 +170,18 @@ type InferenceModelDTO struct { } // InferenceAgentDTO represents an agent that supports inference. +// +// Status reflects the host-utility probe outcome at the time of the request. +// The frontend uses it to render a contextual note ("sign in to Claude", +// "Claude CLI not installed", "setting up Claude…") and a Refresh button when +// Models is empty, instead of showing a silently-disabled Model picker. type InferenceAgentDTO struct { - ID string `json:"id"` - Name string `json:"name"` - DisplayName string `json:"display_name"` - Models []InferenceModelDTO `json:"models"` + ID string `json:"id"` + Name string `json:"name"` + DisplayName string `json:"display_name"` + Models []InferenceModelDTO `json:"models"` + Status string `json:"status"` + StatusMessage string `json:"status_message,omitempty"` } // InferenceAgentsResponse is the response for listing inference agents. diff --git a/apps/backend/internal/utility/handlers/handlers.go b/apps/backend/internal/utility/handlers/handlers.go index ec05167869..24051277b8 100644 --- a/apps/backend/internal/utility/handlers/handlers.go +++ b/apps/backend/internal/utility/handlers/handlers.go @@ -4,6 +4,7 @@ import ( "context" "errors" "net/http" + "regexp" "strconv" "github.com/gin-gonic/gin" @@ -32,6 +33,7 @@ type InferenceExecutor interface { type HostUtilityExecutor interface { ExecutePrompt(ctx context.Context, agentType, model, mode, prompt string) (*hostutility.PromptResult, error) Get(agentType string) (hostutility.AgentCapabilities, bool) + Refresh(ctx context.Context, agentType string) (hostutility.AgentCapabilities, error) } // UserSettingsProvider provides user settings for default utility agent/model. @@ -73,6 +75,7 @@ func RegisterRoutes(router *gin.Engine, ctrl *controller.Controller, executor In api.POST("/execute", handlers.httpExecutePrompt) api.GET("/agents/:id/calls", handlers.httpListCalls) api.GET("/inference-agents", handlers.httpListInferenceAgents) + api.POST("/inference-agents/:id/refresh", handlers.httpRefreshInferenceAgent) } func (h *Handlers) httpListAgents(c *gin.Context) { @@ -313,14 +316,15 @@ func (h *Handlers) httpListInferenceAgents(c *gin.Context) { inferenceAgents := h.executor.ListInferenceAgentsWithContext(c.Request.Context()) // Build the response from the host utility capability cache (boot-time - // ACP probe). Only include agents whose probe reached StatusOK — an - // agent that isn't authenticated, not installed, or still probing - // can't actually run a utility prompt, so showing it in the picker - // just leads the user into a dead end. + // ACP probe). Every registered ACP-capable agent is included, even when + // its probe didn't reach StatusOK — the frontend uses the per-agent + // `status` field to render an inline note ("sign in to Claude", + // "setting up Claude…", "Claude CLI not installed") and a Refresh + // button instead of silently empty Model picker. // // hostExecutor is an optional dependency (see executeSessionless); - // without it we can't check health or surface models, so the list is - // empty by design rather than showing unusable options. + // without it we can't check probe state at all, so the list is empty + // by design rather than showing unusable options. result := make([]dto.InferenceAgentDTO, 0, len(inferenceAgents)) if h.hostExecutor == nil { c.JSON(http.StatusOK, dto.InferenceAgentsResponse{Agents: result}) @@ -331,25 +335,106 @@ func (h *Handlers) httpListInferenceAgents(c *gin.Context) { // ag.Name() — built-in ACP agents return distinct strings for the // two (e.g. "claude-acp" vs "Claude ACP Agent"). caps, ok := h.hostExecutor.Get(ia.ID) - if !ok || caps.Status != hostutility.StatusOK { - continue - } - models := make([]dto.InferenceModelDTO, 0, len(caps.Models)) - for _, m := range caps.Models { - models = append(models, dto.InferenceModelDTO{ - ID: m.ID, - Name: m.Name, - Description: m.Description, - IsDefault: m.ID == caps.CurrentModelID, - Meta: m.Meta, - }) - } - result = append(result, dto.InferenceAgentDTO{ - ID: ia.ID, - Name: ia.Name, - DisplayName: ia.DisplayName, - Models: models, - }) + result = append(result, inferenceAgentDTOFromCaps(ia, caps, ok)) } c.JSON(http.StatusOK, dto.InferenceAgentsResponse{Agents: result}) } + +// httpRefreshInferenceAgent re-probes an agent type and returns the updated +// capabilities. Used by the settings page Refresh button so the user can +// recover from a transient probe failure (sign-in race, agent not yet +// installed at boot, network blip) without restarting kandev. +func (h *Handlers) httpRefreshInferenceAgent(c *gin.Context) { + if h.hostExecutor == nil { + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "host utility not configured"}) + return + } + agentID := c.Param("id") + ctx := c.Request.Context() + + // Find the registered InferenceAgentInfo for this id so the DTO carries + // the same name/display_name as the list endpoint. Refusing unknown ids + // avoids spinning up probes for typos / arbitrary input. + var info *lifecycle.InferenceAgentInfo + for _, ia := range h.executor.ListInferenceAgentsWithContext(ctx) { + if ia.ID == agentID { + info = &ia + break + } + } + if info == nil { + c.JSON(http.StatusNotFound, gin.H{"error": "agent not found"}) + return + } + + caps, err := h.hostExecutor.Refresh(ctx, agentID) + if err != nil { + // Refresh already records the failure in the cache (status + error) + // via Manager.Refresh, so we surface the same shape as a GET would + // rather than a generic 500 — the UI can re-render the inline note + // with the latest probe error instead of going blank. + h.logger.Warn("failed to refresh inference agent", + zap.String("error", sanitizeStatusMessage(err.Error())), + zap.String("agent_id", agentID)) + latest, ok := h.hostExecutor.Get(agentID) + c.JSON(http.StatusOK, inferenceAgentDTOFromCaps(*info, latest, ok)) + return + } + c.JSON(http.StatusOK, inferenceAgentDTOFromCaps(*info, caps, true)) +} + +// inferenceAgentDTOFromCaps maps a (registry, cache) pair into the wire DTO. +// hasCaps=false (cache miss) is reported as "probing" — the cache only misses +// before the boot-time probe lands, which is the same state the UI surfaces +// during a refresh. +func inferenceAgentDTOFromCaps(ia lifecycle.InferenceAgentInfo, caps hostutility.AgentCapabilities, hasCaps bool) dto.InferenceAgentDTO { + models := make([]dto.InferenceModelDTO, 0, len(caps.Models)) + for _, m := range caps.Models { + models = append(models, dto.InferenceModelDTO{ + ID: m.ID, + Name: m.Name, + Description: m.Description, + IsDefault: m.ID == caps.CurrentModelID, + Meta: m.Meta, + }) + } + status := string(caps.Status) + if !hasCaps || status == "" { + status = string(hostutility.StatusProbing) + } + return dto.InferenceAgentDTO{ + ID: ia.ID, + Name: ia.Name, + DisplayName: ia.DisplayName, + Models: models, + Status: status, + StatusMessage: sanitizeStatusMessage(caps.Error), + } +} + +// sanitizeStatusMessage strips obvious credential-looking substrings from a +// probe error before exposing it on the wire. ACP probe errors usually carry +// the upstream CLI's stderr verbatim; a misconfigured key can end up echoed +// back ("...invalid api key=sk-..."). Belt-and-braces — the frontend never +// shows the raw value, but the response is also reachable via /api/v1. +// +// Split into two patterns so prose like "access token was revoked" is left +// alone: kw=val / kw:val requires a real separator (claude bot review), and +// the separator is restricted to horizontal whitespace so a stderr line like +// "invalid token\ncaused by network" does not eat words across lines +// (greptile review). "api key" with a literal space is matched alongside +// api_key / api-key (cubic review). The bare-space "bearer " form gets +// its own anchored rule so the kv-only matcher can stay strict. +var ( + credentialKVPattern = regexp.MustCompile(`(?i)((?:api[ _-]?key|token|secret|password))[^\S\n]*[:=][^\S\n]*\S+`) + credentialBearerPattern = regexp.MustCompile(`(?i)(bearer)[^\S\n]+\S+`) +) + +func sanitizeStatusMessage(msg string) string { + if msg == "" { + return "" + } + msg = credentialKVPattern.ReplaceAllString(msg, "${1}=") + msg = credentialBearerPattern.ReplaceAllString(msg, "${1}=") + return msg +} diff --git a/apps/backend/internal/utility/handlers/handlers_integration_test.go b/apps/backend/internal/utility/handlers/handlers_integration_test.go new file mode 100644 index 0000000000..26432a2aa7 --- /dev/null +++ b/apps/backend/internal/utility/handlers/handlers_integration_test.go @@ -0,0 +1,335 @@ +package handlers + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/agent/hostutility" + "github.com/kandev/kandev/internal/agent/runtime/lifecycle" + "github.com/kandev/kandev/internal/common/logger" +) + +// statefulHostStub is a goroutine-safe stub that models the real probe +// cache more faithfully than the basic stubHostUtility used elsewhere: +// Get/Refresh share the same map, Refresh atomically swaps state into +// the cache, and concurrent calls are serialized through a mutex (the +// real Manager.Refresh uses singleflight, which is even stricter). +type statefulHostStub struct { + mu sync.Mutex + caps map[string]hostutility.AgentCapabilities + nextRefresh map[string]hostutility.AgentCapabilities + refreshCalls int +} + +func newStatefulHostStub() *statefulHostStub { + return &statefulHostStub{ + caps: map[string]hostutility.AgentCapabilities{}, + nextRefresh: map[string]hostutility.AgentCapabilities{}, + } +} + +func (h *statefulHostStub) ExecutePrompt(_ context.Context, _, _, _, _ string) (*hostutility.PromptResult, error) { + return nil, nil +} + +func (h *statefulHostStub) Get(agentType string) (hostutility.AgentCapabilities, bool) { + h.mu.Lock() + defer h.mu.Unlock() + c, ok := h.caps[agentType] + return c, ok +} + +func (h *statefulHostStub) Refresh(_ context.Context, agentType string) (hostutility.AgentCapabilities, error) { + h.mu.Lock() + defer h.mu.Unlock() + h.refreshCalls++ + next, ok := h.nextRefresh[agentType] + if !ok { + next = hostutility.AgentCapabilities{AgentType: agentType, Status: hostutility.StatusOK} + } + h.caps[agentType] = next + return next, nil +} + +func newIntegrationServer(t *testing.T, agents []lifecycle.InferenceAgentInfo, host *statefulHostStub) *httptest.Server { + t.Helper() + gin.SetMode(gin.TestMode) + r := gin.New() + log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) + + // Register only the two routes under test directly. RegisterRoutes + // requires a real Controller (full service stack); for integration + // against the inference-agents surface we only need these two. + h := &Handlers{ + executor: &stubInferenceExecutor{agents: agents}, + hostExecutor: host, + logger: log, + } + r.GET("/api/v1/utility/inference-agents", h.httpListInferenceAgents) + r.POST("/api/v1/utility/inference-agents/:id/refresh", h.httpRefreshInferenceAgent) + + return httptest.NewServer(r) +} + +func getJSON(t *testing.T, url string) (int, http.Header, map[string]any) { + t.Helper() + resp, err := http.Get(url) //nolint:noctx + if err != nil { + t.Fatalf("GET %s: %v", url, err) + } + defer func() { _ = resp.Body.Close() }() + body, _ := io.ReadAll(resp.Body) + var out map[string]any + if len(body) > 0 { + if err := json.Unmarshal(body, &out); err != nil { + t.Fatalf("GET %s: invalid JSON: %v; body=%s", url, err, body) + } + } + return resp.StatusCode, resp.Header, out +} + +// postEmptyE issues a POST with an empty body. Returns an error rather +// than calling t.Fatalf so it is safe to invoke from goroutines spawned +// by TestIntegration_ConcurrentRefresh — t.Fatalf is only legal from the +// test's main goroutine and panics elsewhere on Go 1.21+ (cubic review). +func postEmptyE(url string) (int, map[string]any, error) { + req, _ := http.NewRequest(http.MethodPost, url, nil) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return 0, nil, fmt.Errorf("POST %s: %w", url, err) + } + defer func() { _ = resp.Body.Close() }() + body, _ := io.ReadAll(resp.Body) + var out map[string]any + _ = json.Unmarshal(body, &out) + return resp.StatusCode, out, nil +} + +// postEmpty is the main-goroutine convenience wrapper. Concurrent callers +// must use postEmptyE. +func postEmpty(t *testing.T, url string) (int, map[string]any) { + t.Helper() + code, body, err := postEmptyE(url) + if err != nil { + t.Fatalf("%v", err) + } + return code, body +} + +// TestIntegration_ProbeStateMachine drives GET → POST refresh → GET +// against the real gin router with one ACP-capable agent. Verifies the +// status field transitions, the cache reflects the refresh result, and +// the wire format (Content-Type, JSON shape) is correct. +func TestIntegration_ProbeStateMachine(t *testing.T) { + host := newStatefulHostStub() + host.caps["claude-acp"] = hostutility.AgentCapabilities{ + AgentType: "claude-acp", + Status: hostutility.StatusAuthRequired, + Error: "please run `claude login`", + } + host.nextRefresh["claude-acp"] = hostutility.AgentCapabilities{ + AgentType: "claude-acp", + Status: hostutility.StatusOK, + CurrentModelID: "sonnet", + Models: []hostutility.Model{ + {ID: "sonnet", Name: "Sonnet"}, + {ID: "opus", Name: "Opus"}, + }, + } + ts := newIntegrationServer(t, + []lifecycle.InferenceAgentInfo{{ID: "claude-acp", Name: "Claude ACP Agent", DisplayName: "Claude"}}, + host) + defer ts.Close() + + code, hdr, body := getJSON(t, ts.URL+"/api/v1/utility/inference-agents") + if code != http.StatusOK { + t.Fatalf("initial GET: code=%d body=%v", code, body) + } + if ct := hdr.Get("Content-Type"); !strings.HasPrefix(ct, "application/json") { + t.Fatalf("Content-Type = %q", ct) + } + agents := body["agents"].([]any) + if len(agents) != 1 { + t.Fatalf("want 1 agent, got %d", len(agents)) + } + a := agents[0].(map[string]any) + if a["status"] != "auth_required" { + t.Fatalf("initial status: %v", a["status"]) + } + if len(a["models"].([]any)) != 0 { + t.Fatalf("initial models should be empty, got %v", a["models"]) + } + if !strings.Contains(a["status_message"].(string), "claude login") { + t.Fatalf("status_message: %v", a["status_message"]) + } + + code, body = postEmpty(t, ts.URL+"/api/v1/utility/inference-agents/claude-acp/refresh") + if code != http.StatusOK || body["status"] != "ok" { + t.Fatalf("refresh: code=%d body=%v", code, body) + } + + _, _, body = getJSON(t, ts.URL+"/api/v1/utility/inference-agents") + a = body["agents"].([]any)[0].(map[string]any) + if a["status"] != "ok" { + t.Fatalf("post-refresh status: %v", a["status"]) + } + models := a["models"].([]any) + if len(models) != 2 { + t.Fatalf("want 2 models, got %d", len(models)) + } + first := models[0].(map[string]any) + if first["id"] != "sonnet" || first["is_default"] != true { + t.Fatalf("first model: %+v", first) + } +} + +// TestIntegration_RefreshUnknownAgent verifies the handler 404s for an +// id not in the registry and does NOT call Refresh (no probe storm via +// arbitrary path params). +func TestIntegration_RefreshUnknownAgent(t *testing.T) { + host := newStatefulHostStub() + ts := newIntegrationServer(t, + []lifecycle.InferenceAgentInfo{{ID: "claude-acp", Name: "Claude ACP Agent", DisplayName: "Claude"}}, + host) + defer ts.Close() + + code, _ := postEmpty(t, ts.URL+"/api/v1/utility/inference-agents/bogus-id/refresh") + if code != http.StatusNotFound { + t.Fatalf("want 404, got %d", code) + } + if host.refreshCalls != 0 { + t.Fatalf("refresh should not be invoked for unknown agent; got %d calls", host.refreshCalls) + } +} + +// TestIntegration_RefreshPathFuzz hits the refresh route with +// path-traversal-ish ids. None should cause a server error, and none +// should be treated as a valid agent. +func TestIntegration_RefreshPathFuzz(t *testing.T) { + host := newStatefulHostStub() + ts := newIntegrationServer(t, + []lifecycle.InferenceAgentInfo{{ID: "claude-acp", Name: "Claude ACP Agent", DisplayName: "Claude"}}, + host) + defer ts.Close() + + cases := []string{ + "/api/v1/utility/inference-agents/..%2Fclaude-acp/refresh", + "/api/v1/utility/inference-agents/claude-acp%00/refresh", + "/api/v1/utility/inference-agents/" + strings.Repeat("a", 512) + "/refresh", + } + for _, p := range cases { + t.Run(p, func(t *testing.T) { + code, _ := postEmpty(t, ts.URL+p) + if code >= 500 { + t.Fatalf("path %q: server error %d", p, code) + } + }) + } + if host.refreshCalls != 0 { + t.Fatalf("none of the fuzzed ids should have reached Refresh; got %d", host.refreshCalls) + } +} + +// TestIntegration_ConcurrentRefresh hammers the refresh endpoint to +// verify race-freedom. The stub has no singleflight (that lives in the +// real Manager), so every one of the N requests must reach the stub — +// the assertion below pins refreshCalls == N. The primary signal is +// still that 50 concurrent goroutines drain without panic or data +// race, which is what -race exercises. +func TestIntegration_ConcurrentRefresh(t *testing.T) { + host := newStatefulHostStub() + host.caps["claude-acp"] = hostutility.AgentCapabilities{AgentType: "claude-acp", Status: hostutility.StatusAuthRequired} + host.nextRefresh["claude-acp"] = hostutility.AgentCapabilities{ + AgentType: "claude-acp", Status: hostutility.StatusOK, + Models: []hostutility.Model{{ID: "sonnet", Name: "Sonnet"}}, + } + ts := newIntegrationServer(t, + []lifecycle.InferenceAgentInfo{{ID: "claude-acp", Name: "Claude ACP Agent", DisplayName: "Claude"}}, + host) + defer ts.Close() + + const N = 50 + errs := make(chan error, N) + var wg sync.WaitGroup + for i := 0; i < N; i++ { + wg.Add(1) + go func() { + defer wg.Done() + code, body, err := postEmptyE(ts.URL + "/api/v1/utility/inference-agents/claude-acp/refresh") + if err != nil { + errs <- err + return + } + if code != http.StatusOK || body["status"] != "ok" { + errs <- fmt.Errorf("code=%d status=%v", code, body["status"]) + } + }() + } + wg.Wait() + close(errs) + for err := range errs { + t.Fatal(err) + } + if host.refreshCalls != N { + t.Fatalf("want %d refresh calls, got %d", N, host.refreshCalls) + } +} + +// TestIntegration_StatusMessageSanitizationOnTheWire feeds a 100KB +// probe error full of api_key= occurrences plus a unicode probe error +// through the GET endpoint. The redacted form must reach the wire +// without any raw secret material and without exploding the response. +func TestIntegration_StatusMessageSanitizationOnTheWire(t *testing.T) { + t.Run("large payload", func(t *testing.T) { + host := newStatefulHostStub() + host.caps["claude-acp"] = hostutility.AgentCapabilities{ + AgentType: "claude-acp", + Status: hostutility.StatusFailed, + Error: strings.Repeat("api_key=sk-secret-xyz ", 5000), + } + ts := newIntegrationServer(t, + []lifecycle.InferenceAgentInfo{{ID: "claude-acp", Name: "Claude ACP Agent", DisplayName: "Claude"}}, + host) + defer ts.Close() + + code, _, body := getJSON(t, ts.URL+"/api/v1/utility/inference-agents") + if code != http.StatusOK { + t.Fatalf("code=%d", code) + } + msg := body["agents"].([]any)[0].(map[string]any)["status_message"].(string) + if strings.Contains(msg, "sk-secret-xyz") { + t.Fatalf("secret leaked through sanitizer; len=%d head=%q", len(msg), msg[:80]) + } + }) + + t.Run("unicode probe error", func(t *testing.T) { + host := newStatefulHostStub() + host.caps["claude-acp"] = hostutility.AgentCapabilities{ + AgentType: "claude-acp", + Status: hostutility.StatusFailed, + Error: "认证失败: token=abc123 / Müllerverfahren", + } + ts := newIntegrationServer(t, + []lifecycle.InferenceAgentInfo{{ID: "claude-acp", Name: "Claude ACP Agent", DisplayName: "Claude"}}, + host) + defer ts.Close() + + _, _, body := getJSON(t, ts.URL+"/api/v1/utility/inference-agents") + msg := body["agents"].([]any)[0].(map[string]any)["status_message"].(string) + if strings.Contains(msg, "abc123") { + t.Fatalf("unicode secret leaked: %v", msg) + } + if !strings.Contains(msg, "认证失败") { + t.Fatalf("unicode prose was lost in sanitization: %v", msg) + } + }) +} diff --git a/apps/backend/internal/utility/handlers/handlers_test.go b/apps/backend/internal/utility/handlers/handlers_test.go index 9fcaddf698..a2f46ebe5c 100644 --- a/apps/backend/internal/utility/handlers/handlers_test.go +++ b/apps/backend/internal/utility/handlers/handlers_test.go @@ -3,9 +3,11 @@ package handlers import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "strings" + "sync/atomic" "testing" "github.com/gin-gonic/gin" @@ -29,7 +31,10 @@ func (s *stubInferenceExecutor) ListInferenceAgentsWithContext(_ context.Context } type stubHostUtility struct { - caps map[string]hostutility.AgentCapabilities + caps map[string]hostutility.AgentCapabilities + refreshErr map[string]error + refreshCaps map[string]hostutility.AgentCapabilities + refreshCalls atomic.Int32 } func (s *stubHostUtility) ExecutePrompt(_ context.Context, _, _, _, _ string) (*hostutility.PromptResult, error) { @@ -41,6 +46,34 @@ func (s *stubHostUtility) Get(agentType string) (hostutility.AgentCapabilities, return c, ok } +func (s *stubHostUtility) Refresh(_ context.Context, agentType string) (hostutility.AgentCapabilities, error) { + s.refreshCalls.Add(1) + if err, ok := s.refreshErr[agentType]; ok && err != nil { + // Mirror Manager.Refresh: cache is updated with the failure state. + if s.caps == nil { + s.caps = make(map[string]hostutility.AgentCapabilities) + } + s.caps[agentType] = hostutility.AgentCapabilities{ + AgentType: agentType, + Status: hostutility.StatusFailed, + Error: err.Error(), + } + return hostutility.AgentCapabilities{}, err + } + c, ok := s.refreshCaps[agentType] + if !ok { + c = hostutility.AgentCapabilities{ + AgentType: agentType, + Status: hostutility.StatusOK, + } + } + if s.caps == nil { + s.caps = make(map[string]hostutility.AgentCapabilities) + } + s.caps[agentType] = c + return c, nil +} + // wantModel is the expected shape of a single model in the response for a // given agent. type wantModel struct { @@ -49,18 +82,27 @@ type wantModel struct { meta map[string]any } +// wantAgent is the expected shape of a single agent entry in the response. +type wantAgent struct { + status string + hasMsg bool + models []wantModel +} + // TestHttpListInferenceAgents covers the full /api/v1/utility/inference-agents // response contract: // -// - Only agents whose host utility probe reached StatusOK are included — -// an agent that needs auth, isn't installed, or is still probing can't -// actually run a utility prompt, so it must be filtered out of the -// picker rather than leading the user into a dead end. +// - Every registered ACP-capable agent is included regardless of probe +// status. The frontend uses `status` to render an inline note + Refresh +// button when models aren't available, rather than silently dropping the +// agent and leaving the user staring at an empty Model picker. // - `models` is always a JSON array, never null — a null slice would // crash the frontend's flatMap over `ia.models`. // - `is_default` is set on the model matching CurrentModelID. // - `meta` (e.g. Copilot's `copilotUsage` cost multiplier) propagates // through to the DTO so the model combobox can render cost badges. +// - `status_message` is sanitized so a credential-looking substring in +// the upstream probe error never reaches the wire verbatim. func TestHttpListInferenceAgents(t *testing.T) { gin.SetMode(gin.TestMode) @@ -75,7 +117,7 @@ func TestHttpListInferenceAgents(t *testing.T) { agents []lifecycle.InferenceAgentInfo caps map[string]hostutility.AgentCapabilities nilHost bool - wantByAgent map[string][]wantModel + wantByAgent map[string]wantAgent }{ { name: "healthy agent with cached models is included with is_default", @@ -91,50 +133,55 @@ func TestHttpListInferenceAgents(t *testing.T) { }, }, }, - wantByAgent: map[string][]wantModel{ + wantByAgent: map[string]wantAgent{ "Claude ACP Agent": { - {id: "sonnet", isDefault: true}, - {id: "opus", isDefault: false}, + status: "ok", + models: []wantModel{ + {id: "sonnet", isDefault: true}, + {id: "opus", isDefault: false}, + }, }, }, }, { - // Primary UX bug this filter guards against: Codex/Auggie with - // lock icons (auth_required) appearing in the utility picker. - name: "auth_required agent is filtered out", - agents: []lifecycle.InferenceAgentInfo{claude, codex}, + // Previously: filter dropped non-OK agents from the response. + // New contract: include them so the UI can render an inline + // "sign in to Claude" / "Claude CLI not installed" note rather + // than an empty Model picker with no explanation. + name: "auth_required, failed, and probing agents are included with status", + agents: []lifecycle.InferenceAgentInfo{claude, codex, copilot}, caps: map[string]hostutility.AgentCapabilities{ "claude-acp": { AgentType: "claude-acp", - Status: hostutility.StatusOK, - Models: []hostutility.Model{{ID: "sonnet", Name: "Sonnet"}}, + Status: hostutility.StatusAuthRequired, + Error: "please run `claude login`", }, "codex-acp": { AgentType: "codex-acp", - Status: hostutility.StatusAuthRequired, + Status: hostutility.StatusFailed, + Error: "probe crashed", + }, + "copilot-acp": { + AgentType: "copilot-acp", + Status: hostutility.StatusProbing, }, }, - wantByAgent: map[string][]wantModel{ - "Claude ACP Agent": {{id: "sonnet", isDefault: false}}, + wantByAgent: map[string]wantAgent{ + "Claude ACP Agent": {status: "auth_required", hasMsg: true, models: []wantModel{}}, + "Codex ACP Agent": {status: "failed", hasMsg: true, models: []wantModel{}}, + "Copilot ACP Agent": {status: "probing", models: []wantModel{}}, }, }, { - name: "failed and probing agents are filtered out", - agents: []lifecycle.InferenceAgentInfo{claude, codex, copilot}, - caps: map[string]hostutility.AgentCapabilities{ - "claude-acp": {Status: hostutility.StatusFailed}, - "codex-acp": {Status: hostutility.StatusProbing}, - "copilot-acp": {Status: hostutility.StatusOK, Models: []hostutility.Model{{ID: "gpt-5", Name: "GPT-5"}}}, + // Cache miss (registry knows about the agent but the probe + // hasn't landed yet) is surfaced as "probing" so the UI shows + // the same "Setting up X…" affordance. + name: "agent with no cache entry is reported as probing", + agents: []lifecycle.InferenceAgentInfo{claude}, + caps: nil, + wantByAgent: map[string]wantAgent{ + "Claude ACP Agent": {status: "probing", models: []wantModel{}}, }, - wantByAgent: map[string][]wantModel{ - "Copilot ACP Agent": {{id: "gpt-5", isDefault: false}}, - }, - }, - { - name: "agent with no cache entry is filtered out", - agents: []lifecycle.InferenceAgentInfo{claude}, - caps: nil, - wantByAgent: map[string][]wantModel{}, }, { name: "model meta (copilot cost) propagates to DTO", @@ -150,10 +197,13 @@ func TestHttpListInferenceAgents(t *testing.T) { }, }, }, - wantByAgent: map[string][]wantModel{ + wantByAgent: map[string]wantAgent{ "Copilot ACP Agent": { - {id: "gpt-5", isDefault: true, meta: map[string]any{"copilotUsage": "1x"}}, - {id: "gpt-5-mini", isDefault: false, meta: map[string]any{"copilotUsage": "0.33x"}}, + status: "ok", + models: []wantModel{ + {id: "gpt-5", isDefault: true, meta: map[string]any{"copilotUsage": "1x"}}, + {id: "gpt-5-mini", isDefault: false, meta: map[string]any{"copilotUsage": "0.33x"}}, + }, }, }, }, @@ -161,16 +211,16 @@ func TestHttpListInferenceAgents(t *testing.T) { name: "no inference agents returns empty list", agents: nil, caps: nil, - wantByAgent: map[string][]wantModel{}, + wantByAgent: map[string]wantAgent{}, }, { // hostExecutor is optional throughout the package (see the nil // guard in executeSessionless). Without it we can't check - // health, so the list is empty — never a panic. + // probe state, so the list is empty — never a panic. name: "nil hostExecutor yields empty list, no panic", agents: []lifecycle.InferenceAgentInfo{claude}, nilHost: true, - wantByAgent: map[string][]wantModel{}, + wantByAgent: map[string]wantAgent{}, }, } @@ -205,8 +255,10 @@ func TestHttpListInferenceAgents(t *testing.T) { var resp struct { Agents []struct { - Name string `json:"name"` - Models []struct { + Name string `json:"name"` + Status string `json:"status"` + StatusMessage string `json:"status_message,omitempty"` + Models []struct { ID string `json:"id"` IsDefault bool `json:"is_default"` Meta map[string]any `json:"meta,omitempty"` @@ -230,19 +282,28 @@ func TestHttpListInferenceAgents(t *testing.T) { t.Errorf("unexpected agent %q in response", a.Name) continue } - if len(a.Models) != len(want) { - t.Errorf("agent %q: got %d models, want %d", a.Name, len(a.Models), len(want)) + if a.Status != want.status { + t.Errorf("agent %q: got status=%q, want %q", a.Name, a.Status, want.status) + } + if want.hasMsg && a.StatusMessage == "" { + t.Errorf("agent %q: expected non-empty status_message", a.Name) + } + if !want.hasMsg && a.StatusMessage != "" { + t.Errorf("agent %q: expected empty status_message, got %q", a.Name, a.StatusMessage) + } + if len(a.Models) != len(want.models) { + t.Errorf("agent %q: got %d models, want %d", a.Name, len(a.Models), len(want.models)) continue } for i, m := range a.Models { - if m.ID != want[i].id { - t.Errorf("agent %q model[%d]: got id %q, want %q", a.Name, i, m.ID, want[i].id) + if m.ID != want.models[i].id { + t.Errorf("agent %q model[%d]: got id %q, want %q", a.Name, i, m.ID, want.models[i].id) } - if m.IsDefault != want[i].isDefault { - t.Errorf("agent %q model %q: got is_default=%v, want %v", a.Name, m.ID, m.IsDefault, want[i].isDefault) + if m.IsDefault != want.models[i].isDefault { + t.Errorf("agent %q model %q: got is_default=%v, want %v", a.Name, m.ID, m.IsDefault, want.models[i].isDefault) } - if !equalMeta(m.Meta, want[i].meta) { - t.Errorf("agent %q model %q: got meta=%v, want %v", a.Name, m.ID, m.Meta, want[i].meta) + if !equalMeta(m.Meta, want.models[i].meta) { + t.Errorf("agent %q model %q: got meta=%v, want %v", a.Name, m.ID, m.Meta, want.models[i].meta) } } } @@ -250,9 +311,178 @@ func TestHttpListInferenceAgents(t *testing.T) { } } +// TestSanitizeStatusMessage guards the wire-level redaction of credentials in +// upstream probe errors. An ACP agent that echoes the offending value back +// must not leak it through /api/v1/utility/inference-agents. +func TestSanitizeStatusMessage(t *testing.T) { + t.Parallel() + cases := []struct { + in string + want string + }{ + {in: "", want: ""}, + {in: "agent crashed", want: "agent crashed"}, + {in: "invalid api_key=sk-deadbeef", want: "invalid api_key="}, + {in: "auth failed: token = abc123", want: "auth failed: token="}, + {in: "bearer sk-xyz expired", want: "bearer= expired"}, + {in: "API-KEY: SECRETVAL", want: "API-KEY="}, + // Prose without a real separator must not be mangled — guards against + // the kw=val matcher eating the next word. + {in: "access token was revoked", want: "access token was revoked"}, + {in: "the secret handshake failed", want: "the secret handshake failed"}, + // "api key" with a literal space must still be redacted (cubic review). + {in: "invalid api key=sk-deadbeef", want: "invalid api key="}, + // Newline must NOT count as a separator — otherwise a multi-line + // stderr ("invalid token\ncaused by network") would consume the next + // word on the next line (greptile review). + { + in: "invalid token\ncaused by network timeout", + want: "invalid token\ncaused by network timeout", + }, + // Tabs are valid horizontal whitespace separators. + {in: "token\t=\tabc123", want: "token="}, + } + for _, tc := range cases { + got := sanitizeStatusMessage(tc.in) + if got != tc.want { + t.Errorf("sanitizeStatusMessage(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +// TestHttpRefreshInferenceAgent covers the POST /inference-agents/:id/refresh +// route: success path, unknown agent id, error path (still surfaces the +// latest cached state instead of erroring out so the UI can re-render). +func TestHttpRefreshInferenceAgent(t *testing.T) { + gin.SetMode(gin.TestMode) + claude := lifecycle.InferenceAgentInfo{ID: "claude-acp", Name: "Claude ACP Agent", DisplayName: "Claude"} + + t.Run("success returns updated capabilities", func(t *testing.T) { + log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) + host := &stubHostUtility{ + caps: map[string]hostutility.AgentCapabilities{ + "claude-acp": {AgentType: "claude-acp", Status: hostutility.StatusAuthRequired, Error: "please login"}, + }, + refreshCaps: map[string]hostutility.AgentCapabilities{ + "claude-acp": { + AgentType: "claude-acp", + Status: hostutility.StatusOK, + CurrentModelID: "sonnet", + Models: []hostutility.Model{{ID: "sonnet", Name: "Sonnet"}}, + }, + }, + } + h := &Handlers{ + executor: &stubInferenceExecutor{agents: []lifecycle.InferenceAgentInfo{claude}}, + hostExecutor: host, + logger: log, + } + router := gin.New() + router.POST("/api/v1/utility/inference-agents/:id/refresh", h.httpRefreshInferenceAgent) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/utility/inference-agents/claude-acp/refresh", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", rec.Code) + } + if host.refreshCalls.Load() != 1 { + t.Fatalf("refresh called %d times, want 1", host.refreshCalls.Load()) + } + var got struct { + ID string `json:"id"` + Status string `json:"status"` + Models []struct { + ID string `json:"id"` + } `json:"models"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got.ID != "claude-acp" || got.Status != "ok" || len(got.Models) != 1 || got.Models[0].ID != "sonnet" { + t.Fatalf("unexpected response: %+v", got) + } + }) + + t.Run("unknown agent id returns 404", func(t *testing.T) { + log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) + h := &Handlers{ + executor: &stubInferenceExecutor{agents: []lifecycle.InferenceAgentInfo{claude}}, + hostExecutor: &stubHostUtility{}, + logger: log, + } + router := gin.New() + router.POST("/api/v1/utility/inference-agents/:id/refresh", h.httpRefreshInferenceAgent) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/utility/inference-agents/bogus/refresh", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", rec.Code) + } + }) + + t.Run("refresh error surfaces latest cached state", func(t *testing.T) { + log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) + host := &stubHostUtility{ + refreshErr: map[string]error{"claude-acp": errors.New("connection refused")}, + } + h := &Handlers{ + executor: &stubInferenceExecutor{agents: []lifecycle.InferenceAgentInfo{claude}}, + hostExecutor: host, + logger: log, + } + router := gin.New() + router.POST("/api/v1/utility/inference-agents/:id/refresh", h.httpRefreshInferenceAgent) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/utility/inference-agents/claude-acp/refresh", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (UI re-renders even on probe failure), body=%s", rec.Code, rec.Body.String()) + } + var got struct { + Status string `json:"status"` + StatusMessage string `json:"status_message"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got.Status != "failed" { + t.Fatalf("got status=%q, want failed", got.Status) + } + if got.StatusMessage == "" { + t.Fatalf("expected non-empty status_message on probe failure") + } + }) + + t.Run("nil host executor returns 503", func(t *testing.T) { + log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) + h := &Handlers{ + executor: &stubInferenceExecutor{agents: []lifecycle.InferenceAgentInfo{claude}}, + logger: log, + } + router := gin.New() + router.POST("/api/v1/utility/inference-agents/:id/refresh", h.httpRefreshInferenceAgent) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/utility/inference-agents/claude-acp/refresh", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("status = %d, want 503", rec.Code) + } + }) +} + func agentNames(agents []struct { - Name string `json:"name"` - Models []struct { + Name string `json:"name"` + Status string `json:"status"` + StatusMessage string `json:"status_message,omitempty"` + Models []struct { ID string `json:"id"` IsDefault bool `json:"is_default"` Meta map[string]any `json:"meta,omitempty"` diff --git a/apps/web/components/settings/inference-agent-status.test.tsx b/apps/web/components/settings/inference-agent-status.test.tsx new file mode 100644 index 0000000000..bb94bebbe6 --- /dev/null +++ b/apps/web/components/settings/inference-agent-status.test.tsx @@ -0,0 +1,124 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { InferenceAgentStatusNote } from "./inference-agent-status"; +import type { InferenceAgent, InferenceAgentStatus } from "@/lib/api/domains/utility-api"; + +afterEach(() => { + cleanup(); +}); + +function makeAgent(overrides: Partial = {}): InferenceAgent { + return { + id: "claude-acp", + name: "Claude ACP Agent", + display_name: "Claude", + models: [], + status: "ok", + ...overrides, + }; +} + +describe("InferenceAgentStatusNote", () => { + it("renders nothing when agent is healthy with models", () => { + const { container } = render( + {}} + />, + ); + expect(container.firstChild).toBeNull(); + }); + + it.each<[InferenceAgentStatus, string]>([ + ["probing", "Setting up Claude"], + ["auth_required", "Sign in to Claude"], + ["not_installed", "Claude CLI is not installed"], + ["not_configured", "Claude is not configured"], + ["failed", "Probe failed for Claude"], + ])("maps status %s to the expected note copy", (status, expectedSubstring) => { + render( {}} />); + expect(screen.getByText(new RegExp(expectedSubstring))).toBeTruthy(); + }); + + it("treats ok-with-no-models as an empty-advertised-models state", () => { + render( + {}} + />, + ); + expect(screen.getByText(/Claude advertised no models/)).toBeTruthy(); + }); + + it("renders fallback copy when agent is missing from the list", () => { + render( + {}} />, + ); + expect(screen.getByText(/claude-acp is no longer available/)).toBeTruthy(); + }); + + it("hides the Refresh button for non-refreshable statuses", () => { + render( + {}} + />, + ); + expect(screen.queryByTestId("inference-agent-refresh")).toBeNull(); + }); + + it("invokes onRefresh and disables the button while refreshing", async () => { + let resolve: (() => void) | undefined; + const onRefresh = vi.fn( + () => + new Promise((r) => { + resolve = r; + }), + ); + render( + , + ); + const button = screen.getByTestId("inference-agent-refresh") as HTMLButtonElement; + fireEvent.click(button); + expect(onRefresh).toHaveBeenCalledTimes(1); + await waitFor(() => { + expect(button.disabled).toBe(true); + }); + resolve?.(); + await waitFor(() => { + expect(button.disabled).toBe(false); + }); + }); + + it("treats an agent with no status field as healthy when models are present", () => { + const { container } = render( + {}} + />, + ); + expect(container.firstChild).toBeNull(); + }); + + it("exposes the sanitized status_message detail line", () => { + render( + {}} + />, + ); + expect(screen.getByText("subprocess exited with code 1")).toBeTruthy(); + }); +}); diff --git a/apps/web/components/settings/inference-agent-status.tsx b/apps/web/components/settings/inference-agent-status.tsx new file mode 100644 index 0000000000..97d1f6a3de --- /dev/null +++ b/apps/web/components/settings/inference-agent-status.tsx @@ -0,0 +1,110 @@ +"use client"; + +import { useState } from "react"; +import { IconRefresh } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import type { InferenceAgent, InferenceAgentStatus } from "@/lib/api/domains/utility-api"; + +type Props = { + /** + * The agent whose status to render. `null`/`undefined` means the agent + * selected in the form is not present in the latest `/inference-agents` + * response — e.g. the user picked Claude but Claude was just uninstalled, + * or the cache hasn't caught up yet. The fallback note is "agent is no + * longer available". + */ + agent: InferenceAgent | null | undefined; + /** + * Fallback agent display name for the not-available case. Without it the + * note would read "Agent is no longer available" with no identifier. + */ + fallbackName?: string; + /** + * Re-probe handler. Called when the user clicks Refresh. The parent owns + * loading state on the InferenceAgent list; this component only manages + * the in-flight spinner on the button itself so simultaneous clicks + * don't double-fire. + */ + onRefresh: () => Promise | void; +}; + +type Note = { text: string; refreshable: boolean }; + +function noteForStatus(status: InferenceAgentStatus | undefined, name: string): Note { + switch (status) { + case "probing": + return { text: `Setting up ${name}…`, refreshable: true }; + case "auth_required": + return { text: `Sign in to ${name} to load models.`, refreshable: true }; + case "not_installed": + return { text: `${name} CLI is not installed on this machine.`, refreshable: true }; + case "not_configured": + return { text: `${name} is not configured for inference.`, refreshable: false }; + case "failed": + return { text: `Probe failed for ${name}.`, refreshable: true }; + case "ok": + default: + return { text: `${name} advertised no models.`, refreshable: true }; + } +} + +function RefreshButton({ onRefresh }: { onRefresh: () => Promise | void }) { + const [refreshing, setRefreshing] = useState(false); + const handleClick = async () => { + if (refreshing) return; + setRefreshing(true); + try { + await onRefresh(); + } finally { + setRefreshing(false); + } + }; + return ( + + ); +} + +// Treat a missing `status` field as healthy when models are present — +// older backends (or non-OK API consumers) that don't populate `status` +// should not show a spurious warning. Real probe states ("auth_required", +// "probing", …) still fall through to noteForStatus. Per cubic review. +function isAgentHealthy(agent: InferenceAgent | null | undefined): boolean { + if (!agent || (agent.models?.length ?? 0) === 0) return false; + return agent.status === "ok" || agent.status === undefined; +} + +export function InferenceAgentStatusNote({ agent, fallbackName, onRefresh }: Props) { + if (isAgentHealthy(agent)) { + return null; + } + + const name = agent?.display_name ?? fallbackName ?? "this agent"; + const note: Note = agent + ? noteForStatus(agent.status, name) + : { text: `${name} is no longer available.`, refreshable: true }; + const detail = agent?.status_message?.trim(); + + return ( +
+
+

{note.text}

+ {detail &&

{detail}

} +
+ {note.refreshable && } +
+ ); +} diff --git a/apps/web/components/settings/use-inference-agents.ts b/apps/web/components/settings/use-inference-agents.ts new file mode 100644 index 0000000000..f00ca1cd5b --- /dev/null +++ b/apps/web/components/settings/use-inference-agents.ts @@ -0,0 +1,40 @@ +"use client"; + +import { useCallback, useState } from "react"; +import { + listInferenceAgents, + refreshInferenceAgent, + type InferenceAgent, +} from "@/lib/api/domains/utility-api"; + +/** + * Owns the list of inference agents fetched from the backend and the + * refresh-one / refresh-all flows the settings page invokes from the + * inline status note. Refresh is best-effort — failures are absorbed and + * the next render shows the latest cached state (Manager.Refresh writes + * the failure into the cache on its own). + */ +export function useInferenceAgents() { + const [inferenceAgents, setInferenceAgents] = useState([]); + + const refreshAgent = useCallback( + async (agentId: string) => { + if (!agentId) return; + try { + const known = inferenceAgents.some((a) => a.id === agentId); + if (known) { + const updated = await refreshInferenceAgent(agentId); + setInferenceAgents((prev) => prev.map((a) => (a.id === updated.id ? updated : a))); + } else { + const { agents: refetched } = await listInferenceAgents({ cache: "no-store" }); + setInferenceAgents(refetched); + } + } catch { + // best-effort + } + }, + [inferenceAgents], + ); + + return { inferenceAgents, setInferenceAgents, refreshAgent }; +} diff --git a/apps/web/components/settings/utility-agent-dialog.tsx b/apps/web/components/settings/utility-agent-dialog.tsx index ae384c692b..430d1d02de 100644 --- a/apps/web/components/settings/utility-agent-dialog.tsx +++ b/apps/web/components/settings/utility-agent-dialog.tsx @@ -17,6 +17,8 @@ import { type InferenceAgent, type InferenceModel, } from "@/lib/api/domains/utility-api"; +import { InferenceAgentStatusNote } from "./inference-agent-status"; +import { useInferenceAgents } from "./use-inference-agents"; import { ScriptEditor } from "./profile-edit/script-editor"; import type { ScriptPlaceholder } from "./profile-edit/script-editor-completions"; @@ -56,47 +58,58 @@ type AgentModelSelectProps = { agentId: string; model: string; inferenceAgents: InferenceAgent[]; + selectedAgent: InferenceAgent | undefined; availableModels: InferenceModel[]; onAgentChange: (agentId: string) => void; onModelChange: (model: string) => void; + onRefresh: () => Promise | void; }; function AgentModelSelect({ agentId, model, inferenceAgents, + selectedAgent, availableModels, onAgentChange, onModelChange, + onRefresh, }: AgentModelSelectProps) { return ( -
-
- - -
-
- - m.is_default)?.id} - placeholder="Select model..." - disabled={availableModels.length === 0} - /> +
+
+
+ + +
+
+ + m.is_default)?.id} + placeholder="Select model..." + disabled={availableModels.length === 0} + /> +
+
); } @@ -106,8 +119,10 @@ type UtilityAgentFormProps = { setForm: React.Dispatch>; isBuiltin: boolean; inferenceAgents: InferenceAgent[]; + selectedAgent: InferenceAgent | undefined; availableModels: InferenceModel[]; placeholders: ScriptPlaceholder[]; + onRefreshAgent: () => Promise | void; }; function UtilityAgentForm({ @@ -115,8 +130,10 @@ function UtilityAgentForm({ setForm, isBuiltin, inferenceAgents, + selectedAgent, availableModels, placeholders, + onRefreshAgent, }: UtilityAgentFormProps) { return (
@@ -143,9 +160,11 @@ function UtilityAgentForm({ agentId={form.agent_id} model={form.model} inferenceAgents={inferenceAgents} + selectedAgent={selectedAgent} availableModels={availableModels} onAgentChange={(v) => setForm((f) => ({ ...f, agent_id: v, model: "" }))} onModelChange={(v) => setForm((f) => ({ ...f, model: v }))} + onRefresh={onRefreshAgent} />
@@ -171,7 +190,7 @@ export function UtilityAgentDialog({ open, onOpenChange, agent, onSuccess }: Pro const [form, setForm] = useState(defaultFormState); const [saving, setSaving] = useState(false); const [placeholders, setPlaceholders] = useState([]); - const [inferenceAgents, setInferenceAgents] = useState([]); + const { inferenceAgents, setInferenceAgents, refreshAgent } = useInferenceAgents(); const isEdit = Boolean(agent); // Fetch template variables and inference agents @@ -183,15 +202,14 @@ export function UtilityAgentDialog({ open, onOpenChange, agent, onSuccess }: Pro listInferenceAgents() .then(({ agents }) => setInferenceAgents(agents)) .catch(() => setInferenceAgents([])); - }, []); + }, [setInferenceAgents]); - // Get models for the selected agent const selectedAgent = useMemo( () => inferenceAgents.find((a) => a.id === form.agent_id), [inferenceAgents, form.agent_id], ); - const availableModels = selectedAgent?.models ?? []; + const refreshCurrentAgent = () => refreshAgent(form.agent_id); // Auto-select default model when agent changes useEffect(() => { @@ -258,8 +276,10 @@ export function UtilityAgentDialog({ open, onOpenChange, agent, onSuccess }: Pro setForm={setForm} isBuiltin={agent?.builtin ?? false} inferenceAgents={inferenceAgents} + selectedAgent={selectedAgent} availableModels={availableModels} placeholders={placeholders} + onRefreshAgent={refreshCurrentAgent} />
+ {defaultAgentId && ( + onRefreshAgent(defaultAgentId)} + /> + )}
); } diff --git a/apps/web/e2e/tests/settings/utility-agents.spec.ts b/apps/web/e2e/tests/settings/utility-agents.spec.ts index cfe05b747f..1f4cbc0c13 100644 --- a/apps/web/e2e/tests/settings/utility-agents.spec.ts +++ b/apps/web/e2e/tests/settings/utility-agents.spec.ts @@ -114,9 +114,11 @@ test.describe("Utility Agents settings page", () => { // Regression guard for "I select an agent but can't select a model". // The mock-agent binary advertises `mock-fast` (default) and `mock-smart` // in its session/new response, so the boot-time ACP probe populates the - // host utility capability cache. The backend filters out agents whose - // probe didn't reach StatusOK, so in E2E (KANDEV_MOCK_AGENT=only) the - // Agent dropdown should show exactly one option: Mock. + // host utility capability cache. In E2E (KANDEV_MOCK_AGENT=only) only + // `mock-agent` is registered in the inference-agent registry, so the + // Agent dropdown shows exactly one option: Mock. (Non-OK agents are no + // longer filtered out — see #1269 — but in this profile no other agent + // is registered to begin with.) const pageErrors: Error[] = []; testPage.on("pageerror", (err) => pageErrors.push(err)); @@ -193,6 +195,87 @@ test.describe("Utility Agents settings page", () => { ); }); + test("non-ok agent renders status note + Refresh re-probes", async ({ testPage }) => { + // Regression guard for "claude shown in picker but no models, with no + // explanation". The backend now surfaces probe status so the UI can + // render an inline note + Refresh button instead of a silently-empty + // Model picker. Stub both endpoints to drive the state machine. + const pageErrors: Error[] = []; + testPage.on("pageerror", (err) => pageErrors.push(err)); + + let refreshCount = 0; + await testPage.route("**/api/v1/utility/inference-agents", (route) => { + if (route.request().method() !== "GET") { + return route.fallback(); + } + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + agents: [ + { + id: "stub-acp", + name: "Stub ACP Agent", + display_name: "Stub", + status: refreshCount === 0 ? "auth_required" : "ok", + status_message: refreshCount === 0 ? "please run `stub login`" : "", + models: + refreshCount === 0 + ? [] + : [{ id: "stub-fast", name: "Stub Fast", description: "", is_default: true }], + }, + ], + }), + }); + }); + + await testPage.route("**/api/v1/utility/inference-agents/stub-acp/refresh", (route) => { + refreshCount += 1; + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + id: "stub-acp", + name: "Stub ACP Agent", + display_name: "Stub", + status: "ok", + models: [{ id: "stub-fast", name: "Stub Fast", description: "", is_default: true }], + }), + }); + }); + + await testPage.goto("/settings/utility-agents"); + await expect( + testPage.getByRole("heading", { name: "Utility Agents", exact: true }), + ).toBeVisible({ timeout: 15_000 }); + + // Pick Stub from the Agent dropdown so the Default Model row binds + // selectedAgent to the auth_required entry. + const agentSelect = testPage + .locator('div:has(> label:text-is("Agent"))') + .first() + .getByRole("combobox"); + await agentSelect.click(); + await testPage.getByRole("option", { name: "Stub", exact: true }).click(); + + // Auth_required status copy renders, Refresh visible. + const note = testPage.getByTestId("inference-agent-status-note").first(); + await expect(note).toBeVisible(); + await expect(note).toContainText("Sign in to Stub"); + + const refresh = testPage.getByTestId("inference-agent-refresh").first(); + await expect(refresh).toBeVisible(); + await refresh.click(); + + // After Refresh, status note disappears (status:"ok" + 1 model). + await expect(testPage.getByTestId("inference-agent-status-note").first()).not.toBeVisible(); + expect(refreshCount).toBe(1); + + expect(pageErrors, `uncaught errors: ${pageErrors.map((e) => e.message).join("; ")}`).toEqual( + [], + ); + }); + test("Configuration Chat Agent section lives here, not on the agents page", async ({ testPage, }) => { diff --git a/apps/web/lib/api/domains/utility-api.ts b/apps/web/lib/api/domains/utility-api.ts index 7dce107446..4d4d006bfc 100644 --- a/apps/web/lib/api/domains/utility-api.ts +++ b/apps/web/lib/api/domains/utility-api.ts @@ -51,11 +51,31 @@ export type InferenceModel = { meta?: Record; }; +/** + * Probe outcome for the host-utility agentctl instance backing this agent. + * Mirrors `hostutility.Status` (`apps/backend/internal/agent/hostutility/types.go`). + * The settings page renders an inline status note + Refresh button when + * `status !== "ok"` or `models` is empty, instead of a silently-disabled + * Model picker. + */ +export type InferenceAgentStatus = + | "ok" + | "probing" + | "auth_required" + | "not_installed" + | "failed" + | "not_configured"; + export type InferenceAgent = { id: string; name: string; display_name: string; models: InferenceModel[]; + // Optional so older backends (or non-OK API consumers) that omit the + // field decode without forcing a default; the UI treats missing status + // as healthy when models are present. + status?: InferenceAgentStatus; + status_message?: string; }; export type ExecutePromptRequest = { @@ -141,6 +161,22 @@ export async function listInferenceAgents( return fetchJson<{ agents: InferenceAgent[] }>("/api/v1/utility/inference-agents", options); } +/** + * Re-probe a single inference agent and return its updated capabilities. + * Used by the settings-page Refresh button so the user can recover from a + * transient probe failure (sign-in race, agent not yet installed at boot, + * network blip) without restarting kandev. + */ +export async function refreshInferenceAgent( + id: string, + options?: ApiRequestOptions, +): Promise { + return fetchJson(`/api/v1/utility/inference-agents/${id}/refresh`, { + ...options, + init: { method: "POST", ...(options?.init ?? {}) }, + }); +} + export async function executeUtilityPrompt( req: ExecutePromptRequest, options?: ApiRequestOptions,