From 57054ff7d0dae30549884db26bda482caa85c4ff Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Fri, 14 Aug 2026 09:57:26 -0400 Subject: [PATCH 1/2] refactor(wizard): let the wizard own its keyboard nav instead of toggling the stepper's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every configuration-wizard step mounted with `disableNavigation()` and unmounted with `enableNavigation()`, so the Stepper's built-in `keyboardNav` bindings were suppressed for the entire lifetime of every step — they never actually ran. Each step already owns its keys: its widget (Select / MultiSelect / BlinkingTextInput) consumes and advances by calling `goNext()` from `onConfirm`, and each step binds itself. Declare that directly with `keyboardNav={false}` on the Stepper and drop the five identical disable/enable effects. Net behavior on `main` is unchanged. This also unblocks the ink-stepper 0.2.3 bump (#142). Through 0.2.1, `disableNavigation()` only gated the Stepper's own `useInput` handler, so a programmatic `goNext()` still worked. 0.2.3 added an `isBlocked()` guard to `goNext`/`goBack`/`goTo`, so the same flag now also swallows the step's explicit `goNext()` — the wizard could never advance past step 1 and the four walkthrough tests in `ink/main.test.tsx` failed. Not depending on that flag makes the wizard behave identically under both versions. Verified locally on Node 24.18.0, full suite 344/344 under ink-stepper 0.2.1 (main's lockfile) and 0.2.3; lint, format, and `npm run build` clean. --- ink/configurationWizard/ApiKeyStep.test.tsx | 12 ----------- ink/configurationWizard/ApiKeyStep.tsx | 10 +-------- ink/configurationWizard/ApiUrlStep.test.tsx | 12 ----------- ink/configurationWizard/ApiUrlStep.tsx | 10 +-------- .../ConfigurationWizard.tsx | 8 ++++++- .../EnvironmentSettingsStep.test.tsx | 12 ----------- .../EnvironmentSettingsStep.tsx | 10 +-------- .../ModelSelectionStep.test.tsx | 12 ----------- .../ModelSelectionStep.tsx | 9 +------- ink/configurationWizard/ProviderStep.test.tsx | 21 ------------------- ink/configurationWizard/ProviderStep.tsx | 9 +------- 11 files changed, 12 insertions(+), 113 deletions(-) diff --git a/ink/configurationWizard/ApiKeyStep.test.tsx b/ink/configurationWizard/ApiKeyStep.test.tsx index add5303..ecb106e 100644 --- a/ink/configurationWizard/ApiKeyStep.test.tsx +++ b/ink/configurationWizard/ApiKeyStep.test.tsx @@ -1,14 +1,9 @@ import { useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; import { render } from 'ink-testing-library'; import React from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ApiKeyStep } from './ApiKeyStep'; -vi.mock('ink-stepper', () => ({ - useStepperInput: vi.fn(), -})); - vi.mock('ink', async () => { const actual = await vi.importActual('ink'); return { @@ -18,15 +13,8 @@ vi.mock('ink', async () => { }); describe('ApiKeyStep', () => { - const mockDisableNavigation = vi.fn(); - const mockEnableNavigation = vi.fn(); - beforeEach(() => { vi.clearAllMocks(); - (useStepperInput as any).mockReturnValue({ - disableNavigation: mockDisableNavigation, - enableNavigation: mockEnableNavigation, - }); }); it('renders API key prompt for OpenAI', () => { diff --git a/ink/configurationWizard/ApiKeyStep.tsx b/ink/configurationWizard/ApiKeyStep.tsx index 9ccaca6..2886534 100644 --- a/ink/configurationWizard/ApiKeyStep.tsx +++ b/ink/configurationWizard/ApiKeyStep.tsx @@ -1,6 +1,5 @@ import { Box, Text, useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; -import React, { useEffect } from 'react'; +import React from 'react'; import { BlinkingTextInput } from '../components/BlinkingTextInput'; import { emitToListeners } from '../emitters/listener'; import type { ModelProvider } from '../models/config'; @@ -13,13 +12,6 @@ export function ApiKeyStep( onBack: () => void; }, ) { - const { disableNavigation, enableNavigation } = useStepperInput(); - - useEffect(() => { - disableNavigation(); - return () => enableNavigation(); - }, [disableNavigation, enableNavigation]); - useInput((input, key) => { if (key.escape) { onBack(); diff --git a/ink/configurationWizard/ApiUrlStep.test.tsx b/ink/configurationWizard/ApiUrlStep.test.tsx index dcd85e4..4684cca 100644 --- a/ink/configurationWizard/ApiUrlStep.test.tsx +++ b/ink/configurationWizard/ApiUrlStep.test.tsx @@ -1,14 +1,9 @@ import { useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; import { render } from 'ink-testing-library'; import React from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ApiUrlStep } from './ApiUrlStep'; -vi.mock('ink-stepper', () => ({ - useStepperInput: vi.fn(), -})); - vi.mock('ink', async () => { const actual = await vi.importActual('ink'); return { @@ -18,15 +13,8 @@ vi.mock('ink', async () => { }); describe('ApiUrlStep', () => { - const mockDisableNavigation = vi.fn(); - const mockEnableNavigation = vi.fn(); - beforeEach(() => { vi.clearAllMocks(); - (useStepperInput as any).mockReturnValue({ - disableNavigation: mockDisableNavigation, - enableNavigation: mockEnableNavigation, - }); }); it('renders API URL prompt for Ollama', () => { diff --git a/ink/configurationWizard/ApiUrlStep.tsx b/ink/configurationWizard/ApiUrlStep.tsx index 3090ee6..b67505f 100644 --- a/ink/configurationWizard/ApiUrlStep.tsx +++ b/ink/configurationWizard/ApiUrlStep.tsx @@ -1,6 +1,5 @@ import { Box, Text, useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; -import React, { useEffect } from 'react'; +import React from 'react'; import { BlinkingTextInput } from '../components/BlinkingTextInput'; import { emitToListeners } from '../emitters/listener'; import type { ModelProvider } from '../models/config'; @@ -13,13 +12,6 @@ export function ApiUrlStep( onBack: () => void; }, ) { - const { disableNavigation, enableNavigation } = useStepperInput(); - - useEffect(() => { - disableNavigation(); - return () => enableNavigation(); - }, [disableNavigation, enableNavigation]); - useInput((input, key) => { if (key.escape) { onBack(); diff --git a/ink/configurationWizard/ConfigurationWizard.tsx b/ink/configurationWizard/ConfigurationWizard.tsx index 652c3f3..0064ead 100644 --- a/ink/configurationWizard/ConfigurationWizard.tsx +++ b/ink/configurationWizard/ConfigurationWizard.tsx @@ -53,12 +53,18 @@ export function ConfigurationWizard({ onComplete }: Props) { ? [...new Set([...ollamaModels, ...compactorModelsByProvider[provider]])] : compactorModelsByProvider[provider]; + // Every step owns its own keys: its input widget consumes and advances by calling + // goNext() from onConfirm, and each step binds itself. The Stepper's built-in + // bindings would double-handle on top of that, which is why every step used to + // switch them off via disableNavigation() for its whole lifetime. Saying keyboardNav={false} + // once states that directly, and leaves goNext() free of however the library happens to + // gate its own navigation — ink-stepper 0.2.3 extended that gate to programmatic calls. return ( diff --git a/ink/configurationWizard/EnvironmentSettingsStep.test.tsx b/ink/configurationWizard/EnvironmentSettingsStep.test.tsx index 9707638..e01089d 100644 --- a/ink/configurationWizard/EnvironmentSettingsStep.test.tsx +++ b/ink/configurationWizard/EnvironmentSettingsStep.test.tsx @@ -1,14 +1,9 @@ import { useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; import { render } from 'ink-testing-library'; import React from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { EnvironmentSettingsStep } from './EnvironmentSettingsStep'; -vi.mock('ink-stepper', () => ({ - useStepperInput: vi.fn(), -})); - vi.mock('ink', async () => { const actual = await vi.importActual('ink'); return { @@ -18,15 +13,8 @@ vi.mock('ink', async () => { }); describe('EnvironmentSettingsStep', () => { - const mockDisableNavigation = vi.fn(); - const mockEnableNavigation = vi.fn(); - beforeEach(() => { vi.clearAllMocks(); - (useStepperInput as any).mockReturnValue({ - disableNavigation: mockDisableNavigation, - enableNavigation: mockEnableNavigation, - }); }); it('renders settings options', () => { diff --git a/ink/configurationWizard/EnvironmentSettingsStep.tsx b/ink/configurationWizard/EnvironmentSettingsStep.tsx index 3463cb6..22c7f99 100644 --- a/ink/configurationWizard/EnvironmentSettingsStep.tsx +++ b/ink/configurationWizard/EnvironmentSettingsStep.tsx @@ -1,7 +1,6 @@ import { MultiSelect } from '@inkjs/ui'; import { Box, Text, useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; -import React, { useEffect } from 'react'; +import React from 'react'; import { updateEnv } from '../../utils/files/updateEnv'; interface Props { @@ -38,13 +37,6 @@ const SETTINGS = [ ]; export function EnvironmentSettingsStep({ onConfirm, onBack }: Props) { - const { disableNavigation, enableNavigation } = useStepperInput(); - - useEffect(() => { - disableNavigation(); - return () => enableNavigation(); - }, [disableNavigation, enableNavigation]); - useInput((_input, key) => { if (key.escape) { onBack(); diff --git a/ink/configurationWizard/ModelSelectionStep.test.tsx b/ink/configurationWizard/ModelSelectionStep.test.tsx index a853c8d..4cb2dd3 100644 --- a/ink/configurationWizard/ModelSelectionStep.test.tsx +++ b/ink/configurationWizard/ModelSelectionStep.test.tsx @@ -1,14 +1,9 @@ import { useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; import { render } from 'ink-testing-library'; import React from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ModelSelectionStep } from './ModelSelectionStep'; -vi.mock('ink-stepper', () => ({ - useStepperInput: vi.fn(), -})); - vi.mock('ink', async () => { const actual = await vi.importActual('ink'); return { @@ -18,15 +13,8 @@ vi.mock('ink', async () => { }); describe('ModelSelectionStep', () => { - const mockDisableNavigation = vi.fn(); - const mockEnableNavigation = vi.fn(); - beforeEach(() => { vi.clearAllMocks(); - (useStepperInput as any).mockReturnValue({ - disableNavigation: mockDisableNavigation, - enableNavigation: mockEnableNavigation, - }); }); it('renders title and models', () => { diff --git a/ink/configurationWizard/ModelSelectionStep.tsx b/ink/configurationWizard/ModelSelectionStep.tsx index 7b379a8..0aa4083 100644 --- a/ink/configurationWizard/ModelSelectionStep.tsx +++ b/ink/configurationWizard/ModelSelectionStep.tsx @@ -1,7 +1,6 @@ import { Select } from '@inkjs/ui'; import { Box, Text, useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useMemo, useState } from 'react'; import { BlinkingTextInput } from '../components/BlinkingTextInput'; import { emitToListeners } from '../emitters/listener'; @@ -18,14 +17,8 @@ export function ModelSelectionStep({ onConfirm: (model: string) => void; onBack: () => void; }) { - const { disableNavigation, enableNavigation } = useStepperInput(); const [isCustom, setIsCustom] = useState(false); - useEffect(() => { - disableNavigation(); - return () => enableNavigation(); - }, [isCustom, disableNavigation, enableNavigation]); - useInput((input, key) => { if (key.escape) { if (isCustom) { diff --git a/ink/configurationWizard/ProviderStep.test.tsx b/ink/configurationWizard/ProviderStep.test.tsx index 3ae8a67..d46c856 100644 --- a/ink/configurationWizard/ProviderStep.test.tsx +++ b/ink/configurationWizard/ProviderStep.test.tsx @@ -1,23 +1,11 @@ -import { useStepperInput } from 'ink-stepper'; import { render } from 'ink-testing-library'; import React from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ProviderStep } from './ProviderStep'; -vi.mock('ink-stepper', () => ({ - useStepperInput: vi.fn(), -})); - describe('ProviderStep', () => { - const mockDisableNavigation = vi.fn(); - const mockEnableNavigation = vi.fn(); - beforeEach(() => { vi.clearAllMocks(); - (useStepperInput as any).mockReturnValue({ - disableNavigation: mockDisableNavigation, - enableNavigation: mockEnableNavigation, - }); }); it('renders provider selection message', () => { @@ -26,13 +14,4 @@ describe('ProviderStep', () => { expect(lastFrame()).toContain('What model provider would you like to use today?'); }); - - it('disables stepper navigation on mount and enables on unmount', () => { - const onConfirm = vi.fn(); - const { unmount } = render(); - - expect(mockDisableNavigation).toHaveBeenCalled(); - unmount(); - expect(mockEnableNavigation).toHaveBeenCalled(); - }); }); diff --git a/ink/configurationWizard/ProviderStep.tsx b/ink/configurationWizard/ProviderStep.tsx index dc439b2..09b15da 100644 --- a/ink/configurationWizard/ProviderStep.tsx +++ b/ink/configurationWizard/ProviderStep.tsx @@ -1,7 +1,6 @@ import { Select } from '@inkjs/ui'; import { Box, Text, useInput } from 'ink'; -import { useStepperInput } from 'ink-stepper'; -import React, { useEffect, useMemo } from 'react'; +import React, { useMemo } from 'react'; import type { ModelProvider } from '../models/config'; import { providers } from './providers'; @@ -12,7 +11,6 @@ export function ProviderStep( onExit: () => void; }, ) { - const { disableNavigation, enableNavigation } = useStepperInput(); const sortedProviders = useMemo( () => [...providers].sort((a, b) => { @@ -23,11 +21,6 @@ export function ProviderStep( [defaultValue], ); - useEffect(() => { - disableNavigation(); - return () => enableNavigation(); - }, [disableNavigation, enableNavigation]); - useInput((input, key) => { if (key.escape) { onExit(); From 06fe73d3a072cfdf55691f5ad39c57f766ee28dd Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 17 Aug 2026 14:32:26 -0400 Subject: [PATCH 2/2] fix(wizard): let each step's own back-navigation ConfigurationWizard's own useInput also handled by calling onComplete(), and Ink delivers input to every mounted useInput, so on any step past the first it double-fired with that step's own handler: the step called onBack() (or left its custom sub-mode) while the wizard completed on top of it, so effectively exited the wizard instead of going back. Drop the redundant branch (keeping ctrl+x -> ExitUI) so each step owns as intended - the first step exits, the rest go back. Resolves the review finding on #147. Co-Authored-By: Claude Opus 4.8 --- ink/configurationWizard/ConfigurationWizard.test.tsx | 10 ++++++++++ ink/configurationWizard/ConfigurationWizard.tsx | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/ink/configurationWizard/ConfigurationWizard.test.tsx b/ink/configurationWizard/ConfigurationWizard.test.tsx index 7602bf1..8ddc513 100644 --- a/ink/configurationWizard/ConfigurationWizard.test.tsx +++ b/ink/configurationWizard/ConfigurationWizard.test.tsx @@ -39,4 +39,14 @@ describe('ConfigurationWizard', () => { expect(emitToListeners).toHaveBeenCalledWith('ExitUI', undefined); }); + + it('does not complete the wizard on (each step owns back-navigation)', () => { + const onComplete = vi.fn(); + render(); + + const inputHandler = (useInput as any).mock.calls[0][0]; + inputHandler('', { escape: true }); + + expect(onComplete).not.toHaveBeenCalled(); + }); }); diff --git a/ink/configurationWizard/ConfigurationWizard.tsx b/ink/configurationWizard/ConfigurationWizard.tsx index 0064ead..ed55d42 100644 --- a/ink/configurationWizard/ConfigurationWizard.tsx +++ b/ink/configurationWizard/ConfigurationWizard.tsx @@ -37,13 +37,13 @@ export function ConfigurationWizard({ onComplete }: Props) { } }, [provider]); + // is owned per-step (back-navigation, or leaving a sub-mode); the wizard only binds + // global exit keys here. Handling at this level too would double-fire with the active + // step — Ink delivers input to every mounted useInput — and complete the wizard instead. useInput((input, key) => { if (key.ctrl && input === 'x') { emitToListeners('ExitUI', undefined); } - if (key.escape) { - onComplete(); - } }); const models = provider === 'Ollama' && ollamaModels.length > 0