Repository navigation
Add typed JSON output to app bulk execute - #8783
Conversation
|
/snapit |
|
🫰✨ Thanks @isaacroldan! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261006083344Caution After installing, validate the version by running |
d7aaae1 to
2a27ac7
Compare
|
/snapit |
|
🫰✨ Thanks @isaacroldan! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261006100618Caution After installing, validate the version by running |
2816253 to
58364d0
Compare
dmerand
left a comment
There was a problem hiding this comment.
This is working overall in my tests + code changes LGTM. Agent smoke-test found an edge case that's worth double-checking.
| } | ||
| if (!operation) throw new BugError('Bulk operation response returned no operation.') | ||
|
|
||
| const partial = operation.type === 'MUTATION' && results !== undefined && resultsContainUserErrors(results) |
There was a problem hiding this comment.
[P2] Detect aliased mutation errors before reporting success
A mutation can select errors: userErrors { message }. A completed bulk operation can then contain a failed row such as:
{"data":{"productUpdate":{"errors":[{"message":"Rejected"}]}}}resultsContainUserErrors() checks only the literal nested key userErrors. For this row, partial is false:
- With
--watch --json, the command reportsstatus: "success"instead of"partial". - With
--watch --json --output-file, it writes the failed row and emits the file receipt. - Both paths leave
process.exitCodeunset (normal exit 0), instead of setting it to 1.
I reproduced both cases through the command, service, schema, and output writer. The tests use real temporary files and simulated external authentication/API responses. No live store operation was run. The same tests pass with non-aliased userErrors and correctly set exit code 1.
Please use the submitted mutation's selection to identify error-response keys, including aliases. Add regression tests for inline and file output, and preserve the native JSONL bytes. Do not treat every field named errors as an error: query data can use that name.
There was a problem hiding this comment.
Fixed in af72281470. The JSON result now uses the submitted mutation selection to find userErrors response keys, including field/root aliases and fragments.
Inline output reports partial and exits 1. File output keeps the exact JSONL bytes and emits its receipt with exit 1. Ordinary fields aliased as errors or userErrors are not counted as mutation errors.
Added command regressions for inline/file output and fragments. All four fail with the old check restored. All 149 bulk tests, app build/type checks, lint, and Knip pass. No live store operation was run.
🤖 Generated by the review-pr skill
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/system.d.ts@@ -104,9 +104,9 @@ export declare function sleep(seconds: number): Promise<void>;
*/
export declare function terminalSupportsHyperlinks(): boolean;
/**
- * Check if standard input and standard error are terminals that support prompting.
+ * Check if the standard input and output streams support prompting.
*
- * @returns True if standard input and standard error support prompting.
+ * @returns True if the standard input and output streams support prompting.
*/
export declare function terminalSupportsPrompting(): boolean;
/**
packages/cli-kit/dist/public/node/ui.d.ts@@ -21,7 +21,6 @@ interface UIDebugOptions {
skipTTYCheck?: boolean;
}
export interface RenderConcurrentOptions extends PartialBy<ConcurrentOutputProps, 'abortSignal'> {
- /** Ink options for terminal UI. Finite JSON output uses the command event channel on stderr instead. */
renderOptions?: RenderOptions;
}
/**
packages/cli-kit/dist/private/node/ui/components/ConcurrentOutput.d.ts@@ -1,4 +1,4 @@
-import { type OutputProcess } from '../../../../public/node/output.js';
+import { OutputProcess } from '../../../../public/node/output.js';
import { AbortSignal } from '../../../../public/node/abort.js';
import { FunctionComponent } from 'react';
export interface ConcurrentOutputProps {
@@ -6,21 +6,14 @@ export interface ConcurrentOutputProps {
prefixColumnSize?: number;
abortSignal: AbortSignal;
showTimestamps?: boolean;
- /**
- * Keeps terminal UI running after all processes finish. Defaults to false.
- * In JSON mode, false uses finite progress/diagnostic events; true retains streaming terminal UI.
- */
keepRunningAfterProcessesResolve?: boolean;
useAlternativeColorPalette?: boolean;
}
interface ConcurrentOutputContext {
outputPrefix?: string;
- /** Controls ANSI stripping for terminal output. JSON diagnostics are always unstyled. */
stripAnsi?: boolean;
}
declare function useConcurrentOutputContext<T>(context: ConcurrentOutputContext, callback: () => T): T;
-/** Runs finite processes concurrently and routes their output through the shared diagnostic context. */
-export declare function runConcurrentProcessesForJson({ processes, abortSignal, }: Pick<ConcurrentOutputProps, 'processes' | 'abortSignal'>): Promise<void>;
/**
* Renders output from concurrent processes to the terminal.
* Output will be divided in a three column layout
|
af72281 to
60eeaeb
Compare
WHY are these changes introduced?
Related to https://github.com/shop/issues-develop/issues/23664
Expose a finite, typed JSON result for bulk execution while keeping the existing raw JSONL and file formats available.
WHAT is this pull request doing?
Add JSON output for
app bulk executeon top of the earlier stack layers. Separate preparation, execution, and final presentation. Reuse the shared operation projection and diagnostic events.Unwatched execution returns operation metadata. Watched JSON output includes native data in
resultsJsonl; file output emits an absolutepath/formatreceipt. Preserve the existing text-mode output and exact JSONL file contents. Treat query records as data and completed zero-match queries as successful empty exports. Detect mutation row errors through the selected userErrors fields, including aliases and fragments, so watched JSON execution exits nonzero when a row fails.Representative text output:
Matching JSON output:
{ "storeDomain": "shop.myshopify.com", "apiVersion": "2026-01", "status": "success", "operation": { "gid": "gid://shopify/BulkOperation/123", "type": "QUERY", "status": "RUNNING", "errorCode": null, "createdAt": "2026-09-01T00:00:00Z", "completedAt": null, "objectCount": "2", "url": null, "partialDataUrl": null } }How to manually test your changes?
Use a linked app installed on a development store.
pnpm shopify app bulk execute --store <store.myshopify.com> --query 'query { products { edges { node { id } } } }', then repeat with--json.--watch. Without JSON mode, confirm stdout still contains raw JSONL. With JSON mode, confirm one result object containsresultsJsonl.--output-file ./results.jsonlto the watched JSON command. Confirm stdout contains only an absolute JSONL file receipt and the file contains native JSONL.query { products(query: "id:0") { edges { node { id } } } }. Confirm exit 0, an empty file, and a file receipt.id errors: tagsfor products with tags. Confirm watched JSON output reports success rather than treating tags as errors.pnpm shopify app bulk execute --json-schema.Checklist