Skip to content

Guide users to use bulk status when an operation is still running in the background #6678

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

Merged
jordanverasamy merged 1 commit into from
Dec 4, 2025
Merged

Guide users to use bulk status when an operation is still running in the background #6678

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
36 changes: 33 additions & 3 deletions packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {downloadBulkOperationResults} from './download-bulk-operation-results.js
import {BulkOperationRunQueryMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-query.js'
import {BulkOperationRunMutationMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-mutation.js'
import {OrganizationApp} from '../../models/organization.js'
import {renderSuccess, renderWarning, renderError} from '@shopify/cli-kit/node/ui'
import {renderSuccess, renderWarning, renderError, renderInfo} from '@shopify/cli-kit/node/ui'
import {ensureAuthenticatedAdminAsApp} from '@shopify/cli-kit/node/session'
import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'
Expand Down Expand Up @@ -355,14 +355,45 @@ describe('executeBulkOperation', () => {
watch: true,
})

expect(watchBulkOperation).toHaveBeenCalledWith(mockAdminSession, createdBulkOperation.id)
Copy link
Contributor Author

@jordanverasamy jordanverasamy Dec 4, 2025

Choose a reason for hiding this comment

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

(I don't think this assertion was providing actual value, since the thing we actually care about is that the success banner was rendered; I opted to just remove it instead of fixing the parameters on the assertion.)

expect(renderSuccess).toHaveBeenCalledWith(
expect.objectContaining({
headline: expect.stringContaining('Bulk operation succeeded:'),
}),
)
})

test('renders help message in an info banner when watch is provided and user aborts', async () => {
const query = '{ products { edges { node { id } } } }'
const initialResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = {
bulkOperation: createdBulkOperation,
userErrors: [],
}
const runningOperation = {
...createdBulkOperation,
status: 'RUNNING' as const,
objectCount: '100',
}

vi.mocked(runBulkOperationQuery).mockResolvedValue(initialResponse)
vi.mocked(watchBulkOperation).mockImplementation(async (_session, _id, signal, onAbort) => {
onAbort()
return runningOperation
})

await executeBulkOperation({
remoteApp: mockRemoteApp,
storeFqdn,
query,
watch: true,
})

expect(renderInfo).toHaveBeenCalledWith({
headline: `Bulk operation ${createdBulkOperation.id} is still running in the background.`,
body: ['Monitor its progress with:', {command: expect.stringContaining('shopify app bulk status')}],
})
expect(downloadBulkOperationResults).not.toHaveBeenCalled()
})

test('writes results to file when --output-file flag is provided', async () => {
const query = '{ products { edges { node { id } } } }'
const outputFile = '/tmp/results.jsonl'
Expand Down Expand Up @@ -450,7 +481,6 @@ describe('executeBulkOperation', () => {
watch: true,
})

expect(watchBulkOperation).toHaveBeenCalledWith(mockAdminSession, createdBulkOperation.id)
Copy link
Contributor Author

@jordanverasamy jordanverasamy Dec 4, 2025

Choose a reason for hiding this comment

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

(Same as https://github.com/Shopify/cli/pull/6678/files#r2587231786.)

expect(renderError).toHaveBeenCalledWith(
expect.objectContaining({
headline: expect.any(String),
Expand Down
28 changes: 24 additions & 4 deletions packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import {watchBulkOperation, type BulkOperation} from './watch-bulk-operation.js'
import {formatBulkOperationStatus} from './format-bulk-operation-status.js'
import {downloadBulkOperationResults} from './download-bulk-operation-results.js'
import {OrganizationApp} from '../../models/organization.js'
import {renderSuccess, renderInfo, renderError, renderWarning} from '@shopify/cli-kit/node/ui'
import {renderSuccess, renderInfo, renderError, renderWarning, TokenItem} from '@shopify/cli-kit/node/ui'
import {outputContent, outputToken, outputResult} from '@shopify/cli-kit/node/output'
import {ensureAuthenticatedAdminAsApp} from '@shopify/cli-kit/node/session'
import {AbortError, BugError} from '@shopify/cli-kit/node/error'
import {AbortController} from '@shopify/cli-kit/node/abort'
import {parse} from 'graphql'
import {readFile, writeFile, fileExists} from '@shopify/cli-kit/node/fs'

Expand Down Expand Up @@ -76,8 +77,19 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr
const createdOperation = bulkOperationResponse?.bulkOperation
if (createdOperation) {
if (watch) {
const finishedOperation = await watchBulkOperation(adminSession, createdOperation.id)
await renderBulkOperationResult(finishedOperation, outputFile)
const abortController = new AbortController()
const operation = await watchBulkOperation(adminSession, createdOperation.id, abortController.signal, () =>
abortController.abort(),
)

if (abortController.signal.aborted) {
renderInfo({
Copy link
Contributor Author

@jordanverasamy jordanverasamy Dec 4, 2025

Choose a reason for hiding this comment

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

Made this an info banner per the brief, but I do think it looks a bit odd being all grey, maybe a success banner would more appropriately convey to the user "nothing went wrong, all is still good and running nicely". I'm not really sure. Let me know what you think is correct!

Copy link
Contributor

@gonzaloriestra gonzaloriestra Dec 4, 2025

Choose a reason for hiding this comment

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

I agree a success banner would be better. Actually, why not showing exactly the same message as when you run it without --watch?

╭─ success ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                                                                                           │
│  Bulk operation started.                                                                                                                                  │
│                                                                                                                                                           │
│  Monitor its progress with:                                                                                                                               │
│  shopify app bulk status --id="gid://shopify/BulkOperation/5749231780082"                                                                                 │
│                                                                                                                                                           │
│    • ID: gid://shopify/BulkOperation/5749231780082                                                                                                        │
│    • Status: CREATED                                                                                                                                      │
│    • Created at: 2025-12-04T10:37:57Z                                                                                                                     │
│                                                                                                                                                           │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

Copy link
Contributor Author

@jordanverasamy jordanverasamy Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I considered this but decided it was a slightly different context so the most helpful possible messaging to the user is slightly different. I'll log this for Nick to think about though! TY!

Copy link
Contributor

@nickwesselman nickwesselman Dec 5, 2025

Choose a reason for hiding this comment

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

Since this is the Ctrl+C banner, I think info is correct, since it’s just informing you that the query is still running.

headline: `Bulk operation ${operation.id} is still running in the background.`,
body: statusCommandHelpMessage(operation.id),
})
} else {
await renderBulkOperationResult(operation, outputFile)
}
} else {
await renderBulkOperationResult(createdOperation, outputFile)
}
Expand Down Expand Up @@ -105,7 +117,11 @@ async function renderBulkOperationResult(operation: BulkOperation, outputFile?:

switch (operation.status) {
case 'CREATED':
renderSuccess({headline: 'Bulk operation started.', customSections})
renderSuccess({
headline: 'Bulk operation started.',
body: statusCommandHelpMessage(operation.id),
customSections,
})
break
case 'COMPLETED':
if (operation.url) {
Expand Down Expand Up @@ -147,6 +163,10 @@ function validateGraphQLDocument(graphqlOperation: string, variablesJsonl?: stri
}
}

function statusCommandHelpMessage(operationId: string): TokenItem {
return ['Monitor its progress with:', {command: `shopify app bulk status --id="${operationId}}"`}]
}

function isMutation(graphqlOperation: string): boolean {
const document = parse(graphqlOperation)
const operation = document.definitions.find((def) => def.kind === 'OperationDefinition')
Expand Down
51 changes: 37 additions & 14 deletions packages/app/src/cli/services/bulk-operations/watch-bulk-operation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {sleep} from '@shopify/cli-kit/node/system'
import {renderSingleTask} from '@shopify/cli-kit/node/ui'
import {describe, test, expect, vi, beforeEach} from 'vitest'
import {outputContent} from '@shopify/cli-kit/node/output'
import {AbortController} from '@shopify/cli-kit/node/abort'

vi.mock('./format-bulk-operation-status.js')
vi.mock('@shopify/cli-kit/node/api/admin')
Expand All @@ -14,6 +15,7 @@ vi.mock('@shopify/cli-kit/node/ui')
describe('watchBulkOperation', () => {
const mockAdminSession = {token: 'test-token', storeFqdn: 'test.myshopify.com'}
const operationId = 'gid://shopify/BulkOperation/123'
let abortController: AbortController

const runningOperation = {
id: operationId,
Expand All @@ -30,8 +32,13 @@ describe('watchBulkOperation', () => {
}

beforeEach(() => {
abortController = new AbortController()
vi.mocked(sleep).mockResolvedValue()
vi.mocked(formatBulkOperationStatus).mockReturnValue(outputContent`formatted status`)
vi.mocked(renderSingleTask).mockImplementation(async ({task, onAbort}) => {
if (onAbort) onAbort()
return task(() => {})
})
})

test('polls until operation completes and returns the final operation', async () => {
Expand All @@ -40,11 +47,7 @@ describe('watchBulkOperation', () => {
.mockResolvedValueOnce({bulkOperation: runningOperation})
.mockResolvedValueOnce({bulkOperation: completedOperation})

vi.mocked(renderSingleTask).mockImplementation(async ({task}) => {
return task(() => {})
})

const result = await watchBulkOperation(mockAdminSession, operationId)
const result = await watchBulkOperation(mockAdminSession, operationId, abortController.signal, () => {})

expect(result).toEqual(completedOperation)
expect(adminRequestDoc).toHaveBeenCalledTimes(3)
Expand All @@ -65,11 +68,7 @@ describe('watchBulkOperation', () => {
.mockResolvedValueOnce({bulkOperation: runningOperation})
.mockResolvedValueOnce({bulkOperation: terminalOperation})

vi.mocked(renderSingleTask).mockImplementation(async ({task}) => {
return task(() => {})
})

const result = await watchBulkOperation(mockAdminSession, operationId)
const result = await watchBulkOperation(mockAdminSession, operationId, abortController.signal, () => {})

expect(result).toEqual(terminalOperation)
expect(adminRequestDoc).toHaveBeenCalledTimes(3)
Expand Down Expand Up @@ -97,7 +96,7 @@ describe('watchBulkOperation', () => {
return task(mockUpdateStatus)
})

await watchBulkOperation(mockAdminSession, operationId)
await watchBulkOperation(mockAdminSession, operationId, abortController.signal, () => {})

expect(mockUpdateStatus).toHaveBeenNthCalledWith(1, outputContent`processed 10 objects`)
expect(mockUpdateStatus).toHaveBeenNthCalledWith(2, outputContent`processed 20 objects`)
Expand All @@ -107,10 +106,34 @@ describe('watchBulkOperation', () => {
test('throws when operation not found', async () => {
vi.mocked(adminRequestDoc).mockResolvedValue({bulkOperation: null})

vi.mocked(renderSingleTask).mockImplementation(async ({task}) => {
return task(() => {})
await expect(watchBulkOperation(mockAdminSession, operationId, abortController.signal, () => {})).rejects.toThrow(
'bulk operation not found',
)
})

describe('when signal is aborted during polling', () => {
beforeEach(() => {
let callCount = 0
vi.mocked(adminRequestDoc).mockImplementation(async () => {
callCount++
if (callCount === 2) {
abortController.abort()
}
return {bulkOperation: runningOperation}
})
})

await expect(watchBulkOperation(mockAdminSession, operationId)).rejects.toThrow('bulk operation not found')
test('returns current state of the operation, even if it is not terminal', async () => {
const result = await watchBulkOperation(mockAdminSession, operationId, abortController.signal, () => {})

expect(result.status).toBe('RUNNING')
expect(result).toEqual(runningOperation)
})

test('calls the onAbort callback', async () => {
const onAbort = vi.fn()
await watchBulkOperation(mockAdminSession, operationId, abortController.signal, onAbort)
expect(onAbort).toHaveBeenCalled()
})
})
})
19 changes: 15 additions & 4 deletions packages/app/src/cli/services/bulk-operations/watch-bulk-operation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,24 @@ import {sleep} from '@shopify/cli-kit/node/system'
import {AdminSession} from '@shopify/cli-kit/node/session'
import {outputContent} from '@shopify/cli-kit/node/output'
import {renderSingleTask} from '@shopify/cli-kit/node/ui'
import {AbortSignal} from '@shopify/cli-kit/node/abort'

const TERMINAL_STATUSES = ['COMPLETED', 'FAILED', 'CANCELED', 'EXPIRED']
const POLL_INTERVAL_SECONDS = 5
const API_VERSION = '2026-01'

export type BulkOperation = NonNullable<GetBulkOperationByIdQuery['bulkOperation']>

export async function watchBulkOperation(adminSession: AdminSession, operationId: string): Promise<BulkOperation> {
export async function watchBulkOperation(
adminSession: AdminSession,
operationId: string,
abortSignal: AbortSignal,
onAbort: () => void,
): Promise<BulkOperation> {
return renderSingleTask<BulkOperation>({
title: outputContent`Polling bulk operation...`,
task: async (updateStatus) => {
const poller = pollBulkOperation(adminSession, operationId)
const poller = pollBulkOperation(adminSession, operationId, abortSignal)

while (true) {
// eslint-disable-next-line no-await-in-loop
Expand All @@ -31,12 +37,14 @@ export async function watchBulkOperation(adminSession: AdminSession, operationId
}
}
},
onAbort,
})
}

async function* pollBulkOperation(
adminSession: AdminSession,
operationId: string,
abortSignal: AbortSignal,
): AsyncGenerator<BulkOperation, BulkOperation> {
while (true) {
// eslint-disable-next-line no-await-in-loop
Expand All @@ -48,14 +56,17 @@ async function* pollBulkOperation(

const latestOperationState = response.bulkOperation

if (TERMINAL_STATUSES.includes(latestOperationState.status)) {
if (TERMINAL_STATUSES.includes(latestOperationState.status) || abortSignal.aborted) {
return latestOperationState
} else {
yield latestOperationState
}

// eslint-disable-next-line no-await-in-loop
await sleep(POLL_INTERVAL_SECONDS)
await Promise.race([
sleep(POLL_INTERVAL_SECONDS),
new Promise((resolve) => abortSignal.addEventListener('abort', resolve)),
])
}
}

Expand Down
20 changes: 16 additions & 4 deletions packages/cli-kit/src/private/node/ui/components/SingleTask.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,33 @@
import {LoadingBar} from './LoadingBar.js'
import {useExitOnCtrlC} from '../hooks/use-exit-on-ctrl-c.js'
import {handleCtrlC} from '../../ui.js'
import {TokenizedString} from '../../../../public/node/output.js'
import React, {useEffect, useState} from 'react'
import {useApp} from 'ink'
import {useApp, useInput, useStdin} from 'ink'

interface SingleTaskProps<T> {
title: TokenizedString
task: (updateStatus: (status: TokenizedString) => void) => Promise<T>
onComplete?: (result: T) => void
onAbort?: () => void
noColor?: boolean
}

const SingleTask = <T,>({task, title, onComplete, noColor}: SingleTaskProps<T>) => {
const SingleTask = <T,>({task, title, onComplete, onAbort, noColor}: SingleTaskProps<T>) => {
const [status, setStatus] = useState(title)
const [isDone, setIsDone] = useState(false)
const {exit: unmountInk} = useApp()
useExitOnCtrlC()
const {isRawModeSupported} = useStdin()

useInput(
(input, key) => {
if (onAbort) {
handleCtrlC(input, key, onAbort)
} else {
handleCtrlC(input, key)
}
},
{isActive: Boolean(isRawModeSupported)},
Copy link
Contributor Author

@jordanverasamy jordanverasamy Dec 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This part is basically saying: only activate this behaviour if we're in an interactive terminal.

If STDIN isn't a TTY, for example if input is being piped, we're running in CI, or it's otherwise non-interactive, then isRawModeSupported will be false and the useInput hook will helpfully not do anything.

)

useEffect(() => {
task(setStatus)
Expand Down
10 changes: 8 additions & 2 deletions packages/cli-kit/src/public/node/ui.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ export async function renderTasks<TContext>(
export interface RenderSingleTaskOptions<T> {
title: TokenizedString
task: (updateStatus: (status: TokenizedString) => void) => Promise<T>
onAbort?: () => void
renderOptions?: RenderOptions
}

Expand All @@ -504,10 +505,15 @@ export interface RenderSingleTaskOptions<T> {
* ▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
* Loading app ...
*/
export async function renderSingleTask<T>({title, task, renderOptions}: RenderSingleTaskOptions<T>): Promise<T> {
export async function renderSingleTask<T>({
title,
task,
onAbort,
renderOptions,
}: RenderSingleTaskOptions<T>): Promise<T> {
// eslint-disable-next-line max-params
return new Promise<T>((resolve, reject) => {
render(<SingleTask title={title} task={task} onComplete={resolve} />, {
render(<SingleTask title={title} task={task} onComplete={resolve} onAbort={onAbort} />, {
...renderOptions,
exitOnCtrlC: false,
}).catch(reject)
Expand Down