Add TOML config regression tests

[ryancbahan]

WHY are these changes introduced?

We want to catch regressions in shopify.app.toml parsing and validation. The CLI should accept all documented config options and reject invalid configurations with clear errors.

WHAT is this pull request doing?

Static test data in data/:

• data/valid-app/shopify.app.toml — comprehensive toml with all documented sections (access_scopes, webhooks, app_proxy, pos, build, auth, app_preferences, access.admin)
• data/valid-app/package.json — minimal package.json
• data/invalid-tomls/ — 5 invalid toml files (bad syntax, wrong types, unknown section, missing webhook uri, invalid app_proxy prefix)

New fixture:

• setup/toml-app.ts — copies data/valid-app/ to a temp dir, injects client_id from env

Tests:

• toml-config.spec.ts:
  • Deploy succeeds with fully populated toml
  • Dev starts with fully populated toml (waits for ready, presses q to quit)
• toml-config-invalid.spec.ts:
  • Auto-discovers all .toml files in data/invalid-tomls/
  • Asserts app deploy --force fails for each with non-zero exit code
  • Logs full CLI output for diagnostics

cd packages/e2e && npx playwright test tests/toml-config.spec.ts tests/toml-config-invalid.spec.ts

How to test your changes?

cd packages/e2e && npx playwright test tests/toml-config.spec.ts

Measuring impact

n/a - test infrastructure

[ryancbahan]

• Add TOML config regression tests #6940 👈 (View in Graphite)
• Add cleanup script for e2e test org #6907
• Add e2e tests to PR CI workflow #6903
• Add e2e tests for app init, deploy, and dev server #6900 : 1 other dependent PR (#6901 )
• Add e2e test infrastructure with Playwright + node-pty #6899
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[binks-code-reviewer]

🤖 Code Review · #projects-dev-ai for questions
React with 👍/👎 or reply — all feedback helps improve the agent.

✅ Complete - No issues

📋 History

✅ No issues → ✅ No issues

[github-actions]

Coverage report

St.❔	Category	Percentage	Covered / Total
🟡	Statements	79.23%	14599/18426
🟡	Branches	73.49%	7249/9864
🟡	Functions	79.37%	3717/4683
🟡	Lines	79.56%	13795/17339

Test suite run success

3822 tests passing in 1464 suites.

Report generated by 🧪jest coverage report action from 9097f3f

[dmerand]

AI pair review found a few assertion weaknesses that are addressable.

---

Review assisted by pair-review

[dmerand]

invalid-prefix.toml reads like a local config-validation case, but the CLI does not currently validate app_proxy.prefix beyond “must be a string” packages/app/src/cli/models/extensions/specifications/app_config_app_proxy.ts:12-21. The deploy path forwards the prefix unchanged into the remote app proxy config packages/app/src/cli/models/extensions/specifications/app_config_app_proxy.ts:30-45, and manifest tests show arbitrary prefix strings are preserved packages/app/src/cli/models/app/app.test.ts:890-900. So if this fixture fails, it is likely exercising downstream/server-side validation rather than the CLI rejecting an invalid TOML value. Useful integration coverage maybe. But the current naming and grouping imply stronger client-side validation coverage than the test actually provides.

[dmerand]

unknown-section.toml does not look invalid in the code path app deploy actually uses. Linked apps are loaded with getAppVersionedSchema(specifications), which defaults to .passthrough(), so unknown top-level sections are accepted instead of rejected packages/app/src/cli/models/app/app.ts:172-183, packages/app/src/cli/models/app/loader.ts:999-1018. Then deploy sends a generated app manifest rather than the raw TOML packages/app/src/cli/models/app/app.ts:448-470, packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts:748-756, so completely_made_up_section may never be validated server-side either. That makes this fixture a likely false-negative/false-positive case: it may not fail for the reason the test name implies, and could even succeed.

[dmerand]

💡 Improvement: The test suite includes 5 invalid TOML cases but is missing several categories of validation issues that exist in the actual codebase. Additional valuable test cases could include: URL validation failures (invalid URLs in app_proxy.url, auth.redirect_urls), webhook API version validation, array vs string type mismatches, invalid enum values (e.g., access.admin.direct_api_mode accepts only 'online'/'offline'), and field length validations. More comprehensive coverage would provide stronger regression protection.

Suggestion: Consider adding test files for additional validation scenarios:

• invalid-url.toml - malformed URL in application_url or redirect_urls
• invalid-api-version.toml - invalid webhooks.api_version
• invalid-direct-api-mode.toml - access.admin.direct_api_mode = "invalid"
• wrong-array-type.toml - auth.redirect_urls as string instead of array

These would provide more comprehensive coverage of the validation rules implemented in the codebase.

[dmerand]

💡 Improvement: The PR description states the CLI should 'reject invalid configurations with clear errors', but the test only validates that the command fails (non-zero exit code) without verifying that error messages are actually clear and helpful. The test captures output for diagnostics but doesn't assert anything about error message content or quality. The CLI could return unhelpful errors like 'Unknown error' and the test would still pass.

Suggestion: Add assertions to verify that error messages are present and meaningful:

expect(result.exitCode, `expected deploy to fail for ${label}, but it succeeded:\n${output}`).not.toBe(0)
expect(output.toLowerCase()).toMatch(/error|invalid|failed/)
expect(output).toContain('shopify.app.toml')

Or create specific assertions for each invalid case (e.g., verify 'bad-syntax' mentions syntax errors, 'invalid-webhook' mentions missing uri).

[ryancbahan]

@dmerand i did an initial pass at narrowing, but overall i think this is something we'll want to iterate on a bit and the value for shipping this is high enough as-is. wdyt?