authorgravatar for encriptandost@gmail.comLeonel Rivas <encriptandost@gmail.com> 2026-07-22 06:54:20-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-07-22 15:54:20+02:00
log282ecb317877e6cce296976a03278cc388b7ca37
tree01c05f7c4432ec01f219392e7ded448141bf0ba4
parent32c6012dabdbd0eb178b25ea4225d889ec8f6475
signature Signed by PGP key B5690EEEBB952194

fix(settings): validate the add-provider wizard step before advancing (#2813) (#3100)

Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com>

5 files changed, 254 insertions(+), 72 deletions(-)

apps/web/src/components/settings/AddProviderInstanceDialog.logic.ts created+36
...@@ -0,0 +1,36 @@
1export type WizardNavigation =
2 | { readonly kind: "navigate"; readonly step: number }
3 | { readonly kind: "blocked"; readonly step: number; readonly error: string };
4
5const IDENTITY_STEP = 1;
6
7export const ADD_PROVIDER_WIZARD_STEPS = ["Driver", "Identity", "Config"] as const;
8
9/**
10 * Resolve navigation within the add-provider wizard.
11 *
12 * Moving forward past Identity requires a valid instance id, whether the user
13 * advances one step at a time or skips directly to Config from a step header.
14 * A blocked skip lands on Identity so its existing inline validation is
15 * visible. Backward navigation is always preserved.
16 */
17export function resolveWizardNavigation(
18 currentStep: number,
19 requestedStep: number,
20 stepCount: number,
21 validation: { readonly instanceIdError: string | null },
22): WizardNavigation {
23 const lastStep = Math.max(0, stepCount - 1);
24 const targetStep = Math.max(0, Math.min(lastStep, requestedStep));
25 const movesForwardPastIdentity = currentStep <= IDENTITY_STEP && targetStep > IDENTITY_STEP;
26
27 if (movesForwardPastIdentity && validation.instanceIdError !== null) {
28 return {
29 kind: "blocked",
30 step: Math.min(IDENTITY_STEP, lastStep),
31 error: validation.instanceIdError,
32 };
33 }
34
35 return { kind: "navigate", step: targetStep };
36}
apps/web/src/components/settings/AddProviderInstanceDialog.test.ts created+44
...@@ -0,0 +1,44 @@
1import { describe, expect, it } from "vite-plus/test";
2
3import { resolveWizardNavigation } from "./AddProviderInstanceDialog.logic";
4
5describe("resolveWizardNavigation", () => {
6 const invalidId = { instanceIdError: "Instance ID is required." };
7 const validId = { instanceIdError: null };
8
9 it("allows moving from Driver to Identity before the instance id is valid", () => {
10 expect(resolveWizardNavigation(0, 1, 3, invalidId)).toEqual({ kind: "navigate", step: 1 });
11 });
12
13 it("blocks Next from Identity to Config while the instance id is invalid", () => {
14 expect(resolveWizardNavigation(1, 2, 3, invalidId)).toEqual({
15 kind: "blocked",
16 step: 1,
17 error: "Instance ID is required.",
18 });
19 });
20
21 it("stops a direct Driver-to-Config skip at Identity and surfaces its error", () => {
22 expect(resolveWizardNavigation(0, 2, 3, invalidId)).toEqual({
23 kind: "blocked",
24 step: 1,
25 error: "Instance ID is required.",
26 });
27 });
28
29 it("allows advancing and skipping forward once the instance id is valid", () => {
30 expect(resolveWizardNavigation(1, 2, 3, validId)).toEqual({ kind: "navigate", step: 2 });
31 expect(resolveWizardNavigation(0, 2, 3, validId)).toEqual({ kind: "navigate", step: 2 });
32 });
33
34 it("always preserves backward Driver and Identity navigation", () => {
35 expect(resolveWizardNavigation(2, 1, 3, invalidId)).toEqual({ kind: "navigate", step: 1 });
36 expect(resolveWizardNavigation(2, 0, 3, invalidId)).toEqual({ kind: "navigate", step: 0 });
37 expect(resolveWizardNavigation(1, 0, 3, invalidId)).toEqual({ kind: "navigate", step: 0 });
38 });
39
40 it("clamps requested steps to the wizard bounds", () => {
41 expect(resolveWizardNavigation(2, 8, 3, validId)).toEqual({ kind: "navigate", step: 2 });
42 expect(resolveWizardNavigation(0, -1, 3, invalidId)).toEqual({ kind: "navigate", step: 0 });
43 });
44});
apps/web/src/components/settings/AddProviderInstanceDialog.tsx+43-72
...@@ -1,8 +1,7 @@...@@ -1,8 +1,7 @@
1"use client";1"use client";
22
3import { CheckIcon } from "lucide-react";
4import { Radio as RadioPrimitive } from "@base-ui/react/radio";3import { Radio as RadioPrimitive } from "@base-ui/react/radio";
5import { useCallback, useMemo, useState } from "react";4import { useMemo, useState } from "react";
6import {5import {
7 ProviderInstanceId,6 ProviderInstanceId,
8 ProviderDriverKind,7 ProviderDriverKind,
...@@ -29,6 +28,12 @@ import { toastManager } from "../ui/toast";...@@ -29,6 +28,12 @@ import { toastManager } from "../ui/toast";
29import { DRIVER_OPTION_BY_VALUE, DRIVER_OPTIONS } from "./providerDriverMeta";28import { DRIVER_OPTION_BY_VALUE, DRIVER_OPTIONS } from "./providerDriverMeta";
30import { ProviderSettingsForm, deriveProviderSettingsFields } from "./ProviderSettingsForm";29import { ProviderSettingsForm, deriveProviderSettingsFields } from "./ProviderSettingsForm";
31import { AnimatedHeight } from "../AnimatedHeight";30import { AnimatedHeight } from "../AnimatedHeight";
31import {
32 ADD_PROVIDER_WIZARD_STEPS,
33 resolveWizardNavigation,
34 type WizardNavigation,
35} from "./AddProviderInstanceDialog.logic";
36import { AddProviderInstanceWizardSteps } from "./AddProviderInstanceWizardSteps";
3237
33const PROVIDER_ACCENT_SWATCHES = [38const PROVIDER_ACCENT_SWATCHES = [
34 "#2563eb",39 "#2563eb",
...@@ -143,26 +148,37 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns...@@ -143,26 +148,37 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns
143 const instanceIdError = validateInstanceId(instanceId, existingIds);148 const instanceIdError = validateInstanceId(instanceId, existingIds);
144 const showInstanceIdError = hasAttemptedSubmit && instanceIdError !== null;149 const showInstanceIdError = hasAttemptedSubmit && instanceIdError !== null;
145 const previewLabel = label.trim() || `${driverOption.label} Workspace`;150 const previewLabel = label.trim() || `${driverOption.label} Workspace`;
146 const wizardSteps = ["Driver", "Identity", "Config"] as const;
147 const wizardStepSummaries = [driverOption.label, previewLabel, null] as const;151 const wizardStepSummaries = [driverOption.label, previewLabel, null] as const;
148152
149 const configDraft = configByDriver[driver] ?? EMPTY_CONFIG_DRAFT;153 const configDraft = configByDriver[driver] ?? EMPTY_CONFIG_DRAFT;
150 const setConfigDraft = useCallback(154 const setConfigDraft = (config: Record<string, unknown> | undefined) => {
151 (config: Record<string, unknown> | undefined) => {155 setConfigByDriver((existing) => {
152 setConfigByDriver((existing) => {156 const next = { ...existing };
153 const next = { ...existing };157 if (config === undefined || Object.keys(config).length === 0) {
154 if (config === undefined || Object.keys(config).length === 0) {158 delete next[driver];
155 delete next[driver];159 } else {
156 } else {160 next[driver] = config;
157 next[driver] = config;161 }
158 }162 return next;
159 return next;163 });
160 });164 };
161 },165
162 [driver],166 const applyWizardNavigation = (navigation: WizardNavigation) => {
163 );167 if (navigation.kind === "blocked") {
168 setHasAttemptedSubmit(true);
169 }
170 setWizardStep(navigation.step);
171 };
164172
165 const handleSave = useCallback(() => {173 const navigateToStep = (requestedStep: number) => {
174 applyWizardNavigation(
175 resolveWizardNavigation(wizardStep, requestedStep, ADD_PROVIDER_WIZARD_STEPS.length, {
176 instanceIdError,
177 }),
178 );
179 };
180
181 const handleSave = () => {
166 setHasAttemptedSubmit(true);182 setHasAttemptedSubmit(true);
167 if (instanceIdError !== null) return;183 if (instanceIdError !== null) return;
168184
...@@ -201,18 +217,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns...@@ -201,18 +217,7 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns
201 description: error instanceof Error ? error.message : "Update failed.",217 description: error instanceof Error ? error.message : "Update failed.",
202 });218 });
203 }219 }
204 }, [220 };
205 driver,
206 driverOption,
207 configByDriver,
208 instanceId,
209 instanceIdError,
210 label,
211 accentColor,
212 onOpenChange,
213 settings.providerInstances,
214 updateSettings,
215 ]);
216221
217 return (222 return (
218 <Dialog open={open} onOpenChange={onOpenChange}>223 <Dialog open={open} onOpenChange={onOpenChange}>
...@@ -224,46 +229,12 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns...@@ -224,46 +229,12 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns
224 Configure an additional provider instance — for example, a second Codex install229 Configure an additional provider instance — for example, a second Codex install
225 pointed at a different workspace.230 pointed at a different workspace.
226 </DialogDescription>231 </DialogDescription>
227 <div className="grid grid-cols-3 gap-2">232 <AddProviderInstanceWizardSteps
228 {wizardSteps.map((step, index) => (233 currentStep={wizardStep}
229 <button234 summaries={wizardStepSummaries}
230 key={step}235 instanceIdError={instanceIdError}
231 type="button"236 onNavigation={applyWizardNavigation}
232 className={cn(237 />
233 "grid min-w-0 grid-cols-[1rem_minmax(0,1fr)] gap-x-2 rounded-lg border px-3 py-2 text-left",
234 index === wizardStep
235 ? "border-primary bg-primary/10 ring-1 ring-primary/25"
236 : index < wizardStep
237 ? "border-border bg-background"
238 : "border-border bg-muted/40",
239 )}
240 onClick={() => setWizardStep(index)}
241 >
242 <span
243 className={cn(
244 "row-span-2 mt-0.5 grid size-4 place-items-center rounded-full border",
245 index < wizardStep
246 ? "border-primary bg-primary text-primary-foreground"
247 : index === wizardStep
248 ? "border-primary bg-background"
249 : "border-muted-foreground/35 bg-background",
250 )}
251 aria-hidden
252 >
253 {index < wizardStep ? <CheckIcon className="size-3" /> : null}
254 </span>
255 <span className="text-[10px] font-medium uppercase text-muted-foreground">
256 Step {index + 1}
257 </span>
258 <span className="truncate text-xs font-semibold text-foreground">
259 {step}
260 {index < wizardStep && wizardStepSummaries[index]
261 ? `: ${wizardStepSummaries[index]}`
262 : ""}
263 </span>
264 </button>
265 ))}
266 </div>
267 </DialogHeader>238 </DialogHeader>
268239
269 <div240 <div
...@@ -452,8 +423,8 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns...@@ -452,8 +423,8 @@ export function AddProviderInstanceDialog({ open, onOpenChange }: AddProviderIns
452 >423 >
453 {wizardStep === 0 ? "Cancel" : "Back"}424 {wizardStep === 0 ? "Cancel" : "Back"}
454 </Button>425 </Button>
455 {wizardStep < wizardSteps.length - 1 ? (426 {wizardStep < ADD_PROVIDER_WIZARD_STEPS.length - 1 ? (
456 <Button size="sm" onClick={() => setWizardStep((step) => Math.min(2, step + 1))}>427 <Button size="sm" onClick={() => navigateToStep(wizardStep + 1)}>
457 Next428 Next
458 </Button>429 </Button>
459 ) : (430 ) : (
apps/web/src/components/settings/AddProviderInstanceWizardSteps.test.tsx created+61
...@@ -0,0 +1,61 @@
1import { Children, isValidElement, type ReactElement } from "react";
2import { describe, expect, it, vi } from "vite-plus/test";
3
4import { ADD_PROVIDER_WIZARD_STEPS } from "./AddProviderInstanceDialog.logic";
5import { AddProviderInstanceWizardSteps } from "./AddProviderInstanceWizardSteps";
6
7interface StepButtonProps {
8 readonly "aria-current"?: string;
9 readonly onClick: () => void;
10}
11
12function renderStepButtons(
13 currentStep: number,
14 instanceIdError: string | null,
15 onNavigation: Parameters<typeof AddProviderInstanceWizardSteps>[0]["onNavigation"],
16): ReactElement<StepButtonProps>[] {
17 const header = AddProviderInstanceWizardSteps({
18 currentStep,
19 summaries: ["Codex", "Codex Workspace", null],
20 instanceIdError,
21 onNavigation,
22 });
23
24 return Children.toArray(header.props.children).filter(
25 (child): child is ReactElement<StepButtonProps> => isValidElement(child),
26 );
27}
28
29describe("AddProviderInstanceWizardSteps", () => {
30 it("gates the actual Config header click through Identity validation", () => {
31 const onNavigation = vi.fn();
32 const buttons = renderStepButtons(0, "Instance ID is required.", onNavigation);
33
34 expect(buttons).toHaveLength(ADD_PROVIDER_WIZARD_STEPS.length);
35 buttons[2]!.props.onClick();
36
37 expect(onNavigation).toHaveBeenCalledOnce();
38 expect(onNavigation).toHaveBeenCalledWith({
39 kind: "blocked",
40 step: 1,
41 error: "Instance ID is required.",
42 });
43 });
44
45 it("marks the wizard step separately from the clicked button focus", () => {
46 const buttons = renderStepButtons(1, "Instance ID is required.", vi.fn());
47
48 expect(buttons[0]!.props["aria-current"]).toBeUndefined();
49 expect(buttons[1]!.props["aria-current"]).toBe("step");
50 expect(buttons[2]!.props["aria-current"]).toBeUndefined();
51 });
52
53 it("preserves the actual backward header click", () => {
54 const onNavigation = vi.fn();
55 const buttons = renderStepButtons(2, "Instance ID is required.", onNavigation);
56
57 buttons[0]!.props.onClick();
58
59 expect(onNavigation).toHaveBeenCalledWith({ kind: "navigate", step: 0 });
60 });
61});
apps/web/src/components/settings/AddProviderInstanceWizardSteps.tsx created+70
...@@ -0,0 +1,70 @@
1import { CheckIcon } from "lucide-react";
2
3import { cn } from "../../lib/utils";
4import {
5 ADD_PROVIDER_WIZARD_STEPS,
6 resolveWizardNavigation,
7 type WizardNavigation,
8} from "./AddProviderInstanceDialog.logic";
9
10interface AddProviderInstanceWizardStepsProps {
11 readonly currentStep: number;
12 readonly summaries: readonly (string | null)[];
13 readonly instanceIdError: string | null;
14 readonly onNavigation: (navigation: WizardNavigation) => void;
15}
16
17export function AddProviderInstanceWizardSteps({
18 currentStep,
19 summaries,
20 instanceIdError,
21 onNavigation,
22}: AddProviderInstanceWizardStepsProps) {
23 return (
24 <div className="grid grid-cols-3 gap-2">
25 {ADD_PROVIDER_WIZARD_STEPS.map((step, index) => (
26 <button
27 key={step}
28 type="button"
29 className={cn(
30 "grid min-w-0 grid-cols-[1rem_minmax(0,1fr)] gap-x-2 rounded-lg border px-3 py-2 text-left",
31 index === currentStep
32 ? "border-primary bg-primary/10 ring-1 ring-primary/25"
33 : index < currentStep
34 ? "border-border bg-background"
35 : "border-border bg-muted/40",
36 )}
37 aria-current={index === currentStep ? "step" : undefined}
38 onClick={() =>
39 onNavigation(
40 resolveWizardNavigation(currentStep, index, ADD_PROVIDER_WIZARD_STEPS.length, {
41 instanceIdError,
42 }),
43 )
44 }
45 >
46 <span
47 className={cn(
48 "row-span-2 mt-0.5 grid size-4 place-items-center rounded-full border",
49 index < currentStep
50 ? "border-primary bg-primary text-primary-foreground"
51 : index === currentStep
52 ? "border-primary bg-background"
53 : "border-muted-foreground/35 bg-background",
54 )}
55 aria-hidden
56 >
57 {index < currentStep ? <CheckIcon className="size-3" /> : null}
58 </span>
59 <span className="text-[10px] font-medium uppercase text-muted-foreground">
60 Step {index + 1}
61 </span>
62 <span className="truncate text-xs font-semibold text-foreground">
63 {step}
64 {index < currentStep && summaries[index] ? `: ${summaries[index]}` : ""}
65 </span>
66 </button>
67 ))}
68 </div>
69 );
70}