Skip to content

fix(core): localize config normalization diagnostics to the failing field - #50832

Open
argszero wants to merge 1 commit into
anomalyco:v2from
argszero:config-field-diagnostic
Open

argszero wants to merge 1 commit into
anomalyco:v2from
argszero:config-field-diagnostic

Conversation

@argszero

@argszero argszero commented Sep 23, 2026 •

Copy link
Copy Markdown

Issue for this PR

Related to #50340, and to the diagnostic issue #50756 that its reporter closed after folding it into this thread.

This is the diagnostic half of that report. It does not change which configs load, so it does not close #50340 on its own — the schema-side PRs #50702 and #49940 handle that. Whichever of those lands, any remaining silent skip stays debuggable with this change.

Type of change

  • Bug fix

What does this PR do?

When a config value fails to decode, normalize.ts reports a diagnostic at the enclosing path with the message "skipped malformed recognized value". The failing field is lost: decodeValue/decodeEncoded decode with Schema.decodeUnknownOption, which returns None on failure, so the parse error that carries the offending path is discarded before anything can log it.

The effect is visible in #50340, #49912 and #50756: three different malformed values (a capabilities object missing tools, an invalid legacy package id) all collapse to the same provider-root warning, which tells a user that something is wrong but not what.

This switches those decoders to SchemaParser.decodeUnknownResult and walks the resulting issue to recover the failing sub-path, so the warning names the field:

path=$.providers.<id>.models.<model>.capabilities.tools

Two limits are deliberate, and both are covered by tests:

  • Only the path is carried, never the issue message or the value. PR feat(core): normalize mixed config formats #40919 defines this normalizer as emitting "source-aware, value-redacted diagnostics" and two existing tests enforce it, so the message stays exactly "skipped malformed recognized value" and only schema positions are added. Effect issue formatting would have echoed the input.
  • Union members are not entered. For AnyOf/OneOf Effect records an issue per branch with no preferred member, so guessing one would report an arbitrary field. The path therefore localizes formatter entries but not lsp entries; the new test documents this rather than hiding it.

Behaviour is otherwise unchanged: the value is still skipped exactly as before. That is why the existing assertions stay green, apart from three that now report a more precise path.

Note: packages/core/src/config/normalize.ts exists only on v2, which is why this targets v2.

How did you verify your code works?

  • bun test test/config in packages/core: 209 pass, 0 fail (28 tests in the normalization file, one of them new).
  • New regression test localizing the field that invalidates a native provider model.
  • The three updated assertions report the exact paths ["commands","fallback","template"], ["provider","azure","env"] and ["formatter","prettier","command","0"].
  • bun run typecheck (tsgo) clean, oxlint clean, prettier --check clean.

Screenshots / recordings

Not applicable — log output only.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

…ield

Config normalization discarded the parse issue that explained why a value
was rejected, so an invalid `capabilities` object reported only the enclosing
provider path.

Decode through `SchemaParser.decodeUnknownResult` instead of the `Option`
adapters and carry the issue into the diagnostic, appending the failing field
path so the log names the offending field. Value redaction is unchanged: the
diagnostic message stays generic and only schema positions are added.

Closes anomalyco#50340
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@argszero

argszero commented Sep 23, 2026 •

Copy link
Copy Markdown
Author

Note on the needs:issue label from pr-standards, so it is not mistaken for a missing issue.

This PR targets v2, not the default branch, and GitHub only populates the linked-issue field for PRs targeting the default branch. The check that runs through pull_request_target resolves the workflow from dev, which does not yet carry the v2 fallback that the v2 copy has (hasBodyIssueRef is absent there), so it reads the empty field and flags. This is visible on the other v2-based fix PRs, for example #50499, #50525 and #50517, which carry the same label; the only v2 PR here without it is a feat: title, which the workflow skips by design.

The reference above is deliberate. This change localizes the diagnostic that #50340 and #50756 both report, but it does not change which configs load, so it should not close #50340 — the schema-side PRs #50702 and #49940 do that. I am leaving the issue unlinked rather than forcing a closing link that the change does not earn.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant