From efed6784af92e0551d4017c6bd09857b9b5284a6 Mon Sep 17 00:00:00 2001 From: wenjie Date: Mon, 9 Mar 2026 10:58:58 +0800 Subject: [PATCH] feat: add startup readiness checks and propagate start availability to UI - add gateway precondition validation for default model and credentials - auto-start gateway on backend boot when conditions are met - include gateway_start_allowed and gateway_start_reason in status updates - prevent frontend start actions when gateway cannot be started --- web/backend/api/gateway.go | 168 +++++++++++++++--- web/backend/api/gateway_test.go | 122 +++++++++++++ web/backend/main.go | 6 + web/frontend/src/api/gateway.ts | 2 + web/frontend/src/components/app-header.tsx | 10 +- .../src/components/chat/chat-composer.tsx | 11 +- web/frontend/src/hooks/use-gateway.ts | 67 +++++-- web/frontend/src/store/gateway.ts | 4 +- 8 files changed, 338 insertions(+), 52 deletions(-) create mode 100644 web/backend/api/gateway_test.go diff --git a/web/backend/api/gateway.go b/web/backend/api/gateway.go index 46c638a8b..f4b953929 100644 --- a/web/backend/api/gateway.go +++ b/web/backend/api/gateway.go @@ -13,6 +13,7 @@ import ( "path/filepath" "runtime" "strconv" + "strings" "sync" "syscall" "time" @@ -40,29 +41,70 @@ func (h *Handler) registerGatewayRoutes(mux *http.ServeMux) { mux.HandleFunc("POST /api/gateway/restart", h.handleGatewayRestart) } -// handleGatewayStart starts the picoclaw gateway subprocess. -// -// POST /api/gateway/start -func (h *Handler) handleGatewayStart(w http.ResponseWriter, r *http.Request) { +// TryAutoStartGateway checks whether gateway start preconditions are met and +// starts it when possible. Intended to be called by the backend at startup. +func (h *Handler) TryAutoStartGateway() { gateway.mu.Lock() defer gateway.mu.Unlock() - // Prevent duplicate starts + if isGatewayProcessAliveLocked() { + return + } if gateway.cmd != nil && gateway.cmd.Process != nil { - // Check if process is still alive (signal 0 doesn't kill, just checks) - if err := gateway.cmd.Process.Signal(syscall.Signal(0)); err == nil { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusConflict) - json.NewEncoder(w).Encode(map[string]any{ - "status": "already_running", - "pid": gateway.cmd.Process.Pid, - }) - return - } - // Process is dead, clean up gateway.cmd = nil } + ready, reason, err := h.gatewayStartReady() + if err != nil { + log.Printf("Skip auto-starting gateway: %v", err) + return + } + if !ready { + log.Printf("Skip auto-starting gateway: %s", reason) + return + } + + pid, err := h.startGatewayLocked() + if err != nil { + log.Printf("Failed to auto-start gateway: %v", err) + return + } + log.Printf("Gateway auto-started (PID: %d)", pid) +} + +// gatewayStartReady validates whether current config can start the gateway. +func (h *Handler) gatewayStartReady() (bool, string, error) { + cfg, err := config.LoadConfig(h.configPath) + if err != nil { + return false, "", fmt.Errorf("failed to load config: %w", err) + } + + modelName := strings.TrimSpace(cfg.Agents.Defaults.GetModelName()) + if modelName == "" { + return false, "no default model configured", nil + } + + modelCfg, err := cfg.GetModelConfig(modelName) + if err != nil { + return false, fmt.Sprintf("default model %q is invalid", modelName), nil + } + + hasCredential := strings.TrimSpace(modelCfg.APIKey) != "" || + strings.TrimSpace(modelCfg.AuthMethod) != "" + if !hasCredential { + return false, fmt.Sprintf("default model %q has no credentials configured", modelName), nil + } + + return true, "", nil +} + +func isGatewayProcessAliveLocked() bool { + return gateway.cmd != nil && + gateway.cmd.Process != nil && + gateway.cmd.Process.Signal(syscall.Signal(0)) == nil +} + +func (h *Handler) startGatewayLocked() (int, error) { // Locate the picoclaw executable execPath := findPicoclawBinary() @@ -70,14 +112,12 @@ func (h *Handler) handleGatewayStart(w http.ResponseWriter, r *http.Request) { stdoutPipe, err := cmd.StdoutPipe() if err != nil { - http.Error(w, fmt.Sprintf("Failed to create stdout pipe: %v", err), http.StatusInternalServerError) - return + return 0, fmt.Errorf("failed to create stdout pipe: %w", err) } stderrPipe, err := cmd.StderrPipe() if err != nil { - http.Error(w, fmt.Sprintf("Failed to create stderr pipe: %v", err), http.StatusInternalServerError) - return + return 0, fmt.Errorf("failed to create stderr pipe: %w", err) } // Clear old logs for this new run @@ -90,8 +130,7 @@ func (h *Handler) handleGatewayStart(w http.ResponseWriter, r *http.Request) { } if err := cmd.Start(); err != nil { - http.Error(w, fmt.Sprintf("Failed to start gateway: %v", err), http.StatusInternalServerError) - return + return 0, fmt.Errorf("failed to start gateway: %w", err) } gateway.cmd = cmd @@ -158,10 +197,59 @@ func (h *Handler) handleGatewayStart(w http.ResponseWriter, r *http.Request) { } }() + return pid, nil +} + +// handleGatewayStart starts the picoclaw gateway subprocess. +// +// POST /api/gateway/start +func (h *Handler) handleGatewayStart(w http.ResponseWriter, r *http.Request) { + gateway.mu.Lock() + defer gateway.mu.Unlock() + + // Prevent duplicate starts + if isGatewayProcessAliveLocked() { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusConflict) + json.NewEncoder(w).Encode(map[string]any{ + "status": "already_running", + "pid": gateway.cmd.Process.Pid, + }) + return + } + if gateway.cmd != nil && gateway.cmd.Process != nil { + gateway.cmd = nil + } + + ready, reason, err := h.gatewayStartReady() + if err != nil { + http.Error( + w, + fmt.Sprintf("Failed to validate gateway start conditions: %v", err), + http.StatusInternalServerError, + ) + return + } + if !ready { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + json.NewEncoder(w).Encode(map[string]any{ + "status": "precondition_failed", + "message": reason, + }) + return + } + + pid, err := h.startGatewayLocked() + if err != nil { + http.Error(w, fmt.Sprintf("Failed to start gateway: %v", err), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]any{ "status": "ok", - "pid": cmd.Process.Pid, + "pid": pid, }) } @@ -292,6 +380,17 @@ func (h *Handler) handleGatewayStatus(w http.ResponseWriter, r *http.Request) { } } + ready, reason, readyErr := h.gatewayStartReady() + if readyErr != nil { + data["gateway_start_allowed"] = false + data["gateway_start_reason"] = readyErr.Error() + } else { + data["gateway_start_allowed"] = ready + if !ready { + data["gateway_start_reason"] = reason + } + } + // Append incremental log data appendGatewayLogs(r, data) @@ -385,16 +484,29 @@ func (h *Handler) currentGatewayStatus() string { gateway.mu.Lock() defer gateway.mu.Unlock() - event := GatewayEvent{Status: "stopped"} + data := map[string]any{ + "gateway_status": "stopped", + } if gateway.cmd != nil && gateway.cmd.Process != nil { if err := gateway.cmd.Process.Signal(syscall.Signal(0)); err == nil { - event.Status = "running" - event.PID = gateway.cmd.Process.Pid + data["gateway_status"] = "running" + data["pid"] = gateway.cmd.Process.Pid } } - data, _ := json.Marshal(event) - return string(data) + ready, reason, readyErr := h.gatewayStartReady() + if readyErr != nil { + data["gateway_start_allowed"] = false + data["gateway_start_reason"] = readyErr.Error() + } else { + data["gateway_start_allowed"] = ready + if !ready { + data["gateway_start_reason"] = reason + } + } + + encoded, _ := json.Marshal(data) + return string(encoded) } // findPicoclawBinary locates the picoclaw executable. diff --git a/web/backend/api/gateway_test.go b/web/backend/api/gateway_test.go new file mode 100644 index 000000000..336bb6a0c --- /dev/null +++ b/web/backend/api/gateway_test.go @@ -0,0 +1,122 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "github.com/sipeed/picoclaw/pkg/config" +) + +func TestGatewayStartReady_NoDefaultModel(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + + ready, reason, err := h.gatewayStartReady() + if err != nil { + t.Fatalf("gatewayStartReady() error = %v", err) + } + if ready { + t.Fatalf("gatewayStartReady() ready = true, want false") + } + if reason != "no default model configured" { + t.Fatalf("gatewayStartReady() reason = %q, want %q", reason, "no default model configured") + } +} + +func TestGatewayStartReady_InvalidDefaultModel(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + cfg := config.DefaultConfig() + cfg.Agents.Defaults.Model = "missing-model" + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + ready, reason, err := h.gatewayStartReady() + if err != nil { + t.Fatalf("gatewayStartReady() error = %v", err) + } + if ready { + t.Fatalf("gatewayStartReady() ready = true, want false") + } + if reason == "" { + t.Fatalf("gatewayStartReady() reason is empty") + } +} + +func TestGatewayStartReady_ValidDefaultModel(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + cfg := config.DefaultConfig() + cfg.Agents.Defaults.ModelName = cfg.ModelList[0].ModelName + cfg.ModelList[0].APIKey = "test-key" + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + ready, reason, err := h.gatewayStartReady() + if err != nil { + t.Fatalf("gatewayStartReady() error = %v", err) + } + if !ready { + t.Fatalf("gatewayStartReady() ready = false, want true (reason=%q)", reason) + } +} + +func TestGatewayStartReady_DefaultModelWithoutCredential(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + cfg := config.DefaultConfig() + cfg.Agents.Defaults.ModelName = cfg.ModelList[0].ModelName + cfg.ModelList[0].APIKey = "" + cfg.ModelList[0].AuthMethod = "" + if err := config.SaveConfig(configPath, cfg); err != nil { + t.Fatalf("SaveConfig() error = %v", err) + } + + h := NewHandler(configPath) + ready, reason, err := h.gatewayStartReady() + if err != nil { + t.Fatalf("gatewayStartReady() error = %v", err) + } + if ready { + t.Fatalf("gatewayStartReady() ready = true, want false") + } + if !strings.Contains(reason, "no credentials configured") { + t.Fatalf("gatewayStartReady() reason = %q, want contains %q", reason, "no credentials configured") + } +} + +func TestGatewayStatusIncludesStartConditionWhenNotReady(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.json") + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/gateway/status", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + + allowed, ok := body["gateway_start_allowed"].(bool) + if !ok { + t.Fatalf("gateway_start_allowed missing or not bool: %#v", body["gateway_start_allowed"]) + } + if allowed { + t.Fatalf("gateway_start_allowed = true, want false") + } + if _, ok := body["gateway_start_reason"].(string); !ok { + t.Fatalf("gateway_start_reason missing or not string: %#v", body["gateway_start_reason"]) + } +} diff --git a/web/backend/main.go b/web/backend/main.go index 281930fd2..6dd025e4a 100644 --- a/web/backend/main.go +++ b/web/backend/main.go @@ -107,6 +107,12 @@ func main() { }() } + // Auto-start gateway after backend starts listening. + go func() { + time.Sleep(1 * time.Second) + apiHandler.TryAutoStartGateway() + }() + // Start the Server if err := http.ListenAndServe(addr, handler); err != nil { log.Fatalf("Server failed to start: %v", err) diff --git a/web/frontend/src/api/gateway.ts b/web/frontend/src/api/gateway.ts index 497c510cc..5a58d48f0 100644 --- a/web/frontend/src/api/gateway.ts +++ b/web/frontend/src/api/gateway.ts @@ -2,6 +2,8 @@ interface GatewayStatusResponse { gateway_status: "running" | "starting" | "stopped" | "error" + gateway_start_allowed?: boolean + gateway_start_reason?: string pid?: number logs?: string[] log_total?: number diff --git a/web/frontend/src/components/app-header.tsx b/web/frontend/src/components/app-header.tsx index 0c715819c..1c2ca672e 100644 --- a/web/frontend/src/components/app-header.tsx +++ b/web/frontend/src/components/app-header.tsx @@ -40,7 +40,7 @@ export function AppHeader() { const { state: gwState, loading: gwLoading, - isInitialized, + canStart, start, stop, } = useGateway() @@ -48,11 +48,13 @@ export function AppHeader() { const isRunning = gwState === "running" const isStarting = gwState === "starting" const isStopped = gwState === "stopped" || gwState === "unknown" + const showNotConnectedHint = + canStart && (gwState === "stopped" || gwState === "error") const [showStopDialog, setShowStopDialog] = React.useState(false) const handleGatewayToggle = () => { - if (gwLoading) return + if (gwLoading || (!isRunning && !canStart)) return if (isRunning) { setShowStopDialog(true) } else { @@ -80,7 +82,7 @@ export function AppHeader() { {/* Center prominent connection status */}
- {isInitialized && !isRunning && !isStarting && ( + {showNotConnectedHint && (
@@ -133,7 +135,7 @@ export function AppHeader() { : "" }`} onClick={handleGatewayToggle} - disabled={gwLoading || isStarting} + disabled={gwLoading || isStarting || (!isRunning && !canStart)} > {gwLoading || isStarting ? ( diff --git a/web/frontend/src/components/chat/chat-composer.tsx b/web/frontend/src/components/chat/chat-composer.tsx index 0433c256c..e8bae89b8 100644 --- a/web/frontend/src/components/chat/chat-composer.tsx +++ b/web/frontend/src/components/chat/chat-composer.tsx @@ -4,6 +4,7 @@ import { useTranslation } from "react-i18next" import TextareaAutosize from "react-textarea-autosize" import { Button } from "@/components/ui/button" +import { cn } from "@/lib/utils" interface ChatComposerProps { input: string @@ -21,6 +22,7 @@ export function ChatComposer({ hasDefaultModel, }: ChatComposerProps) { const { t } = useTranslation() + const canInput = isConnected && hasDefaultModel const handleKeyDown = (e: KeyboardEvent) => { if (e.nativeEvent.isComposing) return @@ -32,14 +34,17 @@ export function ChatComposer({ return (
-
+
onInputChange(e.target.value)} onKeyDown={handleKeyDown} placeholder={t("chat.placeholder")} - disabled={!isConnected || !hasDefaultModel} - className="max-h-[200px] min-h-[60px] resize-none border-0 bg-transparent px-2 py-1 text-[15px] shadow-none focus-visible:ring-0 focus-visible:outline-none dark:bg-transparent" + disabled={!canInput} + className={cn( + "max-h-[200px] min-h-[60px] resize-none border-0 bg-transparent px-2 py-1 text-[15px] shadow-none transition-colors focus-visible:ring-0 focus-visible:outline-none dark:bg-transparent", + !canInput && "cursor-not-allowed", + )} minRows={1} maxRows={8} /> diff --git a/web/frontend/src/hooks/use-gateway.ts b/web/frontend/src/hooks/use-gateway.ts index ca305f077..097dc3598 100644 --- a/web/frontend/src/hooks/use-gateway.ts +++ b/web/frontend/src/hooks/use-gateway.ts @@ -1,43 +1,72 @@ import { useAtom } from "jotai" import { useCallback, useEffect, useState } from "react" -import { getGatewayStatus, startGateway, stopGateway } from "@/api/gateway" +import { + type GatewayStatusResponse, + getGatewayStatus, + startGateway, + stopGateway, +} from "@/api/gateway" import { gatewayAtom } from "@/store" // Global variable to ensure we only have one SSE connection let sseInitialized = false export function useGateway() { - const [{ status: state, isInitialized }, setGateway] = useAtom(gatewayAtom) + const [{ status: state, canStart }, setGateway] = useAtom(gatewayAtom) const [loading, setLoading] = useState(false) + const applyGatewayStatus = useCallback( + (data: GatewayStatusResponse) => { + setGateway((prev) => ({ + ...prev, + status: data.gateway_status ?? "unknown", + canStart: data.gateway_start_allowed ?? true, + })) + }, + [setGateway], + ) + // Initialize global SSE connection once useEffect(() => { if (sseInitialized) return sseInitialized = true getGatewayStatus() - .then((data) => { - setGateway({ - status: data.gateway_status ?? "unknown", - isInitialized: true, - }) - }) + .then((data) => applyGatewayStatus(data)) .catch(() => { setGateway({ status: "unknown", - isInitialized: true, + canStart: true, }) }) + const statusPoll = window.setInterval(() => { + getGatewayStatus() + .then((data) => applyGatewayStatus(data)) + .catch(() => { + // ignore polling errors + }) + }, 5000) + // Subscribe to SSE for real-time updates globally const es = new EventSource("/api/gateway/events") es.onmessage = (event) => { try { const data = JSON.parse(event.data) - if (data.gateway_status) { - setGateway((prev) => ({ ...prev, status: data.gateway_status })) + if ( + data.gateway_status || + typeof data.gateway_start_allowed === "boolean" + ) { + setGateway((prev) => ({ + ...prev, + status: data.gateway_status ?? prev.status, + canStart: + typeof data.gateway_start_allowed === "boolean" + ? data.gateway_start_allowed + : prev.canStart, + })) } } catch { // ignore @@ -50,12 +79,15 @@ export function useGateway() { } return () => { + window.clearInterval(statusPoll) es.close() sseInitialized = false } - }, [setGateway]) + }, [applyGatewayStatus, setGateway]) const start = useCallback(async () => { + if (!canStart) return + setLoading(true) try { await startGateway() @@ -63,11 +95,16 @@ export function useGateway() { setGateway((prev) => ({ ...prev, status: "starting" })) } catch (err) { console.error("Failed to start gateway:", err) - setGateway((prev) => ({ ...prev, status: "unknown" })) + try { + const status = await getGatewayStatus() + applyGatewayStatus(status) + } catch { + setGateway((prev) => ({ ...prev, status: "unknown" })) + } } finally { setLoading(false) } - }, [setGateway]) + }, [applyGatewayStatus, canStart, setGateway]) const stop = useCallback(async () => { setLoading(true) @@ -80,5 +117,5 @@ export function useGateway() { } }, []) - return { state, loading, isInitialized, start, stop } + return { state, loading, canStart, start, stop } } diff --git a/web/frontend/src/store/gateway.ts b/web/frontend/src/store/gateway.ts index 1426bbb97..ebe132746 100644 --- a/web/frontend/src/store/gateway.ts +++ b/web/frontend/src/store/gateway.ts @@ -8,12 +8,12 @@ export type GatewayState = | "unknown" export interface GatewayStoreState { - isInitialized: boolean status: GatewayState + canStart: boolean } // Global atom for gateway state export const gatewayAtom = atom({ - isInitialized: false, status: "unknown", + canStart: true, })