cli icon indicating copy to clipboard operation
cli copied to clipboard

fix: correct resolver configuration merge in ValidationService

Open Kartikayy007 opened this issue 1 month ago • 11 comments

Description

Fixes external file resolution via $ref that broke in v3.3.0. The issue was caused by incorrect object spread order in the ValidationService constructor.

Problem: Since the 3.3.0 refactor, the ValidationService constructor has been merging __unstable config in the wrong order. The spread of parserOptions.__unstable?.resolver overwrites the resolver array, breaking local file reference resolution

BEFORE (bug):

__unstable: {
  resolver: {
    resolvers: [createHttpWithAuthResolver(), ...]  // Was added
  },
  ...parserOptions.__unstable?.resolver,  // Overwrites above
}

Solution: Reordered spreads so base config is spread first, then custom resolvers are set last:

AFTER (fixed):

__unstable: {
  ...parserOptions.__unstable,
  resolver: {
    ...parserOptions.__unstable?.resolver,
    resolvers: [createHttpWithAuthResolver(), ...]  // Set last
  }
}

Testing:

  • Done All existing tests pass (200 passing)
  • Local file references now resolve correctly
  • GitHub resolver with auth still works

Related issue(s)

Fixes #1839

Kartikayy007 avatar Nov 04 '25 19:11 Kartikayy007

⚠️ No Changeset found

Latest commit: 81d12760a711124fcce028393740175803207bbd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Nov 04 '25 19:11 changeset-bot[bot]

Changeset has been generated for this PR as part of auto-changeset workflow.

Please review the changeset before merging the PR.
---
'@asyncapi/cli': patch
---

fix: correct resolver configuration merge in ValidationService

- 29c22f1: fix: correct resolver configuration merge in ValidationService

Fixes #1839
- b4529e5: test: add regression test for resolver config merge bug
- 8246049: test: add integration test for external file reference resolution

If you are a maintainer or the author of the PR, you can change the changeset by clicking here

[!TIP] If you don't want auto-changeset to run on this PR, you can add the label skip-changeset to the PR or remove the changeset and change PR title to something other than fix: or feat:.

github-actions[bot] avatar Nov 04 '25 19:11 github-actions[bot]

Thanks for finding the root cause @Kartikayy007. Could you possibly add a test targeting #1839 so that such scenarios don't happen later?

Shurtu-gal avatar Nov 05 '25 04:11 Shurtu-gal

Thanks for finding the root cause @Kartikayy007. Could you possibly add a test targeting #1839 so that such scenarios don't happen later?

ready for another review @Shurtu-gal

Kartikayy007 avatar Nov 05 '25 13:11 Kartikayy007

@Kartikayy007 What I meant was having a set of asyncapi files with refs between them. That should provide us good sense of security.

Shurtu-gal avatar Nov 05 '25 17:11 Shurtu-gal

@Shurtu-gal please review, also please take a look at #1887 whenever you get time

Kartikayy007 avatar Nov 07 '25 18:11 Kartikayy007

@Shurtu-gal ready to merge?

Kartikayy007 avatar Nov 19 '25 12:11 Kartikayy007

/rtm

Kartikayy007 avatar Nov 20 '25 21:11 Kartikayy007

I believe after PR https://github.com/asyncapi/cli/pull/1875 adding the custom resolver support, the spread of parserOptions.__unstable?.resolver was overwriting the custom resolvers array that was breaking local file reference resolution causing bug.

Just curious, PR #1875 was merged on Oct 27 but the issue #1839 was reported on Aug 22.

neoandmatrix avatar Nov 27 '25 08:11 neoandmatrix

Just curious, PR #1875 was merged on Oct 27 but the issue #1839 was reported on Aug 22.

Thank you for that the bug came in the v3.3.0 itself not in the #1875 the PR inherited the issue, ill update the PR description

Kartikayy007 avatar Nov 27 '25 20:11 Kartikayy007