From 5d101dff51ffd863d9ef68071375310e1b0bc725 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Fri, 24 Jul 2026 15:20:18 +0800 Subject: [PATCH 01/17] fix: keep sibling keywords when a referenced schema has a root-level $ref --- .changeset/root-ref-sibling-keywords.md | 7 +++ packages/core/src/__tests__/bundle.test.ts | 45 +++++++++++++++++++ .../openapi-root-ref-siblings.yaml | 24 ++++++++++ .../sibling-refs/schemas/BadRequest.yaml | 10 +++++ .../sibling-refs/schemas/BaseProblem.yaml | 4 ++ packages/core/src/__tests__/resolve.test.ts | 28 ++++++++++++ packages/core/src/bundle/bundle-visitor.ts | 11 +++-- packages/core/src/resolve.ts | 4 +- ...quired-schema-properties-undefined.test.ts | 38 ++++++++++++++++ .../src/rules/common/__tests__/struct.test.ts | 30 +++++++++++++ ...no-required-schema-properties-undefined.ts | 13 ++++++ packages/core/src/rules/common/struct.ts | 24 ++++++---- packages/core/src/walk.ts | 5 +++ .../openapi.yaml | 21 +++++++++ .../redocly.yaml | 3 ++ .../schemas/BadRequest.yaml | 10 +++++ .../schemas/BaseProblem.yaml | 4 ++ .../snapshot.txt | 36 +++++++++++++++ .../sibling-refs-root-in-file/openapi.yaml | 33 ++++++++++++++ .../sibling-refs-root-in-file/redocly.yaml | 3 ++ .../sibling-refs-root-in-file/snapshot.txt | 36 +++++++++++++++ 21 files changed, 376 insertions(+), 13 deletions(-) create mode 100644 .changeset/root-ref-sibling-keywords.md create mode 100644 packages/core/src/__tests__/fixtures/sibling-refs/openapi-root-ref-siblings.yaml create mode 100644 packages/core/src/__tests__/fixtures/sibling-refs/schemas/BadRequest.yaml create mode 100644 packages/core/src/__tests__/fixtures/sibling-refs/schemas/BaseProblem.yaml create mode 100644 tests/e2e/bundle/sibling-refs-root-external-file/openapi.yaml create mode 100644 tests/e2e/bundle/sibling-refs-root-external-file/redocly.yaml create mode 100644 tests/e2e/bundle/sibling-refs-root-external-file/schemas/BadRequest.yaml create mode 100644 tests/e2e/bundle/sibling-refs-root-external-file/schemas/BaseProblem.yaml create mode 100644 tests/e2e/bundle/sibling-refs-root-external-file/snapshot.txt create mode 100644 tests/e2e/bundle/sibling-refs-root-in-file/openapi.yaml create mode 100644 tests/e2e/bundle/sibling-refs-root-in-file/redocly.yaml create mode 100644 tests/e2e/bundle/sibling-refs-root-in-file/snapshot.txt diff --git a/.changeset/root-ref-sibling-keywords.md b/.changeset/root-ref-sibling-keywords.md new file mode 100644 index 0000000000..8999bc0696 --- /dev/null +++ b/.changeset/root-ref-sibling-keywords.md @@ -0,0 +1,7 @@ +--- +'@redocly/openapi-core': patch +'@redocly/cli': patch +--- + +Fixed an issue where the `bundle` command dropped sibling keywords of a root-level `$ref` when the referenced schema itself started with a `$ref` (for example, an external schema file composing another file). +Such schemas are now preserved as separate components with their sibling keywords intact, and the `lint` command validates the sibling keywords instead of silently skipping them. diff --git a/packages/core/src/__tests__/bundle.test.ts b/packages/core/src/__tests__/bundle.test.ts index 447a7cbe14..f1767a483e 100644 --- a/packages/core/src/__tests__/bundle.test.ts +++ b/packages/core/src/__tests__/bundle.test.ts @@ -976,6 +976,51 @@ describe('sibling $ref resolution by spec', () => { expect(problems).toHaveLength(0); expect(res.parsed).toMatchSnapshot(); }); + + it('should keep sibling keywords when an external schema file has a root-level $ref', async () => { + const { bundle: res, problems } = await bundle({ + config: await createConfig({}), + ref: path.join(__dirname, 'fixtures/sibling-refs/openapi-root-ref-siblings.yaml'), + }); + + expect(problems).toHaveLength(0); + expect((res.parsed as any).components.schemas.BadRequest).toEqual({ + title: 'Bad request', + type: 'object', + $ref: '#/components/schemas/BaseProblem', + properties: { + status: { type: 'integer', minimum: 400, maximum: 400 }, + }, + required: ['status'], + }); + expect( + (res.parsed as any).paths['/demo'].get.responses['400'].content['application/json'].schema + ).toEqual({ $ref: '#/components/schemas/BadRequest' }); + }); + + it('should keep sibling keywords when a referenced component has a root-level $ref', async () => { + const { bundle: res, problems } = await bundle({ + config: await createConfig({}), + ref: path.join(__dirname, 'fixtures/sibling-refs/openapi-root-ref-siblings.yaml'), + }); + + expect(problems).toHaveLength(0); + const schemas = (res.parsed as any).components.schemas; + expect(schemas.Problem).toEqual({ + $ref: '#/components/schemas/ProblemAlias', + description: 'Composed problem', + }); + expect(schemas.ProblemAlias).toEqual({ + $ref: '#/components/schemas/BaseProblem', + title: 'Problem alias', + }); + expect(schemas.BaseProblem).toEqual({ + type: 'object', + properties: { + title: { type: 'string' }, + }, + }); + }); }); describe('bundle with --component-names-strategy title', () => { diff --git a/packages/core/src/__tests__/fixtures/sibling-refs/openapi-root-ref-siblings.yaml b/packages/core/src/__tests__/fixtures/sibling-refs/openapi-root-ref-siblings.yaml new file mode 100644 index 0000000000..140409bb07 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/sibling-refs/openapi-root-ref-siblings.yaml @@ -0,0 +1,24 @@ +openapi: 3.1.0 +info: + title: Root-level $ref with sibling keywords + version: 1.0.0 +paths: + /demo: + get: + responses: + '400': + description: Bad request + content: + application/json: + schema: + $ref: ./schemas/BadRequest.yaml +components: + schemas: + BadRequest: + $ref: ./schemas/BadRequest.yaml + ProblemAlias: + $ref: ./schemas/BaseProblem.yaml + title: Problem alias + Problem: + $ref: '#/components/schemas/ProblemAlias' + description: Composed problem diff --git a/packages/core/src/__tests__/fixtures/sibling-refs/schemas/BadRequest.yaml b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/BadRequest.yaml new file mode 100644 index 0000000000..a6a8c05c72 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/BadRequest.yaml @@ -0,0 +1,10 @@ +title: Bad request +type: object +$ref: ./BaseProblem.yaml +properties: + status: + type: integer + minimum: 400 + maximum: 400 +required: + - status diff --git a/packages/core/src/__tests__/fixtures/sibling-refs/schemas/BaseProblem.yaml b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/BaseProblem.yaml new file mode 100644 index 0000000000..0a27ba2939 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/BaseProblem.yaml @@ -0,0 +1,4 @@ +type: object +properties: + title: + type: string diff --git a/packages/core/src/__tests__/resolve.test.ts b/packages/core/src/__tests__/resolve.test.ts index 40b8b4828e..fbe9eaf0c2 100644 --- a/packages/core/src/__tests__/resolve.test.ts +++ b/packages/core/src/__tests__/resolve.test.ts @@ -411,6 +411,34 @@ describe('collect refs', () => { expect(Array.from(resolvedRefs.values()).pop()!.node).toEqual({ type: 'string' }); }); + it('should stop following transitive refs at a $ref with sibling keys', async () => { + const rootDocument = parseYamlToDocument( + outdent` + openapi: 3.0.0 + info: + $ref: "#/aliased" + aliased: + $ref: '#/target' + description: alias with sibling + target: + contact: {} + `, + 'foobar.yaml' + ); + + const resolvedRefs = await resolveDocument({ + rootDocument, + externalRefResolver: new BaseResolver(), + rootType: normalizeTypes(Oas3Types).Root, + }); + + expect(resolvedRefs.get('foobar.yaml::#/aliased')!.node).toEqual({ + $ref: '#/target', + description: 'alias with sibling', + }); + expect(resolvedRefs.get('foobar.yaml::#/target')!.node).toEqual({ contact: {} }); + }); + it('should throw error if ref is folder', async () => { const cwd = path.join(__dirname, 'fixtures/resolve'); const rootDocument = parseYamlToDocument( diff --git a/packages/core/src/bundle/bundle-visitor.ts b/packages/core/src/bundle/bundle-visitor.ts index 77c2b68aa4..4de639a615 100644 --- a/packages/core/src/bundle/bundle-visitor.ts +++ b/packages/core/src/bundle/bundle-visitor.ts @@ -263,14 +263,17 @@ export function makeBundleVisitor({ } function resolveBundledComponent(node: OasRef, resolved: ResolveResult, ctx: UserContext) { - const newRefId = makeRefId(ctx.location.source.absoluteRef, node.$ref); - resolvedRefMap.set(newRefId, { + const resolvedRef = { document: rootDocument, isRemote: false, node: resolved.node, nodePointer: node.$ref, - resolved: true, - }); + resolved: true as const, + }; + resolvedRefMap.set(makeRefId(ctx.location.source.absoluteRef, node.$ref), resolvedRef); + // The node may be walked again as part of the bundled document (e.g. a component saved + // from an external file), so register the rewritten ref from the root source as well. + resolvedRefMap.set(makeRefId(rootDocument.source.absoluteRef, node.$ref), resolvedRef); } function saveComponent(componentType: string, target: ComponentTarget, ctx: UserContext) { diff --git a/packages/core/src/resolve.ts b/packages/core/src/resolve.ts index aac6c68e6c..ae23903e41 100644 --- a/packages/core/src/resolve.ts +++ b/packages/core/src/resolve.ts @@ -476,7 +476,9 @@ export async function resolveDocument(opts: { resolvedRef.node = target; resolvedRef.document = targetDoc; const refId = makeRefId(document.source.absoluteRef, ref.$ref); - if (resolvedRef.document && isRef(target)) { + // A $ref with sibling keys is a composition, not a transparent alias — resolution stops + // there so the siblings survive; only pure refs are followed further. + if (resolvedRef.document && isRef(target) && Object.keys(target).length === 1) { resolvedRef = await followRef(resolvedRef.document, target, pushRef(refStack, target)); } resolvedRefMap.set(refId, resolvedRef); diff --git a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts index cdc8c35aeb..6a345a84da 100644 --- a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts +++ b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts @@ -950,4 +950,42 @@ describe('no-required-schema-properties-undefined', () => { ] `); }); + + it('should not report a required property defined as a sibling of $ref', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + BadRequest: + $ref: '#/components/schemas/Base' + properties: + status: + type: integer + required: + - status + - title + Base: + type: object + properties: + title: + type: string + Usage: + $ref: '#/components/schemas/BadRequest' + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); }); diff --git a/packages/core/src/rules/common/__tests__/struct.test.ts b/packages/core/src/rules/common/__tests__/struct.test.ts index 758dbc2350..993a39e60f 100644 --- a/packages/core/src/rules/common/__tests__/struct.test.ts +++ b/packages/core/src/rules/common/__tests__/struct.test.ts @@ -826,4 +826,34 @@ describe('AsyncAPI bindings struct', () => { expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); + + it('should not report $ref on a referenced schema with sibling keywords', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + Composed: + $ref: '#/components/schemas/Base' + title: Composed + Base: + type: object + Usage: + $ref: '#/components/schemas/Composed' + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { struct: 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); }); diff --git a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts index 0da4453f7b..b2e4c8b23a 100644 --- a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts +++ b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts @@ -1,7 +1,9 @@ +import { isRef } from '../../ref-utils.js'; import type { Oas3Schema, Oas3_1Schema } from '../../typings/openapi.js'; import type { Oas2Schema } from '../../typings/swagger.js'; import { getOwn } from '../../utils/get-own.js'; import { isNotEmptyArray } from '../../utils/is-not-empty-array.js'; +import { isPlainObject } from '../../utils/is-plain-object.js'; import type { Async2Rule, Async3Rule, Arazzo1Rule, Oas2Rule, Oas3Rule } from '../../visitors.js'; import type { UserContext } from '../../walk.js'; import { resolveSchema } from '../utils.js'; @@ -33,6 +35,17 @@ export const NoRequiredSchemaPropertiesUndefined: visited: Set, resolveFrom?: string ): boolean => { + if (isRef(schemaOrRef)) { + // a JSON Schema 2020-12 $ref can carry sibling keywords, so check them too + const siblingProperties = (schemaOrRef as AnySchema).properties; + if ( + isPlainObject(siblingProperties) && + getOwn(siblingProperties, propertyName) !== undefined + ) { + return true; + } + } + const { schema, location } = resolveSchema(schemaOrRef, ctx, resolveFrom); if (!schema || visited.has(schema)) return false; visited.add(schema); diff --git a/packages/core/src/rules/common/struct.ts b/packages/core/src/rules/common/struct.ts index 66a30156f0..4ccb8fabb8 100644 --- a/packages/core/src/rules/common/struct.ts +++ b/packages/core/src/rules/common/struct.ts @@ -53,16 +53,21 @@ export const Struct: return; } + // A $ref with sibling keys composes its target, so required fields can't be judged locally. + const nodeComposesRef = isRef(node); + const required = typeof type.required === 'function' ? type.required(node, key) : type.required; - for (const propName of required || []) { - if (!(node as object).hasOwnProperty(propName)) { - report({ - message: `The field \`${propName}\` must be present on this level.`, - from: refLocation, - location: [{ reportOnKey: true }], - }); + if (!nodeComposesRef) { + for (const propName of required || []) { + if (!(node as object).hasOwnProperty(propName)) { + report({ + message: `The field \`${propName}\` must be present on this level.`, + from: refLocation, + location: [{ reportOnKey: true }], + }); + } } } @@ -85,7 +90,7 @@ export const Struct: } const requiredOneOf = type.requiredOneOf || null; - if (requiredOneOf) { + if (requiredOneOf && !nodeComposesRef) { let hasProperty = false; for (const propName of requiredOneOf || []) { if ((node as object).hasOwnProperty(propName)) { @@ -103,6 +108,9 @@ export const Struct: } for (const propName of Object.keys(node)) { + if (propName === '$ref' && nodeComposesRef) { + continue; + } const propLocation = location.child([propName]); let propValue = node[propName]; diff --git a/packages/core/src/walk.ts b/packages/core/src/walk.ts index 539ba90853..e9feb4bda2 100644 --- a/packages/core/src/walk.ts +++ b/packages/core/src/walk.ts @@ -367,6 +367,11 @@ export function walkDocument(opts: { walkNode(value, propType, loc.child([propName]), resolvedNode, propName); } } + + if (isRef(resolvedNode) && resolvedNode !== node) { + // resolution stopped at a $ref with sibling keys; walk it as a ref from its own location + walkNode(resolvedNode, type, resolvedLocation, parent, key); + } } const currentLeaveVisitors = diff --git a/tests/e2e/bundle/sibling-refs-root-external-file/openapi.yaml b/tests/e2e/bundle/sibling-refs-root-external-file/openapi.yaml new file mode 100644 index 0000000000..501b360418 --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-external-file/openapi.yaml @@ -0,0 +1,21 @@ +openapi: 3.1.0 +info: + title: Root-level $ref with siblings across files + version: 1.0.0 +paths: + /demo: + get: + operationId: getDemo + responses: + '400': + description: Bad request + content: + application/json: + schema: + $ref: ./schemas/BadRequest.yaml +components: + schemas: + BadRequest: + $ref: ./schemas/BadRequest.yaml + BaseProblem: + $ref: ./schemas/BaseProblem.yaml diff --git a/tests/e2e/bundle/sibling-refs-root-external-file/redocly.yaml b/tests/e2e/bundle/sibling-refs-root-external-file/redocly.yaml new file mode 100644 index 0000000000..f6f9f27544 --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-external-file/redocly.yaml @@ -0,0 +1,3 @@ +apis: + main: + root: ./openapi.yaml diff --git a/tests/e2e/bundle/sibling-refs-root-external-file/schemas/BadRequest.yaml b/tests/e2e/bundle/sibling-refs-root-external-file/schemas/BadRequest.yaml new file mode 100644 index 0000000000..a6a8c05c72 --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-external-file/schemas/BadRequest.yaml @@ -0,0 +1,10 @@ +title: Bad request +type: object +$ref: ./BaseProblem.yaml +properties: + status: + type: integer + minimum: 400 + maximum: 400 +required: + - status diff --git a/tests/e2e/bundle/sibling-refs-root-external-file/schemas/BaseProblem.yaml b/tests/e2e/bundle/sibling-refs-root-external-file/schemas/BaseProblem.yaml new file mode 100644 index 0000000000..0a27ba2939 --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-external-file/schemas/BaseProblem.yaml @@ -0,0 +1,4 @@ +type: object +properties: + title: + type: string diff --git a/tests/e2e/bundle/sibling-refs-root-external-file/snapshot.txt b/tests/e2e/bundle/sibling-refs-root-external-file/snapshot.txt new file mode 100644 index 0000000000..6b1cb9c18c --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-external-file/snapshot.txt @@ -0,0 +1,36 @@ +openapi: 3.1.0 +info: + title: Root-level $ref with siblings across files + version: 1.0.0 +paths: + /demo: + get: + operationId: getDemo + responses: + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/BadRequest' +components: + schemas: + BadRequest: + title: Bad request + type: object + $ref: '#/components/schemas/BaseProblem' + properties: + status: + type: integer + minimum: 400 + maximum: 400 + required: + - status + BaseProblem: + type: object + properties: + title: + type: string + +bundling openapi.yaml using configuration for api 'main'... +📦 Created a bundle for openapi.yaml at stdout ms. diff --git a/tests/e2e/bundle/sibling-refs-root-in-file/openapi.yaml b/tests/e2e/bundle/sibling-refs-root-in-file/openapi.yaml new file mode 100644 index 0000000000..523404def6 --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-in-file/openapi.yaml @@ -0,0 +1,33 @@ +openapi: 3.1.0 +info: + title: Root-level $ref with siblings in the same file + version: 1.0.0 +paths: + /demo: + get: + operationId: getDemo + responses: + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/BadRequest' +components: + schemas: + BadRequest: + title: Bad request + type: object + $ref: '#/components/schemas/BaseProblem' + properties: + status: + type: integer + minimum: 400 + maximum: 400 + required: + - status + BaseProblem: + type: object + properties: + title: + type: string diff --git a/tests/e2e/bundle/sibling-refs-root-in-file/redocly.yaml b/tests/e2e/bundle/sibling-refs-root-in-file/redocly.yaml new file mode 100644 index 0000000000..f6f9f27544 --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-in-file/redocly.yaml @@ -0,0 +1,3 @@ +apis: + main: + root: ./openapi.yaml diff --git a/tests/e2e/bundle/sibling-refs-root-in-file/snapshot.txt b/tests/e2e/bundle/sibling-refs-root-in-file/snapshot.txt new file mode 100644 index 0000000000..419063e563 --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-in-file/snapshot.txt @@ -0,0 +1,36 @@ +openapi: 3.1.0 +info: + title: Root-level $ref with siblings in the same file + version: 1.0.0 +paths: + /demo: + get: + operationId: getDemo + responses: + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/BadRequest' +components: + schemas: + BadRequest: + title: Bad request + type: object + $ref: '#/components/schemas/BaseProblem' + properties: + status: + type: integer + minimum: 400 + maximum: 400 + required: + - status + BaseProblem: + type: object + properties: + title: + type: string + +bundling openapi.yaml using configuration for api 'main'... +📦 Created a bundle for openapi.yaml at stdout ms. From 4e46cf288b57883baabe2fcb682e9a381cba3baf Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Fri, 24 Jul 2026 15:26:19 +0800 Subject: [PATCH 02/17] chore: simplify changeset wording --- .changeset/root-ref-sibling-keywords.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/root-ref-sibling-keywords.md b/.changeset/root-ref-sibling-keywords.md index 8999bc0696..1a1b9c146b 100644 --- a/.changeset/root-ref-sibling-keywords.md +++ b/.changeset/root-ref-sibling-keywords.md @@ -3,5 +3,5 @@ '@redocly/cli': patch --- -Fixed an issue where the `bundle` command dropped sibling keywords of a root-level `$ref` when the referenced schema itself started with a `$ref` (for example, an external schema file composing another file). -Such schemas are now preserved as separate components with their sibling keywords intact, and the `lint` command validates the sibling keywords instead of silently skipping them. +Fixed the `bundle` command losing schema keywords (such as `title`, `properties`, or `required`) written next to a `$ref` when the referenced schema starts with its own `$ref`. +These keywords are now kept in the bundled output, and the `lint` command validates them instead of skipping them. From 113c97d5f6b74b36be93fec60022c3f93a5da50c Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Fri, 24 Jul 2026 15:28:32 +0800 Subject: [PATCH 03/17] Apply suggestion from @RomanHotsiy --- .changeset/root-ref-sibling-keywords.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/root-ref-sibling-keywords.md b/.changeset/root-ref-sibling-keywords.md index 1a1b9c146b..8c3187515c 100644 --- a/.changeset/root-ref-sibling-keywords.md +++ b/.changeset/root-ref-sibling-keywords.md @@ -4,4 +4,3 @@ --- Fixed the `bundle` command losing schema keywords (such as `title`, `properties`, or `required`) written next to a `$ref` when the referenced schema starts with its own `$ref`. -These keywords are now kept in the bundled output, and the `lint` command validates them instead of skipping them. From 420e2e9de7c3b9286805fa87c712374e4173fe64 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Fri, 24 Jul 2026 15:46:55 +0800 Subject: [PATCH 04/17] fix: follow composed $ref chains in no-required-schema-properties-undefined --- ...quired-schema-properties-undefined.test.ts | 129 ++++++++++++++++++ ...no-required-schema-properties-undefined.ts | 57 +++++--- 2 files changed, 165 insertions(+), 21 deletions(-) diff --git a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts index 6a345a84da..92cdc0fbeb 100644 --- a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts +++ b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts @@ -988,4 +988,133 @@ describe('no-required-schema-properties-undefined', () => { expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); + + it('should not report a required property defined further down a composed $ref chain', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + Usage: + $ref: '#/components/schemas/Middle' + Middle: + $ref: '#/components/schemas/Mid2' + title: Middle + required: + - deepProp + Mid2: + $ref: '#/components/schemas/Leaf' + title: Mid2 + Leaf: + type: object + properties: + deepProp: + type: string + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should not report a required property defined in an allOf sibling of a referenced composed $ref', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + Usage: + $ref: '#/components/schemas/Composed' + Composed: + $ref: '#/components/schemas/Base' + allOf: + - type: object + properties: + extra: + type: string + required: + - extra + Base: + type: object + properties: + title: + type: string + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + + it('should still report a genuinely undefined property in a composed $ref chain', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + Usage: + $ref: '#/components/schemas/Middle' + Middle: + $ref: '#/components/schemas/Leaf' + title: Middle + required: + - missingProp + Leaf: + type: object + properties: + deepProp: + type: string + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Middle/required/0", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Required property 'missingProp' is not defined.", + "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", + "ruleId": "no-required-schema-properties-undefined", + "severity": "error", + "suggest": [], + }, + ] + `); + }); }); diff --git a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts index b2e4c8b23a..f4d2c3fb33 100644 --- a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts +++ b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts @@ -3,7 +3,6 @@ import type { Oas3Schema, Oas3_1Schema } from '../../typings/openapi.js'; import type { Oas2Schema } from '../../typings/swagger.js'; import { getOwn } from '../../utils/get-own.js'; import { isNotEmptyArray } from '../../utils/is-not-empty-array.js'; -import { isPlainObject } from '../../utils/is-plain-object.js'; import type { Async2Rule, Async3Rule, Arazzo1Rule, Oas2Rule, Oas3Rule } from '../../visitors.js'; import type { UserContext } from '../../walk.js'; import { resolveSchema } from '../utils.js'; @@ -29,45 +28,30 @@ export const NoRequiredSchemaPropertiesUndefined: parents.push(currentSchema); if (!currentSchema.required) return; - const hasProperty = ( - schemaOrRef: AnySchema | undefined, + const definesProperty = ( + schema: AnySchema, propertyName: string, visited: Set, resolveFrom?: string ): boolean => { - if (isRef(schemaOrRef)) { - // a JSON Schema 2020-12 $ref can carry sibling keywords, so check them too - const siblingProperties = (schemaOrRef as AnySchema).properties; - if ( - isPlainObject(siblingProperties) && - getOwn(siblingProperties, propertyName) !== undefined - ) { - return true; - } - } - - const { schema, location } = resolveSchema(schemaOrRef, ctx, resolveFrom); - if (!schema || visited.has(schema)) return false; - visited.add(schema); - if (schema.properties && getOwn(schema.properties, propertyName) !== undefined) { return true; } - if (schema.allOf?.some((s) => hasProperty(s, propertyName, visited, location))) { + if (schema.allOf?.some((s) => hasProperty(s, propertyName, visited, resolveFrom))) { return true; } if ( isNotEmptyArray(schema.anyOf) && - schema.anyOf.every((s) => hasProperty(s, propertyName, new Set(visited), location)) + schema.anyOf.every((s) => hasProperty(s, propertyName, new Set(visited), resolveFrom)) ) { return true; } if ( isNotEmptyArray(schema.oneOf) && - schema.oneOf.every((s) => hasProperty(s, propertyName, new Set(visited), location)) + schema.oneOf.every((s) => hasProperty(s, propertyName, new Set(visited), resolveFrom)) ) { return true; } @@ -75,6 +59,37 @@ export const NoRequiredSchemaPropertiesUndefined: return false; }; + const hasProperty = ( + schemaOrRef: AnySchema | undefined, + propertyName: string, + visited: Set, + resolveFrom?: string + ): boolean => { + // A JSON Schema 2020-12 $ref can carry sibling keywords that compose the target, + // so check them before the $ref is resolved away. + if ( + isRef(schemaOrRef) && + definesProperty(schemaOrRef, propertyName, visited, resolveFrom) + ) { + return true; + } + + const { schema, location } = resolveSchema(schemaOrRef, ctx, resolveFrom); + if (!schema || visited.has(schema)) return false; + visited.add(schema); + + if (definesProperty(schema, propertyName, visited, location)) { + return true; + } + + // The resolved schema may itself be a $ref with siblings; follow the chain. + if (isRef(schema)) { + return hasProperty(schema, propertyName, visited, location); + } + + return false; + }; + const isCompositionChild = (parent: AnySchema, child: AnySchema): boolean => { const matchesChild = (s: AnySchema) => resolveSchema(s, ctx).schema === child; return !!( From 232e3e304b4da333b416b14aafc188e6b10e46c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacek=20=C5=81=C4=99kawa?= <164185257+JLekawa@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:44:25 +0200 Subject: [PATCH 05/17] Apply suggestion from @JLekawa --- .changeset/root-ref-sibling-keywords.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/root-ref-sibling-keywords.md b/.changeset/root-ref-sibling-keywords.md index 8c3187515c..1928e04657 100644 --- a/.changeset/root-ref-sibling-keywords.md +++ b/.changeset/root-ref-sibling-keywords.md @@ -3,4 +3,4 @@ '@redocly/cli': patch --- -Fixed the `bundle` command losing schema keywords (such as `title`, `properties`, or `required`) written next to a `$ref` when the referenced schema starts with its own `$ref`. +Fixed the `bundle` command losing schema keywords (such as `title`, `properties`, or `required`) written next to a `$ref` when the referenced schemas started with their own `$ref`. From 2cb68571b6d4de3c15d82eac39a759de76d18bb1 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Mon, 27 Jul 2026 17:14:51 +0800 Subject: [PATCH 06/17] feat: expose composed $ref chains on the resolve result instead of stopping resolution --- .changeset/root-ref-sibling-keywords.md | 5 +- packages/core/src/__tests__/bundle.test.ts | 14 ++ packages/core/src/__tests__/resolve.test.ts | 8 +- packages/core/src/bundle/bundle-visitor.ts | 49 +++-- packages/core/src/resolve.ts | 34 +++- ...no-required-schema-properties-undefined.ts | 188 +++++++++++------- packages/core/src/rules/common/struct.ts | 24 +-- .../__tests__/no-unused-components.test.ts | 37 ++++ packages/core/src/rules/utils.ts | 9 +- packages/core/src/walk.ts | 102 +++++++--- 10 files changed, 333 insertions(+), 137 deletions(-) diff --git a/.changeset/root-ref-sibling-keywords.md b/.changeset/root-ref-sibling-keywords.md index 1928e04657..3e7abbc585 100644 --- a/.changeset/root-ref-sibling-keywords.md +++ b/.changeset/root-ref-sibling-keywords.md @@ -1,6 +1,7 @@ --- -'@redocly/openapi-core': patch -'@redocly/cli': patch +'@redocly/openapi-core': minor +'@redocly/cli': minor --- Fixed the `bundle` command losing schema keywords (such as `title`, `properties`, or `required`) written next to a `$ref` when the referenced schemas started with their own `$ref`. +Ref resolution now exposes such composed `$ref`s as a `chain` field on the resolve result, so custom rules and decorators can inspect them. diff --git a/packages/core/src/__tests__/bundle.test.ts b/packages/core/src/__tests__/bundle.test.ts index f1767a483e..d7aee01ab8 100644 --- a/packages/core/src/__tests__/bundle.test.ts +++ b/packages/core/src/__tests__/bundle.test.ts @@ -998,6 +998,20 @@ describe('sibling $ref resolution by spec', () => { ).toEqual({ $ref: '#/components/schemas/BadRequest' }); }); + it('should keep referenced composed schemas when removing unused components', async () => { + const { bundle: res, problems } = await bundle({ + config: await createConfig({}), + ref: path.join(__dirname, 'fixtures/sibling-refs/openapi-root-ref-siblings.yaml'), + removeUnusedComponents: true, + }); + + expect(problems).toHaveLength(0); + expect(Object.keys((res.parsed as any).components.schemas).sort()).toEqual([ + 'BadRequest', + 'BaseProblem', + ]); + }); + it('should keep sibling keywords when a referenced component has a root-level $ref', async () => { const { bundle: res, problems } = await bundle({ config: await createConfig({}), diff --git a/packages/core/src/__tests__/resolve.test.ts b/packages/core/src/__tests__/resolve.test.ts index fbe9eaf0c2..3267845c85 100644 --- a/packages/core/src/__tests__/resolve.test.ts +++ b/packages/core/src/__tests__/resolve.test.ts @@ -411,7 +411,7 @@ describe('collect refs', () => { expect(Array.from(resolvedRefs.values()).pop()!.node).toEqual({ type: 'string' }); }); - it('should stop following transitive refs at a $ref with sibling keys', async () => { + it('should record a $ref with sibling keys as a chain hop while following transitive refs', async () => { const rootDocument = parseYamlToDocument( outdent` openapi: 3.0.0 @@ -432,10 +432,14 @@ describe('collect refs', () => { rootType: normalizeTypes(Oas3Types).Root, }); - expect(resolvedRefs.get('foobar.yaml::#/aliased')!.node).toEqual({ + const aliasedRef = resolvedRefs.get('foobar.yaml::#/aliased')!; + expect(aliasedRef.node).toEqual({ contact: {} }); + expect(aliasedRef.chain).toHaveLength(1); + expect(aliasedRef.chain![0].node).toEqual({ $ref: '#/target', description: 'alias with sibling', }); + expect(aliasedRef.chain![0].nodePointer).toEqual('#/aliased'); expect(resolvedRefs.get('foobar.yaml::#/target')!.node).toEqual({ contact: {} }); }); diff --git a/packages/core/src/bundle/bundle-visitor.ts b/packages/core/src/bundle/bundle-visitor.ts index 4de639a615..abe132ed67 100644 --- a/packages/core/src/bundle/bundle-visitor.ts +++ b/packages/core/src/bundle/bundle-visitor.ts @@ -151,17 +151,24 @@ export function makeBundleVisitor({ reportUnresolvedRef(resolved, ctx.report, ctx.location); return; } + + // a composed $ref (with sibling keys) in the chain is the effective target, + // so the composition survives in the bundled output + const target = resolved.chain?.length + ? { node: resolved.chain[0].node, location: resolved.chain[0].location } + : { node: resolved.node, location: resolved.location }; + if ( - resolved.location.source === rootDocument.source && - resolved.location.source === ctx.location.source && + target.location.source === rootDocument.source && + target.location.source === ctx.location.source && ctx.type.name !== 'scalar' && !dereference ) { // Normalize explicit self-file refs (api.yaml#/x -> #/x). Already-internal refs // stay as authored: rewriting them would collapse $ref chains, and AsyncAPI 3 // operation messages must point to channel messages, not to components. - if (parseRef(node.$ref).uri !== null && node.$ref !== resolved.location.pointer) { - node.$ref = resolved.location.pointer; + if (parseRef(node.$ref).uri !== null && node.$ref !== target.location.pointer) { + node.$ref = target.location.pointer; } return; @@ -173,14 +180,14 @@ export function makeBundleVisitor({ const componentType = mapTypeToComponent(ctx.type.name, version); if (!componentType) { - replaceRef(node, resolved, ctx); + replaceRef(node, target, ctx); } else { if (dereference) { - saveComponent(componentType, resolved, ctx); - replaceRef(node, resolved, ctx); + saveComponent(componentType, target, ctx); + replaceRef(node, target, ctx); } else { - node.$ref = saveComponent(componentType, resolved, ctx); - resolveBundledComponent(node, resolved, ctx); + node.$ref = saveComponent(componentType, target, ctx); + resolveBundledComponent(node, target, ctx); } } }, @@ -240,7 +247,11 @@ export function makeBundleVisitor({ return; } - discriminator.defaultMapping = saveComponent(schemaComponentType, resolved, ctx); + discriminator.defaultMapping = saveComponent( + schemaComponentType, + resolved.chain?.[0] ?? resolved, + ctx + ); }, DiscriminatorMapping: { leave(mapping, ctx) { @@ -255,7 +266,11 @@ export function makeBundleVisitor({ return; } - mapping[name] = saveComponent(schemaComponentType, resolved, ctx); + mapping[name] = saveComponent( + schemaComponentType, + resolved.chain?.[0] ?? resolved, + ctx + ); } }, }, @@ -293,12 +308,12 @@ export function makeBundleVisitor({ } function isEqualOrEqualRef(node: unknown, target: ComponentTarget, ctx: UserContext) { - if ( - isRef(node) && - ctx.resolve(node, rootLocation.absolutePointer).location?.absolutePointer === - target.location.absolutePointer - ) { - return true; + if (isRef(node)) { + const resolved = ctx.resolve(node, rootLocation.absolutePointer); + const effectiveLocation = resolved.chain?.[0]?.location ?? resolved.location; + if (effectiveLocation?.absolutePointer === target.location.absolutePointer) { + return true; + } } return dequal(node, target.node); diff --git a/packages/core/src/resolve.ts b/packages/core/src/resolve.ts index ae23903e41..41b5ea18b4 100644 --- a/packages/core/src/resolve.ts +++ b/packages/core/src/resolve.ts @@ -181,6 +181,15 @@ export class BaseResolver { } } +// A $ref target that is itself a $ref with sibling keys (a JSON Schema 2020-12 composition). +// Resolution chases through it to the chain end, but the hop is recorded so consumers +// that care about the composition (the bundler, chain-aware rules) can access it. +export type ResolvedRefChainHop = { + node: unknown; + document: Document; + nodePointer: string; +}; + export type ResolvedRef = | { resolved: false; @@ -190,6 +199,7 @@ export type ResolvedRef = source?: Source; error?: ResolveError | YamlParseError; node?: any; + chain?: undefined; } | { resolved: true; @@ -198,6 +208,7 @@ export type ResolvedRef = nodePointer: string; isRemote: boolean; error?: undefined; + chain?: ResolvedRefChainHop[]; }; export type ResolvedRefMap = Map; @@ -344,6 +355,10 @@ export async function resolveDocument(opts: { resolvedRef.nodePointer!, type ); + // chain hops carry sibling keys that can contain refs of their own + for (const chainHop of resolvedRef.chain ?? []) { + resolveRefsInParallel(chainHop.node, chainHop.document, chainHop.nodePointer, type); + } } }); resolvePromises.push(promise); @@ -449,6 +464,8 @@ export async function resolveDocument(opts: { ); } else if (isRef(target)) { resolvedRef = await followRef(targetDoc, target, pushRef(refStack, target)); + // a chain collected while traversing into a ref does not belong to the outer ref + resolvedRef.chain = undefined; targetDoc = resolvedRef.document || targetDoc; if (isPlainObject(resolvedRef.node)) { @@ -476,10 +493,21 @@ export async function resolveDocument(opts: { resolvedRef.node = target; resolvedRef.document = targetDoc; const refId = makeRefId(document.source.absoluteRef, ref.$ref); - // A $ref with sibling keys is a composition, not a transparent alias — resolution stops - // there so the siblings survive; only pure refs are followed further. - if (resolvedRef.document && isRef(target) && Object.keys(target).length === 1) { + if (resolvedRef.document && isRef(target)) { + // A $ref with sibling keys is a composition (JSON Schema 2020-12), not a transparent + // alias — record it as a chain hop so the composition survives for consumers that need it. + const chainHop: ResolvedRefChainHop | undefined = + Object.keys(target).length > 1 + ? { + node: target, + document: resolvedRef.document, + nodePointer: resolvedRef.nodePointer!, + } + : undefined; resolvedRef = await followRef(resolvedRef.document, target, pushRef(refStack, target)); + if (chainHop && resolvedRef.resolved) { + resolvedRef = { ...resolvedRef, chain: [chainHop, ...(resolvedRef.chain ?? [])] }; + } } resolvedRefMap.set(refId, resolvedRef); return { ...resolvedRef }; diff --git a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts index f4d2c3fb33..4f64ad61cf 100644 --- a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts +++ b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts @@ -1,10 +1,11 @@ -import { isRef } from '../../ref-utils.js'; -import type { Oas3Schema, Oas3_1Schema } from '../../typings/openapi.js'; +import { isRef, type Location } from '../../ref-utils.js'; +import type { Oas3Schema, Oas3_1Schema, OasRef } from '../../typings/openapi.js'; import type { Oas2Schema } from '../../typings/swagger.js'; import { getOwn } from '../../utils/get-own.js'; import { isNotEmptyArray } from '../../utils/is-not-empty-array.js'; +import { isPlainObject } from '../../utils/is-plain-object.js'; import type { Async2Rule, Async3Rule, Arazzo1Rule, Oas2Rule, Oas3Rule } from '../../visitors.js'; -import type { UserContext } from '../../walk.js'; +import type { ResolveResult, UserContext } from '../../walk.js'; import { resolveSchema } from '../utils.js'; type AnySchema = @@ -19,7 +20,122 @@ export const NoRequiredSchemaPropertiesUndefined: | Async2Rule | Arazzo1Rule = () => { const parents: AnySchema[] = []; + const validatedChainHops = new Set(); + + const definesProperty = ( + schema: AnySchema, + propertyName: string, + visited: Set, + ctx: UserContext, + resolveFrom?: string + ): boolean => { + if (schema.properties && getOwn(schema.properties, propertyName) !== undefined) { + return true; + } + + if (schema.allOf?.some((s) => hasProperty(s, propertyName, visited, ctx, resolveFrom))) { + return true; + } + + if ( + isNotEmptyArray(schema.anyOf) && + schema.anyOf.every((s) => hasProperty(s, propertyName, new Set(visited), ctx, resolveFrom)) + ) { + return true; + } + + if ( + isNotEmptyArray(schema.oneOf) && + schema.oneOf.every((s) => hasProperty(s, propertyName, new Set(visited), ctx, resolveFrom)) + ) { + return true; + } + + return false; + }; + + const hasProperty = ( + schemaOrRef: AnySchema | undefined, + propertyName: string, + visited: Set, + ctx: UserContext, + resolveFrom?: string + ): boolean => { + // A JSON Schema 2020-12 $ref can carry sibling keywords that compose the target, + // so check them before the $ref is resolved away. + if ( + isRef(schemaOrRef) && + definesProperty(schemaOrRef as AnySchema, propertyName, visited, ctx, resolveFrom) + ) { + return true; + } + + const { schema, location, chain } = resolveSchema(schemaOrRef, ctx, resolveFrom); + if (!schema || visited.has(schema)) return false; + visited.add(schema); + + if (definesProperty(schema, propertyName, visited, ctx, location)) { + return true; + } + + // composed $refs the resolution chased through contribute their sibling keywords too + for (const chainHop of chain ?? []) { + if ( + isPlainObject(chainHop.node) && + definesProperty( + chainHop.node as AnySchema, + propertyName, + visited, + ctx, + chainHop.location.source.absoluteRef + ) + ) { + return true; + } + } + + return false; + }; + + const reportUndefinedRequired = ( + schema: AnySchema, + schemaLocation: Location, + ctx: UserContext, + resolveFrom?: string + ) => { + if (!schema.required) return; + + for (const [i, requiredProperty] of schema.required.entries()) { + if (!hasProperty(schema, requiredProperty, new Set(), ctx, resolveFrom)) { + ctx.report({ + message: `Required property '${requiredProperty}' is not defined.`, + location: schemaLocation.child(['required', i]), + reference: + 'https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined', + }); + } + } + }; + return { + ref: { + leave(_ref: OasRef, ctx: UserContext, resolved: ResolveResult) { + // composed $refs in the chain are never visited as Schema nodes, so their + // `required` sibling keywords are validated here + for (const chainHop of resolved.chain ?? []) { + if (!isPlainObject(chainHop.node) || validatedChainHops.has(chainHop.node)) { + continue; + } + validatedChainHops.add(chainHop.node); + reportUndefinedRequired( + chainHop.node as AnySchema, + chainHop.location, + ctx, + chainHop.location.source.absoluteRef + ); + } + }, + }, Schema: { leave(_: AnySchema) { parents.pop(); @@ -28,68 +144,6 @@ export const NoRequiredSchemaPropertiesUndefined: parents.push(currentSchema); if (!currentSchema.required) return; - const definesProperty = ( - schema: AnySchema, - propertyName: string, - visited: Set, - resolveFrom?: string - ): boolean => { - if (schema.properties && getOwn(schema.properties, propertyName) !== undefined) { - return true; - } - - if (schema.allOf?.some((s) => hasProperty(s, propertyName, visited, resolveFrom))) { - return true; - } - - if ( - isNotEmptyArray(schema.anyOf) && - schema.anyOf.every((s) => hasProperty(s, propertyName, new Set(visited), resolveFrom)) - ) { - return true; - } - - if ( - isNotEmptyArray(schema.oneOf) && - schema.oneOf.every((s) => hasProperty(s, propertyName, new Set(visited), resolveFrom)) - ) { - return true; - } - - return false; - }; - - const hasProperty = ( - schemaOrRef: AnySchema | undefined, - propertyName: string, - visited: Set, - resolveFrom?: string - ): boolean => { - // A JSON Schema 2020-12 $ref can carry sibling keywords that compose the target, - // so check them before the $ref is resolved away. - if ( - isRef(schemaOrRef) && - definesProperty(schemaOrRef, propertyName, visited, resolveFrom) - ) { - return true; - } - - const { schema, location } = resolveSchema(schemaOrRef, ctx, resolveFrom); - if (!schema || visited.has(schema)) return false; - visited.add(schema); - - if (definesProperty(schema, propertyName, visited, location)) { - return true; - } - - // The resolved schema may itself be a $ref with siblings; follow the chain. - if (isRef(schema)) { - return hasProperty(schema, propertyName, visited, location); - } - - return false; - }; - const isCompositionChild = (parent: AnySchema, child: AnySchema): boolean => { const matchesChild = (s: AnySchema) => resolveSchema(s, ctx).schema === child; return !!( @@ -111,8 +165,8 @@ export const NoRequiredSchemaPropertiesUndefined: for (const [i, requiredProperty] of currentSchema.required.entries()) { if ( - !hasProperty(currentSchema, requiredProperty, new Set()) && - !hasProperty(compositionRoot, requiredProperty, new Set()) + !hasProperty(currentSchema, requiredProperty, new Set(), ctx) && + !hasProperty(compositionRoot, requiredProperty, new Set(), ctx) ) { ctx.report({ message: `Required property '${requiredProperty}' is not defined.`, diff --git a/packages/core/src/rules/common/struct.ts b/packages/core/src/rules/common/struct.ts index 4ccb8fabb8..66a30156f0 100644 --- a/packages/core/src/rules/common/struct.ts +++ b/packages/core/src/rules/common/struct.ts @@ -53,21 +53,16 @@ export const Struct: return; } - // A $ref with sibling keys composes its target, so required fields can't be judged locally. - const nodeComposesRef = isRef(node); - const required = typeof type.required === 'function' ? type.required(node, key) : type.required; - if (!nodeComposesRef) { - for (const propName of required || []) { - if (!(node as object).hasOwnProperty(propName)) { - report({ - message: `The field \`${propName}\` must be present on this level.`, - from: refLocation, - location: [{ reportOnKey: true }], - }); - } + for (const propName of required || []) { + if (!(node as object).hasOwnProperty(propName)) { + report({ + message: `The field \`${propName}\` must be present on this level.`, + from: refLocation, + location: [{ reportOnKey: true }], + }); } } @@ -90,7 +85,7 @@ export const Struct: } const requiredOneOf = type.requiredOneOf || null; - if (requiredOneOf && !nodeComposesRef) { + if (requiredOneOf) { let hasProperty = false; for (const propName of requiredOneOf || []) { if ((node as object).hasOwnProperty(propName)) { @@ -108,9 +103,6 @@ export const Struct: } for (const propName of Object.keys(node)) { - if (propName === '$ref' && nodeComposesRef) { - continue; - } const propLocation = location.child([propName]); let propValue = node[propName]; diff --git a/packages/core/src/rules/oas3/__tests__/no-unused-components.test.ts b/packages/core/src/rules/oas3/__tests__/no-unused-components.test.ts index 558c087ae2..ecc64efd12 100644 --- a/packages/core/src/rules/oas3/__tests__/no-unused-components.test.ts +++ b/packages/core/src/rules/oas3/__tests__/no-unused-components.test.ts @@ -448,4 +448,41 @@ describe('Oas3 no-unused-components', () => { ] `); }); + + it('should not report a referenced schema composing another schema with a root-level $ref', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: + /demo: + get: + responses: + '400': + description: Bad request + content: + application/json: + schema: + $ref: '#/components/schemas/Composed' + components: + schemas: + Composed: + $ref: '#/components/schemas/Base' + title: Composed + Base: + type: object + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-unused-components': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); }); diff --git a/packages/core/src/rules/utils.ts b/packages/core/src/rules/utils.ts index c389c402e0..8002bee51b 100644 --- a/packages/core/src/rules/utils.ts +++ b/packages/core/src/rules/utils.ts @@ -13,7 +13,7 @@ import type { import type { Oas2Tag } from '../typings/swagger.js'; import { isDefined } from '../utils/is-defined.js'; import { isPlainObject } from '../utils/is-plain-object.js'; -import type { NonUndefined, UserContext } from '../walk.js'; +import type { NonUndefined, ResolvedChainHop, UserContext } from '../walk.js'; import type { AjvValidator } from './ajv.js'; export const resolveSchema = ( @@ -23,11 +23,16 @@ export const resolveSchema = ( ): { schema: T | undefined; location: string | undefined; + chain?: ResolvedChainHop[]; } => { if (isRef(schemaOrRef)) { const resolved = ctx.resolve(schemaOrRef, resolveFrom); return resolved - ? { schema: resolved.node, location: resolved.location?.source.absoluteRef } + ? { + schema: resolved.node, + location: resolved.location?.source.absoluteRef, + chain: resolved.chain, + } : { schema: undefined, location: resolveFrom }; } diff --git a/packages/core/src/walk.ts b/packages/core/src/walk.ts index e9feb4bda2..8c0dd68f51 100644 --- a/packages/core/src/walk.ts +++ b/packages/core/src/walk.ts @@ -29,9 +29,22 @@ export type NonUndefined = | object | Record; +// A composed $ref (with sibling keys) the resolution chased through on the way to `node`. +export type ResolvedChainHop = { node: unknown; location: Location }; + export type ResolveResult = - | { node: T; location: Location; error?: ResolveError | YamlParseError } - | { node: undefined; location: undefined; error?: ResolveError | YamlParseError }; + | { + node: T; + location: Location; + error?: ResolveError | YamlParseError; + chain?: ResolvedChainHop[]; + } + | { + node: undefined; + location: undefined; + error?: ResolveError | YamlParseError; + chain?: ResolvedChainHop[]; + }; export type ResolveFn = ( node: Referenced, @@ -132,6 +145,7 @@ export function walkDocument(opts: { }) { const { document, rootType, normalizedVisitors, resolvedRefMap, ctx } = opts; const seenNodesPerType: Record> = {}; + const walkedComposedRefs = new Set(); const ignoredNodes = new Set(); // Pre-compute combined enter/leave arrays per type to avoid per-node array allocations @@ -165,22 +179,40 @@ export function walkDocument(opts: { }; } - const { resolved, node, document, nodePointer, error } = resolvedRef; + const { resolved, node, document, nodePointer, error, chain } = resolvedRef; const newLocation = resolved ? new Location(document!.source, nodePointer!) : error instanceof YamlParseError ? new Location(error.source, '') : undefined; - return { location: newLocation, node, error }; + return { + location: newLocation, + node, + error, + chain: chain?.map((chainHop) => ({ + node: chainHop.node, + location: new Location(chainHop.document.source, chainHop.nodePointer), + })), + }; }; const rawLocation = location; let currentLocation = location; const nodeIsRef = isRef(node); - const { node: resolvedNode, location: resolvedLocation, error } = resolve(node); + const { + node: resolvedNode, + location: resolvedLocation, + error, + chain: resolvedChain, + } = resolve(node); const enteredContexts: Set = new Set(); + if (nodeIsRef && Object.keys(node).length > 1) { + // composed $refs can also be reached as chain hops — record this walk to avoid a repeat + walkedComposedRefs.add(`${type.name}::${location.absolutePointer}`); + } + if (nodeIsRef) { const refEnterVisitors = normalizedVisitors.ref.enter; for (const { visit: visitor, ruleId, severity, message, context } of refEnterVisitors) { @@ -202,7 +234,7 @@ export function walkDocument(opts: { config: ctx.config, getVisitorData: () => getVisitorDataFn(ruleId), }, - { node: resolvedNode, location: resolvedLocation, error } + { node: resolvedNode, location: resolvedLocation, error, chain: resolvedChain } ); if (resolvedLocation?.source.absoluteRef && ctx.refTypes) { ctx.refTypes.set(resolvedLocation?.source.absoluteRef, type); @@ -282,12 +314,18 @@ export function walkDocument(opts: { } } - if (visitedBySome || !isNodeSeen) { - seenNodesPerType[type.name] = seenNodesPerType[type.name] || new Set(); - seenNodesPerType[type.name].add(resolvedNode); + const shouldWalkChildren = visitedBySome || !isNodeSeen; + const refSiblingProps = + nodeIsRef && isPlainObject(node) ? Object.keys(node).filter((k) => k !== '$ref') : []; + + if (shouldWalkChildren || refSiblingProps.length > 0) { + if (shouldWalkChildren) { + seenNodesPerType[type.name] = seenNodesPerType[type.name] || new Set(); + seenNodesPerType[type.name].add(resolvedNode); + } if (Array.isArray(resolvedNode)) { - const itemsType = type.items; + const itemsType = shouldWalkChildren ? type.items : undefined; if (itemsType !== undefined) { const isTypeAFunction = typeof itemsType === 'function'; for (let i = 0; i < resolvedNode.length; i++) { @@ -308,20 +346,24 @@ export function walkDocument(opts: { } } } else if (isPlainObject(resolvedNode)) { - // visit in order from type-tree first - const props = Object.keys(type.properties); - if (type.additionalProperties) { - props.push(...Object.keys(resolvedNode).filter((k) => !props.includes(k))); - } else if (type.extensionsPrefix) { - props.push( - ...Object.keys(resolvedNode).filter((k) => - k.startsWith(type.extensionsPrefix as string) - ) - ); - } - - if (nodeIsRef) { - props.push(...Object.keys(node).filter((k) => k !== '$ref' && !props.includes(k))); // properties on the same level as $ref + let props: string[]; + if (shouldWalkChildren) { + // visit in order from type-tree first + props = Object.keys(type.properties); + if (type.additionalProperties) { + props.push(...Object.keys(resolvedNode).filter((k) => !props.includes(k))); + } else if (type.extensionsPrefix) { + props.push( + ...Object.keys(resolvedNode).filter((k) => + k.startsWith(type.extensionsPrefix as string) + ) + ); + } + // properties on the same level as $ref + props.push(...refSiblingProps.filter((k) => !props.includes(k))); + } else { + // the resolved node was already visited, but this ref's sibling keys still need walking + props = refSiblingProps; } for (const propName of props) { @@ -367,10 +409,14 @@ export function walkDocument(opts: { walkNode(value, propType, loc.child([propName]), resolvedNode, propName); } } + } - if (isRef(resolvedNode) && resolvedNode !== node) { - // resolution stopped at a $ref with sibling keys; walk it as a ref from its own location - walkNode(resolvedNode, type, resolvedLocation, parent, key); + if (nodeIsRef && resolvedChain?.length) { + // walk the composed $ref hop from its own location so its siblings and inner $ref + // are processed; the hop's own resolution carries the rest of the chain + const chainHop = resolvedChain[0]; + if (!walkedComposedRefs.has(`${type.name}::${chainHop.location.absolutePointer}`)) { + walkNode(chainHop.node, type, chainHop.location, parent, key); } } @@ -424,7 +470,7 @@ export function walkDocument(opts: { config: ctx.config, getVisitorData: () => getVisitorDataFn(ruleId), }, - { node: resolvedNode, location: resolvedLocation, error } + { node: resolvedNode, location: resolvedLocation, error, chain: resolvedChain } ); } } From 1c03c85fabe2f8eccf9c50d724c2a55e6a856560 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Mon, 27 Jul 2026 17:31:19 +0800 Subject: [PATCH 07/17] fix: validate required on inline composed $refs and ignore non-array required --- ...quired-schema-properties-undefined.test.ts | 89 +++++++++++++++++++ ...no-required-schema-properties-undefined.ts | 31 +++---- 2 files changed, 105 insertions(+), 15 deletions(-) diff --git a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts index 92cdc0fbeb..3e9076ad0e 100644 --- a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts +++ b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts @@ -1066,6 +1066,95 @@ describe('no-required-schema-properties-undefined', () => { expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); + it('should report an undefined required property written next to an inline $ref', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + Payload: + type: object + properties: + data: + $ref: '#/components/schemas/Base' + required: + - name + - missing + Base: + type: object + properties: + name: + type: string + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Payload/properties/data/required/1", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Required property 'missing' is not defined.", + "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", + "ruleId": "no-required-schema-properties-undefined", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + it('should ignore a non-array required on a composed parameter $ref', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: + /demo: + get: + parameters: + - $ref: '#/components/parameters/Alias' + responses: {} + components: + parameters: + Alias: + $ref: '#/components/parameters/Base' + required: true + Base: + name: q + in: query + schema: + type: string + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + it('should still report a genuinely undefined property in a composed $ref chain', async () => { const document = parseYamlToDocument( outdent` diff --git a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts index 4f64ad61cf..c460ada4d8 100644 --- a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts +++ b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts @@ -20,7 +20,7 @@ export const NoRequiredSchemaPropertiesUndefined: | Async2Rule | Arazzo1Rule = () => { const parents: AnySchema[] = []; - const validatedChainHops = new Set(); + const validatedRefNodes = new Set(); const definesProperty = ( schema: AnySchema, @@ -103,7 +103,7 @@ export const NoRequiredSchemaPropertiesUndefined: ctx: UserContext, resolveFrom?: string ) => { - if (!schema.required) return; + if (!isNotEmptyArray(schema.required)) return; for (const [i, requiredProperty] of schema.required.entries()) { if (!hasProperty(schema, requiredProperty, new Set(), ctx, resolveFrom)) { @@ -119,20 +119,21 @@ export const NoRequiredSchemaPropertiesUndefined: return { ref: { - leave(_ref: OasRef, ctx: UserContext, resolved: ResolveResult) { - // composed $refs in the chain are never visited as Schema nodes, so their - // `required` sibling keywords are validated here - for (const chainHop of resolved.chain ?? []) { - if (!isPlainObject(chainHop.node) || validatedChainHops.has(chainHop.node)) { + leave(refNode: OasRef, ctx: UserContext, resolved: ResolveResult) { + if (ctx.type.name !== 'Schema') return; + + // composed $refs are never visited as Schema nodes, so the `required` sibling + // keywords of the ref itself and of the chain hops are validated here + const composedSchemas = [ + { node: refNode as unknown, location: ctx.location }, + ...(resolved.chain ?? []), + ]; + for (const { node, location } of composedSchemas) { + if (!isPlainObject(node) || validatedRefNodes.has(node)) { continue; } - validatedChainHops.add(chainHop.node); - reportUndefinedRequired( - chainHop.node as AnySchema, - chainHop.location, - ctx, - chainHop.location.source.absoluteRef - ); + validatedRefNodes.add(node); + reportUndefinedRequired(node as AnySchema, location, ctx, location.source.absoluteRef); } }, }, @@ -142,7 +143,7 @@ export const NoRequiredSchemaPropertiesUndefined: }, enter(currentSchema: AnySchema, ctx: UserContext) { parents.push(currentSchema); - if (!currentSchema.required) return; + if (!isNotEmptyArray(currentSchema.required)) return; const isCompositionChild = (parent: AnySchema, child: AnySchema): boolean => { const matchesChild = (s: AnySchema) => resolveSchema(s, ctx).schema === child; From 094ff48af9020f830a182271cce32ffa065345fb Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Mon, 27 Jul 2026 21:58:10 +0800 Subject: [PATCH 08/17] fix: walk $ref sibling values that collide with resolved node keys --- packages/core/src/__tests__/bundle.test.ts | 24 +++++++++++ .../openapi-sibling-collision.yaml | 25 +++++++++++ packages/core/src/walk.ts | 43 +++++++++++-------- 3 files changed, 75 insertions(+), 17 deletions(-) create mode 100644 packages/core/src/__tests__/fixtures/sibling-refs/openapi-sibling-collision.yaml diff --git a/packages/core/src/__tests__/bundle.test.ts b/packages/core/src/__tests__/bundle.test.ts index d7aee01ab8..edcf9da91d 100644 --- a/packages/core/src/__tests__/bundle.test.ts +++ b/packages/core/src/__tests__/bundle.test.ts @@ -998,6 +998,30 @@ describe('sibling $ref resolution by spec', () => { ).toEqual({ $ref: '#/components/schemas/BadRequest' }); }); + it('should bundle refs inside $ref siblings that collide with resolved schema keys', async () => { + const { bundle: res, problems } = await bundle({ + config: await createConfig({}), + ref: path.join(__dirname, 'fixtures/sibling-refs/openapi-sibling-collision.yaml'), + }); + + expect(problems).toHaveLength(0); + const schema = (res.parsed as any).paths['/demo'].get.responses['200'].content[ + 'application/json' + ].schema; + expect(schema).toEqual({ + $ref: '#/components/schemas/Test', + properties: { + second: { $ref: '#/components/schemas/BaseProblem' }, + }, + }); + expect((res.parsed as any).components.schemas.BaseProblem).toEqual({ + type: 'object', + properties: { + title: { type: 'string' }, + }, + }); + }); + it('should keep referenced composed schemas when removing unused components', async () => { const { bundle: res, problems } = await bundle({ config: await createConfig({}), diff --git a/packages/core/src/__tests__/fixtures/sibling-refs/openapi-sibling-collision.yaml b/packages/core/src/__tests__/fixtures/sibling-refs/openapi-sibling-collision.yaml new file mode 100644 index 0000000000..7fdcad52d9 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/sibling-refs/openapi-sibling-collision.yaml @@ -0,0 +1,25 @@ +openapi: 3.1.0 +info: + title: Sibling properties colliding with the referenced schema keys + version: 1.0.0 +paths: + /demo: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + $ref: '#/components/schemas/Test' + properties: + second: + $ref: ./schemas/BaseProblem.yaml +components: + schemas: + Test: + type: object + title: Test schema + properties: + first: + type: integer diff --git a/packages/core/src/walk.ts b/packages/core/src/walk.ts index 8c0dd68f51..ed4a0da97d 100644 --- a/packages/core/src/walk.ts +++ b/packages/core/src/walk.ts @@ -366,16 +366,12 @@ export function walkDocument(opts: { props = refSiblingProps; } - for (const propName of props) { - let value = resolvedNode[propName]; - - let loc = resolvedLocation; - - if (value === undefined) { - value = node[propName]; - loc = location; // properties on the same level as $ref should resolve against original location, not target - } - + const walkProp = ( + propName: string, + value: unknown, + loc: Location, + valueParent: unknown + ) => { let propType = getOwn(type.properties, propName); if (propType === undefined) propType = type.additionalProperties; if (typeof propType === 'function') propType = propType(value, propName); @@ -397,16 +393,29 @@ export function walkDocument(opts: { propType = { name: 'scalar', properties: {} }; } - if (isRef(node[propName]) && propType?.name === 'scalar') { - walkNode(node[propName], propType, location.child([propName]), node, propName); - continue; - } - if (!isNamedType(propType) || (propType.name === 'scalar' && !isRef(value))) { - continue; + return; } - walkNode(value, propType, loc.child([propName]), resolvedNode, propName); + walkNode(value, propType, loc.child([propName]), valueParent, propName); + }; + + // the isRef check narrows `node` to OasRef, but sibling keys are read from it too + const rawNode = node as Record; + + for (const propName of props) { + const resolvedValue = resolvedNode[propName]; + // a property on the same level as $ref composes with the resolved node even + // when both define it, and resolves against the original location, not target + const siblingValue = + nodeIsRef && rawNode[propName] !== resolvedValue ? rawNode[propName] : undefined; + + if (shouldWalkChildren && resolvedValue !== undefined) { + walkProp(propName, resolvedValue, resolvedLocation, resolvedNode); + } + if (siblingValue !== undefined) { + walkProp(propName, siblingValue, location, node); + } } } } From 71b7bc5f8c9f1d5716f0a8c00ec6d01cf8725ff4 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 00:29:15 +0800 Subject: [PATCH 09/17] test: pin per-level chain independence for multi-hop composed refs --- packages/core/src/__tests__/resolve.test.ts | 34 +++++++++++++++++++++ packages/core/src/resolve.ts | 2 ++ 2 files changed, 36 insertions(+) diff --git a/packages/core/src/__tests__/resolve.test.ts b/packages/core/src/__tests__/resolve.test.ts index 3267845c85..115cc178a5 100644 --- a/packages/core/src/__tests__/resolve.test.ts +++ b/packages/core/src/__tests__/resolve.test.ts @@ -443,6 +443,40 @@ describe('collect refs', () => { expect(resolvedRefs.get('foobar.yaml::#/target')!.node).toEqual({ contact: {} }); }); + it('should record each level of a multi-hop composed chain independently', async () => { + const rootDocument = parseYamlToDocument( + outdent` + openapi: 3.0.0 + info: + $ref: "#/first" + first: + $ref: '#/second' + description: first hop + second: + $ref: '#/target' + description: second hop + target: + contact: {} + `, + 'foobar.yaml' + ); + + const resolvedRefs = await resolveDocument({ + rootDocument, + externalRefResolver: new BaseResolver(), + rootType: normalizeTypes(Oas3Types).Root, + }); + + const outerRef = resolvedRefs.get('foobar.yaml::#/first')!; + expect(outerRef.node).toEqual({ contact: {} }); + expect(outerRef.chain!.map((hop) => hop.nodePointer)).toEqual(['#/first', '#/second']); + + // the inner resolution must not be polluted by outer hops + const innerRef = resolvedRefs.get('foobar.yaml::#/second')!; + expect(innerRef.node).toEqual({ contact: {} }); + expect(innerRef.chain!.map((hop) => hop.nodePointer)).toEqual(['#/second']); + }); + it('should throw error if ref is folder', async () => { const cwd = path.join(__dirname, 'fixtures/resolve'); const rootDocument = parseYamlToDocument( diff --git a/packages/core/src/resolve.ts b/packages/core/src/resolve.ts index 41b5ea18b4..ee468f678b 100644 --- a/packages/core/src/resolve.ts +++ b/packages/core/src/resolve.ts @@ -506,6 +506,8 @@ export async function resolveDocument(opts: { : undefined; resolvedRef = await followRef(resolvedRef.document, target, pushRef(refStack, target)); if (chainHop && resolvedRef.resolved) { + // rebuilt rather than mutated: the returned chain array is shared with the + // map entry the inner recursion has just written resolvedRef = { ...resolvedRef, chain: [chainHop, ...(resolvedRef.chain ?? [])] }; } } From 7801c2b215300c66f3c63e7cc5713b676961c64c Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 00:41:27 +0800 Subject: [PATCH 10/17] fix: keep the resolve contract for rewritten bundled refs --- packages/core/src/bundle/bundle-visitor.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/core/src/bundle/bundle-visitor.ts b/packages/core/src/bundle/bundle-visitor.ts index abe132ed67..3fb0058518 100644 --- a/packages/core/src/bundle/bundle-visitor.ts +++ b/packages/core/src/bundle/bundle-visitor.ts @@ -20,7 +20,7 @@ import { isString } from '../utils/is-string.js'; import { makeRefId } from '../utils/make-ref-id.js'; import { toPascalCase } from '../utils/to-pascal-case.js'; import { type Oas3Visitor, type Oas2Visitor } from '../visitors.js'; -import { type UserContext, type ResolveResult, type Problem } from '../walk.js'; +import { type UserContext, type Problem } from '../walk.js'; import { type ComponentNamesStrategy } from './bundle-document.js'; type ComponentTarget = { node: unknown; location: Location }; @@ -186,8 +186,9 @@ export function makeBundleVisitor({ saveComponent(componentType, target, ctx); replaceRef(node, target, ctx); } else { + const originalRefId = makeRefId(ctx.location.source.absoluteRef, node.$ref); node.$ref = saveComponent(componentType, target, ctx); - resolveBundledComponent(node, target, ctx); + resolveBundledComponent(node, originalRefId, ctx); } } }, @@ -277,13 +278,14 @@ export function makeBundleVisitor({ }; } - function resolveBundledComponent(node: OasRef, resolved: ResolveResult, ctx: UserContext) { + function resolveBundledComponent(node: OasRef, originalRefId: string, ctx: UserContext) { + // Re-register the original resolution (chain-end node plus composed hops) under the + // rewritten pointer, so resolving it keeps the same contract as any other ref. const resolvedRef = { + ...resolvedRefMap.get(originalRefId)!, document: rootDocument, isRemote: false, - node: resolved.node, nodePointer: node.$ref, - resolved: true as const, }; resolvedRefMap.set(makeRefId(ctx.location.source.absoluteRef, node.$ref), resolvedRef); // The node may be walked again as part of the bundled document (e.g. a component saved From 5d8b082c52f1c5a2a22b4341fa73a37e5dc03d18 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 00:46:42 +0800 Subject: [PATCH 11/17] refactor: carry a Location on chain hops and simplify walker checks --- packages/core/src/__tests__/resolve.test.ts | 6 +-- packages/core/src/resolve.ts | 13 +++++-- packages/core/src/walk.ts | 29 +++++++------- tests/e2e/bundle/bundle.test.ts | 8 ++++ .../sibling-refs-root-in-file/snapshot_2.txt | 38 +++++++++++++++++++ 5 files changed, 73 insertions(+), 21 deletions(-) create mode 100644 tests/e2e/bundle/sibling-refs-root-in-file/snapshot_2.txt diff --git a/packages/core/src/__tests__/resolve.test.ts b/packages/core/src/__tests__/resolve.test.ts index 115cc178a5..22ea6c2f2e 100644 --- a/packages/core/src/__tests__/resolve.test.ts +++ b/packages/core/src/__tests__/resolve.test.ts @@ -439,7 +439,7 @@ describe('collect refs', () => { $ref: '#/target', description: 'alias with sibling', }); - expect(aliasedRef.chain![0].nodePointer).toEqual('#/aliased'); + expect(aliasedRef.chain![0].location.pointer).toEqual('#/aliased'); expect(resolvedRefs.get('foobar.yaml::#/target')!.node).toEqual({ contact: {} }); }); @@ -469,12 +469,12 @@ describe('collect refs', () => { const outerRef = resolvedRefs.get('foobar.yaml::#/first')!; expect(outerRef.node).toEqual({ contact: {} }); - expect(outerRef.chain!.map((hop) => hop.nodePointer)).toEqual(['#/first', '#/second']); + expect(outerRef.chain!.map((hop) => hop.location.pointer)).toEqual(['#/first', '#/second']); // the inner resolution must not be polluted by outer hops const innerRef = resolvedRefs.get('foobar.yaml::#/second')!; expect(innerRef.node).toEqual({ contact: {} }); - expect(innerRef.chain!.map((hop) => hop.nodePointer)).toEqual(['#/second']); + expect(innerRef.chain!.map((hop) => hop.location.pointer)).toEqual(['#/second']); }); it('should throw error if ref is folder', async () => { diff --git a/packages/core/src/resolve.ts b/packages/core/src/resolve.ts index ee468f678b..01cfaf5aa1 100644 --- a/packages/core/src/resolve.ts +++ b/packages/core/src/resolve.ts @@ -14,6 +14,7 @@ import { isAbsoluteUrl, isAnchor, isExternalValue, + Location, } from './ref-utils.js'; import { isNamedType, SpecExtension, type NormalizedNodeType } from './types/index.js'; import type { OasRef } from './typings/openapi.js'; @@ -184,10 +185,11 @@ export class BaseResolver { // A $ref target that is itself a $ref with sibling keys (a JSON Schema 2020-12 composition). // Resolution chases through it to the chain end, but the hop is recorded so consumers // that care about the composition (the bundler, chain-aware rules) can access it. +// The document is kept alongside the location so hop subtrees can be resolved too. export type ResolvedRefChainHop = { node: unknown; document: Document; - nodePointer: string; + location: Location; }; export type ResolvedRef = @@ -357,7 +359,12 @@ export async function resolveDocument(opts: { ); // chain hops carry sibling keys that can contain refs of their own for (const chainHop of resolvedRef.chain ?? []) { - resolveRefsInParallel(chainHop.node, chainHop.document, chainHop.nodePointer, type); + resolveRefsInParallel( + chainHop.node, + chainHop.document, + chainHop.location.pointer, + type + ); } } }); @@ -501,7 +508,7 @@ export async function resolveDocument(opts: { ? { node: target, document: resolvedRef.document, - nodePointer: resolvedRef.nodePointer!, + location: new Location(resolvedRef.document.source, resolvedRef.nodePointer!), } : undefined; resolvedRef = await followRef(resolvedRef.document, target, pushRef(refStack, target)); diff --git a/packages/core/src/walk.ts b/packages/core/src/walk.ts index ed4a0da97d..8e8dd6c8ad 100644 --- a/packages/core/src/walk.ts +++ b/packages/core/src/walk.ts @@ -2,7 +2,13 @@ import type { Config, RuleSeverity } from './config/index.js'; import { YamlParseError } from './errors/yaml-parse-error.js'; import type { SpecVersion } from './oas-types.js'; import { Location, isRef } from './ref-utils.js'; -import type { ResolveError, Source, ResolvedRefMap, Document } from './resolve.js'; +import type { + ResolveError, + Source, + ResolvedRefMap, + ResolvedRefChainHop, + Document, +} from './resolve.js'; import { isNamedType, SpecExtension, type NormalizedNodeType } from './types/index.js'; import type { Referenced } from './typings/openapi.js'; import { getOwn } from './utils/get-own.js'; @@ -30,7 +36,7 @@ export type NonUndefined = | Record; // A composed $ref (with sibling keys) the resolution chased through on the way to `node`. -export type ResolvedChainHop = { node: unknown; location: Location }; +export type ResolvedChainHop = ResolvedRefChainHop; export type ResolveResult = | { @@ -146,6 +152,8 @@ export function walkDocument(opts: { const { document, rootType, normalizedVisitors, resolvedRefMap, ctx } = opts; const seenNodesPerType: Record> = {}; const walkedComposedRefs = new Set(); + const composedRefWalkId = (type: NormalizedNodeType, location: Location) => + `${type.name}::${location.absolutePointer}`; const ignoredNodes = new Set(); // Pre-compute combined enter/leave arrays per type to avoid per-node array allocations @@ -186,15 +194,7 @@ export function walkDocument(opts: { ? new Location(error.source, '') : undefined; - return { - location: newLocation, - node, - error, - chain: chain?.map((chainHop) => ({ - node: chainHop.node, - location: new Location(chainHop.document.source, chainHop.nodePointer), - })), - }; + return { location: newLocation, node, error, chain }; }; const rawLocation = location; @@ -210,7 +210,7 @@ export function walkDocument(opts: { if (nodeIsRef && Object.keys(node).length > 1) { // composed $refs can also be reached as chain hops — record this walk to avoid a repeat - walkedComposedRefs.add(`${type.name}::${location.absolutePointer}`); + walkedComposedRefs.add(composedRefWalkId(type, location)); } if (nodeIsRef) { @@ -315,8 +315,7 @@ export function walkDocument(opts: { } const shouldWalkChildren = visitedBySome || !isNodeSeen; - const refSiblingProps = - nodeIsRef && isPlainObject(node) ? Object.keys(node).filter((k) => k !== '$ref') : []; + const refSiblingProps = nodeIsRef ? Object.keys(node).filter((k) => k !== '$ref') : []; if (shouldWalkChildren || refSiblingProps.length > 0) { if (shouldWalkChildren) { @@ -424,7 +423,7 @@ export function walkDocument(opts: { // walk the composed $ref hop from its own location so its siblings and inner $ref // are processed; the hop's own resolution carries the rest of the chain const chainHop = resolvedChain[0]; - if (!walkedComposedRefs.has(`${type.name}::${chainHop.location.absolutePointer}`)) { + if (!walkedComposedRefs.has(composedRefWalkId(type, chainHop.location))) { walkNode(chainHop.node, type, chainHop.location, parent, key); } } diff --git a/tests/e2e/bundle/bundle.test.ts b/tests/e2e/bundle/bundle.test.ts index 26312010d9..cb176366dc 100644 --- a/tests/e2e/bundle/bundle.test.ts +++ b/tests/e2e/bundle/bundle.test.ts @@ -190,6 +190,14 @@ describe('bundle with option: dereferenced', () => { const result = getCommandOutput(args, { testPath }); await expect(cleanupOutput(result)).toMatchFileSnapshot(join(testPath, 'snapshot_2.txt')); }); + + it('should compose sibling keywords of a root-level $ref', async () => { + const testPath = join(__dirname, `sibling-refs-root-in-file`); + const args = getParams(indexEntryPoint, ['bundle', 'openapi.yaml', '--dereferenced']); + + const result = getCommandOutput(args, { testPath }); + await expect(cleanupOutput(result)).toMatchFileSnapshot(join(testPath, 'snapshot_2.txt')); + }); }); describe('bundle with long description', () => { diff --git a/tests/e2e/bundle/sibling-refs-root-in-file/snapshot_2.txt b/tests/e2e/bundle/sibling-refs-root-in-file/snapshot_2.txt new file mode 100644 index 0000000000..e2e65e349b --- /dev/null +++ b/tests/e2e/bundle/sibling-refs-root-in-file/snapshot_2.txt @@ -0,0 +1,38 @@ +openapi: 3.1.0 +info: + title: Root-level $ref with siblings in the same file + version: 1.0.0 +paths: + /demo: + get: + operationId: getDemo + responses: + '400': + description: Bad request + content: + application/json: + schema: + title: Bad request + type: object + properties: &ref_0 + status: + type: integer + minimum: 400 + maximum: 400 + required: &ref_1 + - status +components: + schemas: + BadRequest: + title: Bad request + type: object + properties: *ref_0 + required: *ref_1 + BaseProblem: + type: object + properties: + title: + type: string + +bundling openapi.yaml using configuration for api 'main'... +📦 Created a bundle for openapi.yaml at stdout ms. From 7f9f301037c6af36a0f7b03010118586d83b9054 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 00:54:39 +0800 Subject: [PATCH 12/17] fix: check composed chain hops even when the chain end was already visited --- ...quired-schema-properties-undefined.test.ts | 39 +++++++++++++++++++ ...no-required-schema-properties-undefined.ts | 18 ++++++--- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts index 3e9076ad0e..a560e94732 100644 --- a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts +++ b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts @@ -1155,6 +1155,45 @@ describe('no-required-schema-properties-undefined', () => { expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); + it('should find a property on a composed hop even when the chain end was already visited', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.1.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + Cat: + allOf: + - $ref: '#/components/schemas/Leaf' + - $ref: '#/components/schemas/Composed' + required: + - special + Composed: + $ref: '#/components/schemas/Leaf' + properties: + special: + type: string + Leaf: + type: object + properties: + name: + type: string + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + it('should still report a genuinely undefined property in a composed $ref chain', async () => { const document = parseYamlToDocument( outdent` diff --git a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts index c460ada4d8..dbe5703e74 100644 --- a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts +++ b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts @@ -71,17 +71,23 @@ export const NoRequiredSchemaPropertiesUndefined: } const { schema, location, chain } = resolveSchema(schemaOrRef, ctx, resolveFrom); - if (!schema || visited.has(schema)) return false; - visited.add(schema); + if (!schema) return false; - if (definesProperty(schema, propertyName, visited, ctx, location)) { - return true; + if (!visited.has(schema)) { + visited.add(schema); + if (definesProperty(schema, propertyName, visited, ctx, location)) { + return true; + } } - // composed $refs the resolution chased through contribute their sibling keywords too + // composed $refs the resolution chased through contribute their sibling keywords too, + // even when the chain end was already visited through another branch for (const chainHop of chain ?? []) { + if (!isPlainObject(chainHop.node) || visited.has(chainHop.node)) { + continue; + } + visited.add(chainHop.node as AnySchema); if ( - isPlainObject(chainHop.node) && definesProperty( chainHop.node as AnySchema, propertyName, From 1093b7485e5a3c2c9748eeafe9ac53302dc53674 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 15:07:46 +0800 Subject: [PATCH 13/17] refactor: build the bundled ref registration from the resolve result --- packages/core/src/bundle/bundle-visitor.ts | 24 +++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/core/src/bundle/bundle-visitor.ts b/packages/core/src/bundle/bundle-visitor.ts index 3fb0058518..067ba9a0da 100644 --- a/packages/core/src/bundle/bundle-visitor.ts +++ b/packages/core/src/bundle/bundle-visitor.ts @@ -20,7 +20,7 @@ import { isString } from '../utils/is-string.js'; import { makeRefId } from '../utils/make-ref-id.js'; import { toPascalCase } from '../utils/to-pascal-case.js'; import { type Oas3Visitor, type Oas2Visitor } from '../visitors.js'; -import { type UserContext, type Problem } from '../walk.js'; +import { type UserContext, type ResolveResult, type NonUndefined, type Problem } from '../walk.js'; import { type ComponentNamesStrategy } from './bundle-document.js'; type ComponentTarget = { node: unknown; location: Location }; @@ -186,9 +186,8 @@ export function makeBundleVisitor({ saveComponent(componentType, target, ctx); replaceRef(node, target, ctx); } else { - const originalRefId = makeRefId(ctx.location.source.absoluteRef, node.$ref); node.$ref = saveComponent(componentType, target, ctx); - resolveBundledComponent(node, originalRefId, ctx); + resolveBundledComponent(node, resolved, ctx); } } }, @@ -278,18 +277,23 @@ export function makeBundleVisitor({ }; } - function resolveBundledComponent(node: OasRef, originalRefId: string, ctx: UserContext) { - // Re-register the original resolution (chain-end node plus composed hops) under the - // rewritten pointer, so resolving it keeps the same contract as any other ref. + // Makes the rewritten ref resolvable: registers the rewritten pointer in the map + // with the same resolution (chain-end node plus composed hops) the original ref had. + function resolveBundledComponent( + node: OasRef, + resolved: ResolveResult, + ctx: UserContext + ) { const resolvedRef = { - ...resolvedRefMap.get(originalRefId)!, - document: rootDocument, + resolved: true as const, isRemote: false, + node: resolved.node, + chain: resolved.chain, + document: rootDocument, nodePointer: node.$ref, }; resolvedRefMap.set(makeRefId(ctx.location.source.absoluteRef, node.$ref), resolvedRef); - // The node may be walked again as part of the bundled document (e.g. a component saved - // from an external file), so register the rewritten ref from the root source as well. + // saved components are walked again as part of the bundled document resolvedRefMap.set(makeRefId(rootDocument.source.absoluteRef, node.$ref), resolvedRef); } From eded8102bd62c7bfe7c0d42fdffa35f6a353c1b0 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 15:09:25 +0800 Subject: [PATCH 14/17] Apply suggestion from @RomanHotsiy --- .changeset/root-ref-sibling-keywords.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/root-ref-sibling-keywords.md b/.changeset/root-ref-sibling-keywords.md index 3e7abbc585..2002b61ff3 100644 --- a/.changeset/root-ref-sibling-keywords.md +++ b/.changeset/root-ref-sibling-keywords.md @@ -4,4 +4,3 @@ --- Fixed the `bundle` command losing schema keywords (such as `title`, `properties`, or `required`) written next to a `$ref` when the referenced schemas started with their own `$ref`. -Ref resolution now exposes such composed `$ref`s as a `chain` field on the resolve result, so custom rules and decorators can inspect them. From 64e1a0f6d9625a1c2e5b080775fb6fcc8b843f1e Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 15:48:54 +0800 Subject: [PATCH 15/17] refactor: walk each composed hop in its own frame and split sibling walking from resolved children --- packages/core/src/ref-utils.ts | 10 ++- packages/core/src/walk.ts | 136 +++++++++++++++------------------ 2 files changed, 69 insertions(+), 77 deletions(-) diff --git a/packages/core/src/ref-utils.ts b/packages/core/src/ref-utils.ts index 14cebd115f..1023cde3f1 100644 --- a/packages/core/src/ref-utils.ts +++ b/packages/core/src/ref-utils.ts @@ -130,7 +130,15 @@ export function isAnchor(ref: string) { export function replaceRef(ref: OasRef, resolved: ResolveResult, ctx: UserContext) { if (!isPlainObject(resolved.node)) { - ctx.parent[ctx.key] = resolved.node; + if (ctx.parent) { + ctx.parent[ctx.key] = resolved.node; + } else { + // a composed chain hop has no parent to replace, so dereference it in place + // by dropping the $ref and keeping the sibling keys + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete ref.$ref; + } } else { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore diff --git a/packages/core/src/walk.ts b/packages/core/src/walk.ts index 8e8dd6c8ad..7478a1546c 100644 --- a/packages/core/src/walk.ts +++ b/packages/core/src/walk.ts @@ -243,6 +243,38 @@ export function walkDocument(opts: { } if (resolvedNode !== undefined && resolvedLocation && type.name !== 'scalar') { + // the isRef check narrows `node` to OasRef, but sibling keys are read from it too + const rawNode = node as Record; + + const walkProp = (propName: string, value: unknown, loc: Location, valueParent: unknown) => { + let propType = getOwn(type.properties, propName); + if (propType === undefined) propType = type.additionalProperties; + if (typeof propType === 'function') propType = propType(value, propName); + + if ( + propType === undefined && + type.extensionsPrefix && + propName.startsWith(type.extensionsPrefix) + ) { + propType = SpecExtension; + } + + if (!isNamedType(propType) && propType?.directResolveAs) { + propType = propType.directResolveAs; + value = { $ref: value }; + } + + if (propType && propType.name === undefined && propType.resolvable !== false) { + propType = { name: 'scalar', properties: {} }; + } + + if (!isNamedType(propType) || (propType.name === 'scalar' && !isRef(value))) { + return; + } + + walkNode(value, propType, loc.child([propName]), valueParent, propName); + }; + currentLocation = resolvedLocation; const isNodeSeen = seenNodesPerType[type.name]?.has?.(resolvedNode); let visitedBySome = false; @@ -314,17 +346,12 @@ export function walkDocument(opts: { } } - const shouldWalkChildren = visitedBySome || !isNodeSeen; - const refSiblingProps = nodeIsRef ? Object.keys(node).filter((k) => k !== '$ref') : []; - - if (shouldWalkChildren || refSiblingProps.length > 0) { - if (shouldWalkChildren) { - seenNodesPerType[type.name] = seenNodesPerType[type.name] || new Set(); - seenNodesPerType[type.name].add(resolvedNode); - } + if (visitedBySome || !isNodeSeen) { + seenNodesPerType[type.name] = seenNodesPerType[type.name] || new Set(); + seenNodesPerType[type.name].add(resolvedNode); if (Array.isArray(resolvedNode)) { - const itemsType = shouldWalkChildren ? type.items : undefined; + const itemsType = type.items; if (itemsType !== undefined) { const isTypeAFunction = typeof itemsType === 'function'; for (let i = 0; i < resolvedNode.length; i++) { @@ -345,86 +372,43 @@ export function walkDocument(opts: { } } } else if (isPlainObject(resolvedNode)) { - let props: string[]; - if (shouldWalkChildren) { - // visit in order from type-tree first - props = Object.keys(type.properties); - if (type.additionalProperties) { - props.push(...Object.keys(resolvedNode).filter((k) => !props.includes(k))); - } else if (type.extensionsPrefix) { - props.push( - ...Object.keys(resolvedNode).filter((k) => - k.startsWith(type.extensionsPrefix as string) - ) - ); - } - // properties on the same level as $ref - props.push(...refSiblingProps.filter((k) => !props.includes(k))); - } else { - // the resolved node was already visited, but this ref's sibling keys still need walking - props = refSiblingProps; + // visit in order from type-tree first + const props = Object.keys(type.properties); + if (type.additionalProperties) { + props.push(...Object.keys(resolvedNode).filter((k) => !props.includes(k))); + } else if (type.extensionsPrefix) { + props.push( + ...Object.keys(resolvedNode).filter((k) => + k.startsWith(type.extensionsPrefix as string) + ) + ); } - const walkProp = ( - propName: string, - value: unknown, - loc: Location, - valueParent: unknown - ) => { - let propType = getOwn(type.properties, propName); - if (propType === undefined) propType = type.additionalProperties; - if (typeof propType === 'function') propType = propType(value, propName); - - if ( - propType === undefined && - type.extensionsPrefix && - propName.startsWith(type.extensionsPrefix) - ) { - propType = SpecExtension; - } - - if (!isNamedType(propType) && propType?.directResolveAs) { - propType = propType.directResolveAs; - value = { $ref: value }; - } - - if (propType && propType.name === undefined && propType.resolvable !== false) { - propType = { name: 'scalar', properties: {} }; - } - - if (!isNamedType(propType) || (propType.name === 'scalar' && !isRef(value))) { - return; - } - - walkNode(value, propType, loc.child([propName]), valueParent, propName); - }; - - // the isRef check narrows `node` to OasRef, but sibling keys are read from it too - const rawNode = node as Record; - for (const propName of props) { const resolvedValue = resolvedNode[propName]; - // a property on the same level as $ref composes with the resolved node even - // when both define it, and resolves against the original location, not target - const siblingValue = - nodeIsRef && rawNode[propName] !== resolvedValue ? rawNode[propName] : undefined; - - if (shouldWalkChildren && resolvedValue !== undefined) { + if (resolvedValue !== undefined) { walkProp(propName, resolvedValue, resolvedLocation, resolvedNode); } - if (siblingValue !== undefined) { - walkProp(propName, siblingValue, location, node); - } } } } if (nodeIsRef && resolvedChain?.length) { - // walk the composed $ref hop from its own location so its siblings and inner $ref - // are processed; the hop's own resolution carries the rest of the chain + // the target composes another $ref: the hop is walked as a ref node from its own + // location (it has no parent there); its resolution carries the rest of the chain const chainHop = resolvedChain[0]; if (!walkedComposedRefs.has(composedRefWalkId(type, chainHop.location))) { - walkNode(chainHop.node, type, chainHop.location, parent, key); + walkNode(chainHop.node, type, chainHop.location, undefined, key); + } + } + + if (nodeIsRef) { + // $ref sibling keys belong to the ref node itself and walk from its own location; + // the resolved node's (or hops') keys are walked in their own frames + for (const propName of Object.keys(node)) { + if (propName !== '$ref' && rawNode[propName] !== resolvedNode[propName]) { + walkProp(propName, rawNode[propName], location, node); + } } } From 6fbd859fa5632bca4c02bd4464ba54eb11481df6 Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 16:47:28 +0800 Subject: [PATCH 16/17] test: cover discriminator mappings pointing to composed schemas --- packages/core/src/__tests__/bundle.test.ts | 19 +++++++++++++++ .../openapi-discriminator-mapping.yaml | 23 +++++++++++++++++++ .../fixtures/sibling-refs/schemas/Cat.yaml | 5 ++++ .../fixtures/sibling-refs/schemas/Toy.yaml | 1 + 4 files changed, 48 insertions(+) create mode 100644 packages/core/src/__tests__/fixtures/sibling-refs/openapi-discriminator-mapping.yaml create mode 100644 packages/core/src/__tests__/fixtures/sibling-refs/schemas/Cat.yaml create mode 100644 packages/core/src/__tests__/fixtures/sibling-refs/schemas/Toy.yaml diff --git a/packages/core/src/__tests__/bundle.test.ts b/packages/core/src/__tests__/bundle.test.ts index edcf9da91d..21f813151c 100644 --- a/packages/core/src/__tests__/bundle.test.ts +++ b/packages/core/src/__tests__/bundle.test.ts @@ -1022,6 +1022,25 @@ describe('sibling $ref resolution by spec', () => { }); }); + it('should bundle discriminator mappings that point to composed schemas', async () => { + const { bundle: res, problems } = await bundle({ + config: await createConfig({}), + ref: path.join(__dirname, 'fixtures/sibling-refs/openapi-discriminator-mapping.yaml'), + }); + + expect(problems).toHaveLength(0); + const schemas = (res.parsed as any).components.schemas; + expect(schemas.Pet.discriminator.mapping.cat).toEqual('#/components/schemas/Cat'); + expect(schemas.Cat).toEqual({ + title: 'Cat', + $ref: '#/components/schemas/BaseProblem', + properties: { + toy: { $ref: '#/components/schemas/Toy' }, + }, + }); + expect(schemas.Toy).toEqual({ type: 'string' }); + }); + it('should keep referenced composed schemas when removing unused components', async () => { const { bundle: res, problems } = await bundle({ config: await createConfig({}), diff --git a/packages/core/src/__tests__/fixtures/sibling-refs/openapi-discriminator-mapping.yaml b/packages/core/src/__tests__/fixtures/sibling-refs/openapi-discriminator-mapping.yaml new file mode 100644 index 0000000000..94f3219f18 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/sibling-refs/openapi-discriminator-mapping.yaml @@ -0,0 +1,23 @@ +openapi: 3.1.0 +info: + title: Discriminator mapping to a composed schema + version: 1.0.0 +paths: + /pets: + get: + responses: + '200': + description: OK + content: + application/json: + schema: + oneOf: + - $ref: '#/components/schemas/Pet' +components: + schemas: + Pet: + type: object + discriminator: + propertyName: type + mapping: + cat: ./schemas/Cat.yaml diff --git a/packages/core/src/__tests__/fixtures/sibling-refs/schemas/Cat.yaml b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/Cat.yaml new file mode 100644 index 0000000000..abbda1c0b7 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/Cat.yaml @@ -0,0 +1,5 @@ +title: Cat +$ref: ./BaseProblem.yaml +properties: + toy: + $ref: ./Toy.yaml diff --git a/packages/core/src/__tests__/fixtures/sibling-refs/schemas/Toy.yaml b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/Toy.yaml new file mode 100644 index 0000000000..5c21d88b9e --- /dev/null +++ b/packages/core/src/__tests__/fixtures/sibling-refs/schemas/Toy.yaml @@ -0,0 +1 @@ +type: string From b704b0ba99a7dd11f49115ccb6e853d0e4d1425f Mon Sep 17 00:00:00 2001 From: Roman Hotsiy Date: Tue, 28 Jul 2026 16:54:03 +0800 Subject: [PATCH 17/17] fix: do not crash on $ref siblings when the ref resolves to null --- packages/core/src/__tests__/bundle.test.ts | 32 ++++++++++++++++++++++ packages/core/src/walk.ts | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/core/src/__tests__/bundle.test.ts b/packages/core/src/__tests__/bundle.test.ts index 21f813151c..279028221b 100644 --- a/packages/core/src/__tests__/bundle.test.ts +++ b/packages/core/src/__tests__/bundle.test.ts @@ -1022,6 +1022,38 @@ describe('sibling $ref resolution by spec', () => { }); }); + it('should not crash when a $ref with siblings resolves to null', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.0.0 + info: + title: Test + version: 1.0.0 + paths: {} + components: + schemas: + Nullish: + $ref: '#/components/schemas/Target' + description: composed over null + Target: null + `, + '' + ); + + const { bundle: res, problems } = await bundleDocument({ + document, + externalRefResolver: new BaseResolver(), + config: await createConfig({}), + types: Oas3Types, + }); + + expect(problems).toHaveLength(0); + expect((res.parsed as any).components.schemas.Nullish).toEqual({ + $ref: '#/components/schemas/Target', + description: 'composed over null', + }); + }); + it('should bundle discriminator mappings that point to composed schemas', async () => { const { bundle: res, problems } = await bundle({ config: await createConfig({}), diff --git a/packages/core/src/walk.ts b/packages/core/src/walk.ts index 7478a1546c..24c15446f8 100644 --- a/packages/core/src/walk.ts +++ b/packages/core/src/walk.ts @@ -406,7 +406,7 @@ export function walkDocument(opts: { // $ref sibling keys belong to the ref node itself and walk from its own location; // the resolved node's (or hops') keys are walked in their own frames for (const propName of Object.keys(node)) { - if (propName !== '$ref' && rawNode[propName] !== resolvedNode[propName]) { + if (propName !== '$ref' && rawNode[propName] !== resolvedNode?.[propName]) { walkProp(propName, rawNode[propName], location, node); } }