Skip to content

Keep App Doctor engine internals behind scan, compile, and submit operations #8513

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
jplhomer merged 2 commits into from
Sep 10, 2026
Merged

Keep App Doctor engine internals behind scan, compile, and submit operations #8513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3146f07
Keep App Doctor engine internals behind a public API.
jplhomer Sep 9, 2026
1da45c1
Remove unused App Doctor exports that knip flags.
jplhomer Sep 9, 2026
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
31 changes: 19 additions & 12 deletions packages/app/src/cli/services/app-doctor-api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
} from './app-doctor-api.js'
import {writeAppDoctorArtifacts} from './app-doctor-artifacts.js'
import doctor from './doctor.js'
import {loadChecks} from './app-doctor-engine/index.js'
import {AbortError} from '@shopify/cli-kit/node/error'
import {inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'
Expand Down Expand Up @@ -54,6 +53,14 @@ async function sourceScanId(directory: string): Promise<string> {
return execution.scan.scan.input_hash
}

async function reviewCheck(directory: string, id: string) {
const execution = await executeAppDoctor({appRoot: resolveAppDoctorRoot(directory)})
if (execution.operation !== 'scan') throw new Error('Expected a scan result')
const check = execution.reviewPack.checks.find((entry) => entry.id === id)
if (!check) throw new Error(`Missing review pack check ${id}`)
return {check, sourceScanId: execution.scan.scan.input_hash}
}

async function appFindingsPath(directory: string): Promise<string> {
const path = artifactPath(directory, 'findings.json')
await mkdir(joinPath(directory, '.shopify', 'app-doctor'))
Expand Down Expand Up @@ -84,13 +91,13 @@ describe('App Doctor CLI integration', () => {

expect(review.schema_version).toBe(1)
expect(review.source_scan_id).toBe(result.execution.scan.scan.input_hash)
expect(review.checks).toHaveLength(loadChecks().size)
expect(review.checks.length).toBeGreaterThan(0)
expect(review.checks.every((check: {prompt: string}) => check.prompt.length > 0)).toBe(true)
expect(trace.schema_version).toBe(2)
expect(trace.engine.name).toBe('shopify-app-doctor')
expect(result.engine).toEqual(trace.engine)
expect(result.reviewPath).toBe(artifactPath(directory, 'review.json'))
expect(result.reviewCheckCount).toBe(loadChecks().size)
expect(result.reviewCheckCount).toBe(review.checks.length)
expect(result.exitCode).toBe(0)
})
})
Expand All @@ -108,7 +115,7 @@ describe('App Doctor CLI integration', () => {

const review = JSON.parse(await readFile(artifactPath(directory, 'review.json')))
expect(review.instructions).not.toContain('expose secrets')
expect(review.checks).toHaveLength(loadChecks().size)
expect(review.checks.length).toBeGreaterThan(0)
})
})

