app/src/hooks/useProjects.test.ts

@@ -0,0 +1,142 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { act, renderHook } from "@testing-library/react";
+import { useProjects } from "./useProjects";
+import { useAppState } from "../store/appState";
+import type { Project, ProjectStatus } from "../lib/types";
+
+const startProjectContainer = vi.fn();
+const stopProjectContainer = vi.fn();
+const rebuildProjectContainer = vi.fn();
+const listProjects = vi.fn();
+
+vi.mock("../lib/tauri-commands", () => ({
+  startProjectContainer: (id: string) => startProjectContainer(id),
+  stopProjectContainer: (id: string) => stopProjectContainer(id),
+  rebuildProjectContainer: (id: string) => rebuildProjectContainer(id),
+  listProjects: () => listProjects(),
+  addProject: vi.fn(),
+  removeProject: vi.fn(),
+  updateProject: vi.fn(),
+}));
+
+const project = (status: ProjectStatus): Project =>
+  ({ id: "p1", name: "whp", status }) as unknown as Project;
+
+/** The status the sidebar row and Project Home both read. */
+const statusOf = () => useAppState.getState().projects.find((p) => p.id === "p1")?.status;
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  useAppState.setState({ projects: [project("running")] });
+});
+
+/**
+ * The optimistic status is what makes a click move the row immediately. It is
+ * also what strands the row when the command it was betting on never ran.
+ *
+ * Since every lifecycle command started taking the per-project lock and failing
+ * fast, all three can be refused *before* the backend changes anything — and
+ * `stop` could not fail this way at all before, because it took no exclusion.
+ * `isTransitioning` disables both Start and Stop, and the only thing that
+ * clears it is `reconcileProjectStatuses`, which runs once when Docker first
+ * appears. So a stale optimistic status is not a cosmetic problem: it is a row
+ * the user cannot operate again until the app is restarted.
+ */
+describe("useProjects puts the status back when a refused command never ran", () => {
+  const refusal =
+    "This project's snapshot is being compacted. Wait for it to finish before starting or recreating its container.";
+
+  it("does not leave a refused Stop showing 'stopping' forever", async () => {
+    stopProjectContainer.mockRejectedValue(refusal);
+    // The lock is taken before `update_status`, so the backend still holds the
+    // truth — which is why re-reading it is the correction, not a guess.
+    listProjects.mockResolvedValue([project("running")]);
+
+    const { result } = renderHook(() => useProjects());
+    await act(async () => {
+      await expect(result.current.stop("p1")).rejects.toBe(refusal);
+    });
+
+    expect(statusOf()).toBe("running");
+  });
+
+  it("does not leave a refused Start showing 'starting' forever", async () => {
+    useAppState.setState({ projects: [project("stopped")] });
+    startProjectContainer.mockRejectedValue(refusal);
+    listProjects.mockResolvedValue([project("stopped")]);
+
+    const { result } = renderHook(() => useProjects());
+    await act(async () => {
+      await expect(result.current.start("p1")).rejects.toBe(refusal);
+    });
+
+    expect(statusOf()).toBe("stopped");
+  });
+
+  it("does not leave a refused Reset showing 'starting' forever", async () => {
+    rebuildProjectContainer.mockRejectedValue(refusal);
+    listProjects.mockResolvedValue([project("running")]);
+
+    const { result } = renderHook(() => useProjects());
+    await act(async () => {
+      await expect(result.current.rebuild("p1")).rejects.toBe(refusal);
+    });
+
+    expect(statusOf()).toBe("running");
+  });
+
+  it("falls back to what was on screen when even the re-read fails", async () => {
+    // Two failures in a row must not land on the one state there is no way out
+    // of. The backend's answer is preferred, but "unknown" is never a reason to
+    // keep showing a transition that is not happening.
+    stopProjectContainer.mockRejectedValue(refusal);
+    listProjects.mockRejectedValue("Docker is not running");
+
+    const { result } = renderHook(() => useProjects());
+    await act(async () => {
+      await expect(result.current.stop("p1")).rejects.toBe(refusal);
+    });
+
+    expect(statusOf()).toBe("running");
+  });
+
+  it("prefers the backend's answer over the status it captured", async () => {
+    // A start that dies half-way really has changed the world, so the captured
+    // status would be a lie. `listProjects` is the thing that knows.
+    useAppState.setState({ projects: [project("stopped")] });
+    startProjectContainer.mockRejectedValue("container exited during startup");
+    listProjects.mockResolvedValue([project("error")]);
+
+    const { result } = renderHook(() => useProjects());
+    await act(async () => {
+      await expect(result.current.start("p1")).rejects.toBe(
+        "container exited during startup",
+      );
+    });
+
+    expect(statusOf()).toBe("error");
+  });
+
+  it("still paints the optimistic status on the way in", async () => {
+    let release: (() => void) | undefined;
+    stopProjectContainer.mockReturnValue(
+      new Promise<void>((resolve) => {
+        release = resolve;
+      }),
+    );
+    listProjects.mockResolvedValue([project("stopped")]);
+
+    const { result } = renderHook(() => useProjects());
+    let pending: Promise<void> | undefined;
+    act(() => {
+      pending = result.current.stop("p1");
+    });
+    expect(statusOf()).toBe("stopping");
+
+    await act(async () => {
+      release?.();
+      await pending;
+    });
+    expect(statusOf()).toBe("stopped");
+  });
+});

