Refactor app validation errors: result-based parsing, structured error data

[ryancbahan]

What

Refactor app validation error handling from throw/catch with styled OutputMessage to result-based parsing with a plain ConfigurationError data interface.

Why

Prep for app config validate --json (see #7066, #7090, #7106). The original stack added structured error data alongside rendered messages, creating dual-track serialization in the loader. This PR takes the opposite approach: make errors pure data, let callers render. This gives callers more flexibility and removes the job of rendering/formatting from the data flow.

How

• ConfigurationError is now a plain interface ({file, message, path?, code?}), not a class extending AbortError
• parseConfigurationObject, parseConfigurationObjectAgainstSpecification, parseConfigurationFile return {data} | {errors: ConfigurationError[]} instead of throwing
• AppErrors stores ConfigurationError[] with addError, addErrors, getErrors(file?), isEmpty
• formatConfigurationError is the single plain-text formatting function
• Styled output (outputContent/outputToken) applied only at display boundaries: app-context.ts (AbortError), validate.ts (renderError), loader.ts (reloadApp)
• parseStructuredErrors in error-parsing.ts converts Zod issues to ParsedIssue[] with union-variant scoring

No user-facing behavior changes. CLI output preserves styling. Multi-error-per-file now preserved (previously silently overwritten by dictionary key).

[github-actions]

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

[ryancbahan]

• Add errors to TomlFile and Project for TOML parse failure tracking #7130
• Add structured {valid, issues} JSON contract for app config validate #7128
• Refactor app validation errors: result-based parsing, structured error data #7126 👈 (View in Graphite)
• main

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

[isaacroldan]

why only show the first error? Would be useful to show all validation errors.

[ryancbahan]

This is parity with what we do today on main actually. I agree we can broaden, but am avoiding behavioral changes alongside orthogonal features/refactors.

[isaacroldan]

---

Review assisted by pair-review

[isaacroldan]

there are many calls to formatConfigurationError, maybe the AppErrors class expose a getFormattedErrors getter to simplify that?

I've also seen this almost exact line in two places, here and in app-context.ts
(prepending the Validation error as a errorText). If we want to use the same format from different places would be nice to try to unify it somehow.

[ryancbahan]

I'm open to adjusting, but I definitely think that error formatting should be off of AppErrors/loader. Today, we make habit of passing around rendering concerns across the entire lifecycle and it adds complexity/mixes concerns too much. The decision of "how to render" should be as close to the render call as possible, allowing the "backend" to be concerned with data flow. The formatting of structured data to renderable info can still be abstracted when needed, IMO just not as part of the data flow itself.

[isaacroldan]

I get that reasoning, but we end up with the same formatting in multiple places, there must be a better way to do it? :thinking_face:

Seems like a case of consistency vs correctness? We are being more correct, but at the expense of duplicating code and potentially being less consistent if this transformations diverge?

I appreciate the push for correctness though! maybe we can meet in the middle :P

[dmerand]

I think this catch may be a bit too broad for the JSON contract.

linkedAppContext() does more than local config validation — it can also abort during link / remote app lookup / org fetch / spec fetch. For example, appFromIdentifiers() in services/context.ts throws AbortError for cases like “No app with client ID … found”.

So with this change, some operational/setup failures now look like:

{"valid": false, "errors": [{"message": "..."}]}

which reads as “your config is invalid” even when the problem is actually linking/auth/remote resolution.

I think the --json fallback should probably only map actual local configuration failures into the validation result shape, and let unrelated AbortErrors bubble normally. Otherwise machine consumers may start treating non-validation failures as fixable TOML issues.

Apologies if we've litigated this already, but just want to confirm.

[ryancbahan]

I've updated the top pr to address this. It's a little hacky while we only have AbortError today, but I think we can continue to refine.

[dmerand]

This looks like a regression in the human-readable error path for app config loading.

Before, loadAppFromContext() threw an error that included the config file path:

throw new AbortError(
  outputContent`Validation errors in ${outputToken.path(configurationPath)}:\n\n${...}`,
)

Now it throws only:

throw new AbortError(configResult.errors.map(formatConfigurationError).join('\n'))

But formatConfigurationError() doesn’t include error.file, only [path]: message or just message.

So for top-level app config parse/validation failures, we seem to lose which config file actually failed. In multi-config setups that feels materially less actionable than before.

Would it make sense to preserve the file context at this boundary, either by:

• keeping the configurationPath wrapper here, or
• making the formatter include file information when rendering for humans?