From 0389e2e0b0bcc3ccb7a1eca5cfb2c006bdd6723a Mon Sep 17 00:00:00 2001 From: Blomios Date: Wed, 8 Jul 2026 08:11:24 +0200 Subject: [PATCH] =?UTF-8?q?fix(first-run):=20d=C3=A9couple=20`busy`=20de?= =?UTF-8?q?=20la=20d=C3=A9tection=20des=20CLI=20(#28)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Même backend réparé, le wizard restait à la merci d'une détection qui ne répond pas : `reload()` et `detect()` awaitaient `detectProfiles` sous le même drapeau `busy` qui grise « Save and continue » et « Detect installed CLIs ». Une promesse IPC jamais résolue laissait donc les deux boutons grisés à vie — sans issue pour l'utilisateur. La détection redevient ce qu'elle est : une étape best-effort, jamais bloquante. - `busy` ne garde plus que ce dont le wizard ne peut pas se passer (`firstRunState`) ou qui mute l'état (`configureProfiles`). Il ne dépend plus jamais de `detectProfiles`. L'invariant est documenté en tête de module. - Un drapeau `detecting` distinct suit la sonde et n'inhibe aucune action. Il est relâché par un timer (`DETECT_TIMEOUT_MS`), jamais par la seule promesse : celle-ci peut rester pendante indéfiniment. - Un identifiant de tour monotone (`detectRun`) invalide les tours périmés et ceux qui survivent au démontage, évitant un `setState` hors montage. - `reload()` rend les lignes immédiatement puis lance la détection en tâche détachée. Si elle échoue, les lignes restent cochables à la main ; seul le bouton explicite remonte l'erreur. Tests: vitest 59 fichiers / 569 tests verts, tsc --noEmit exit 0. Co-Authored-By: Claude Opus 4.8 --- .../first-run/FirstRunWizard.test.tsx | 66 +++++++++ .../src/features/first-run/FirstRunWizard.tsx | 7 + .../src/features/first-run/useFirstRun.ts | 137 +++++++++++++----- 3 files changed, 172 insertions(+), 38 deletions(-) diff --git a/frontend/src/features/first-run/FirstRunWizard.test.tsx b/frontend/src/features/first-run/FirstRunWizard.test.tsx index 03caf1a..6bde8e6 100644 --- a/frontend/src/features/first-run/FirstRunWizard.test.tsx +++ b/frontend/src/features/first-run/FirstRunWizard.test.tsx @@ -6,6 +6,7 @@ */ import { describe, it, expect, vi } from "vitest"; import { + act, render, screen, waitFor, @@ -13,9 +14,11 @@ import { } from "@testing-library/react"; import { MockProfileGateway } from "@/adapters/mock"; +import type { ProfileAvailability } from "@/domain"; import type { Gateways } from "@/ports"; import { DIProvider } from "@/app/di"; import { FirstRunWizard } from "./FirstRunWizard"; +import { DETECT_TIMEOUT_MS } from "./useFirstRun"; function renderWizard( profile: MockProfileGateway = new MockProfileGateway(), @@ -301,6 +304,69 @@ describe("FirstRunWizard — OpenAI-compatible local/LAN profile (ticket #14)", }); }); +// Ticket #28 — the wizard must survive a detection that never answers. When the +// backend `detect_profiles` command panics, the `invoke` promise is neither +// resolved nor rejected: nothing after `await detectProfiles(...)` ever runs. +// `busy` must therefore never be tied to detection, or both buttons stay greyed +// out for good and the first run cannot be completed. +describe("FirstRunWizard — detection that never answers (ticket #28)", () => { + /** A gateway whose `detectProfiles` promise stays pending forever. */ + class HangingDetectGateway extends MockProfileGateway { + override detectProfiles(): Promise { + return new Promise(() => {}); + } + } + + it("keeps Save and Detect enabled once firstRunState answered", async () => { + renderWizard(new HangingDetectGateway()); + await waitForLoaded(); + + // The rows are rendered, so the blocking load is over: both actions are live. + const save = screen.getByRole("button", { name: "Save and continue" }); + const detect = screen.getByRole("button", { name: "Detect installed CLIs" }); + expect((save as HTMLButtonElement).disabled).toBe(false); + expect((detect as HTMLButtonElement).disabled).toBe(false); + + // Detection is merely reported as in flight, never as a blocking state. + expect(screen.getByRole("status").textContent).toMatch(/detecting/i); + + // Re-probing by hand must not freeze them either. + fireEvent.click(detect); + expect((save as HTMLButtonElement).disabled).toBe(false); + expect((detect as HTMLButtonElement).disabled).toBe(false); + }); + + it("still finishes the first run while detection hangs", async () => { + const profile = new HangingDetectGateway(); + const { onDone } = renderWizard(profile, vi.fn()); + await waitForLoaded(); + + // Nothing got pre-checked (detection never answered): tick a profile by hand. + fireEvent.click(screen.getByLabelText("use Claude Code")); + fireEvent.click(screen.getByRole("button", { name: "Save and continue" })); + + await waitFor(() => expect(onDone).toHaveBeenCalled()); + const saved = await profile.listProfiles(); + expect(saved.map((p) => p.command)).toEqual(["claude"]); + }); + + it("drops the detecting flag on timeout even if the promise never settles", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + renderWizard(new HangingDetectGateway()); + await waitForLoaded(); + expect(screen.getByRole("status").textContent).toMatch(/detecting/i); + + await act(async () => { + vi.advanceTimersByTime(DETECT_TIMEOUT_MS + 1); + }); + expect(screen.queryByRole("status")).toBeNull(); + } finally { + vi.useRealTimers(); + } + }); +}); + describe("FirstRunWizard reopening after the first run (forceOpen)", () => { /** A gateway whose first run is already done (profiles configured). */ async function configuredGateway() { diff --git a/frontend/src/features/first-run/FirstRunWizard.tsx b/frontend/src/features/first-run/FirstRunWizard.tsx index 1476ed0..a84a224 100644 --- a/frontend/src/features/first-run/FirstRunWizard.tsx +++ b/frontend/src/features/first-run/FirstRunWizard.tsx @@ -78,6 +78,8 @@ export function FirstRunWizard({ )} + {/* `detecting` deliberately does NOT disable this button (ticket #28): + detection is best-effort and may never answer. */} + {vm.detecting && ( + + Detecting… + + )}
    diff --git a/frontend/src/features/first-run/useFirstRun.ts b/frontend/src/features/first-run/useFirstRun.ts index 38497cc..ff7b9eb 100644 --- a/frontend/src/features/first-run/useFirstRun.ts +++ b/frontend/src/features/first-run/useFirstRun.ts @@ -3,9 +3,17 @@ * (L5). Owns the wizard state (selectable reference candidates + selection + * availability) and the actions. Consumes the {@link ProfileGateway} * exclusively — never `invoke()` — so the feature is testable with a mock. + * + * **Invariant (ticket #28)**: `busy` gates only the requests the wizard cannot + * render without (`firstRunState`) or that mutate state (`configureProfiles`). + * It NEVER depends on `detectProfiles`: CLI detection is a detached, best-effort + * step tracked by the separate {@link FirstRunViewModel.detecting} flag, bounded + * by {@link DETECT_TIMEOUT_MS}. A detection that panics backend-side (leaving the + * `invoke` promise forever pending) must not freeze "Save and continue" / + * "Detect installed CLIs". */ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import type { AgentProfile, @@ -15,6 +23,9 @@ import type { } from "@/domain"; import { useGateways } from "@/app/di"; +/** How long the UI waits for a detection round before giving up on it. */ +export const DETECT_TIMEOUT_MS = 3_000; + /** One row in the wizard: a candidate profile, selected state, availability. */ export interface WizardEntry { profile: AgentProfile; @@ -31,8 +42,10 @@ export interface FirstRunViewModel { entries: WizardEntry[]; /** Last error message, or null. */ error: string | null; - /** Whether a request is in flight. */ + /** Whether a blocking request is in flight (load / save). Never detection. */ busy: boolean; + /** Whether a best-effort detection round is in flight. Never disables actions. */ + detecting: boolean; /** Toggles selection of a candidate by id. */ toggle: (id: string) => void; /** Replaces a candidate's profile (edited command/args/injection). */ @@ -60,41 +73,101 @@ export function useFirstRun(): FirstRunViewModel { const [entries, setEntries] = useState([]); const [error, setError] = useState(null); const [busy, setBusy] = useState(false); + const [detecting, setDetecting] = useState(false); + + /** Monotonic id of the latest detection round; older rounds are ignored. */ + const detectRun = useRef(0); + const detectTimer = useRef | null>(null); + + useEffect( + () => () => { + // Unmount: invalidate any in-flight round (its promise may never settle). + detectRun.current += 1; + if (detectTimer.current !== null) clearTimeout(detectTimer.current); + }, + [], + ); + + /** + * Best-effort detection round, detached from `busy`. A round is abandoned + * after {@link DETECT_TIMEOUT_MS} — the underlying promise may stay pending + * forever (backend panic ⇒ no IPC response), so `detecting` is released by a + * timer, never by the promise alone. `preselect` ticks the installed CLIs (the + * auto-detection on open); the manual button only refreshes availability. + */ + const runDetection = useCallback( + async ( + candidates: AgentProfile[], + opts: { preselect: boolean; reportError: boolean }, + ) => { + const run = detectRun.current + 1; + detectRun.current = run; + const stale = () => detectRun.current !== run; + + setDetecting(true); + if (detectTimer.current !== null) clearTimeout(detectTimer.current); + detectTimer.current = setTimeout(() => { + if (!stale()) setDetecting(false); + }, DETECT_TIMEOUT_MS); + + try { + const results: ProfileAvailability[] = + await profile.detectProfiles(candidates); + if (stale()) return; + const byId = new Map(results.map((r) => [r.profile.id, r.available])); + setEntries((prev) => + prev.map((e) => { + const available = byId.get(e.profile.id) ?? e.available; + return { + ...e, + available, + selected: opts.preselect ? available === true : e.selected, + }; + }), + ); + } catch (e) { + // Detection unavailable: rows stay as they are (the user can tick them + // by hand and re-run). Only the explicit button surfaces the failure. + if (stale()) return; + if (opts.reportError) setError(describe(e)); + } finally { + if (!stale()) { + if (detectTimer.current !== null) clearTimeout(detectTimer.current); + detectTimer.current = null; + setDetecting(false); + } + } + }, + [profile], + ); const reload = useCallback(async () => { setBusy(true); setError(null); + let refs: AgentProfile[] | null = null; try { const state: FirstRunState = await profile.firstRunState(); setIsFirstRun(state.isFirstRun); - const refs = state.referenceProfiles; + refs = state.referenceProfiles; // Show the rows immediately (nothing pre-checked yet). setEntries( refs.map((p) => ({ profile: p, selected: false, available: null })), ); - // Auto-detect once on open: only the CLIs actually installed end up - // pre-checked. The manual "Detect installed CLIs" button stays available to - // re-probe. If detection is unavailable, rows stay unchecked (the user can - // still tick them by hand and re-run detection). - try { - const results = await profile.detectProfiles(refs); - const byId = new Map(results.map((r) => [r.profile.id, r.available])); - setEntries( - refs.map((p) => { - const available = byId.get(p.id) ?? null; - return { profile: p, selected: available === true, available }; - }), - ); - } catch { - /* detection unavailable: leave rows unchecked */ - } } catch (e) { setError(describe(e)); setIsFirstRun(false); } finally { + // Released as soon as the rows are rendered — detection is not awaited. setBusy(false); } - }, [profile]); + + // Auto-detect once on open, detached: only the CLIs actually installed end + // up pre-checked. The manual "Detect installed CLIs" button stays available + // to re-probe, whatever happens to this round. + if (refs !== null) { + void runDetection(refs, { preselect: true, reportError: false }); + } + }, [profile, runDetection]); useEffect(() => { void reload(); @@ -119,25 +192,12 @@ export function useFirstRun(): FirstRunViewModel { }, []); const detect = useCallback(async () => { - setBusy(true); setError(null); - try { - const candidates = entries.map((e) => e.profile); - const results: ProfileAvailability[] = - await profile.detectProfiles(candidates); - const byId = new Map(results.map((r) => [r.profile.id, r.available])); - setEntries((prev) => - prev.map((e) => ({ - ...e, - available: byId.get(e.profile.id) ?? e.available, - })), - ); - } catch (e) { - setError(describe(e)); - } finally { - setBusy(false); - } - }, [profile, entries]); + await runDetection( + entries.map((e) => e.profile), + { preselect: false, reportError: true }, + ); + }, [runDetection, entries]); const finish = useCallback(async () => { setBusy(true); @@ -158,6 +218,7 @@ export function useFirstRun(): FirstRunViewModel { entries, error, busy, + detecting, toggle, updateProfile, remove,