Skip to content

Commit c76ce3e

Browse files
committed
refactor(ui): self-correct the wizard step when its guard breaks
Replace the connection-reset remount mechanism (a reset epoch plus a keyed wizard remount) with a render-phase reachability clamp. When the active step's entry guard no longer holds, the wizard re-seats to the furthest-reachable step using the same derivation it uses on mount, adjusting state during render rather than via an effect. Reset becomes a pure delete with no navigation.
1 parent 42cec2f commit c76ce3e

7 files changed

Lines changed: 179 additions & 53 deletions

File tree

packages/ui/src/components/ConfigureSSO/ConfigureSSOContext.tsx

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,14 @@ export interface ConfigureSSOData {
4646
*/
4747
primaryEmailAddress: EmailAddressResource | undefined;
4848
/**
49-
* A monotonically-increasing counter bumped once per reset. The top-level
50-
* `<Wizard>` is keyed on it, so a bump remounts the wizard and re-derives the
51-
* furthest-reachable step for the now-no-connection state. Create never bumps
52-
* it, so the create path keeps using a plain `goNext` (no remount, no flash).
53-
*/
54-
resetEpoch: number;
55-
/**
56-
* Deletes the current connection, then bumps {@link resetEpoch} so the keyed
57-
* top-level wizard remounts and re-derives the furthest-reachable step. Reads
58-
* the reverification-wrapped `deleteConnection` mutation off the bundle. A
59-
* no-op when there is no connection. Reset affordances call this instead of
60-
* `useWizard().goToStep`, so they work from ANY footer (including nested SAML
61-
* configure footers) without binding to a nested wizard.
49+
* Deletes the current connection. Reset does NOT navigate: once the delete
50+
* revalidates the source and drops `hasConnection`, the active step's entry
51+
* guard breaks and the wizard self-corrects, re-seating to the
52+
* furthest-reachable step (the same `initialState` derivation it uses on
53+
* mount). Reads the reverification-wrapped `deleteConnection` mutation off the
54+
* bundle. A no-op when there is no connection. Reset affordances call this
55+
* instead of `useWizard().goToStep`, so they work from ANY footer (including
56+
* nested SAML configure footers) without binding to a nested wizard.
6257
*/
6358
resetConnection: () => Promise<void>;
6459
}
@@ -101,23 +96,19 @@ export const ConfigureSSOProvider = ({
10196
primaryEmailAddress,
10297
children,
10398
}: PropsWithChildren<ConfigureSSOProviderProps>): JSX.Element => {
104-
// Bumped once per reset. The keyed top-level `<Wizard>` remounts on a bump and
105-
// re-derives the furthest-reachable step for the now-no-connection state.
106-
const [resetEpoch, setResetEpoch] = React.useState(0);
107-
10899
const { deleteConnection } = mutations;
109100
const connectionId = enterpriseConnection?.id;
110101

102+
// Reset is pure deletion — no navigation. Deleting revalidates the source and
103+
// drops `hasConnection`, which breaks the active step's entry guard; the
104+
// wizard self-corrects in its render phase, re-seating to the
105+
// furthest-reachable step (verified email → select-provider; unverified →
106+
// verify-domain). Never a hardcoded step, never a remount.
111107
const resetConnection = React.useCallback(async () => {
112108
if (!connectionId) {
113109
return;
114110
}
115111
await deleteConnection(connectionId);
116-
// The delete revalidates the source, dropping `hasConnection`. Bumping the
117-
// epoch remounts the keyed wizard, whose `initialState` re-derives the
118-
// furthest-reachable step (verified email → select-provider; unverified →
119-
// verify-domain) — never a hardcoded step, never a blank pane.
120-
setResetEpoch(e => e + 1);
121112
}, [connectionId, deleteConnection]);
122113

123114
const value = React.useMemo<ConfigureSSOData>(
@@ -128,19 +119,9 @@ export const ConfigureSSOProvider = ({
128119
testRuns,
129120
mutations,
130121
primaryEmailAddress,
131-
resetEpoch,
132122
resetConnection,
133123
}),
134-
[
135-
contentRef,
136-
enterpriseConnection,
137-
mutations,
138-
connectionState,
139-
testRuns,
140-
primaryEmailAddress,
141-
resetEpoch,
142-
resetConnection,
143-
],
124+
[contentRef, enterpriseConnection, mutations, connectionState, testRuns, primaryEmailAddress, resetConnection],
144125
);
145126

146127
return <ConfigureSSOContext.Provider value={value}>{children}</ConfigureSSOContext.Provider>;

packages/ui/src/components/ConfigureSSO/ConfigureSSOSteps.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { ConfigureStep, ConfirmationStep, SelectProviderStep, TestConfigurationS
2525
* test satisfied, so a live connection short-circuits straight to confirmation.
2626
*/
2727
export const ConfigureSSOSteps = (): JSX.Element => {
28-
const { connectionState: c, resetEpoch } = useConfigureSSO();
28+
const { connectionState: c } = useConfigureSSO();
2929
const card = useCardState();
3030

3131
const steps = React.useMemo<WizardStepConfig[]>(
@@ -39,18 +39,17 @@ export const ConfigureSSOSteps = (): JSX.Element => {
3939
[c],
4040
);
4141

42-
// Keyed on `resetEpoch`: a reset bumps the epoch, remounting the wizard so its
43-
// `initialState` re-derives the furthest-reachable step for the now-no-
44-
// connection state. Create does NOT bump the epoch, so the create path keeps
45-
// its plain `goNext` (no remount, no create-flash).
42+
// Reset does NOT remount the wizard. Deleting the connection breaks the active
43+
// step's entry guard, and the machine self-corrects in its render phase,
44+
// re-seating to the furthest-reachable step (the same `initialState`
45+
// derivation it uses on mount) — no key, no remount, no create-flash.
4646
//
4747
// `onStepChange` clears any card-level error whenever the active top-level step
4848
// actually changes, so a stale failure from one step doesn't leak into the next
4949
// on a breadcrumb jump or back-nav. The wizard fires it from its dispatch
5050
// handler — no wizard-level `useEffect` involved.
5151
return (
5252
<Wizard
53-
key={resetEpoch}
5453
steps={steps}
5554
onStepChange={() => card.setError(undefined)}
5655
>

packages/ui/src/components/ConfigureSSO/ResetConnectionDialog.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,11 @@ const ResetConnectionDialogContent = withCardStateProvider((props: ResetConnecti
6565
}
6666

6767
try {
68-
// `resetConnection` deletes the connection and bumps the reset epoch, which
69-
// remounts the keyed top-level wizard so it re-derives the furthest-
70-
// reachable step for the now-no-connection state. No `useWizard()` here —
71-
// that lets this dialog be triggered from ANY footer (including the nested
72-
// SAML configure footers) without binding to a nested wizard.
68+
// `resetConnection` deletes the connection — a pure delete, no navigation.
69+
// Dropping `hasConnection` breaks the active step's entry guard, so the
70+
// wizard self-corrects to the furthest-reachable step. No `useWizard()`
71+
// here — that lets this dialog be triggered from ANY footer (including the
72+
// nested SAML configure footers) without binding to a nested wizard.
7373
await resetConnection();
7474
onClose();
7575
} catch (err) {

packages/ui/src/components/ConfigureSSO/__tests__/ResetConnectionDialog.test.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@ import { render, screen, waitFor } from '@/test/utils';
66
import { CardStateProvider } from '@/ui/elements/contexts';
77

88
// The dialog no longer touches the wizard. On confirm it calls the context
9-
// `resetConnection()` — delete + reset-epoch bump — which remounts the keyed
10-
// top-level wizard so it re-derives the furthest-reachable step. That lets the
11-
// dialog be triggered from ANY footer (including nested SAML configure footers)
12-
// without binding to a nested wizard.
9+
// `resetConnection()` — a pure delete, no navigation — and the wizard
10+
// self-corrects to the furthest-reachable step once the active step's guard
11+
// breaks. That lets the dialog be triggered from ANY footer (including nested
12+
// SAML configure footers) without binding to a nested wizard.
1313
const resetConnection = vi.fn();
1414

1515
const connectionMockState = vi.hoisted(() => ({
@@ -21,7 +21,7 @@ vi.mock('../ConfigureSSOContext', () => ({
2121
enterpriseConnection: connectionMockState.current,
2222
contentRef: { current: null },
2323
// The dialog's confirm calls the context `resetConnection`, which owns the
24-
// reverification-wrapped delete plus the reset-epoch bump.
24+
// reverification-wrapped delete. No navigation — the wizard self-corrects.
2525
resetConnection,
2626
}),
2727
}));

packages/ui/src/components/ConfigureSSO/elements/Step.tsx

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,11 @@ FooterContinue.displayName = 'Step.Footer.Continue';
198198
* never on verify-domain / select-provider).
199199
*
200200
* It deliberately does NOT call `useWizard()`. The confirm path calls the
201-
* context `resetConnection()` (delete + epoch bump → keyed wizard remount →
202-
* re-derive to furthest-reachable), so this works from ANY footer — including
203-
* the nested SAML configure footers, which have their own (linear) wizard. That
204-
* is what kills the old per-step nested-binding trap.
201+
* context `resetConnection()` (a pure delete; the wizard then self-corrects to
202+
* the furthest-reachable step when the active step's guard breaks), so this
203+
* works from ANY footer — including the nested SAML configure footers, which
204+
* have their own (linear) wizard. That is what kills the old per-step
205+
* nested-binding trap.
205206
*
206207
* `marginInlineStart: 'auto'` pushes it to the far-left of the `justify='end'`
207208
* footer row, matching the prior destructive affordance.

packages/ui/src/components/ConfigureSSO/elements/Wizard/__tests__/useWizardMachine.test.tsx

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,131 @@ describe('useWizardMachine — first/last and reachability derivations', () => {
183183
});
184184
});
185185

186+
describe('useWizardMachine — reachability clamp (self-correct on a broken guard)', () => {
187+
it('re-seats to the furthest-reachable step when the active step guard breaks between renders', () => {
188+
// Start with c reachable; the machine seeds on c (furthest reachable).
189+
let cOpen = true;
190+
const open = () => cOpen;
191+
const config = cfg([{ id: 'a' }, { id: 'b', guard: () => true }, { id: 'c', guard: open }]);
192+
const { result, rerender } = renderMachine({ config });
193+
expect(result.current.current).toBe('c');
194+
195+
// The connection backing c is deleted: c's guard now breaks. Re-render with a
196+
// fresh config object so the memo sees new descriptors. The machine notices
197+
// its active step is impossible and re-seats to the furthest-reachable step
198+
// (b — a + b still reachable, c gated out).
199+
cOpen = false;
200+
act(() => {
201+
rerender({
202+
config: cfg([{ id: 'a' }, { id: 'b', guard: () => true }, { id: 'c', guard: open }]),
203+
parentWizard: null,
204+
initialStepId: undefined,
205+
onStepChange: undefined,
206+
});
207+
});
208+
expect(result.current.current).toBe('b');
209+
});
210+
211+
it('re-seats all the way to the entry step when every later guard breaks', () => {
212+
let unlocked = true;
213+
const gate = () => unlocked;
214+
const { result, rerender } = renderMachine({
215+
config: cfg([{ id: 'a' }, { id: 'b', guard: gate }, { id: 'c', guard: gate }]),
216+
});
217+
expect(result.current.current).toBe('c');
218+
219+
unlocked = false;
220+
act(() => {
221+
rerender({
222+
config: cfg([{ id: 'a' }, { id: 'b', guard: gate }, { id: 'c', guard: gate }]),
223+
parentWizard: null,
224+
initialStepId: undefined,
225+
onStepChange: undefined,
226+
});
227+
});
228+
expect(result.current.current).toBe('a');
229+
});
230+
231+
it('does NOT move when a guard goes TRUE while the active step still holds (create-style change)', () => {
232+
// Seed on a; b is gated out. Opening b later (a guard going TRUE) must not
233+
// yank the user forward — a's guard still holds, so there is nothing to fix.
234+
let bOpen = false;
235+
const bGuard = () => bOpen;
236+
const { result, rerender } = renderMachine({
237+
config: cfg([{ id: 'a' }, { id: 'b', guard: bGuard }]),
238+
initialStepId: 'a',
239+
});
240+
expect(result.current.current).toBe('a');
241+
242+
bOpen = true;
243+
act(() => {
244+
rerender({
245+
config: cfg([{ id: 'a' }, { id: 'b', guard: bGuard }]),
246+
parentWizard: null,
247+
initialStepId: 'a',
248+
onStepChange: undefined,
249+
});
250+
});
251+
// a still holds — no clamp. The user advances only via an explicit goNext.
252+
expect(result.current.current).toBe('a');
253+
});
254+
255+
it('is a provably one-shot: the clamp re-seats to a guard-passing step and does not loop or re-fire', () => {
256+
let cOpen = true;
257+
const open = () => cOpen;
258+
const { result, rerender } = renderMachine({
259+
config: cfg([{ id: 'a' }, { id: 'b', guard: () => true }, { id: 'c', guard: open }]),
260+
});
261+
expect(result.current.current).toBe('c');
262+
263+
cOpen = false;
264+
act(() => {
265+
rerender({
266+
config: cfg([{ id: 'a' }, { id: 'b', guard: () => true }, { id: 'c', guard: open }]),
267+
parentWizard: null,
268+
initialStepId: undefined,
269+
onStepChange: undefined,
270+
});
271+
});
272+
// Lands on a guard-passing step (b). A subsequent render with the same broken
273+
// config does NOT move it again — b's guard holds, so the clamp is inert.
274+
expect(result.current.current).toBe('b');
275+
act(() => {
276+
rerender({
277+
config: cfg([{ id: 'a' }, { id: 'b', guard: () => true }, { id: 'c', guard: open }]),
278+
parentWizard: null,
279+
initialStepId: undefined,
280+
onStepChange: undefined,
281+
});
282+
});
283+
expect(result.current.current).toBe('b');
284+
});
285+
286+
it('does not clamp a nested wizard (isNested) even when its active guard breaks', () => {
287+
// A nested machine is guard-less in practice, but the clamp is gated on
288+
// !isNested regardless: a nested wizard never re-seats, it bubbles instead.
289+
let open = true;
290+
const gate = () => open;
291+
const { result, rerender } = renderMachine({
292+
config: cfg([{ id: 'n0' }, { id: 'n1', guard: gate }]),
293+
parentWizard: makeParent(),
294+
});
295+
expect(result.current.current).toBe('n1');
296+
297+
open = false;
298+
act(() => {
299+
rerender({
300+
config: cfg([{ id: 'n0' }, { id: 'n1', guard: gate }]),
301+
parentWizard: makeParent(),
302+
initialStepId: undefined,
303+
onStepChange: undefined,
304+
});
305+
});
306+
// Nested: no clamp. current stays put despite the broken guard.
307+
expect(result.current.current).toBe('n1');
308+
});
309+
});
310+
186311
describe('useWizardMachine — onStepChange', () => {
187312
it('fires with the new step id on a goNext / goPrev / goToStep that changes current', () => {
188313
const onStepChange = vi.fn();

packages/ui/src/components/ConfigureSSO/elements/Wizard/useWizardMachine.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,26 @@ export const useWizardMachine = ({
8585
const stateRef = React.useRef(state);
8686
stateRef.current = state;
8787

88+
// Reachability invariant (adjust-state-during-render — NOT a useEffect): if the
89+
// active step's entry guard no longer holds (e.g. the connection backing it was
90+
// deleted from a footer reset, or revoked elsewhere), the wizard is sitting on an
91+
// impossible step. Re-seat to the furthest-reachable step — the SAME derivation
92+
// used on mount (initialState). React discards this render and re-renders before
93+
// paint, so the impossible frame never shows. Do NOT "fix" this by adding a
94+
// goToStep: navigation here is emergent from the guard state, by design. The
95+
// strict condition (current descriptor exists, its guard is false, and the
96+
// re-seated step differs) makes it a provably one-shot — initialState always
97+
// lands on a guard-passing step, so it cannot loop.
98+
if (!isNested) {
99+
const currentDescriptor = config.descriptors.find(d => d.id === state.current);
100+
if (currentDescriptor && !guardHolds(currentDescriptor)) {
101+
const seated = initialState(config);
102+
if (seated.current !== state.current) {
103+
setState(seated);
104+
}
105+
}
106+
}
107+
88108
// Position of `current` in the static descriptor array — the basis for both
89109
// first/last detection and the boundary-bubble decision below.
90110
const indexOfCurrent = (s: WizardState, cfg: WizardConfig): number =>

0 commit comments

Comments
 (0)