From ab1625795e30d69497648b071daecd36f281af58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 7 Oct 2026 11:44:18 +0200 Subject: [PATCH 1/5] Write interactive prompts to stderr by default --- .changeset/quiet-prompt-streams.md | 6 + docs/cli-kit/ui-kit/readme.md | 2 + .../components/Prompts/PromptLayout.test.tsx | 34 +++++- .../ui/components/Prompts/PromptLayout.tsx | 6 +- .../cli-kit/src/public/node/system.test.ts | 38 +++++++ packages/cli-kit/src/public/node/system.ts | 6 +- packages/cli-kit/src/public/node/ui.test.ts | 103 +++++++++++++++++- packages/cli-kit/src/public/node/ui.tsx | 4 + 8 files changed, 192 insertions(+), 7 deletions(-) create mode 100644 .changeset/quiet-prompt-streams.md diff --git a/.changeset/quiet-prompt-streams.md b/.changeset/quiet-prompt-streams.md new file mode 100644 index 00000000000..ca1bcff582c --- /dev/null +++ b/.changeset/quiet-prompt-streams.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli': patch +'@shopify/cli-kit': patch +--- + +Write interactive prompts to stderr by default to keep stdout clear for command results. diff --git a/docs/cli-kit/ui-kit/readme.md b/docs/cli-kit/ui-kit/readme.md index 1b2bcdd399d..c08acebfcf3 100644 --- a/docs/cli-kit/ui-kit/readme.md +++ b/docs/cli-kit/ui-kit/readme.md @@ -153,6 +153,8 @@ when you want to display it to the terminal, for example in your exception handl Prompts interrupt the flow of commands to ask the user for some information. There are two main types of prompts: selects and textual prompts. They all take a `message` property which will be displayed as the title of the prompt to the user and is usually in the form of a question. If you forget to use punctuation at the end of `message` we'll add `?` for you. +Prompts read input from stdin and write to stderr by default, so stdout can contain command results. To use a different output stream, pass it as `renderOptions.stdout`. + For select prompts, if the terminal is not tall enough to render all the options the prompt will resize to fit the terminal window and a message will appear warning the user that only a fraction of the items is being displayed. The user can then cycle through the items, including the hidden ones, by using the arrow keys. #### `renderSelectPrompt` diff --git a/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.test.tsx b/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.test.tsx index 4b4d685af63..1b30dbdb3f1 100644 --- a/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.test.tsx @@ -1,11 +1,43 @@ import {PromptLayout} from './PromptLayout.js' import {PromptState} from '../../hooks/use-prompt.js' import {render} from '../../../testing/ui.js' -import {describe, expect, test} from 'vitest' +import {Stdout} from '../../../ui.js' +import {SelectInput} from '../SelectInput.js' +import {describe, expect, test, vi} from 'vitest' import React from 'react' import {Box, Text} from 'ink' describe('PromptLayout', async () => { + test('renders choices when the output stream has no terminal row count', async () => { + const stdout = new Stdout({columns: 80}) + Reflect.deleteProperty(stdout, 'rows') + const renderInstance = render( + {}} + /> + } + />, + {stdout}, + ) + + try { + await vi.waitFor(() => expect(stdout.lastFrame()).toContain('Second option')) + stdout.emit('resize') + await vi.waitFor(() => expect(stdout.lastFrame()).toContain('First option')) + } finally { + renderInstance.unmount() + renderInstance.cleanup() + } + }) + test("doesn't add unnecessary margins when infoTable is an empty array", async () => { const items = [ {label: 'first', value: 'first'}, diff --git a/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.tsx b/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.tsx index fe035922d80..422427c6367 100644 --- a/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Prompts/PromptLayout.tsx @@ -11,6 +11,8 @@ import {Box, measureElement, Text, useStdout, DOMElement} from 'ink' import figures from 'figures' import type {InlineToken, LinkToken, TokenItem} from '../token-item.js' +const DEFAULT_TERMINAL_ROWS = 24 + export type Message = TokenItem> interface PromptLayoutProps { @@ -38,7 +40,7 @@ const PromptLayout = ({ const [wrapperHeight, setWrapperHeight] = useState(0) const [promptAreaHeight, setPromptAreaHeight] = useState(0) const [inputFixedAreaHeight, setInputFixedAreaHeight] = useState(0) - const currentAvailableLines = stdout.rows - promptAreaHeight - inputFixedAreaHeight + const currentAvailableLines = (stdout.rows ?? DEFAULT_TERMINAL_ROWS) - promptAreaHeight - inputFixedAreaHeight const [availableLines, setAvailableLines] = useState(currentAvailableLines) const wrapperRef = useCallback( @@ -72,7 +74,7 @@ const PromptLayout = ({ useLayoutEffect(() => { function onResize() { - const newAvailableLines = stdout.rows - promptAreaHeight - inputFixedAreaHeight + const newAvailableLines = (stdout.rows ?? DEFAULT_TERMINAL_ROWS) - promptAreaHeight - inputFixedAreaHeight if (newAvailableLines !== availableLines) { setAvailableLines(newAvailableLines) } diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts index a9b362fbe45..807ee554b0d 100644 --- a/packages/cli-kit/src/public/node/system.test.ts +++ b/packages/cli-kit/src/public/node/system.test.ts @@ -18,6 +18,44 @@ vi.mock('fs', async (importOriginal) => { } }) +test.each([ + {name: 'all streams are terminals', stdin: true, stdout: true, stderr: true, expected: true}, + {name: 'stdout is redirected', stdin: true, stdout: false, stderr: true, expected: true}, + {name: 'stderr is redirected', stdin: true, stdout: true, stderr: false, expected: true}, + {name: 'both output streams are redirected', stdin: true, stdout: false, stderr: false, expected: false}, + {name: 'stdin is redirected', stdin: false, stdout: true, stderr: true, expected: false}, + {name: 'CI is enabled', stdin: true, stdout: true, stderr: true, ci: 'true', expected: false}, + {name: 'input is disabled', stdin: true, stdout: true, stderr: true, noInput: 'true', expected: false}, +])('terminalSupportsPrompting when $name', ({stdin, stdout, stderr, ci, noInput, expected}) => { + const streams = [ + {stream: process.stdin, isTTY: stdin}, + {stream: process.stdout, isTTY: stdout}, + {stream: process.stderr, isTTY: stderr}, + ] + const originalProperties = streams.map(({stream}) => ({ + stream, + descriptor: Object.getOwnPropertyDescriptor(stream, 'isTTY'), + })) + vi.stubEnv('CI', ci ?? '') + vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', noInput ?? '') + + try { + for (const {stream, isTTY} of streams) { + Object.defineProperty(stream, 'isTTY', {value: isTTY, configurable: true, writable: true}) + } + expect(system.terminalSupportsPrompting()).toBe(expected) + } finally { + for (const {stream, descriptor} of originalProperties) { + if (descriptor) { + Object.defineProperty(stream, 'isTTY', descriptor) + } else { + Reflect.deleteProperty(stream, 'isTTY') + } + } + vi.unstubAllEnvs() + } +}) + test('terminalSupportsPrompting returns false when input is disabled', () => { vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', 'true') diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index 0659bc0b602..4f89da800ca 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -342,15 +342,15 @@ export function terminalSupportsHyperlinks(): boolean { } /** - * Check if the standard input and output streams support prompting. + * Check if standard input and at least one output stream are terminals that support prompting. * - * @returns True if the standard input and output streams support prompting. + * @returns True if standard input and at least one output stream support prompting. */ export function terminalSupportsPrompting(): boolean { if (isInputDisabled() || isTruthy(process.env.CI)) { return false } - return Boolean(process.stdin.isTTY && process.stdout.isTTY) + return Boolean(process.stdin.isTTY && (process.stderr.isTTY || process.stdout.isTTY)) } /** diff --git a/packages/cli-kit/src/public/node/ui.test.ts b/packages/cli-kit/src/public/node/ui.test.ts index 0276f4cae32..09db6e86f88 100644 --- a/packages/cli-kit/src/public/node/ui.test.ts +++ b/packages/cli-kit/src/public/node/ui.test.ts @@ -7,6 +7,11 @@ import { renderTasks, renderWarning, renderSingleTask, + renderSelectPrompt, + renderConfirmationPrompt, + renderAutocompletePrompt, + renderTextPrompt, + renderDangerousConfirmationPrompt, Task, } from './ui.js' import {AbortSignal} from './abort.js' @@ -14,11 +19,13 @@ import {BugError, FatalError, AbortError, FatalErrorType} from './error.js' import {renderCommandEventAsJson, runWithCommandEvents} from './command-events.js' import {mockAndCaptureOutput, withCapturedStandardStreams} from './testing/output.js' import {TokenizedString} from './output.js' -import {Stdin} from '../../private/node/testing/ui.js' +import {Stdin, waitForInputsToBeReady} from '../../private/node/testing/ui.js' +import {Stdout} from '../../private/node/ui.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' import supportsHyperlinks from 'supports-hyperlinks' import {Writable} from 'stream' +import type {RenderOptions} from 'ink' vi.mock('supports-hyperlinks') @@ -30,6 +37,100 @@ afterEach(() => { mockAndCaptureOutput().clear() }) +describe('prompts', () => { + const choices = [{label: 'First option', value: 'first'}] + const prompts = [ + { + name: 'select', + renderPrompt: (renderOptions: RenderOptions) => + renderSelectPrompt({message: 'Which option?', choices, renderOptions}), + input: '\r', + expected: 'first', + answerLabel: 'First option', + }, + { + name: 'confirmation', + renderPrompt: (renderOptions: RenderOptions) => + renderConfirmationPrompt({message: 'Which option?', renderOptions}), + input: '\r', + expected: true, + answerLabel: 'Yes, confirm', + }, + { + name: 'autocomplete', + renderPrompt: (renderOptions: RenderOptions) => + renderAutocompletePrompt({message: 'Which option?', choices, renderOptions}), + input: '\r', + expected: 'first', + answerLabel: 'First option', + }, + { + name: 'text', + renderPrompt: (renderOptions: RenderOptions) => + renderTextPrompt({message: 'Which option?', defaultValue: 'first', renderOptions}), + input: '\r', + expected: 'first', + answerLabel: 'first', + }, + { + name: 'dangerous confirmation', + renderPrompt: (renderOptions: RenderOptions) => + renderDangerousConfirmationPrompt({message: 'Which option?', confirmation: 'first', renderOptions}), + input: '\u001b', + expected: false, + answerLabel: 'Cancelled', + }, + ] + + describe.each(['text', 'json'] as const)('%s output', (outputMode) => { + test.each(prompts)( + 'renders $name prompts to stderr by default', + async ({renderPrompt, input, expected, answerLabel}) => { + const stdin = new Stdin() + + await withCapturedStandardStreams(async (streams) => { + const result = runWithCommandEvents({outputMode}, () => + renderPrompt({stdin: stdin as unknown as NodeJS.ReadStream, debug: true, patchConsole: false}), + ) + const completion = expect(result).resolves.toBe(expected) + await waitForInputsToBeReady() + stdin.write(input) + await completion + + expect(streams.stdout()).toBe('') + expect(streams.stderr()).toContain('Which option?') + expect(streams.stderr()).toContain(answerLabel) + }) + }, + ) + }) + + test.each(prompts)( + 'preserves the output stream override for $name prompts', + async ({renderPrompt, input, expected}) => { + const stdin = new Stdin() + const stdout = new Stdout({columns: 80, rows: 24}) + + await withCapturedStandardStreams(async (streams) => { + const result = renderPrompt({ + stdin: stdin as unknown as NodeJS.ReadStream, + stdout: stdout as unknown as NodeJS.WriteStream, + debug: true, + patchConsole: false, + }) + const completion = expect(result).resolves.toBe(expected) + await waitForInputsToBeReady() + stdin.write(input) + await completion + + expect(stdout.frames.join('')).toContain('Which option?') + expect(streams.stdout()).toBe('') + expect(streams.stderr()).toBe('') + }) + }, + ) +}) + describe('renderInfo', async () => { test('renders info inside a banner', async () => { // Given diff --git a/packages/cli-kit/src/public/node/ui.tsx b/packages/cli-kit/src/public/node/ui.tsx index ad2bda3c567..593433f0cfb 100644 --- a/packages/cli-kit/src/public/node/ui.tsx +++ b/packages/cli-kit/src/public/node/ui.tsx @@ -292,6 +292,7 @@ export async function renderSelectPrompt( }} />, { + stdout: process.stderr as unknown as NodeJS.WriteStream, ...renderOptions, exitOnCtrlC: false, }, @@ -444,6 +445,7 @@ export async function renderAutocompletePrompt( }} />, { + stdout: process.stderr as unknown as NodeJS.WriteStream, ...renderOptions, exitOnCtrlC: false, }, @@ -662,6 +664,7 @@ export async function renderTextPrompt( }} />, { + stdout: process.stderr as unknown as NodeJS.WriteStream, ...renderOptions, exitOnCtrlC: false, }, @@ -714,6 +717,7 @@ export async function renderDangerousConfirmationPrompt( }} />, { + stdout: process.stderr as unknown as NodeJS.WriteStream, ...renderOptions, exitOnCtrlC: false, }, From c1aa67d40a5ab9fbae6247573e003ce2dd4bad81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 7 Oct 2026 11:52:56 +0200 Subject: [PATCH 2/5] Require stderr terminal support and reduce prompt tests --- docs/cli-kit/ui-kit/readme.md | 2 +- .../src/public/node/base-command.test.ts | 10 +- .../cli-kit/src/public/node/system.test.ts | 2 +- packages/cli-kit/src/public/node/system.ts | 6 +- packages/cli-kit/src/public/node/ui.test.ts | 130 +++++------------- 5 files changed, 47 insertions(+), 103 deletions(-) diff --git a/docs/cli-kit/ui-kit/readme.md b/docs/cli-kit/ui-kit/readme.md index c08acebfcf3..7b4be4f94d8 100644 --- a/docs/cli-kit/ui-kit/readme.md +++ b/docs/cli-kit/ui-kit/readme.md @@ -153,7 +153,7 @@ when you want to display it to the terminal, for example in your exception handl Prompts interrupt the flow of commands to ask the user for some information. There are two main types of prompts: selects and textual prompts. They all take a `message` property which will be displayed as the title of the prompt to the user and is usually in the form of a question. If you forget to use punctuation at the end of `message` we'll add `?` for you. -Prompts read input from stdin and write to stderr by default, so stdout can contain command results. To use a different output stream, pass it as `renderOptions.stdout`. +Prompts read input from stdin and write to stderr by default, so stdout can contain command results. By default, both stdin and stderr must be terminals for prompting. To use a different output stream, pass it as `renderOptions.stdout`. For select prompts, if the terminal is not tall enough to render all the options the prompt will resize to fit the terminal window and a message will appear warning the user that only a fraction of the items is being displayed. The user can then cycle through the items, including the hidden ones, by using the arrow keys. diff --git a/packages/cli-kit/src/public/node/base-command.test.ts b/packages/cli-kit/src/public/node/base-command.test.ts index fb67fd5f482..21c0b43370b 100644 --- a/packages/cli-kit/src/public/node/base-command.test.ts +++ b/packages/cli-kit/src/public/node/base-command.test.ts @@ -14,21 +14,21 @@ import {Flags} from '@oclif/core' import {Ajv} from 'ajv' let originalStdinIsTTY: boolean | undefined -let originalStdoutIsTTY: boolean | undefined +let originalStderrIsTTY: boolean | undefined beforeEach(() => { originalStdinIsTTY = process.stdin.isTTY - originalStdoutIsTTY = process.stdout.isTTY + originalStderrIsTTY = process.stderr.isTTY vi.unstubAllEnvs() // Default: simulate interactive TTY environment Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true}) - Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true}) + Object.defineProperty(process.stderr, 'isTTY', {value: true, configurable: true, writable: true}) vi.stubEnv('CI', '') }) afterEach(() => { Object.defineProperty(process.stdin, 'isTTY', {value: originalStdinIsTTY, configurable: true, writable: true}) - Object.defineProperty(process.stdout, 'isTTY', {value: originalStdoutIsTTY, configurable: true, writable: true}) + Object.defineProperty(process.stderr, 'isTTY', {value: originalStderrIsTTY, configurable: true, writable: true}) mockAndCaptureOutput().clear() }) @@ -631,7 +631,7 @@ describe('applying environments', async () => { runTestInTmpDir('does not throw in TTY mode when a non-TTY required argument is missing', async (tmpDir: string) => { // Given — simulate interactive terminal Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true}) - Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true}) + Object.defineProperty(process.stderr, 'isTTY', {value: true, configurable: true, writable: true}) vi.stubEnv('CI', '') // When diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts index 807ee554b0d..7ab5c4176da 100644 --- a/packages/cli-kit/src/public/node/system.test.ts +++ b/packages/cli-kit/src/public/node/system.test.ts @@ -21,7 +21,7 @@ vi.mock('fs', async (importOriginal) => { test.each([ {name: 'all streams are terminals', stdin: true, stdout: true, stderr: true, expected: true}, {name: 'stdout is redirected', stdin: true, stdout: false, stderr: true, expected: true}, - {name: 'stderr is redirected', stdin: true, stdout: true, stderr: false, expected: true}, + {name: 'stderr is redirected', stdin: true, stdout: true, stderr: false, expected: false}, {name: 'both output streams are redirected', stdin: true, stdout: false, stderr: false, expected: false}, {name: 'stdin is redirected', stdin: false, stdout: true, stderr: true, expected: false}, {name: 'CI is enabled', stdin: true, stdout: true, stderr: true, ci: 'true', expected: false}, diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index 4f89da800ca..3d5659c418a 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -342,15 +342,15 @@ export function terminalSupportsHyperlinks(): boolean { } /** - * Check if standard input and at least one output stream are terminals that support prompting. + * Check if standard input and standard error are terminals that support prompting. * - * @returns True if standard input and at least one output stream support prompting. + * @returns True if standard input and standard error support prompting. */ export function terminalSupportsPrompting(): boolean { if (isInputDisabled() || isTruthy(process.env.CI)) { return false } - return Boolean(process.stdin.isTTY && (process.stderr.isTTY || process.stdout.isTTY)) + return Boolean(process.stdin.isTTY && process.stderr.isTTY) } /** diff --git a/packages/cli-kit/src/public/node/ui.test.ts b/packages/cli-kit/src/public/node/ui.test.ts index 09db6e86f88..ae8c6dd6950 100644 --- a/packages/cli-kit/src/public/node/ui.test.ts +++ b/packages/cli-kit/src/public/node/ui.test.ts @@ -8,7 +8,6 @@ import { renderWarning, renderSingleTask, renderSelectPrompt, - renderConfirmationPrompt, renderAutocompletePrompt, renderTextPrompt, renderDangerousConfirmationPrompt, @@ -20,7 +19,6 @@ import {renderCommandEventAsJson, runWithCommandEvents} from './command-events.j import {mockAndCaptureOutput, withCapturedStandardStreams} from './testing/output.js' import {TokenizedString} from './output.js' import {Stdin, waitForInputsToBeReady} from '../../private/node/testing/ui.js' -import {Stdout} from '../../private/node/ui.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' import supportsHyperlinks from 'supports-hyperlinks' @@ -37,98 +35,44 @@ afterEach(() => { mockAndCaptureOutput().clear() }) -describe('prompts', () => { - const choices = [{label: 'First option', value: 'first'}] - const prompts = [ - { - name: 'select', - renderPrompt: (renderOptions: RenderOptions) => - renderSelectPrompt({message: 'Which option?', choices, renderOptions}), - input: '\r', - expected: 'first', - answerLabel: 'First option', - }, - { - name: 'confirmation', - renderPrompt: (renderOptions: RenderOptions) => - renderConfirmationPrompt({message: 'Which option?', renderOptions}), - input: '\r', - expected: true, - answerLabel: 'Yes, confirm', - }, - { - name: 'autocomplete', - renderPrompt: (renderOptions: RenderOptions) => - renderAutocompletePrompt({message: 'Which option?', choices, renderOptions}), - input: '\r', - expected: 'first', - answerLabel: 'First option', - }, - { - name: 'text', - renderPrompt: (renderOptions: RenderOptions) => - renderTextPrompt({message: 'Which option?', defaultValue: 'first', renderOptions}), - input: '\r', - expected: 'first', - answerLabel: 'first', - }, - { - name: 'dangerous confirmation', - renderPrompt: (renderOptions: RenderOptions) => - renderDangerousConfirmationPrompt({message: 'Which option?', confirmation: 'first', renderOptions}), - input: '\u001b', - expected: false, - answerLabel: 'Cancelled', - }, - ] - - describe.each(['text', 'json'] as const)('%s output', (outputMode) => { - test.each(prompts)( - 'renders $name prompts to stderr by default', - async ({renderPrompt, input, expected, answerLabel}) => { - const stdin = new Stdin() - - await withCapturedStandardStreams(async (streams) => { - const result = runWithCommandEvents({outputMode}, () => - renderPrompt({stdin: stdin as unknown as NodeJS.ReadStream, debug: true, patchConsole: false}), - ) - const completion = expect(result).resolves.toBe(expected) - await waitForInputsToBeReady() - stdin.write(input) - await completion - - expect(streams.stdout()).toBe('') - expect(streams.stderr()).toContain('Which option?') - expect(streams.stderr()).toContain(answerLabel) - }) - }, - ) +test.each([ + { + name: 'select', + renderPrompt: (renderOptions: RenderOptions) => + renderSelectPrompt({message: 'Which option?', choices: [{label: 'First option', value: 'first'}], renderOptions}), + }, + { + name: 'autocomplete', + renderPrompt: (renderOptions: RenderOptions) => + renderAutocompletePrompt({ + message: 'Which option?', + choices: [{label: 'First option', value: 'first'}], + renderOptions, + }), + }, + { + name: 'text', + renderPrompt: (renderOptions: RenderOptions) => + renderTextPrompt({message: 'Which option?', defaultValue: 'first', renderOptions}), + }, + { + name: 'dangerous confirmation', + renderPrompt: (renderOptions: RenderOptions) => + renderDangerousConfirmationPrompt({message: 'Which option?', confirmation: 'first', renderOptions}), + input: '\u001b', + }, +])('renders $name prompts to stderr by default', async ({renderPrompt, input}) => { + const stdin = new Stdin() + + await withCapturedStandardStreams(async (streams) => { + const result = renderPrompt({stdin: stdin as unknown as NodeJS.ReadStream, debug: true, patchConsole: false}) + await waitForInputsToBeReady() + stdin.write(input ?? '\r') + await result + + expect(streams.stdout()).toBe('') + expect(streams.stderr()).toContain('Which option?') }) - - test.each(prompts)( - 'preserves the output stream override for $name prompts', - async ({renderPrompt, input, expected}) => { - const stdin = new Stdin() - const stdout = new Stdout({columns: 80, rows: 24}) - - await withCapturedStandardStreams(async (streams) => { - const result = renderPrompt({ - stdin: stdin as unknown as NodeJS.ReadStream, - stdout: stdout as unknown as NodeJS.WriteStream, - debug: true, - patchConsole: false, - }) - const completion = expect(result).resolves.toBe(expected) - await waitForInputsToBeReady() - stdin.write(input) - await completion - - expect(stdout.frames.join('')).toContain('Which option?') - expect(streams.stdout()).toBe('') - expect(streams.stderr()).toBe('') - }) - }, - ) }) describe('renderInfo', async () => { From 9050cf0f6dff12f126afd2a140dc14e70c9259ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 7 Oct 2026 12:05:03 +0200 Subject: [PATCH 3/5] Remove additional terminal support tests --- .../cli-kit/src/public/node/system.test.ts | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts index 7ab5c4176da..a9b362fbe45 100644 --- a/packages/cli-kit/src/public/node/system.test.ts +++ b/packages/cli-kit/src/public/node/system.test.ts @@ -18,44 +18,6 @@ vi.mock('fs', async (importOriginal) => { } }) -test.each([ - {name: 'all streams are terminals', stdin: true, stdout: true, stderr: true, expected: true}, - {name: 'stdout is redirected', stdin: true, stdout: false, stderr: true, expected: true}, - {name: 'stderr is redirected', stdin: true, stdout: true, stderr: false, expected: false}, - {name: 'both output streams are redirected', stdin: true, stdout: false, stderr: false, expected: false}, - {name: 'stdin is redirected', stdin: false, stdout: true, stderr: true, expected: false}, - {name: 'CI is enabled', stdin: true, stdout: true, stderr: true, ci: 'true', expected: false}, - {name: 'input is disabled', stdin: true, stdout: true, stderr: true, noInput: 'true', expected: false}, -])('terminalSupportsPrompting when $name', ({stdin, stdout, stderr, ci, noInput, expected}) => { - const streams = [ - {stream: process.stdin, isTTY: stdin}, - {stream: process.stdout, isTTY: stdout}, - {stream: process.stderr, isTTY: stderr}, - ] - const originalProperties = streams.map(({stream}) => ({ - stream, - descriptor: Object.getOwnPropertyDescriptor(stream, 'isTTY'), - })) - vi.stubEnv('CI', ci ?? '') - vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', noInput ?? '') - - try { - for (const {stream, isTTY} of streams) { - Object.defineProperty(stream, 'isTTY', {value: isTTY, configurable: true, writable: true}) - } - expect(system.terminalSupportsPrompting()).toBe(expected) - } finally { - for (const {stream, descriptor} of originalProperties) { - if (descriptor) { - Object.defineProperty(stream, 'isTTY', descriptor) - } else { - Reflect.deleteProperty(stream, 'isTTY') - } - } - vi.unstubAllEnvs() - } -}) - test('terminalSupportsPrompting returns false when input is disabled', () => { vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', 'true') From 9e88552f7fa612f4c29e164e6efef456f11eb640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 7 Oct 2026 12:29:07 +0200 Subject: [PATCH 4/5] Update interactive test fixtures to use stderr --- packages/app/src/cli/commands/app/init.test.ts | 8 ++++---- packages/theme/src/cli/utilities/theme-ui.test.ts | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/app/src/cli/commands/app/init.test.ts b/packages/app/src/cli/commands/app/init.test.ts index 90c1d013279..97a95ffe4db 100644 --- a/packages/app/src/cli/commands/app/init.test.ts +++ b/packages/app/src/cli/commands/app/init.test.ts @@ -36,20 +36,20 @@ vi.mock('../../services/init/validate.js') vi.mock('@shopify/cli-kit/node/node-package-manager') let originalStdinIsTTY: boolean | undefined -let originalStdoutIsTTY: boolean | undefined +let originalStderrIsTTY: boolean | undefined beforeEach(() => { originalStdinIsTTY = process.stdin.isTTY - originalStdoutIsTTY = process.stdout.isTTY + originalStderrIsTTY = process.stderr.isTTY vi.unstubAllEnvs() Object.defineProperty(process.stdin, 'isTTY', {value: true, configurable: true, writable: true}) - Object.defineProperty(process.stdout, 'isTTY', {value: true, configurable: true, writable: true}) + Object.defineProperty(process.stderr, 'isTTY', {value: true, configurable: true, writable: true}) vi.stubEnv('CI', '') }) afterEach(() => { Object.defineProperty(process.stdin, 'isTTY', {value: originalStdinIsTTY, configurable: true, writable: true}) - Object.defineProperty(process.stdout, 'isTTY', {value: originalStdoutIsTTY, configurable: true, writable: true}) + Object.defineProperty(process.stderr, 'isTTY', {value: originalStderrIsTTY, configurable: true, writable: true}) }) describe('Init command', () => { diff --git a/packages/theme/src/cli/utilities/theme-ui.test.ts b/packages/theme/src/cli/utilities/theme-ui.test.ts index 9ef1d50018d..db7e96853a8 100644 --- a/packages/theme/src/cli/utilities/theme-ui.test.ts +++ b/packages/theme/src/cli/utilities/theme-ui.test.ts @@ -10,6 +10,7 @@ vi.mock('@shopify/cli-kit/node/ui') beforeEach(() => vi.stubEnv('CI', '')) afterEach(() => { + vi.unstubAllGlobals() vi.unstubAllEnvs() }) @@ -42,7 +43,7 @@ describe('ensureDirectoryConfirmed', () => { vi.stubGlobal('process', { ...process, stdin: {...process.stdin, isTTY: true}, - stdout: {...process.stdout, isTTY: true}, + stderr: {...process.stderr, isTTY: true}, }) vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) @@ -61,7 +62,7 @@ describe('ensureDirectoryConfirmed', () => { vi.stubGlobal('process', { ...process, stdin: {...process.stdin, isTTY: false}, - stdout: {...process.stdout, isTTY: true}, + stderr: {...process.stderr, isTTY: true}, }) const confirmed = await ensureDirectoryConfirmed(false) @@ -103,7 +104,7 @@ describe('ensureLiveThemeConfirmed', () => { vi.stubGlobal('process', { ...process, stdin: {...process.stdin, isTTY: true}, - stdout: {...process.stdout, isTTY: true}, + stderr: {...process.stderr, isTTY: true}, }) }) @@ -154,7 +155,7 @@ describe('ensureLiveThemeConfirmed', () => { vi.stubGlobal('process', { ...process, stdin: {...process.stdin, isTTY: false}, - stdout: {...process.stdout, isTTY: true}, + stderr: {...process.stderr, isTTY: true}, }) const result = await ensureLiveThemeConfirmed(liveTheme, 'start development mode', false) From 22f445572fdcc38730ad8cc898b9410f669c12e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 7 Oct 2026 15:30:43 +0200 Subject: [PATCH 5/5] Require live theme confirmation when prompting is unavailable --- .changeset/strict-live-theme-confirmation.md | 5 +++ .../theme/src/cli/utilities/theme-ui.test.ts | 42 +++++++++++-------- packages/theme/src/cli/utilities/theme-ui.ts | 4 +- 3 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 .changeset/strict-live-theme-confirmation.md diff --git a/.changeset/strict-live-theme-confirmation.md b/.changeset/strict-live-theme-confirmation.md new file mode 100644 index 00000000000..b536a3ca6bf --- /dev/null +++ b/.changeset/strict-live-theme-confirmation.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli': patch +--- + +Require --allow-live for theme dev on a live theme when prompting is unavailable. diff --git a/packages/theme/src/cli/utilities/theme-ui.test.ts b/packages/theme/src/cli/utilities/theme-ui.test.ts index db7e96853a8..771d06e5345 100644 --- a/packages/theme/src/cli/utilities/theme-ui.test.ts +++ b/packages/theme/src/cli/utilities/theme-ui.test.ts @@ -132,35 +132,41 @@ describe('ensureLiveThemeConfirmed', () => { expect(renderConfirmationPrompt).not.toHaveBeenCalled() }) - test('does not prompt for confirmation if acting on a live theme and allowLive flag is true', async () => { - // Given - await ensureLiveThemeConfirmed(liveTheme, 'start development mode', true) - - // Then - expect(renderConfirmationPrompt).not.toHaveBeenCalled() - }) - - test('requires --allow-live when input is disabled', async () => { + test('allows a live theme with --allow-live when prompting is unavailable', async () => { vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', 'true') + vi.stubGlobal('process', { + ...process, + stdin: {...process.stdin, isTTY: false}, + stderr: {...process.stderr, isTTY: false}, + }) - const confirmation = ensureLiveThemeConfirmed(liveTheme, 'start development mode', false) + const result = await ensureLiveThemeConfirmed(liveTheme, 'start development mode', true) - await expect(confirmation).rejects.toThrow( - "Can't start development mode on the live theme when user input is unavailable.", - ) + expect(result).toBe(true) expect(renderConfirmationPrompt).not.toHaveBeenCalled() }) - test('preserves existing behavior for a live theme in a non-interactive environment', async () => { + test.each([ + {name: 'input is disabled', stdin: true, stderr: true, noInput: 'true'}, + {name: 'stdin is redirected', stdin: false, stderr: true}, + {name: 'stderr is redirected', stdin: true, stderr: false}, + {name: 'CI is enabled', stdin: true, stderr: true, ci: 'true'}, + ])('requires --allow-live when $name', async ({stdin, stderr, noInput, ci}) => { + vi.stubEnv('SHOPIFY_FLAG_NO_INPUT', noInput ?? '') + vi.stubEnv('CI', ci ?? '') vi.stubGlobal('process', { ...process, - stdin: {...process.stdin, isTTY: false}, - stderr: {...process.stderr, isTTY: true}, + stdin: {...process.stdin, isTTY: stdin}, + stdout: {...process.stdout, isTTY: true}, + stderr: {...process.stderr, isTTY: stderr}, }) - const result = await ensureLiveThemeConfirmed(liveTheme, 'start development mode', false) + const confirmation = ensureLiveThemeConfirmed(liveTheme, 'start development mode', false) - expect(result).toBe(true) + await expect(confirmation).rejects.toMatchObject({ + message: "Can't start development mode on the live theme when user input is unavailable.", + tryMessage: 'Use `--allow-live` to confirm that you want to continue.', + }) expect(renderConfirmationPrompt).not.toHaveBeenCalled() }) }) diff --git a/packages/theme/src/cli/utilities/theme-ui.ts b/packages/theme/src/cli/utilities/theme-ui.ts index e3f101bf28e..7362eb36e0a 100644 --- a/packages/theme/src/cli/utilities/theme-ui.ts +++ b/packages/theme/src/cli/utilities/theme-ui.ts @@ -65,15 +65,13 @@ export async function ensureLiveThemeConfirmed(theme: Theme, action: string, all return true } - if (isInputDisabled()) { + if (!terminalSupportsPrompting()) { throw new AbortError( `Can't ${action} on the live theme when user input is unavailable.`, 'Use `--allow-live` to confirm that you want to continue.', ) } - if (!terminalSupportsPrompting()) return true - const message = `You're about to ${action} on your live theme "${theme.name}". ` + `This will make changes visible to customers. Are you sure you want to proceed?`