Skip to content

Defer Ink teardown in prompt components #7121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dmerand wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Defer Ink teardown in prompt components #7121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
sendInputAndWaitForContent,
waitForInputsToBeReady,
render,
waitForContent,
} from '../../testing/ui.js'
import {Stdout} from '../../ui.js'
import {AbortController} from '../../../../public/node/abort.js'
Expand Down Expand Up @@ -117,8 +118,13 @@ describe('AutocompletePrompt', async () => {

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, ARROW_DOWN)
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

Comment on lines +123 to +126
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waitForContent yields via setImmediate, and useDeferredUnmount schedules exit via setImmediate as well. That means by the time waitForContent resolves, renderPromise may already be fulfilled while the last frame still includes ✔, causing this assertion to be timing-sensitive. Consider asserting the ordering between “✔ observed” and “exit resolved” directly (e.g., by racing those two conditions) instead of checking isFulfilled() after waitForContent completes.

Suggested change
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))
expect(renderPromise.isFulfilled()).toBe(false)
const contentPromise = waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))
const winner = await Promise.race([
contentPromise.then(() => 'content'),
renderPromise.then(() => 'exit'),
])
expect(winner).toBe('content')

Copilot uses AI. Check for mistakes.
await renderPromise
expect(getLastFrameAfterUnmount(renderInstance)).toMatchInlineSnapshot(`
"? Associate your project with the org Castile Ventures?
✔ second
Expand Down Expand Up @@ -520,8 +526,13 @@ describe('AutocompletePrompt', async () => {
"
`)

await sendInputAndWaitForChange(renderInstance, ENTER)
const renderPromise = renderInstance.waitUntilExit()
renderInstance.stdin.write(ENTER)

expect(renderPromise.isFulfilled()).toBe(false)

