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]; +}