Skip to content

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

Open
jplhomer wants to merge 2 commits into
mainfrom
app-doctor/encapsulate-engine
Open

Keep App Doctor engine internals behind scan, compile, and submit operations#8513
jplhomer wants to merge 2 commits into
mainfrom
app-doctor/encapsulate-engine

Conversation

@jplhomer

@jplhomer jplhomer commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

App Doctor CLI services imported scanners, check registries, merge helpers, trace validators, and redaction functions from the engine. Those internals made engine changes ripple into command-layer files.

WHAT is this pull request doing?

Expose only the operations the CLI needs: locate an app, scan, parse and compile findings, parse a stored trace, build a submission, and load agent instructions. Move compile/merge/findings-parse into the engine. CLI output, artifacts, and submit now use that surface; engine tests still import internal modules directly.

Display redaction stays an engine contract. The report layer renders already-redacted scan results instead of calling engine redact helpers.

No user-facing behavior change.

How to manually test your changes?

From an app directory:

shopify app doctor
shopify app doctor --findings .shopify/app-doctor/findings.json
shopify app doctor submit --dry-run

Scan, compile, and dry-run submit should behave as they do on main.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

CLI services now call scan, compile, parse, and submit operations instead of
scanners, registries, merge helpers, and redaction internals. Engine tests still
import those modules directly.

Co-authored-by: AI (Pi/Grok 4.6) <noreply@pi.dev>
@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Sep 9, 2026
@jplhomer
jplhomer added this pull request to stack #8515 September 9, 2026 20:40
sortIssues moved into CLI output, FindingsDocument is not re-exported from
the CLI adapter, and the unused Trace v1 type is gone with the public API shrink.

Co-authored-by: AI (Pi/Grok 4.6) <noreply@pi.dev>
@jplhomer
jplhomer marked this pull request as ready for review September 9, 2026 21:05
@jplhomer
jplhomer requested a review from a team as a code owner September 9, 2026 21:05
Copilot AI lite review requested due to automatic review settings September 9, 2026 21:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It’s a broad API-surface refactor across engine + CLI layers (including redaction and compile/merge flow relocation) that should receive final human review despite tests being updated.

Pull request overview

This PR refactors the App Doctor “engine” to expose a smaller public API surface (scan/compile/trace parsing/submission/instructions), keeping scanners, registries, merge helpers, and redaction internals encapsulated so engine changes don’t ripple into CLI command-layer code.

Changes:

  • Introduces a new engine-facing “operations” module (run.ts) and updates the engine’s public exports to re-export only supported CLI operations/types.
  • Updates CLI services (output rendering, instructions, artifacts, API orchestration) to use the new engine operations instead of importing engine internals.
  • Adjusts tests (CLI + engine) to import from the new public surface or targeted internal modules, and updates redaction expectations to treat redaction as an engine contract.
File summaries
File Description
packages/app/src/cli/services/doctor-output.ts Stop calling engine redaction helpers directly; render engine-redacted scan/findings and localize issue sorting.
packages/app/src/cli/services/doctor-output.test.ts Update test expectations to assert engine-redacted content is rendered unchanged.
packages/app/src/cli/services/app-doctor-instructions.ts Switch from embedded constant import to getAgentInstructions() operation.
packages/app/src/cli/services/app-doctor-instructions.test.ts Assert getAgentInstructions() matches INSTRUCTIONS.md.
packages/app/src/cli/services/app-doctor-engine/types.ts Remove legacy TraceV1 type from the shared types surface.
packages/app/src/cli/services/app-doctor-engine/tests/trace.test.ts Update tests to import internal modules directly rather than the public engine index.
packages/app/src/cli/services/app-doctor-engine/tests/submission.test.ts Update imports to use submission/trace internals instead of engine index.
packages/app/src/cli/services/app-doctor-engine/tests/scan-contract.test.ts Update imports to use checks/registry/scanners/trace internals directly.
packages/app/src/cli/services/app-doctor-engine/tests/registry.test.ts Update imports to use internal modules (checks/registry/scanners).
packages/app/src/cli/services/app-doctor-engine/tests/deterministic-rules.test.ts Update imports to use internal modules (registry/scanners).
packages/app/src/cli/services/app-doctor-engine/run.ts New engine “operations” module: scan/compile findings, parse findings/trace, and provide agent instructions.
packages/app/src/cli/services/app-doctor-engine/output/format.ts Keep JSON redaction formatting; remove sorting helper export from engine output layer.
packages/app/src/cli/services/app-doctor-engine/index.ts Narrow the public engine API to the new operations + required types/submission builder.
packages/app/src/cli/services/app-doctor-artifacts.ts Use parseTrace() (engine operation) rather than importing trace validator internals.
packages/app/src/cli/services/app-doctor-artifacts.test.ts Use scanApp() to generate a valid trace fixture via the engine surface.
packages/app/src/cli/services/app-doctor-api.ts Move findings parsing/compile orchestration into engine operations; map engine findings errors to CLI AbortError.
packages/app/src/cli/services/app-doctor-api.test.ts Adjust integration assertions to avoid importing internal check registry, while still validating behavior.
Review details
  • Files reviewed: 17/17 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dmerand dmerand left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants