From d4182387bfc9c76c2c3c3b37bd02c9fd064aae54 Mon Sep 17 00:00:00 2001 From: River Date: Fri, 23 Oct 2026 13:42:47 +0000 Subject: [PATCH 1/2] Widen piped tables and word store-not-found advice for tokens Two small things the service account bug bash turned up in the store commands' output. `store list` piped into a file or a CI log came out squeezed: Ink has no terminal width to go on when stdout isn't a terminal, so it laid the table out in 80 columns, cut the "Type" header to "Typ" and wrapped every cell onto a second line. `renderTable` now gives the table the width it needs in that case; a real terminal keeps its own width and wraps as before. `store info`'s not-found advice told a service account to check it was "signed in to an account with access to the store". A token isn't signed in, and the account's store access is set in the admin, so with a service account token the advice now says what that account can see: the stores it created, or any store in an organization where it has Dev store management access. Co-authored-by: Eddie Gao --- .changeset/store-output-polish.md | 6 ++ .../node/ui/components/Table/Table.tsx | 24 ++++++- packages/cli-kit/src/public/node/ui.test.ts | 62 +++++++++++++++++++ packages/cli-kit/src/public/node/ui.tsx | 20 +++++- .../store-lookup/destinations.test.ts | 15 ++++- .../utilities/store-lookup/destinations.ts | 14 ++++- 6 files changed, 133 insertions(+), 8 deletions(-) create mode 100644 .changeset/store-output-polish.md diff --git a/.changeset/store-output-polish.md b/.changeset/store-output-polish.md new file mode 100644 index 00000000000..4ed786f475e --- /dev/null +++ b/.changeset/store-output-polish.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-kit': patch +'@shopify/cli': patch +--- + +Keep tables on one line per row when the output isn't a terminal, and word the store-not-found advice for service account tokens. diff --git a/packages/cli-kit/src/private/node/ui/components/Table/Table.tsx b/packages/cli-kit/src/private/node/ui/components/Table/Table.tsx index aa3764258d7..dabcdfb9ed5 100644 --- a/packages/cli-kit/src/private/node/ui/components/Table/Table.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Table/Table.tsx @@ -1,5 +1,6 @@ import {Row} from './Row.js' import ScalarDict from './ScalarDict.js' +import {Column} from './Column.js' import {unstyled} from '../../../../../public/node/output.js' import React from 'react' import {Box} from 'ink' @@ -14,8 +15,10 @@ export interface TableProps { columns: TableColumn } -function Table({rows, columns: columnsConfiguration}: TableProps) { - const columns = Object.entries(columnsConfiguration).map(([key, {header, color}]) => { +const COLUMN_GAP = 2 + +function tableColumns({rows, columns: columnsConfiguration}: TableProps): Column[] { + return Object.entries(columnsConfiguration).map(([key, {header, color}]) => { const headerWidth = String(header ?? key).length const columnWidths = rows.map((row) => { const value = row[key] @@ -33,6 +36,23 @@ function Table({rows, columns: columnsConfiguration}: Tabl color, } }) +} + +/** + * The width the table needs to show every cell on one line: the widest value or header of each column, plus the + * gap between columns. + * + * @param props - The table's rows and columns. + * @returns The width in characters. + */ +export function tableWidth(props: TableProps): number { + const columns = tableColumns(props) + const gaps = Math.max(columns.length - 1, 0) * COLUMN_GAP + return columns.reduce((width, column) => width + column.width, gaps) +} + +function Table({rows, columns: columnsConfiguration}: TableProps) { + const columns = tableColumns({rows, columns: columnsConfiguration}) const headings = Object.entries(columnsConfiguration).reduce( (headings, [column, {header}]) => ({...headings, [column]: header ?? column}), {}, diff --git a/packages/cli-kit/src/public/node/ui.test.ts b/packages/cli-kit/src/public/node/ui.test.ts index 09f787a45f4..b55addfaad8 100644 --- a/packages/cli-kit/src/public/node/ui.test.ts +++ b/packages/cli-kit/src/public/node/ui.test.ts @@ -11,6 +11,7 @@ import { renderAutocompletePrompt, renderTextPrompt, renderDangerousConfirmationPrompt, + renderTable, Task, } from './ui.js' import {AbortSignal} from './abort.js' @@ -185,6 +186,67 @@ describe('renderInfo', async () => { }) }) +describe('renderTable', async () => { + const rows = [ + { + subdomain: 'parimi-photography-dev-test-store', + name: 'Parimi photography dev test store', + type: 'Dev', + plan: 'Plus', + created: 'Apr 01, 2026', + }, + ] + const columns = { + subdomain: {header: 'Subdomain'}, + name: {header: 'Name'}, + type: {header: 'Type'}, + plan: {header: 'Plan'}, + created: {header: 'Created'}, + } + + test('gives a table wider than 80 columns the room it needs when the output is not a terminal', async () => { + // Given: piped output has no column count. + const mockOutput = mockAndCaptureOutput() + const stdoutColumns = Object.getOwnPropertyDescriptor(process.stdout, 'columns') + Object.defineProperty(process.stdout, 'columns', {value: undefined, configurable: true}) + + try { + // When + renderTable({rows, columns}) + } finally { + if (stdoutColumns) Object.defineProperty(process.stdout, 'columns', stdoutColumns) + else delete (process.stdout as {columns?: number}).columns + } + + // Then: one line per row, with every header and cell whole. + expect(unstyled(mockOutput.info())).toMatchInlineSnapshot(` + "Subdomain Name Type Plan Created + ───────────────────────────────── ───────────────────────────────── ──── ──── ──────────── + parimi-photography-dev-test-store Parimi photography dev test store Dev Plus Apr 01, 2026" + `) + }) + + test("keeps a terminal's own width", async () => { + // Given: a terminal 80 columns wide. + const mockOutput = mockAndCaptureOutput() + const stdoutColumns = Object.getOwnPropertyDescriptor(process.stdout, 'columns') + Object.defineProperty(process.stdout, 'columns', {value: 80, configurable: true}) + + try { + // When + renderTable({rows, columns}) + } finally { + if (stdoutColumns) Object.defineProperty(process.stdout, 'columns', stdoutColumns) + else delete (process.stdout as {columns?: number}).columns + } + + // Then: Ink fits the table to the terminal, as before. + const lines = unstyled(mockOutput.info()).split('\n') + expect(Math.max(...lines.map((line) => line.length))).toBeLessThanOrEqual(80) + expect(lines.length).toBeGreaterThan(3) + }) +}) + describe('renderSuccess', async () => { test('renders a success message 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 3e210187878..b7eef236b76 100644 --- a/packages/cli-kit/src/public/node/ui.tsx +++ b/packages/cli-kit/src/public/node/ui.tsx @@ -11,12 +11,12 @@ import { ConcurrentOutputProps, runConcurrentProcessesForJson, } from '../../private/node/ui/components/ConcurrentOutput.js' -import {handleCtrlC, render, renderOnce} from '../../private/node/ui.js' +import {handleCtrlC, render, renderOnce, Stdout} from '../../private/node/ui.js' import {alert, AlertOptions} from '../../private/node/ui/alert.js' import {CustomSection} from '../../private/node/ui/components/Alert.js' import {FatalError} from '../../private/node/ui/components/FatalError.js' import ScalarDict from '../../private/node/ui/components/Table/ScalarDict.js' -import {Table, TableColumn, TableProps} from '../../private/node/ui/components/Table/Table.js' +import {Table, TableColumn, TableProps, tableWidth} from '../../private/node/ui/components/Table/Table.js' import { tokenItemToString, type InlineToken, @@ -489,9 +489,23 @@ interface RenderTableOptions extends TableProps { * 3 John Smith jon@smith.com */ export function renderTable({renderOptions, ...props}: RenderTableOptions) { - return renderOnce(, {renderOptions}) + return renderOnce(
, {renderOptions: tableRenderOptions(props, renderOptions)}) } +// Output that isn't a terminal (a pipe, a file, a CI log) has no width, and Ink then lays the table out in 80 +// columns, squeezing headers to "Typ" and wrapping cells onto extra lines. Give the table the width it needs +// instead; a terminal keeps its own width. +function tableRenderOptions( + props: TableProps, + renderOptions: RenderOptions | undefined, +): RenderOptions | undefined { + if (renderOptions?.stdout || process.stdout.columns !== undefined) return renderOptions + const columns = Math.max(DEFAULT_OUTPUT_COLUMNS, tableWidth(props)) + return {...renderOptions, stdout: new Stdout({columns}) as unknown as NodeJS.WriteStream} +} + +const DEFAULT_OUTPUT_COLUMNS = 80 + interface RenderTasksOptions { renderOptions?: RenderOptions noProgressBar?: boolean diff --git a/packages/store/src/cli/utilities/store-lookup/destinations.test.ts b/packages/store/src/cli/utilities/store-lookup/destinations.test.ts index 1f96c724266..afec633056b 100644 --- a/packages/store/src/cli/utilities/store-lookup/destinations.test.ts +++ b/packages/store/src/cli/utilities/store-lookup/destinations.test.ts @@ -1,6 +1,6 @@ import {fetchDestinationsContext} from './destinations.js' import {businessPlatformRequestDoc} from '@shopify/cli-kit/node/api/business-platform' -import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' +import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' import {AbortError} from '@shopify/cli-kit/node/error' import {describe, test, expect, vi, beforeEach} from 'vitest' @@ -31,6 +31,19 @@ describe('fetchDestinationsContext', () => { const err = await fetchDestinationsContext({store: SHOP}).catch((error: unknown) => error) expect(err).toBeInstanceOf(AbortError) expect((err as AbortError).message).toContain(SHOP) + expect((err as AbortError).tryMessage).toContain("you're logged in to an account with access to the store") + }) + + test("advises on the service account's access, not a login, when a service account token is set", async () => { + vi.mocked(isServiceAccountTokenSet).mockReturnValue(true) + vi.mocked(businessPlatformRequestDoc).mockResolvedValueOnce({ + currentUserAccount: {destinations: {nodes: []}}, + } as never) + + const err = await fetchDestinationsContext({store: SHOP}).catch((error: unknown) => error) + expect(err).toBeInstanceOf(AbortError) + expect((err as AbortError).tryMessage).toContain('the service account has access to the store') + expect((err as AbortError).tryMessage).not.toContain('logged in') }) test('throws AbortError when domain match is missing from results', async () => { diff --git a/packages/store/src/cli/utilities/store-lookup/destinations.ts b/packages/store/src/cli/utilities/store-lookup/destinations.ts index a9b97dd8bf4..46b9081bec2 100644 --- a/packages/store/src/cli/utilities/store-lookup/destinations.ts +++ b/packages/store/src/cli/utilities/store-lookup/destinations.ts @@ -3,7 +3,7 @@ import {StoreInfoDestinations} from '../../api/graphql/business-platform-destina import {StoreInfoOwningOrg} from '../../api/graphql/business-platform-destinations/generated/store-info-owning-org.js' import {AbortError} from '@shopify/cli-kit/node/error' import {businessPlatformRequestDoc} from '@shopify/cli-kit/node/api/business-platform' -import {ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' +import {ensureAuthenticatedBusinessPlatform, isServiceAccountTokenSet} from '@shopify/cli-kit/node/session' import {numericIdFromEncodedGid} from '@shopify/cli-kit/common/gid' import {extractHost} from '@shopify/cli-kit/common/url' import {outputDebug} from '@shopify/cli-kit/node/output' @@ -59,7 +59,7 @@ export async function fetchDestinationsContext(options: FetchDestinationsContext if (!matchedNode) { throw new StoreLookupStoreNotFoundError( `Couldn't find a store with domain ${options.store} for the current account.`, - 'Verify the domain (must be the canonical `myshopify.com` FQDN) and that you are signed in to an account with access to the store. Inactive shops are not searchable.', + storeNotFoundAdvice(), ) } @@ -68,6 +68,16 @@ export async function fetchDestinationsContext(options: FetchDestinationsContext return {...(owningOrg ? {owningOrg} : {})} } +// A token isn't "signed in", and the account behind it is a service account whose store access is set in the +// Shopify admin, so the advice names that instead of a login. +function storeNotFoundAdvice(): string { + const domain = 'Check the domain (it must be the permanent `myshopify.com` domain)' + if (isServiceAccountTokenSet()) { + return `${domain} and that the service account has access to the store: it can open the stores it created, or any store in an organization where it has Dev store management access. Inactive stores aren't searchable.` + } + return `${domain} and that you're logged in to an account with access to the store. Inactive stores aren't searchable.` +} + async function fetchOwningOrg( destinationPublicId: string, token: string, From 3d97a266737964af55bf043c24b3f98fb20ee4dc Mon Sep 17 00:00:00 2001 From: River Date: Fri, 23 Oct 2026 16:35:05 +0000 Subject: [PATCH 2/2] Update the bulk operations table snapshot for full-width tables The listBulkOperations snapshot had captured the table as Ink squeezed it into 80 columns when the test environment has no terminal width: headers cut to "I" and "COMPLET", cells wrapped onto extra lines. renderTable now gives a table the width it needs when stdout isn't a terminal, so the snapshot shows each row on one line, which is what the change is for. Co-authored-by: Eddie Gao --- .../bulk-operation-status.test.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts index 1e649d1e9bb..3354f29495c 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts @@ -270,7 +270,8 @@ describe('listBulkOperations', () => { .map((line) => line.trimEnd()) .join('\n') - // terminal width in test environment is quite narrow, so values in the snapshot get wrapped + // The test environment has no terminal width, and the table takes the width it needs there, + // so every row stays on one line. expect(outputLinesWithoutTrailingWhitespace).toMatchInlineSnapshot(` "╭─ info ───────────────────────────────────────────────────────────────────────╮ │ │ @@ -282,15 +283,10 @@ describe('listBulkOperations', () => { │ │ ╰──────────────────────────────────────────────────────────────────────────────╯ - I STATUS COUNT DATE CREATED DATE FINISHED RESULTS - - ─ ─────── ───── ────────────── ────────────── ────────────────────────────────── - ─ ──── ──── ──────────── - 1 COMPLET 123.5 2025-11-10 2025-11-10 download ( - D 12:37:52 16:37:12 https://example.com/results.jsonl - ) - 2 RUNNING 100 2025-11-11 - 15:37:52" + ID STATUS COUNT DATE CREATED DATE FINISHED RESULTS + ── ───────── ────── ─────────────────── ─────────────────── ────────────────────────────────────────────── + 1 COMPLETED 123.5K 2025-11-10 12:37:52 2025-11-10 16:37:12 download ( https://example.com/results.jsonl ) + 2 RUNNING 100 2025-11-11 15:37:52" `) })