From 8cdc44de6ec77bfbd4b5a15d7d1a15296fdd9015 Mon Sep 17 00:00:00 2001 From: Blomios Date: Mon, 13 Jul 2026 22:53:46 +0200 Subject: [PATCH] =?UTF-8?q?feat(first-run):=20conserver=20les=20infos=20du?= =?UTF-8?q?=20profil=20courant=20=C3=A0=20l'=C3=A9dition=20(#44)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extrait la logique de dérivation des entrées du wizard dans un module dédié `wizardEntries` afin de préserver les informations du profil courant lors de l'ouverture de l'affichage d'édition des profils, au lieu de repartir d'un état vide. Couvre le comportement par des tests unitaires (wizardEntries + FirstRunWizard). Co-Authored-By: Claude Opus 4.8 --- .../first-run/FirstRunWizard.test.tsx | 189 +++++++++++++++++- .../src/features/first-run/FirstRunWizard.tsx | 5 +- .../src/features/first-run/useFirstRun.ts | 60 ++++-- .../features/first-run/wizardEntries.test.ts | 173 ++++++++++++++++ .../src/features/first-run/wizardEntries.ts | 73 +++++++ 5 files changed, 476 insertions(+), 24 deletions(-) create mode 100644 frontend/src/features/first-run/wizardEntries.test.ts create mode 100644 frontend/src/features/first-run/wizardEntries.ts diff --git a/frontend/src/features/first-run/FirstRunWizard.test.tsx b/frontend/src/features/first-run/FirstRunWizard.test.tsx index 899e66b..cad4762 100644 --- a/frontend/src/features/first-run/FirstRunWizard.test.tsx +++ b/frontend/src/features/first-run/FirstRunWizard.test.tsx @@ -14,7 +14,11 @@ import { } from "@testing-library/react"; import { MockModelServerGateway, MockProfileGateway } from "@/adapters/mock"; -import type { LocalModelServerConfig, ProfileAvailability } from "@/domain"; +import type { + AgentProfile, + LocalModelServerConfig, + ProfileAvailability, +} from "@/domain"; import type { Gateways } from "@/ports"; import { DIProvider } from "@/app/di"; import { FirstRunWizard } from "./FirstRunWizard"; @@ -553,3 +557,186 @@ describe("FirstRunWizard reopening after the first run (forceOpen)", () => { expect(await screen.findByLabelText("first run setup")).toBeTruthy(); }); }); + +// Ticket #44 — reopening the wizard from Settings (`forceOpen` ⇒ edit mode) must +// pre-fill the already-configured profiles, pre-selected with their real config, +// so a save doesn't silently drop them and generic reference candidates don't +// take over. Dedup is by id only; several local OpenCode profiles are preserved. +describe("FirstRunWizard edit mode reopening (#44)", () => { + const injection = { + strategy: "conventionFile" as const, + target: "AGENTS.md", + }; + function openCodeProfile( + id: string, + name: string, + baseURL: string, + model = "qwen3-coder-30b", + ): AgentProfile { + return { + id, + name, + command: "opencode", + args: [], + contextInjection: injection, + detect: "opencode --version", + cwdTemplate: "{projectRoot}", + structuredAdapter: "openCode", + opencode: { baseURL, apiKey: "sk-x", model }, + }; + } + + function renderEdit(profile: MockProfileGateway) { + const gateways = { + profile, + modelServer: new MockModelServerGateway(), + } as unknown as Gateways; + return render( + + + , + ); + } + + it("pre-fills the configured profiles, pre-selected, with config intact", async () => { + const profile = new MockProfileGateway(); + await profile.configureProfiles([ + openCodeProfile("cfg-oc-1", "Local A", "http://localhost:9090/v1", "qwen3-coder-14b"), + ]); + + renderEdit(profile); + await waitForLoaded(); + + // The configured OpenCode comes back checked, with its real endpoint/model. + expect( + (screen.getByLabelText("use Local A") as HTMLInputElement).checked, + ).toBe(true); + expect( + (screen.getByLabelText("Local A base url") as HTMLInputElement).value, + ).toBe("http://localhost:9090/v1"); + expect( + (screen.getByLabelText("Local A model") as HTMLInputElement).value, + ).toBe("qwen3-coder-14b"); + }); + + it("does not duplicate a configured reference (matched by id, configured wins)", async () => { + // The reference OpenCode (id `mock-opencode`) was edited and is now + // configured under the same id — it must appear exactly once, pre-selected. + const profile = new MockProfileGateway(); + await profile.configureProfiles([ + openCodeProfile("mock-opencode", "OpenCode (edited)", "http://localhost:7777/v1"), + ]); + + renderEdit(profile); + await waitForLoaded(); + + // Exactly one row for the edited OpenCode; the generic reference name is gone. + expect(screen.getAllByText("OpenCode (edited)")).toHaveLength(1); + expect(screen.queryByText("OpenCode + llama.cpp")).toBeNull(); + expect( + (screen.getByLabelText("use OpenCode (edited)") as HTMLInputElement).checked, + ).toBe(true); + }); + + it("keeps several distinct OpenCode profiles (same command, different ids)", async () => { + const profile = new MockProfileGateway(); + await profile.configureProfiles([ + openCodeProfile("oc-1", "Local A", "http://localhost:8080/v1"), + openCodeProfile("oc-2", "Local B", "http://localhost:9191/v1"), + ]); + + renderEdit(profile); + await waitForLoaded(); + + // Both distinct profiles are present and pre-selected, none collapsed. + expect( + (screen.getByLabelText("use Local A") as HTMLInputElement).checked, + ).toBe(true); + expect( + (screen.getByLabelText("use Local B") as HTMLInputElement).checked, + ).toBe(true); + expect( + (screen.getByLabelText("Local A base url") as HTMLInputElement).value, + ).toBe("http://localhost:8080/v1"); + expect( + (screen.getByLabelText("Local B base url") as HTMLInputElement).value, + ).toBe("http://localhost:9191/v1"); + }); + + it("saving in edit mode does not drop configured profiles by omission", async () => { + const profile = new MockProfileGateway(); + await profile.configureProfiles([ + openCodeProfile("oc-1", "Local A", "http://localhost:8080/v1"), + openCodeProfile("oc-2", "Local B", "http://localhost:9191/v1"), + ]); + + renderEdit(profile); + await waitForLoaded(); + // Save straight away without touching anything. + fireEvent.click(screen.getByRole("button", { name: "Save and continue" })); + + await waitFor(async () => { + const saved = await profile.listProfiles(); + expect(saved.map((p) => p.id).sort()).toEqual(["oc-1", "oc-2"]); + // Real config preserved through the round-trip. + expect( + saved.find((p) => p.id === "oc-1")?.opencode?.baseURL, + ).toBe("http://localhost:8080/v1"); + }); + }); + + it("unchecking a configured row removes it on save (deliberate deletion)", async () => { + const profile = new MockProfileGateway(); + await profile.configureProfiles([ + openCodeProfile("oc-1", "Local A", "http://localhost:8080/v1"), + openCodeProfile("oc-2", "Local B", "http://localhost:9191/v1"), + ]); + + renderEdit(profile); + await waitForLoaded(); + // Deselect Local B, then save — an explicit removal, not an omission. + fireEvent.click(screen.getByLabelText("use Local B")); + fireEvent.click(screen.getByRole("button", { name: "Save and continue" })); + + await waitFor(async () => { + const saved = await profile.listProfiles(); + expect(saved.map((p) => p.id)).toEqual(["oc-1"]); + }); + }); + + it("auto-detection in edit mode does not re-check or uncheck the initial selection", async () => { + // A configured Codex (not installed per the mock) reuses the reference id, so + // it dedups to one pre-selected row. Claude is installed but only a reference. + const profile = new MockProfileGateway(); + await profile.configureProfiles([ + { + id: "mock-codex", + name: "Codex Configured", + command: "codex", + args: [], + contextInjection: injection, + detect: "codex --version", + cwdTemplate: "{projectRoot}", + }, + ]); + + renderEdit(profile); + await waitForLoaded(); + + // Wait until detection has filled availability (Claude reads as installed). + await waitFor(() => + expect( + screen.getByLabelText("Claude Code availability").textContent, + ).toMatch(/installed/), + ); + + // Selection is untouched by detection: the configured (uninstalled) Codex + // stays checked; the installed Claude reference stays UNchecked. + expect( + (screen.getByLabelText("use Codex Configured") as HTMLInputElement).checked, + ).toBe(true); + expect( + (screen.getByLabelText("use Claude Code") as HTMLInputElement).checked, + ).toBe(false); + }); +}); diff --git a/frontend/src/features/first-run/FirstRunWizard.tsx b/frontend/src/features/first-run/FirstRunWizard.tsx index 3d89460..986e743 100644 --- a/frontend/src/features/first-run/FirstRunWizard.tsx +++ b/frontend/src/features/first-run/FirstRunWizard.tsx @@ -57,7 +57,10 @@ export function FirstRunWizard({ /** Render the wizard even when it is no longer the first run. */ forceOpen?: boolean; }) { - const vm = useFirstRun(); + // Mode is explicit (ticket #44), driven by the entry point — never inferred + // from `isFirstRun`: reopening from Settings (`forceOpen`) is the "edit" mode, + // which pre-loads and pre-selects the already-configured profiles. + const vm = useFirstRun(forceOpen ? "edit" : "firstRun"); const modelServers = useModelServers(); if (vm.isFirstRun === null) return null; diff --git a/frontend/src/features/first-run/useFirstRun.ts b/frontend/src/features/first-run/useFirstRun.ts index 0cc1f8a..c2c0531 100644 --- a/frontend/src/features/first-run/useFirstRun.ts +++ b/frontend/src/features/first-run/useFirstRun.ts @@ -23,18 +23,13 @@ import type { ProfileAvailability, } from "@/domain"; import { useGateways } from "@/app/di"; +import { buildWizardEntries, type WizardEntry, type WizardMode } from "./wizardEntries"; + +export type { WizardEntry, WizardMode } from "./wizardEntries"; /** 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; - selected: boolean; - /** `null` until detection ran; then `true`/`false`. */ - available: boolean | null; -} - /** What the first-run wizard UI needs from this hook. */ export interface FirstRunViewModel { /** Whether the wizard should be shown (null while loading). */ @@ -77,7 +72,13 @@ function describe(e: unknown): string { return String(e); } -export function useFirstRun(): FirstRunViewModel { +/** + * @param mode Which wizard to build (ticket #44): `"firstRun"` (default) for the + * very first launch, or `"edit"` when reopened via *Settings ▸ Configure + * profiles* — the latter pre-loads and pre-selects the already-configured + * profiles and does NOT let detection alter that initial selection. + */ +export function useFirstRun(mode: WizardMode = "firstRun"): FirstRunViewModel { const { profile } = useGateways(); const [isFirstRun, setIsFirstRun] = useState(null); const [entries, setEntries] = useState([]); @@ -154,15 +155,25 @@ export function useFirstRun(): FirstRunViewModel { const reload = useCallback(async () => { setBusy(true); setError(null); - let refs: AgentProfile[] | null = null; + let candidates: AgentProfile[] | null = null; try { - const state: FirstRunState = await profile.firstRunState(); + // In edit mode we also load the already-configured profiles so they come + // back pre-selected with their real config (ticket #44). Fetched together + // so a single blocking window covers the whole load. + const [state, configured]: [FirstRunState, AgentProfile[]] = + await Promise.all([ + profile.firstRunState(), + mode === "edit" ? profile.listProfiles() : Promise.resolve([]), + ]); setIsFirstRun(state.isFirstRun); - refs = state.referenceProfiles; - // Show the rows immediately (nothing pre-checked yet). - setEntries( - refs.map((p) => ({ profile: p, selected: false, available: null })), - ); + const built = buildWizardEntries({ + mode, + configured, + references: state.referenceProfiles, + }); + candidates = built.map((e) => e.profile); + // Show the rows immediately (availability not probed yet). + setEntries(built); } catch (e) { setError(describe(e)); setIsFirstRun(false); @@ -171,13 +182,18 @@ export function useFirstRun(): FirstRunViewModel { setBusy(false); } - // 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 }); + // Auto-detect once on open, detached: only fills availability. In firstRun + // mode it also pre-checks the installed CLIs (`preselect`); in edit mode it + // must NOT touch the initial selection — the configured profiles stay + // selected and references stay unselected (ticket #44), so `preselect:false`. + // The manual "Detect installed CLIs" button stays available to re-probe. + if (candidates !== null) { + void runDetection(candidates, { + preselect: mode === "firstRun", + reportError: false, + }); } - }, [profile, runDetection]); + }, [profile, runDetection, mode]); useEffect(() => { void reload(); diff --git a/frontend/src/features/first-run/wizardEntries.test.ts b/frontend/src/features/first-run/wizardEntries.test.ts new file mode 100644 index 0000000..d3478e6 --- /dev/null +++ b/frontend/src/features/first-run/wizardEntries.test.ts @@ -0,0 +1,173 @@ +/** + * #44 — pure construction of the wizard rows. Covers the two modes and the + * strict id-only deduplication that keeps several OpenCode profiles distinct. + */ +import { describe, it, expect } from "vitest"; + +import type { AgentProfile } from "@/domain"; +import { buildWizardEntries } from "./wizardEntries"; + +/** Minimal profile factory (only the fields the helper reads/carries). */ +function profile(id: string, over: Partial = {}): AgentProfile { + return { + id, + name: id, + command: "claude", + args: [], + contextInjection: { strategy: "conventionFile", target: "CLAUDE.md" }, + detect: "claude --version", + cwdTemplate: "{projectRoot}", + ...over, + }; +} + +const REFERENCES: AgentProfile[] = [ + profile("mock-claude", { name: "Claude Code", command: "claude" }), + profile("mock-codex", { name: "Codex", command: "codex" }), + profile("mock-opencode", { + name: "OpenCode", + command: "opencode", + structuredAdapter: "openCode", + }), +]; + +describe("buildWizardEntries — firstRun mode", () => { + it("maps references to unselected rows with unknown availability", () => { + const entries = buildWizardEntries({ + mode: "firstRun", + configured: [], + references: REFERENCES, + }); + expect(entries).toHaveLength(3); + expect(entries.every((e) => e.selected === false)).toBe(true); + expect(entries.every((e) => e.available === null)).toBe(true); + expect(entries.map((e) => e.profile.id)).toEqual([ + "mock-claude", + "mock-codex", + "mock-opencode", + ]); + }); + + it("ignores configured profiles entirely in firstRun mode", () => { + const entries = buildWizardEntries({ + mode: "firstRun", + configured: [profile("mock-claude")], + references: REFERENCES, + }); + // Still exactly the reference rows, all unselected. + expect(entries.map((e) => e.profile.id)).toEqual([ + "mock-claude", + "mock-codex", + "mock-opencode", + ]); + expect(entries.every((e) => e.selected === false)).toBe(true); + }); +}); + +describe("buildWizardEntries — edit mode", () => { + it("puts configured profiles first, pre-selected, config intact", () => { + const configured = [ + profile("cfg-oc-1", { + name: "Local A", + command: "opencode", + structuredAdapter: "openCode", + opencode: { + baseURL: "http://localhost:9090/v1", + apiKey: "sk-a", + model: "qwen3-coder-14b", + }, + }), + ]; + const entries = buildWizardEntries({ + mode: "edit", + configured, + references: REFERENCES, + }); + + const first = entries[0]; + expect(first.profile.id).toBe("cfg-oc-1"); + expect(first.selected).toBe(true); + // Real config carried through untouched. + expect(first.profile.opencode?.baseURL).toBe("http://localhost:9090/v1"); + expect(first.profile.opencode?.model).toBe("qwen3-coder-14b"); + // Then all three references follow, unselected. + expect(entries.slice(1).map((e) => e.profile.id)).toEqual([ + "mock-claude", + "mock-codex", + "mock-opencode", + ]); + expect(entries.slice(1).every((e) => e.selected === false)).toBe(true); + }); + + it("deduplicates by id only: a configured reference is not shown twice", () => { + // The user edited the OpenCode reference and it is now configured under the + // SAME id — it must appear once (the configured, pre-selected version wins). + const configured = [ + profile("mock-opencode", { + name: "OpenCode (edited)", + command: "opencode", + structuredAdapter: "openCode", + }), + ]; + const entries = buildWizardEntries({ + mode: "edit", + configured, + references: REFERENCES, + }); + + const ocRows = entries.filter((e) => e.profile.id === "mock-opencode"); + expect(ocRows).toHaveLength(1); + expect(ocRows[0].selected).toBe(true); + expect(ocRows[0].profile.name).toBe("OpenCode (edited)"); // configured wins + // The other references remain (unselected). + expect(entries.map((e) => e.profile.id)).toEqual([ + "mock-opencode", + "mock-claude", + "mock-codex", + ]); + }); + + it("keeps several distinct OpenCode profiles that share command/adapter", () => { + // Same command + structuredAdapter, different ids ⇒ distinct identities: all + // preserved. Never matched/collapsed by command/adapter/endpoint/model. + const configured = [ + profile("oc-1", { + command: "opencode", + structuredAdapter: "openCode", + opencode: { baseURL: "http://a/v1", model: "m", apiKey: "" }, + }), + profile("oc-2", { + command: "opencode", + structuredAdapter: "openCode", + opencode: { baseURL: "http://b/v1", model: "m", apiKey: "" }, + }), + ]; + const entries = buildWizardEntries({ + mode: "edit", + configured, + references: REFERENCES, + }); + + const ocIds = entries + .filter((e) => e.profile.structuredAdapter === "openCode") + .map((e) => e.profile.id); + // Both configured OpenCode + the reference OpenCode (different id) survive. + expect(ocIds).toEqual(["oc-1", "oc-2", "mock-opencode"]); + expect(entries[0].selected).toBe(true); + expect(entries[1].selected).toBe(true); + }); + + it("with no configured profiles behaves like references-only, still unselected", () => { + const entries = buildWizardEntries({ + mode: "edit", + configured: [], + references: REFERENCES, + }); + expect(entries.map((e) => e.profile.id)).toEqual([ + "mock-claude", + "mock-codex", + "mock-opencode", + ]); + expect(entries.every((e) => e.selected === false)).toBe(true); + }); +}); diff --git a/frontend/src/features/first-run/wizardEntries.ts b/frontend/src/features/first-run/wizardEntries.ts new file mode 100644 index 0000000..e6beffb --- /dev/null +++ b/frontend/src/features/first-run/wizardEntries.ts @@ -0,0 +1,73 @@ +/** + * Pure construction of the first-run/edit wizard rows (ticket #44). + * + * The wizard runs in one of two **modes**, passed explicitly (never inferred + * from `isFirstRun`): + * + * - `"firstRun"` — the very first launch. Rows come from the selectable + * reference catalogue only, all unselected, availability unknown. Detection + * later pre-checks the installed CLIs. + * - `"edit"` — reopened via *Settings ▸ Configure profiles*. The user already + * configured profiles (possibly several local OpenCode ones with distinct + * endpoints); those must come back **pre-selected with their real config**, so + * a save doesn't silently drop them. Reference profiles are still offered, but + * only those not already configured — deduplicated **by `id` only**. + * + * Deduplication is by `profile.id` and nothing else: several OpenCode profiles + * legitimately share the same `command`/`structuredAdapter` while differing in + * endpoint/model, so their product identity is the id. A reference is "already + * configured" iff a configured profile carries the same id. + */ + +import type { AgentProfile } from "@/domain"; + +/** Which wizard the rows are being built for (ticket #44). */ +export type WizardMode = "firstRun" | "edit"; + +/** One row in the wizard: a candidate profile, selected state, availability. */ +export interface WizardEntry { + profile: AgentProfile; + selected: boolean; + /** `null` until detection ran; then `true`/`false`. */ + available: boolean | null; +} + +/** + * Builds the wizard rows for the given mode. Pure — no I/O, no React — so every + * invariant (pre-selection, id-only dedup, ordering) is unit-testable. + * + * `firstRun`: references only, all `selected:false`, `available:null`. + * `edit`: the configured profiles first (pre-selected, real config intact), then + * the reference profiles whose id is not already configured (`selected:false`). + */ +export function buildWizardEntries({ + mode, + configured, + references, +}: { + mode: WizardMode; + configured: AgentProfile[]; + references: AgentProfile[]; +}): WizardEntry[] { + if (mode === "firstRun") { + return references.map((profile) => ({ + profile, + selected: false, + available: null, + })); + } + + // Edit mode: configured profiles come first, pre-selected and untouched. + const configuredIds = new Set(configured.map((p) => p.id)); + const configuredEntries: WizardEntry[] = configured.map((profile) => ({ + profile, + selected: true, + available: null, + })); + // Then the references not already configured — matched by id ONLY. + const referenceEntries: WizardEntry[] = references + .filter((p) => !configuredIds.has(p.id)) + .map((profile) => ({ profile, selected: false, available: null })); + + return [...configuredEntries, ...referenceEntries]; +}