await waitForContent(renderInstance, '✔')
await renderPromise
expect(getLastFrameAfterUnmount(renderInstance)).toMatchInlineSnapshot(`
"? Associate your project with the org Castile Ventures?
✔ fifth
Expand Down
7 changes: 4 additions & 3 deletions packages/cli-kit/src/private/node/ui/components/AutocompletePrompt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import {InfoMessageProps} from './Prompts/InfoMessage.js'
import {Message, PromptLayout} from './Prompts/PromptLayout.js'
import {throttle} from '../../../../public/common/function.js'
import {AbortSignal} from '../../../../public/node/abort.js'
import useDeferredUnmount from '../hooks/use-deferred-unmount.js'
import usePrompt, {PromptState} from '../hooks/use-prompt.js'

import React, {ReactElement, useCallback, useEffect, useRef, useState} from 'react'
import {Box, useApp} from 'ink'
import {Box} from 'ink'

export interface SearchResults<T> {
data: SelectItem<T>[]
Expand Down Expand Up @@ -42,7 +43,7 @@ function AutocompletePrompt<T>({
infoMessage,
groupOrder,
}: React.PropsWithChildren<AutocompletePromptProps<T>>): ReactElement | null {
const {exit: unmountInk} = useApp()
const unmountInk = useDeferredUnmount()
const [searchTerm, setSearchTerm] = useState('')
const [searchResults, setSearchResults] = useState<SelectItem<T>[]>(choices)
const canSearch = choices.length > MIN_NUMBER_OF_ITEMS_FOR_SEARCH
Expand Down Expand Up @@ -72,8 +73,8 @@ function AutocompletePrompt<T>({
useEffect(() => {
if (promptState === PromptState.Submitted && answer) {
setSearchTerm('')
unmountInk()
onSubmit(answer.value)
unmountInk()
}
}, [answer, onSubmit, promptState, unmountInk])

Expand Down
25 changes: 20 additions & 5 deletions packages/cli-kit/src/private/node/ui/components/DangerousConfirmationPrompt.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {DangerousConfirmationPrompt} from './DangerousConfirmationPrompt.js'
import {getLastFrameAfterUnmount, sendInputAndWaitForChange, waitForInputsToBeReady, render} from '../../testing/ui.js'
import {
getLastFrameAfterUnmount,
sendInputAndWaitForChange,
waitForContent,
waitForInputsToBeReady,
render,
} from '../../testing/ui.js'
import {unstyled} from '../../../../public/node/output.js'
import React from 'react'

Expand Down Expand Up @@ -61,7 +67,13 @@ describe('DangerousConfirmationPrompt', () => {

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, 'yes')
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)
Comment on lines +71 to +74
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isFulfilled() assertion is performed after awaiting waitForContent, which can yield long enough for the deferred exit (scheduled with setImmediate) to run. That can make this check flaky even if the final ✔/✘ frame rendered correctly. Consider validating ordering (final frame observed before waitUntilExit settles) instead of checking isFulfilled() after waitForContent.

Suggested change
const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))
expect(renderPromise.isFulfilled()).toBe(false)
let exitSettled = false
const renderPromise = renderInstance.waitUntilExit().then(() => {
exitSettled = true
})
await waitForContent(renderInstance, '✔', () => {
expect(exitSettled).toBe(false)
renderInstance.stdin.write(ENTER)
})

Copilot uses AI. Check for mistakes.

await renderPromise
expect(onSubmit).toHaveBeenCalledWith(true)
expect(unstyled(getLastFrameAfterUnmount(renderInstance)!)).toMatchInlineSnapshot(`
"? Test question:
Expand Down Expand Up @@ -112,15 +124,18 @@ describe('DangerousConfirmationPrompt', () => {
<DangerousConfirmationPrompt onSubmit={onSubmit} message="Test question" confirmation="yes" />,
)
await waitForInputsToBeReady()
const promise = renderInstance.waitUntilExit()
await sendInputAndWaitForChange(renderInstance, ESC)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✘', () => renderInstance.stdin.write(ESC))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(unstyled(getLastFrameAfterUnmount(renderInstance)!)).toMatchInlineSnapshot(`
"? Test question:
✘ Cancelled
"
`)
await expect(promise).resolves.toEqual(undefined)
expect(onSubmit).toHaveBeenCalledWith(false)
})
})
5 changes: 3 additions & 2 deletions packages/cli-kit/src/private/node/ui/components/DangerousConfirmationPrompt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import useLayout from '../hooks/use-layout.js'
import {messageWithPunctuation} from '../utilities.js'
import {AbortSignal} from '../../../../public/node/abort.js'
import useAbortSignal from '../hooks/use-abort-signal.js'
import useDeferredUnmount from '../hooks/use-deferred-unmount.js'
import usePrompt, {PromptState} from '../hooks/use-prompt.js'

import React, {FunctionComponent, useCallback, useEffect, useState} from 'react'
import {Box, useApp, useInput, Text} from 'ink'
import {Box, useInput, Text} from 'ink'
import figures from 'figures'

export interface DangerousConfirmationPromptProps {
Expand Down Expand Up @@ -38,7 +39,7 @@ const DangerousConfirmationPrompt: FunctionComponent<DangerousConfirmationPrompt
const {promptState, setPromptState, answer, setAnswer} = usePrompt<string>({
initialAnswer: '',
})
const {exit: unmountInk} = useApp()
const unmountInk = useDeferredUnmount()
const [error, setError] = useState<TokenItem<InlineToken> | undefined>(undefined)
const color = promptState === PromptState.Error ? 'red' : 'cyan'
const underline = new Array(oneThird - 3).fill('▔')
Expand Down
36 changes: 31 additions & 5 deletions packages/cli-kit/src/private/node/ui/components/SelectPrompt.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {SelectPrompt} from './SelectPrompt.js'
import {getLastFrameAfterUnmount, sendInputAndWaitForChange, waitForInputsToBeReady, render} from '../../testing/ui.js'
import {
getLastFrameAfterUnmount,
sendInputAndWaitForChange,
waitForContent,
waitForInputsToBeReady,
render,
} from '../../testing/ui.js'
import {unstyled} from '../../../../public/node/output.js'
import {Stdout} from '../../ui.js'
import {AbortController} from '../../../../public/node/abort.js'
Expand Down Expand Up @@ -53,8 +59,13 @@ describe('SelectPrompt', async () => {

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, ARROW_DOWN)
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

Comment on lines +66 to +67
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same potential flakiness pattern: waitForContent yields through setImmediate, and the new unmount is also scheduled via setImmediate. By the time this assertion runs, waitUntilExit() may already have resolved even though the final frame was rendered, causing intermittent failures. Consider asserting that the final frame is observed before the exit promise settles (ordering check) rather than asserting isFulfilled() === false after waitForContent returns.

Suggested change
expect(renderPromise.isFulfilled()).toBe(false)

Copilot uses AI. Check for mistakes.
await renderPromise
expect(getLastFrameAfterUnmount(renderInstance)).toMatchInlineSnapshot(`
"? Associate your project with the org Castile Ventures?
✔ second
Expand Down Expand Up @@ -258,8 +269,13 @@ describe('SelectPrompt', async () => {
`)

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(getLastFrameAfterUnmount(renderInstance)).toMatchInlineSnapshot(`
"? Test question?
✔ b
Expand Down Expand Up @@ -288,8 +304,13 @@ describe('SelectPrompt', async () => {
`)

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(getLastFrameAfterUnmount(renderInstance)).toMatchInlineSnapshot(`
"? Test question?
✔ a
Expand Down Expand Up @@ -318,8 +339,13 @@ describe('SelectPrompt', async () => {
`)

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, 'b')

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write('b'))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(getLastFrameAfterUnmount(renderInstance)).toMatchInlineSnapshot(`
"? Test question?
✔ b
Expand Down
6 changes: 3 additions & 3 deletions packages/cli-kit/src/private/node/ui/components/SelectPrompt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import {InfoTableProps} from './Prompts/InfoTable.js'
import {InfoMessageProps} from './Prompts/InfoMessage.js'
import {Message, PromptLayout} from './Prompts/PromptLayout.js'
import {AbortSignal} from '../../../../public/node/abort.js'
import useDeferredUnmount from '../hooks/use-deferred-unmount.js'
import usePrompt, {PromptState} from '../hooks/use-prompt.js'

import React, {ReactElement, useCallback, useEffect} from 'react'
import {useApp} from 'ink'

export interface SelectPromptProps<T> {
message: Message
Expand All @@ -32,7 +32,7 @@ function SelectPrompt<T>({
if (choices.length === 0) {
throw new Error('SelectPrompt requires at least one choice')
}
const {exit: unmountInk} = useApp()
const unmountInk = useDeferredUnmount()
const {promptState, setPromptState, answer, setAnswer} = usePrompt<SelectItem<T> | undefined>({
initialAnswer: undefined,
})
Expand All @@ -47,8 +47,8 @@ function SelectPrompt<T>({

useEffect(() => {
if (promptState === PromptState.Submitted && answer) {
unmountInk()
onSubmit(answer.value)
unmountInk()
}
}, [answer, onSubmit, promptState, unmountInk])

Expand Down
51 changes: 44 additions & 7 deletions packages/cli-kit/src/private/node/ui/components/TextPrompt.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import {TextPrompt} from './TextPrompt.js'
import {getLastFrameAfterUnmount, sendInputAndWaitForChange, waitForInputsToBeReady, render} from '../../testing/ui.js'
import {
getLastFrameAfterUnmount,
sendInputAndWaitForChange,
waitForContent,
waitForInputsToBeReady,
render,
} from '../../testing/ui.js'
import {unstyled} from '../../../../public/node/output.js'
import {AbortController} from '../../../../public/node/abort.js'
import colors from '../../../../public/node/colors.js'
Expand Down Expand Up @@ -72,7 +78,13 @@ describe('TextPrompt', () => {

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, 'A')
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
Comment on lines +82 to +87
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion that renderPromise is still unresolved is checked after awaiting waitForContent. Since waitForContent yields via setImmediate/setTimeout(0) and the new deferred unmount also schedules exit via setImmediate, it’s possible for the prompt to exit before this assertion runs (while the last frame still contains ✔), making this check timing-sensitive. Prefer asserting ordering directly (ensure the ✔ frame is observed before waitUntilExit settles) rather than checking isFulfilled() after waitForContent completes.

Copilot uses AI. Check for mistakes.
expect(onSubmit).toHaveBeenCalledWith('A')
expect(unstyled(getLastFrameAfterUnmount(renderInstance)!)).toMatchInlineSnapshot(`
"? Test question:
Expand All @@ -86,7 +98,13 @@ describe('TextPrompt', () => {
const renderInstance = render(<TextPrompt onSubmit={onSubmit} message="Test question" defaultValue="A" />)

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(onSubmit).toHaveBeenCalledWith('A')
expect(unstyled(getLastFrameAfterUnmount(renderInstance)!)).toMatchInlineSnapshot(`
"? Test question:
Expand All @@ -102,7 +120,13 @@ describe('TextPrompt', () => {
)

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(onSubmit).toHaveBeenCalledWith('')
expect(unstyled(getLastFrameAfterUnmount(renderInstance)!)).toMatchInlineSnapshot(`
"? Test question:
Expand All @@ -124,7 +148,13 @@ describe('TextPrompt', () => {
)

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, ENTER)

const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(onSubmit).toHaveBeenCalledWith('A')
expect(unstyled(getLastFrameAfterUnmount(renderInstance)!)).toMatchInlineSnapshot(`
"? Test question:
Expand All @@ -151,7 +181,8 @@ describe('TextPrompt', () => {
})

test("masking the input if it's a password", async () => {
const renderInstance = render(<TextPrompt onSubmit={() => {}} message="Test question" password />)
const onSubmit = vi.fn()
const renderInstance = render(<TextPrompt onSubmit={onSubmit} message="Test question" password />)

await waitForInputsToBeReady()
await sendInputAndWaitForChange(renderInstance, 'ABC')
Expand All @@ -162,7 +193,13 @@ describe('TextPrompt', () => {
"
`)

await sendInputAndWaitForChange(renderInstance, ENTER)
const renderPromise = renderInstance.waitUntilExit()
await waitForContent(renderInstance, '✔', () => renderInstance.stdin.write(ENTER))

expect(renderPromise.isFulfilled()).toBe(false)

await renderPromise
expect(onSubmit).toHaveBeenCalledWith('ABC')
expect(unstyled(getLastFrameAfterUnmount(renderInstance)!)).toMatchInlineSnapshot(`
"? Test question:
✔ ***
Expand Down
5 changes: 3 additions & 2 deletions packages/cli-kit/src/private/node/ui/components/TextPrompt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import useLayout from '../hooks/use-layout.js'
import {messageWithPunctuation} from '../utilities.js'
import {AbortSignal} from '../../../../public/node/abort.js'
import useAbortSignal from '../hooks/use-abort-signal.js'
import useDeferredUnmount from '../hooks/use-deferred-unmount.js'
import usePrompt, {PromptState} from '../hooks/use-prompt.js'
import React, {FunctionComponent, useCallback, useEffect, useState} from 'react'

import {Box, useApp, useInput, Text} from 'ink'
import {Box, useInput, Text} from 'ink'
import figures from 'figures'

export interface TextPromptProps {
Expand Down Expand Up @@ -60,7 +61,7 @@ const TextPrompt: FunctionComponent<TextPromptProps> = ({
const answerOrDefault = answer.length > 0 ? answer : defaultValue
const displayEmptyValue = answerOrDefault === ''
const displayedAnswer = displayEmptyValue ? emptyDisplayedValue : answerOrDefault
const {exit: unmountInk} = useApp()
const unmountInk = useDeferredUnmount()
const [error, setError] = useState<string | undefined>(undefined)
const color = promptState === PromptState.Error ? 'red' : 'cyan'
const underline = new Array(oneThird - 3).fill('▔')
Expand Down
Loading
Loading