Skip to content

Commit 12cc647

Browse files
authored
Exit 0 when app security check's no-configuration prompt is declined
Answering No to "Scan it without app configuration?" is a choice, not a failure. With `--json` it was already a cancelled result with exit 0; in the terminal it aborted with exit 1. It now shows the same message and next step as information, and exits 0 in both formats. Without prompts, a missing app configuration still aborts: nothing was declined.
1 parent c00505a commit 12cc647

5 files changed

Lines changed: 37 additions & 20 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@shopify/app': patch
3+
'@shopify/cli': patch
4+
---
5+
6+
Exit 0 instead of failing when you decline to scan without app configuration in `app security check`.

‎packages/app/src/cli/commands/app/security/check.integration.test.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -718,20 +718,19 @@ describe('app security check prompts', () => {
718718
})
719719
})
720720

721-
test('scans nothing when scanning without app configuration is declined', async () => {
721+
test('scans nothing and exits 0 when scanning without app configuration is declined', async () => {
722722
await inTemporaryDirectory(async (directory) => {
723723
vi.mocked(renderConfirmationPrompt).mockResolvedValue(false)
724724

725725
const result = await runInTerminal(['--path', directory], {json})
726726

727727
expect(renderConfirmationPrompt).toHaveBeenCalledOnce()
728728
expect(renderSelectPrompt).not.toHaveBeenCalled()
729+
expect(result.exitCode).toBe(0)
729730
if (json) {
730-
expect(result.exitCode).toBe(0)
731731
expect(JSON.parse(result.stdout)).toStrictEqual({status: 'cancelled'})
732732
expect(result.stderr).toBe('')
733733
} else {
734-
expect(result.exitCode).toBe(1)
735734
expect(result.stdout).toBe('')
736735
expect(boxText(result.stderr)).toContain('No app configuration found at or above')
737736
expect(boxText(result.stderr)).toContain('--without-app-config --client-id <client-id>')

‎packages/app/src/cli/services/app-security-selection.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -358,13 +358,19 @@ export function mergeScanDirectories(
358358
}
359359
}
360360

361-
/** No app configuration at or above `directory`. In the terminal, declining to scan without one ends with it too. */
362-
export class MissingAppConfigurationError extends AbortError {
363-
constructor(directory: string) {
364-
super(
365-
`No app configuration found at or above ${directory}.`,
361+
/** That there's no app configuration at or above `directory`, and how to scan anyway. */
362+
export function missingAppConfigurationMessage(directory: string): {headline: string; nextStep: string} {
363+
return {
364+
headline: `No app configuration found at or above ${directory}.`,
365+
nextStep:
366366
'Pass `--path` to your app directory, or scan without app configuration with `--without-app-config --client-id <client-id>`.',
367-
)
367+
}
368+
}
369+
370+
class MissingAppConfigurationError extends AbortError {
371+
constructor(directory: string) {
372+
const {headline, nextStep} = missingAppConfigurationMessage(directory)
373+
super(headline, nextStep)
368374
}
369375
}
370376

‎packages/app/src/cli/services/security-check/result.test.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {appSecurityInstructions} from '../app-security-instructions.js'
1717
import {validAppConfiguration} from '../app-security-selection.test-data.js'
1818
import {committedSecretIssue, scan, scanExecution} from '../security-check.test-data.js'
1919
import {runWithCommandEventsForCommand} from '@shopify/cli-kit/node/command-events'
20-
import {AbortError} from '@shopify/cli-kit/node/error'
2120
import {fileRealPath, inTemporaryDirectory, matchGlob, mkdir, writeFile} from '@shopify/cli-kit/node/fs'
2221
import {unstyled} from '@shopify/cli-kit/node/output'
2322
import {joinPath} from '@shopify/cli-kit/node/path'
@@ -494,14 +493,17 @@ describe('renderSecurityCheckResult for --list-files', () => {
494493
})
495494

496495
describe('renderSecurityCheckResult when scanning without app configuration was declined', () => {
497-
test('aborts as when no app configuration is found', async () => {
498-
const error: unknown = await present({kind: 'cancelled'}).catch((caught: unknown) => caught)
496+
test('shows that no app configuration was found, with the next step, and leaves the exit code alone', async () => {
497+
process.exitCode = undefined
498+
499+
const {stdout, stderr} = await present({kind: 'cancelled'})
499500

500-
expect(error).toBeInstanceOf(AbortError)
501-
expect((error as AbortError).message).toBe(`No app configuration found at or above ${appDirectory}.`)
502-
expect((error as AbortError).tryMessage).toBe(
501+
expect(stdout).toBe('')
502+
expect(boxText(stderr)).toContain(`No app configuration found at or above ${appDirectory}.`)
503+
expect(boxText(stderr)).toContain(
503504
'Pass `--path` to your app directory, or scan without app configuration with `--without-app-config --client-id <client-id>`.',
504505
)
506+
expect(process.exitCode).toBeUndefined()
505507
})
506508
})
507509

‎packages/app/src/cli/services/security-check/result.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
shellForPlatform,
2222
type AppSecurityCommands,
2323
} from '../app-security-commands.js'
24-
import {MissingAppConfigurationError} from '../app-security-selection.js'
24+
import {missingAppConfigurationMessage} from '../app-security-selection.js'
2525
import {renderSecurityReport} from '../security-output.js'
2626
import {fileRealPath} from '@shopify/cli-kit/node/fs'
2727
import {outputResult} from '@shopify/cli-kit/node/output'
@@ -93,8 +93,8 @@ export function renderSecurityCheckPromptsTip(commands: AppSecurityCommands): vo
9393
* Presents the outcome of `securityCheck` as JSON, or in the terminal. Gathering warnings come first, on stderr. Then
9494
* the listed files, or the scan: in the terminal, its report followed by the coding-agent instructions; as JSON, the
9595
* instructions prompt followed by the result. Either way, sets exit code 1 after the output when the scan breaches
96-
* `--blocking`. Declining to scan without app configuration is a cancelled result as JSON. In the terminal it aborts,
97-
* as when there is no app configuration and prompts are off.
96+
* `--blocking`. Declining to scan without app configuration exits 0: as JSON, it's a cancelled result. In the
97+
* terminal, it shows the message that aborts the command when there's no app configuration and prompts are off.
9898
*/
9999
export async function renderSecurityCheckResult(
100100
check: SecurityCheckOutcome,
@@ -103,8 +103,12 @@ export async function renderSecurityCheckResult(
103103
dependencies: SecurityCheckPresenterDependencies = defaultDependencies,
104104
): Promise<void> {
105105
if (check.kind === 'cancelled') {
106-
if (format === 'text') throw new MissingAppConfigurationError(options.directory)
107-
outputResult(appSecurityCheckJsonOutputSchema.encode({status: 'cancelled'}))
106+
if (format === 'json') {
107+
outputResult(appSecurityCheckJsonOutputSchema.encode({status: 'cancelled'}))
108+
} else {
109+
const {headline, nextStep} = missingAppConfigurationMessage(options.directory)
110+
renderInfo({headline, body: nextStep})
111+
}
108112
return
109113
}
110114

0 commit comments

Comments
 (0)