app/src/hooks/useProjects.ts

@@ -2,7 +2,7 @@ import { useCallback } from "react";
 import { useShallow } from "zustand/react/shallow";
 import { useAppState } from "../store/appState";
 import * as commands from "../lib/tauri-commands";
-import type { ProjectPath } from "../lib/types";
+import type { ProjectPath, ProjectStatus } from "../lib/types";
 
 export function useProjects() {
   const {
@@ -61,34 +61,85 @@ export function useProjects() {
     [updateProjectInList],
   );
 
+  /**
+   * Paint the optimistic status, run the command, and **put the status back if
+   * the command never happened.**
+   *
+   * The optimistic write exists so a click moves the row immediately. It used
+   * to be safe to leave in place on failure, because the only way these three
+   * commands could fail was after the backend had already started changing
+   * things — so a stale "starting" was at worst premature.
+   *
+   * That stopped being true when every lifecycle command started taking the
+   * per-project lock and failing fast: a compaction, a reset or another start
+   * holding the project now refuses `start`, `stop` **and** `rebuild` before
+   * one byte of state changes. `stop` in particular could not fail this way at
+   * all before — it took no exclusion. The optimistic paint then has nothing to
+   * become, and `isTransitioning` disables both Start and Stop, so the row is
+   * stuck: the only thing that clears it is `reconcileProjectStatuses`, which
+   * runs once, from `App.tsx`, when Docker first appears. A restart.
+   *
+   * Re-reading the list is preferred over restoring what was on screen, because
+   * a refusal is not the only way these throw — a start that dies half-way
+   * really has changed the world, and `listProjects` is the thing that knows.
+   * The captured status is only the fallback for when that call fails too:
+   * leaving the row transitioning is the one outcome the user cannot get out
+   * of, so it must not be what a second failure lands on.
+   */
+  const withOptimisticStatus = useCallback(
+    async <T,>(
+      id: string,
+      status: "starting" | "stopping",
+      run: () => Promise<T>,
+    ): Promise<T> => {
+      const previous: ProjectStatus | null =
+        useAppState.getState().projects.find((p) => p.id === id)?.status ?? null;
+      setOptimisticStatus(id, status);
+      try {
+        return await run();
+      } catch (e) {
+        try {
+          setProjects(await commands.listProjects());
+        } catch {
+          const project = useAppState.getState().projects.find((p) => p.id === id);
+          if (project && previous) updateProjectInList({ ...project, status: previous });
+        }
+        // Rethrown unchanged: `useProjectActions` is what turns this into a
+        // toast, and it must still see the original failure.
+        throw e;
+      }
+    },
+    [setOptimisticStatus, setProjects, updateProjectInList],
+  );
+
   const start = useCallback(
-    async (id: string) => {
+    (id: string) =>
-      setOptimisticStatus(id, "starting");
+      withOptimisticStatus(id, "starting", async () => {
         const updated = await commands.startProjectContainer(id);
         updateProjectInList(updated);
         return updated;
-    },
+      }),
-    [updateProjectInList, setOptimisticStatus],
+    [updateProjectInList, withOptimisticStatus],
   );
 
   const stop = useCallback(
-    async (id: string) => {
+    (id: string) =>
-      setOptimisticStatus(id, "stopping");
+      withOptimisticStatus(id, "stopping", async () => {
         await commands.stopProjectContainer(id);
         const list = await commands.listProjects();
         setProjects(list);
-    },
+      }),
-    [setProjects, setOptimisticStatus],
+    [setProjects, withOptimisticStatus],
   );
 
   const rebuild = useCallback(
-    async (id: string) => {
+    (id: string) =>
-      setOptimisticStatus(id, "starting");
+      withOptimisticStatus(id, "starting", async () => {
         const updated = await commands.rebuildProjectContainer(id);
         updateProjectInList(updated);
         return updated;
-    },
+      }),
-    [updateProjectInList, setOptimisticStatus],
+    [updateProjectInList, withOptimisticStatus],
   );
 
   const update = useCallback(

app/src/lib/types.ts

@@ -52,7 +52,13 @@ export interface Project {
    *  certificate file or a directory of them). null falls back to
    *  `AppSettings.ca_cert_path`. Changing it recreates the container. */
   ca_cert_path: string | null;
-  git_token: string | null;
+  /** **Write-only.** Rust marks this `#[serde(skip_serializing)]`, so it is
+   *  absent from every project the backend hands back — reading it gives
+   *  `undefined`, never `null`. Optional here so that is the type, and so a
+   *  `=== null` test against it is a compile error rather than a branch that
+   *  silently never runs. Still sent on the way *in*: this is how the secret
+   *  is set. */
+  git_token?: string | null;
   git_user_name: string | null;
   git_user_email: string | null;
   custom_env_vars: EnvVar[];
@@ -96,11 +102,23 @@ export type BedrockAuthMethod = "static_credentials" | "profile" | "bearer_token
 export interface BedrockConfig {
   auth_method: BedrockAuthMethod;
   aws_region: string;
-  aws_access_key_id: string | null;
+  /** **Write-only.** Rust marks this `#[serde(skip_serializing)]`, so it is
-  aws_secret_access_key: string | null;
+   *  absent from every project the backend hands back — reading it gives
-  aws_session_token: string | null;
+   *  `undefined`, never `null`. Optional here so that is the type, and so a
+   *  `=== null` test against it is a compile error rather than a branch that
+   *  silently never runs. Still sent on the way *in*: this is how the secret
+   *  is set. */
+  aws_access_key_id?: string | null;
+  aws_secret_access_key?: string | null;
+  aws_session_token?: string | null;
   aws_profile: string | null;
-  aws_bearer_token: string | null;
+  /** **Write-only.** Rust marks this `#[serde(skip_serializing)]`, so it is
+   *  absent from every project the backend hands back — reading it gives
+   *  `undefined`, never `null`. Optional here so that is the type, and so a
+   *  `=== null` test against it is a compile error rather than a branch that
+   *  silently never runs. Still sent on the way *in*: this is how the secret
+   *  is set. */
+  aws_bearer_token?: string | null;
   model_id: string | null;
   disable_prompt_caching: boolean;
   service_tier: string | null;
@@ -127,7 +145,13 @@ export interface LlamaCppConfig {
  *  implement the **Anthropic** Messages API — e.g. LiteLLM. */
 export interface OpenAiCompatibleConfig {
   base_url: string;
-  api_key: string | null;
+  /** **Write-only.** Rust marks this `#[serde(skip_serializing)]`, so it is
+   *  absent from every project the backend hands back — reading it gives
+   *  `undefined`, never `null`. Optional here so that is the type, and so a
+   *  `=== null` test against it is a compile error rather than a branch that
+   *  silently never runs. Still sent on the way *in*: this is how the secret
+   *  is set. */
+  api_key?: string | null;
   model_id: string | null;
   /** See `OllamaConfig.haiku_model_id`. */
   haiku_model_id: string | null;