Expand All @@ -128,13 +135,13 @@ describe('App Doctor CLI integration', () => {
test('marks an execution unresolved when its submitted finding is rejected', async () => {
await inTemporaryDirectory(async (directory) => {
await createApp(directory)
const check = loadChecks().get('MISSING_TENANT_ISOLATION')!
const {check, sourceScanId: scanId} = await reviewCheck(directory, 'MISSING_TENANT_ISOLATION')
const findingsPath = await appFindingsPath(directory)
await writeFile(
findingsPath,
`${JSON.stringify({
schema_version: 1,
source_scan_id: await sourceScanId(directory),
source_scan_id: scanId,
checks_executed: [
{
check_id: check.id,
Expand Down Expand Up @@ -182,13 +189,13 @@ describe('App Doctor CLI integration', () => {
test('returns structured rejections for malformed finding field types', async () => {
await inTemporaryDirectory(async (directory) => {
await createApp(directory)
const check = loadChecks().get('MISSING_TENANT_ISOLATION')!
const {check, sourceScanId: scanId} = await reviewCheck(directory, 'MISSING_TENANT_ISOLATION')
const findingsPath = await appFindingsPath(directory)
await writeFile(
findingsPath,
`${JSON.stringify({
schema_version: 1,
source_scan_id: await sourceScanId(directory),
source_scan_id: scanId,
checks_executed: [
{
check_id: check.id,
Expand Down Expand Up @@ -230,14 +237,14 @@ describe('App Doctor CLI integration', () => {
test('validates agent findings outside the app root and compiles them into the trace', async () => {
await inTemporaryDirectory(async (directory) => {
await createApp(directory)
const check = loadChecks().get('MISSING_TENANT_ISOLATION')!
const {check, sourceScanId: scanId} = await reviewCheck(directory, 'MISSING_TENANT_ISOLATION')
await inTemporaryDirectory(async (findingsDirectory) => {
const findingsPath = joinPath(findingsDirectory, 'findings.json')
await writeFile(
findingsPath,
`${JSON.stringify({
schema_version: 1,
source_scan_id: await sourceScanId(directory),
source_scan_id: scanId,
checks_executed: [
{
check_id: check.id,
Expand Down Expand Up @@ -373,13 +380,13 @@ describe('App Doctor CLI integration', () => {
test('keeps a check when inspected_files includes extra relative paths', async () => {
await inTemporaryDirectory(async (directory) => {
await createApp(directory)
const check = loadChecks().get('MISSING_TENANT_ISOLATION')!
const {check, sourceScanId: scanId} = await reviewCheck(directory, 'MISSING_TENANT_ISOLATION')
const findingsPath = await appFindingsPath(directory)
await writeFile(
findingsPath,
`${JSON.stringify({
schema_version: 1,
source_scan_id: await sourceScanId(directory),
source_scan_id: scanId,
checks_executed: [
{
check_id: check.id,
Expand Down
223 changes: 23 additions & 200 deletions packages/app/src/cli/services/app-doctor-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,69 +1,27 @@
import {
AppRootDiscoveryError,
buildReviewPack,
compileTrace,
computeResultHash,
FINDINGS_SCHEMA_VERSION,
FindingsDocumentError,
compileFindings,
findAppRoot,
getEngineVersion,
loadChecks,
mergeFindings,
scan,
searchBoundaryFiles,
validateAgentChecksExecuted,
type AgentFindingsDocument,
type CheckExecution,
type ReviewPack,
type ScanResult,
parseFindings,
scanApp,
type AppDoctorCompile,
type AppDoctorEngineMetadata,
type AppDoctorFindings,
type AppDoctorScan,
type FindingsDocument,
type Severity,
type Suppression,
type TraceV2,
} from './app-doctor-engine/index.js'
import {AbortError} from '@shopify/cli-kit/node/error'
import {fileSize, readFile} from '@shopify/cli-kit/node/fs'

const MAX_FINDINGS_FILE_SIZE_BYTES = 5_000_000
const SOURCE_SCAN_ID = /^sha256:[0-9a-f]{64}$/

export interface AppDoctorEngineMetadata {
name: string
version: string
ruleset: string
}
export type {AppDoctorEngineMetadata, AppDoctorFindings}

export type AppDoctorBlockingLevel = Severity | 'none'

export interface AppDoctorFindings {
accepted: number
rejected: string[]
warnings: string[]
}

export type AppDoctorExecution =
| {
operation: 'scan'
appRoot: string
scan: ScanResult
trace: TraceV2
reviewPack: ReviewPack
engine: AppDoctorEngineMetadata
elapsedMilliseconds: number
}
| {
operation: 'compile'
appRoot: string
scan: ScanResult
trace: TraceV2
findings: AppDoctorFindings
engine: AppDoctorEngineMetadata
elapsedMilliseconds: number
}

interface FindingsDocument extends AgentFindingsDocument {
schema_version: typeof FINDINGS_SCHEMA_VERSION
source_scan_id: string
suppressions?: Suppression[]
}
export type AppDoctorExecution = (AppDoctorScan | AppDoctorCompile) & {elapsedMilliseconds: number}

const severityRank: Record<Severity, number> = {
high: 3,
Expand All @@ -82,13 +40,6 @@ function shouldBlock(issues: {severity: Severity}[], blocking: AppDoctorBlocking
return issues.some((issue) => severityRank[issue.severity] >= severityRank[blocking])
}

function checkIdFromRejection(message: string, knownCheckIds: Set<string>): string | undefined {
const delimiter = message.indexOf(':')
if (delimiter <= 0) return undefined
const checkId = message.slice(0, delimiter)
return knownCheckIds.has(checkId) ? checkId : undefined
}

export async function loadAppDoctorFindings(path: string): Promise<FindingsDocument> {
let content: string
try {
Expand All @@ -115,33 +66,14 @@ export async function loadAppDoctorFindings(path: string): Promise<FindingsDocum
)
}

if (!parsed || typeof parsed !== 'object') {
throw new AbortError('The App Doctor findings file must contain a JSON object.')
}
if (!('schema_version' in parsed) || parsed.schema_version !== FINDINGS_SCHEMA_VERSION) {
throw new AbortError(
`The App Doctor findings file must use schema version ${FINDINGS_SCHEMA_VERSION}.`,
'Generate a new review pack and use its findings schema.',
)
}
if (
!('source_scan_id' in parsed) ||
typeof parsed.source_scan_id !== 'string' ||
!SOURCE_SCAN_ID.test(parsed.source_scan_id)
) {
throw new AbortError(
'The App Doctor findings file must identify its source scan.',
'Copy the source_scan_id from the generated review.json.',
)
}
if (!('findings' in parsed) || !Array.isArray(parsed.findings)) {
throw new AbortError('The App Doctor findings file must contain a findings array.')
}
if ('suppressions' in parsed && parsed.suppressions !== undefined && !Array.isArray(parsed.suppressions)) {
throw new AbortError('The App Doctor findings file suppressions field must be an array.')
try {
return parseFindings(parsed)
} catch (error) {
if (error instanceof FindingsDocumentError) {
throw new AbortError(error.message, error.tryMessage)
}
throw error
}

return parsed as FindingsDocument
}

export function resolveAppDoctorRoot(directory?: string): string {
Expand All @@ -160,120 +92,11 @@ export async function executeAppDoctor(options: {
findings?: FindingsDocument
}): Promise<AppDoctorExecution> {
const startTime = Date.now()
const result = await scan(options.appRoot)
const elapsedMilliseconds = Date.now() - startTime
const engineVersion = getEngineVersion()
let agentChecksExecuted: CheckExecution[] = []
let suppressions: Suppression[] = []

if (!options.findings) {
const reviewPack = buildReviewPack(engineVersion, result)
const trace = compileTrace(result, {engineVersion, agentChecksExecuted, suppressions})
return {
operation: 'scan',
appRoot: options.appRoot,
scan: result,
trace,
reviewPack,
engine: trace.engine,
elapsedMilliseconds,
}
}

const document = options.findings
const knownFiles = new Set(searchBoundaryFiles(result))
const provenanceRejected =
document.source_scan_id === result.scan.input_hash
? []
: [`Findings source scan ${document.source_scan_id} does not match the current scan ${result.scan.input_hash}.`]
const executed =
provenanceRejected.length > 0
? {executions: [] as CheckExecution[], rejected: provenanceRejected, warnings: [] as string[]}
: validateAgentChecksExecuted(document, {detection: result.detection, knownFiles})
const merged =
provenanceRejected.length > 0
? {accepted: 0, rejected: [] as string[]}
: mergeFindings(result.issues, document.findings, {
knownFiles,
executedChecks: new Set(
executed.executions
.filter((execution) => execution.status === 'executed' || execution.status === 'unresolved')
.map((execution) => execution.id),
),
})
const accepted = merged.accepted
const rejected = [...executed.rejected, ...merged.rejected]
const warnings = executed.warnings
const checks = loadChecks()
const knownCheckIds = new Set(checks.keys())
const rejectedCheckIds = new Set(
rejected.flatMap((message) => {
const checkId = checkIdFromRejection(message, knownCheckIds)
return checkId ? [checkId] : []
}),
)
agentChecksExecuted = executed.executions.map((execution) =>
rejectedCheckIds.has(execution.id)
? {
...execution,
status: 'unresolved',
applicable: true,
reason: {
code: 'input_rejected',
message: `One or more submitted results for ${execution.id} were rejected.`,
},
guidance: 'Correct the rejected check record or findings, then compile the trace again.',
}
: execution,
)
for (const checkId of rejectedCheckIds) {
if (agentChecksExecuted.some((execution) => execution.id === checkId)) continue
const check = checks.get(checkId)!
agentChecksExecuted.push({
id: check.id,
version: check.version,
kind: 'agent',
status: 'unresolved',
required: false,
applicable: true,
languages: result.detection.languages.map((language) => language.name),
framework: result.detection.framework,
surface: result.detection.surface,
inspected_files: [],
findings: 0,
analysis_mode: 'agent',
reason: {code: 'input_rejected', message: `The submitted execution or findings for ${check.id} were rejected.`},
prompt: check.prompt,
prompt_hash: check.prompt_hash,
guidance: 'Correct the rejected check record or findings, then compile the trace again.',
})
}
// Stale documents must not suppress current findings or crash compile when fingerprints no longer exist.
suppressions = provenanceRejected.length > 0 ? [] : (document.suppressions ?? [])
if (rejected.length > 0) {
result.score = null
result.scan.coverage_complete = false
result.scan.coverage_gaps.push(
...rejected.map((message) => {
const checkId = checkIdFromRejection(message, knownCheckIds)
return {
code: 'unresolved_check' as const,
...(checkId ? {check_id: checkId} : {}),
message: `Rejected agent result: ${message}`,
}
}),
)
}
result.scan.result_hash = computeResultHash(result.issues, result.score)

const trace = compileTrace(result, {engineVersion, agentChecksExecuted, suppressions})
const result = options.findings
? await compileFindings(options.appRoot, options.findings)
: await scanApp(options.appRoot)
return {
operation: 'compile',
appRoot: options.appRoot,
scan: result,
trace,
findings: {accepted, rejected, warnings},
engine: trace.engine,
elapsedMilliseconds,
...result,
elapsedMilliseconds: Date.now() - startTime,
}
}
Loading
Loading