Refactor the app security JSON codecs after review

packages/app/src/cli/services/app-security-results.ts

@@ -10,7 +10,7 @@ import {
   resolveAppSecurityCommands,
   type AppSecurityCommands,
 } from './app-security-commands.js'
-import {projectAppSecurityPath} from './app-security-json-output.js'
+import {projectAppSecurityPath} from './app-security/result.js'
 import {resultsKey, type AppSecuritySelection} from './app-security-selection.js'
 import {
   combineFindings,

packages/app/src/cli/services/app-security-json-output.test.ts → packages/app/src/cli/services/app-security/result.test.ts

@@ -1,20 +1,15 @@
 import {
-  appSecurityAppSchema,
-  appSecurityBlockingSchema,
-  appSecurityInstructionsSchema,
-  appSecurityScanDirectorySchema,
-  appSecurityScopeSchema,
   projectAppSecurityApp,
   projectAppSecurityBlocking,
   projectAppSecurityInstructions,
   projectAppSecurityPath,
   projectAppSecurityRelativePath,
   projectAppSecurityScanDirectory,
   projectAppSecurityScope,
-} from './app-security-json-output.js'
+} from './result.js'
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {describe, expect, test} from 'vitest'
-import type {AppSecuritySelection} from './app-security-selection.js'
+import type {AppSecuritySelection} from '../app-security-selection.js'
 
 // Selections hold the app directory's real path and paths joined from it with forward slashes, even on Windows.
 const onWindows = process.platform === 'win32'
@@ -91,24 +86,6 @@ describe('projectAppSecurityApp', () => {
   })
 })
 
