fix: harden Pi management readiness

2026-08-05 06:20:25 +02:00
parent 7a8bcacbfe
commit 7df21b5f21
24 changed files with 577 additions and 274 deletions
@@ -10,6 +10,7 @@ import {
  savePiManagementConfig,
  type PiInstallationConfig,
  type PiManagementOptions,
+  type PiManagementStatus,
 } from "../api/pi-management";
 import { Button } from "../components/ui/button";
 import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from "../components/ui/dialog";
@@ -18,9 +19,9 @@ const UPDATE_COMMAND = "thothctl pi update";
 const fieldClass = "h-9 w-full rounded-md border border-input bg-background px-2.5 text-sm shadow-xs outline-none focus-visible:ring-3 focus-visible:ring-ring/25 disabled:cursor-not-allowed disabled:opacity-60";

 type Feedback = { tone: "success" | "error"; message: string } | undefined;
-type CredentialState = "present" | "missing" | undefined;
+type SmokeState = "passed" | "failed" | undefined;

-function configFrom(status: { config: Partial<PiInstallationConfig> }, options: PiManagementOptions): PiInstallationConfig | undefined {
+function suggestedConfig(status: { config: Partial<PiInstallationConfig> }, options: PiManagementOptions): PiInstallationConfig | undefined {
  const provider = status.config.provider && options.providers.includes(status.config.provider)
    ? status.config.provider
    : options.providers[0];
@@ -33,27 +34,55 @@ function configFrom(status: { config: Partial<PiInstallationConfig> }, options:
  return provider && model && reasoning ? { provider, model, reasoning } : undefined;
 }

+function isSupportedConfig(
+  config: Partial<PiInstallationConfig> | undefined,
+  options: PiManagementOptions | undefined,
+): config is PiInstallationConfig {
+  return Boolean(
+    config?.provider
+    && config.model
+    && config.reasoning
+    && options?.providers.includes(config.provider)
+    && options.models.some((model) => model.provider === config.provider && model.id === config.model)
+    && options.reasoning.includes(config.reasoning),
+  );
+}
+
+function sameConfig(a: Partial<PiInstallationConfig> | undefined, b: PiInstallationConfig | undefined): boolean {
+  return Boolean(
+    a?.provider === b?.provider
+    && a?.model === b?.model
+    && a?.reasoning === b?.reasoning,
+  );
+}
+
 function errorMessage(error: unknown, fallback: string): string {
  return asPiManagementApiError(error)?.message ?? fallback;
 }

-function ReadinessRail({ ready, configured, credentialState }: {
+function ReadinessRail({ ready, configured, credentials, smokeState }: {
  ready: boolean;
  configured: boolean;
-  credentialState: CredentialState;
+  credentials: "present" | "missing";
+  smokeState: SmokeState;
 }) {
  return (
    <section
      data-testid="pi-readiness-rail"
      aria-label="Pi readiness sequence"
-      className="grid gap-3 border-l-2 border-border pl-4 sm:grid-cols-3"
+      className="grid gap-3 border-l-2 border-border pl-4 sm:grid-cols-4"
    >
      <RailItem label="Runtime" value={ready ? "Runtime ready" : "Runtime needs attention"} state={ready ? "ready" : "attention"} />
      <RailItem label="Defaults" value={configured ? "Defaults ready" : "Defaults incomplete"} state={configured ? "ready" : "attention"} />
      <RailItem
-        label="Provider smoke"
-        value={credentialState === "present" ? "Credentials present" : credentialState === "missing" ? "Credentials missing" : "Run smoke test"}
-        state={credentialState === "missing" ? "attention" : credentialState === "present" ? "ready" : "idle"}
+        label="Credentials"
+        value={`Credentials ${credentials}`}
+        state={credentials === "present" ? "ready" : "attention"}
+      />
+      <RailItem
+        label="Saved-config test"
+        value={smokeState === "passed" ? "Saved configuration test passed" : smokeState === "failed" ? "Saved configuration test failed" : "Saved configuration test not run"}
+        state={smokeState === "passed" ? "ready" : smokeState === "failed" ? "attention" : "idle"}
      />
    </section>
  );
@@ -78,7 +107,7 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () =>
  const queryClient = useQueryClient();
  const [draft, setDraft] = useState<PiInstallationConfig>();
  const [feedback, setFeedback] = useState<Feedback>();
-  const [credentialState, setCredentialState] = useState<CredentialState>();
+  const [smokeState, setSmokeState] = useState<SmokeState>();
  const [logsRequested, setLogsRequested] = useState(false);
  const statusQuery = useQuery({ queryKey: ["pi-management", "status"], queryFn: getPiManagementStatus, enabled: open });
  const optionsQuery = useQuery({ queryKey: ["pi-management", "options"], queryFn: getPiManagementOptions, enabled: open });
@@ -87,44 +116,59 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () =>
    () => optionsQuery.data?.models.filter((model) => model.provider === draft?.provider) ?? [],
    [draft?.provider, optionsQuery.data?.models],
  );
-  const configured = Boolean(draft?.provider && draft.model && draft.reasoning);
-  const valid = Boolean(
-    draft
-    && optionsQuery.data?.providers.includes(draft.provider)
-    && optionsQuery.data.models.some((model) => model.provider === draft.provider && model.id === draft.model)
-    && optionsQuery.data.reasoning.includes(draft.reasoning),
+  const initialDraft = useMemo(
+    () => statusQuery.data && optionsQuery.data
+      ? suggestedConfig(statusQuery.data, optionsQuery.data)
+      : undefined,
+    [optionsQuery.data, statusQuery.data],
  );
+  const persistedReady = isSupportedConfig(statusQuery.data?.config, optionsQuery.data);
+  const validDraft = isSupportedConfig(draft, optionsQuery.data);
+  const dirty = Boolean(draft && !sameConfig(statusQuery.data?.config, draft));

  useEffect(() => {
    if (!open) {
      setDraft(undefined);
      setFeedback(undefined);
-      setCredentialState(undefined);
+      setSmokeState(undefined);
      setLogsRequested(false);
      queryClient.removeQueries({ queryKey: ["pi-management"] });
    }
  }, [open, queryClient]);

  useEffect(() => {
-    if (draft || !statusQuery.data || !optionsQuery.data) return;
-    setDraft(configFrom(statusQuery.data, optionsQuery.data));
-  }, [draft, optionsQuery.data, statusQuery.data]);
+    if (draft || !initialDraft) return;
+    setDraft(initialDraft);
+  }, [draft, initialDraft]);

  const saveMutation = useMutation({
    mutationFn: savePiManagementConfig,
-    onSuccess: () => {
+    onSuccess: (saved) => {
+      const config = { provider: saved.provider, model: saved.model, reasoning: saved.reasoning };
+      queryClient.setQueryData<PiManagementStatus>(["pi-management", "status"], (current) => (
+        current ? { ...current, config } : current
+      ));
+      setDraft(config);
+      setSmokeState(undefined);
      setFeedback({ tone: "success", message: "Defaults saved." });
-      void queryClient.invalidateQueries({ queryKey: ["pi-management", "status"] });
    },
    onError: (error) => setFeedback({ tone: "error", message: errorMessage(error, "Could not save Pi defaults.") }),
  });
  const smokeMutation = useMutation({
    mutationFn: runPiManagementTest,
    onSuccess: (result) => {
-      setCredentialState(result.ready ? "present" : "missing");
-      setFeedback({ tone: result.ready ? "success" : "error", message: result.message ?? (result.ready ? "Pi smoke test passed." : "Pi smoke test failed.") });
+      setSmokeState(result.ready ? "passed" : "failed");
+      setFeedback({
+        tone: result.ready ? "success" : "error",
+        message: result.ready
+          ? "Saved configuration test passed."
+          : `Saved configuration test failed.${result.message ? ` ${result.message}` : ""}`,
+      });
+    },
+    onError: (error) => {
+      setSmokeState("failed");
+      setFeedback({ tone: "error", message: errorMessage(error, "Could not test the saved Pi defaults.") });
    },
-    onError: (error) => setFeedback({ tone: "error", message: errorMessage(error, "Could not run the Pi smoke test.") }),
  });

  async function copyUpdateCommand() {
@@ -138,15 +182,29 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () =>
  }

  function saveDefaults() {
-    if (!draft || !valid) {
+    if (!draft || !validDraft) {
      setFeedback({ tone: "error", message: "Choose a supported provider, model, and reasoning level." });
      return;
    }
    saveMutation.mutate(draft);
  }

+  function updateDraft(update: (current: PiInstallationConfig) => PiInstallationConfig) {
+    setDraft((current) => current ? update(current) : current);
+    setSmokeState(undefined);
+    setFeedback(undefined);
+  }
+
+  function testSavedDefaults() {
+    if (!persistedReady || dirty) {
+      setFeedback({ tone: "error", message: "Save supported defaults before running the test." });
+      return;
+    }
+    smokeMutation.mutate();
+  }
+
  const forbidden = asPiManagementApiError(statusQuery.error)?.code === "pi_management_forbidden";
-  const loading = statusQuery.isLoading || optionsQuery.isLoading || !draft;
+  const loading = statusQuery.isLoading || optionsQuery.isLoading || Boolean(!draft && initialDraft);
  const unavailable = statusQuery.isError || optionsQuery.isError;

  return (
@@ -155,7 +213,7 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () =>
        <DialogHeader className="border-b border-border/70 px-5 pb-4 pt-5 pr-12">
          <p className="thot-label">Installation controls</p>
          <DialogTitle>Pi management</DialogTitle>
-          <DialogDescription>Review the bundled runtime, set safe defaults, and run a sanitized provider check.</DialogDescription>
+          <DialogDescription>Review the bundled runtime, set safe defaults, and test the saved provider configuration.</DialogDescription>
        </DialogHeader>
        <Button aria-label="Close Pi management" className="absolute right-2 top-2" size="icon-sm" variant="ghost" onClick={onClose}><X /></Button>
        <div className="max-h-[calc(100vh-11rem)] overflow-y-auto px-5 py-5">
@@ -169,7 +227,7 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () =>
              <p className="font-semibold">{errorMessage(statusQuery.error ?? optionsQuery.error, "Pi management is unavailable")}</p>
              <Button className="mt-3" size="sm" variant="outline" onClick={() => { void statusQuery.refetch(); void optionsQuery.refetch(); }}>Retry</Button>
            </div>
-          ) : loading ? <p className="text-sm text-muted-foreground">Loading Pi management…</p> : statusQuery.data && optionsQuery.data && draft && (
+          ) : loading ? <p className="text-sm text-muted-foreground">Loading Pi management…</p> : statusQuery.data && optionsQuery.data && (
            <div className="grid gap-6">
              <div className="flex flex-wrap items-start justify-between gap-3">
                <div>
@@ -181,7 +239,12 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () =>
                </p>
              </div>

-              <ReadinessRail ready={statusQuery.data.ready} configured={configured} credentialState={credentialState} />
+              <ReadinessRail
+                ready={statusQuery.data.ready}
+                configured={persistedReady}
+                credentials={statusQuery.data.credentials ?? "missing"}
+                smokeState={smokeState}
+              />

              {statusQuery.data.message && <p role="status" aria-live="polite" className="rounded-md border border-amber-500/30 bg-amber-500/10 px-3 py-2 text-sm text-amber-900 dark:text-amber-200">{statusQuery.data.message}</p>}
              {feedback && <p role="status" aria-label="Pi management feedback" aria-live="polite" className={`flex items-start gap-2 rounded-md border px-3 py-2 text-sm ${feedback.tone === "error" ? "border-destructive/30 bg-destructive/5" : "border-border bg-muted/50"}`}>
@@ -193,30 +256,55 @@ export function PiManagement({ open, onClose }: { open: boolean; onClose: () =>
                <div className="flex flex-wrap items-baseline justify-between gap-2">
                  <div><h3 className="font-heading text-sm font-semibold">Installation defaults</h3><p className="mt-1 text-sm text-muted-foreground">These choices apply to new Pi work. Credentials remain outside this browser.</p></div>
                </div>
-                <div className="mt-4 grid gap-3 sm:grid-cols-3">
-                  <Field label="Provider">
-                    <select aria-label="Provider" className={fieldClass} value={draft.provider} onChange={(event) => {
-                      const provider = event.target.value;
-                      setDraft((current) => current ? { ...current, provider, model: optionsQuery.data.models.find((model) => model.provider === provider)?.id ?? "" } : current);
-                    }}>
-                      {optionsQuery.data.providers.map((provider) => <option key={provider} value={provider}>{provider}</option>)}
-                    </select>
-                  </Field>
-                  <Field label="Model">
-                    <select aria-label="Model" className={fieldClass} value={draft.model} onChange={(event) => setDraft((current) => current ? { ...current, model: event.target.value } : current)}>
-                      {models.map((model) => <option key={model.id} value={model.id}>{model.id === "glm-5.2" ? "GLM 5.2" : model.id}</option>)}
-                    </select>
-                  </Field>
-                  <Field label="Reasoning level">
-                    <select aria-label="Reasoning level" className={fieldClass} value={draft.reasoning} onChange={(event) => setDraft((current) => current ? { ...current, reasoning: event.target.value as PiInstallationConfig["reasoning"] } : current)}>
-                      {optionsQuery.data.reasoning.map((reasoning) => <option key={reasoning} value={reasoning}>{reasoning}</option>)}
-                    </select>
-                  </Field>
-                </div>
-                <div className="mt-4 flex flex-wrap gap-2">
-                  <Button size="sm" variant="outline" disabled={saveMutation.isPending} onClick={saveDefaults}>{saveMutation.isPending ? <LoaderCircle className="animate-spin motion-reduce:animate-none" /> : <ClipboardCheck />}Save defaults</Button>
-                  <Button size="sm" variant="outline" disabled={smokeMutation.isPending} onClick={() => smokeMutation.mutate()}>{smokeMutation.isPending ? <LoaderCircle className="animate-spin motion-reduce:animate-none" /> : <FlaskConical />}Run smoke test</Button>
-                </div>
+                {draft ? (
+                  <>
+                    <div className="mt-4 grid gap-3 sm:grid-cols-3">
+                      <Field label="Provider">
+                        <select aria-label="Provider" className={fieldClass} value={draft.provider} onChange={(event) => {
+                          const provider = event.target.value;
+                          updateDraft((current) => ({
+                            ...current,
+                            provider,
+                            model: optionsQuery.data.models.find((model) => model.provider === provider)?.id ?? "",
+                          }));
+                        }}>
+                          {optionsQuery.data.providers.map((provider) => <option key={provider} value={provider}>{provider}</option>)}
+                        </select>
+                      </Field>
+                      <Field label="Model">
+                        <select aria-label="Model" className={fieldClass} value={draft.model} onChange={(event) => updateDraft((current) => ({ ...current, model: event.target.value }))}>
+                          {models.map((model) => <option key={model.id} value={model.id}>{model.id === "glm-5.2" ? "GLM 5.2" : model.id}</option>)}
+                        </select>
+                      </Field>
+                      <Field label="Reasoning level">
+                        <select aria-label="Reasoning level" className={fieldClass} value={draft.reasoning} onChange={(event) => updateDraft((current) => ({ ...current, reasoning: event.target.value as PiInstallationConfig["reasoning"] }))}>
+                          {optionsQuery.data.reasoning.map((reasoning) => <option key={reasoning} value={reasoning}>{reasoning}</option>)}
+                        </select>
+                      </Field>
+                    </div>
+                    {dirty && <p className="mt-3 text-sm text-amber-800 dark:text-amber-300">
+                      {persistedReady ? "Changes are not saved yet." : "Suggested choices are not saved yet."}
+                    </p>}
+                    <div className="mt-4 flex flex-wrap gap-2">
+                      <Button size="sm" variant="outline" disabled={saveMutation.isPending || !validDraft || !dirty} onClick={saveDefaults}>{saveMutation.isPending ? <LoaderCircle className="animate-spin motion-reduce:animate-none" /> : <ClipboardCheck />}Save defaults</Button>
+                      <Button size="sm" variant="outline" disabled={smokeMutation.isPending || dirty || !persistedReady} onClick={testSavedDefaults}>{smokeMutation.isPending ? <LoaderCircle className="animate-spin motion-reduce:animate-none" /> : <FlaskConical />}Test saved defaults</Button>
+                    </div>
+                    {dirty && <p className="mt-2 text-xs text-muted-foreground">Save these changes before testing. The test always uses saved defaults.</p>}
+                    {!dirty && !persistedReady && <p className="mt-2 text-xs text-muted-foreground">Save supported defaults before testing. The test always uses saved defaults.</p>}
+                  </>
+                ) : (
+                  <div className="mt-4">
+                    <div role="alert" aria-label="Pi configuration incomplete" className="rounded-md border border-amber-500/30 bg-amber-500/10 p-4 text-sm text-amber-900 dark:text-amber-200">
+                      <p className="font-semibold">No enabled provider and model choices are available.</p>
+                      <p className="mt-1">Check the host-managed Pi model configuration, then retry this panel.</p>
+                    </div>
+                    <div className="mt-4 flex flex-wrap gap-2">
+                      <Button size="sm" variant="outline" disabled><ClipboardCheck />Save defaults</Button>
+                      <Button size="sm" variant="outline" disabled><FlaskConical />Test saved defaults</Button>
+                      <Button size="sm" variant="ghost" onClick={() => { void statusQuery.refetch(); void optionsQuery.refetch(); }}>Retry choices</Button>
+                    </div>
+                  </div>
+                )}
              </section>

              <section aria-label="Sanitized diagnostics" className="border-t border-border/70 pt-5">