-describe('appSecurityAppSchema', () => {
-  const app = {directory: appDirectory, configurationPath: nativeConfigurationPath, clientId: 'toml-client-id'}
-
-  test('accepts a projected app', () => {
-    expect(appSecurityAppSchema.parse(app)).toStrictEqual(app)
-  })
-
-  test.each([
-    ['an extra field', {...app, name: 'My app'}],
-    ['a missing field', {directory: app.directory, clientId: app.clientId}],
-    ['a relative directory', {...app, directory: 'my-app'}],
-    ['a relative configuration path', {...app, configurationPath: 'shopify.app.toml'}],
-    ['an empty client ID', {...app, clientId: ''}],
-  ])('rejects %s', (_description, value) => {
-    expect(appSecurityAppSchema.safeParse(value).success).toBe(false)
-  })
-})
-
 describe('projectAppSecurityInstructions', () => {
   test.each([
     ['printed', {copy: false}, false],
@@ -121,23 +98,6 @@ describe('projectAppSecurityInstructions', () => {
   })
 })
 
-describe('appSecurityInstructionsSchema', () => {
-  const instructions = {content: '# Instructions', copied: true}
-
-  test('accepts projected instructions', () => {
-    expect(appSecurityInstructionsSchema.parse(instructions)).toStrictEqual(instructions)
-  })
-
-  test.each([
-    ['an extra field', {...instructions, path: '/apps/my-app/instructions.md'}],
-    ['a missing field', {content: instructions.content}],
-    ['empty content', {...instructions, content: ''}],
-    ['a copied flag that is not a boolean', {...instructions, copied: 'true'}],
-  ])('rejects %s', (_description, value) => {
-    expect(appSecurityInstructionsSchema.safeParse(value).success).toBe(false)
-  })
-})
-
 describe('projectAppSecurityScope', () => {
   test('keeps the values exactly as typed', () => {
     expect(
@@ -146,21 +106,6 @@ describe('projectAppSecurityScope', () => {
   })
 })
 
-describe('appSecurityScopeSchema', () => {
-  const scope = {includeDirs: ['../backend'], excludes: [], noGitIgnore: false}
-
-  test('accepts a projected scope', () => {
-    expect(appSecurityScopeSchema.parse(scope)).toStrictEqual(scope)
-  })
-
-  test.each([
-    ['an extra field', {...scope, include_dirs: []}],
-    ['a missing field', {includeDirs: [], excludes: []}],
-  ])('rejects %s', (_description, value) => {
-    expect(appSecurityScopeSchema.safeParse(value).success).toBe(false)
-  })
-})
-
 describe('projectAppSecurityScanDirectory', () => {
   test.each([
     ['app_directory', '.', 'app-directory', appDirectory],
@@ -188,22 +133,6 @@ describe('projectAppSecurityScanDirectory', () => {
   })
 })
 
-describe('appSecurityScanDirectorySchema', () => {
-  const scanDirectory = {directory: appDirectory, origin: 'app-directory'}
-
-  test('accepts a projected scan directory', () => {
-    expect(appSecurityScanDirectorySchema.parse(scanDirectory)).toStrictEqual(scanDirectory)
-  })
-
-  test.each([
-    ['an extra field', {...scanDirectory, path: appDirectory}],
-    ['a relative directory', {...scanDirectory, directory: '.'}],
-    ['a stored origin', {...scanDirectory, origin: 'app_directory'}],
-  ])('rejects %s', (_description, value) => {
-    expect(appSecurityScanDirectorySchema.safeParse(value).success).toBe(false)
-  })
-})
-
 describe('projectAppSecurityBlocking', () => {
   test.each([
     ['high', 2],
@@ -212,18 +141,3 @@ describe('projectAppSecurityBlocking', () => {
     expect(projectAppSecurityBlocking({level, blockedChecks})).toStrictEqual({level, blockedCheckCount: blockedChecks})
   })
 })
-
-describe('appSecurityBlockingSchema', () => {
-  test.each(['high', 'medium', 'low', 'none'])('accepts the %s level', (level) => {
-    expect(appSecurityBlockingSchema.parse({level, blockedCheckCount: 0})).toStrictEqual({level, blockedCheckCount: 0})
-  })
-
-  test.each([
-    ['an extra field', {level: 'high', blockedCheckCount: 1, blockedChecks: 1}],
-    ['an unknown level', {level: 'critical', blockedCheckCount: 1}],
-    ['a negative count', {level: 'high', blockedCheckCount: -1}],
-    ['a fractional count', {level: 'high', blockedCheckCount: 0.5}],
-  ])('rejects %s', (_description, value) => {
-    expect(appSecurityBlockingSchema.safeParse(value).success).toBe(false)
-  })
-})

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

@@ -0,0 +1,74 @@
+import {effectiveClientId, type AppSecuritySelection} from '../app-security-selection.js'
+import {resolvePath} from '@shopify/cli-kit/node/path'
+// eslint-disable-next-line no-restricted-imports -- JSON file paths use native platform separators.
+import {resolve} from 'node:path'
+import type {
+  AppSecurityApp,
+  AppSecurityBlocking,
+  AppSecurityInstructions,
+  AppSecurityScanDirectory,
+  AppSecurityScope,
+} from './types.js'
+import type {AppSecurityBlockingLevel} from '../app-security-api.js'
+import type {AppSecurityScope as StoredAppSecurityScope, CoverageScanDirectory} from '../app-security-engine/index.js'
+import type {AppSecurityInstructionsDelivery} from '../app-security-instructions.js'
+
+/**
+ * The projections from the app security commands' data to the shared parts of their JSON output, in types.ts. Some
+ * of that data has the stored results files' own format, from app-security-engine/results/schema.ts: the projections
+ * convert it, so that format never appears in the public contract.
+ */
+
+const SCAN_DIRECTORY_ORIGINS: Record<CoverageScanDirectory['origin'], AppSecurityScanDirectory['origin']> = {
+  app_directory: 'app-directory',
+  include_dir: 'include-dir',
+  app_config_directory: 'app-config-directory',
+}
+
+/** CLI Kit joins paths with forward slashes, even on Windows. JSON output shows them absolute, with native separators. */
+export function projectAppSecurityPath(path: string): string {
+  return resolve(path)
+}
+
+/** Projects a path that is relative to the app directory, as results files record them. An absolute path stays as it is. */
+export function projectAppSecurityRelativePath(appDirectory: string, path: string): string {
+  return projectAppSecurityPath(resolvePath(appDirectory, path))
+}
+
+export function projectAppSecurityApp(selection: AppSecuritySelection): AppSecurityApp {
+  return {
+    directory: projectAppSecurityPath(selection.appDirectory),
+    configurationPath: selection.kind === 'config' ? projectAppSecurityPath(selection.appConfigFilePath) : null,
+    clientId: effectiveClientId(selection) ?? null,
+  }
+}
+
+/** Projects delivered instructions: `delivery.copy` means that they're on the clipboard. */
+export function projectAppSecurityInstructions(
+  instructions: string,
+  delivery: AppSecurityInstructionsDelivery,
+): AppSecurityInstructions {
+  return {content: instructions, copied: delivery.copy}
+}
+
+export function projectAppSecurityScope(scope: StoredAppSecurityScope): AppSecurityScope {
+  return {includeDirs: scope.include_dirs, excludes: scope.excludes, noGitIgnore: scope.no_git_ignore}
+}
+
+/** Projects a scan directory, either absolute or relative to the app directory as results files record them. */
+export function projectAppSecurityScanDirectory(
+  appDirectory: string,
+  scanDirectory: CoverageScanDirectory,
+): AppSecurityScanDirectory {
+  return {
+    directory: projectAppSecurityRelativePath(appDirectory, scanDirectory.directory),
+    origin: SCAN_DIRECTORY_ORIGINS[scanDirectory.origin],
+  }
+}
+
+export function projectAppSecurityBlocking(blocking: {
+  level: AppSecurityBlockingLevel
+  blockedChecks: number
+}): AppSecurityBlocking {
+  return {level: blocking.level, blockedCheckCount: blocking.blockedChecks}
+}