From 15d6f38a137426432249c3a3fb1f679837e8b346 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:15:49 +0000 Subject: [PATCH 1/3] refactor(downgrader): reuse aliasEnd, skip the second pass for path item fields, fix oracle drift - convertContentEntry follows a media type alias chain with ctx.aliasEnd instead of a hand-rolled walk; skipAliases is no longer exported. - A reference into a field every Path Item conversion drops (query, additionalOperations) is now known to dangle from the input, so it no longer costs a second pass over the whole document. Path Items that are copied rather than converted (extensions of paths, callbacks that are Reference Objects) are excluded, and links and mappings still go by the removed prefixes alone, so outputs are unchanged. - The test oracle keeps its own pointer resolver on purpose, now without resolving array properties such as `length` or plain-name fragments. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GZa6QbyudtUSAeHt5W2B8A --- packages/downgrader/src/shared.ts | 54 ++++++++++++++++--- packages/downgrader/src/v3.1-to-v3.0.ts | 4 +- packages/downgrader/src/v3.2-to-v3.1.ts | 11 ++-- .../v3.2-to-v3.1/spec/removed-parts.test.ts | 42 ++++++++++++++- packages/downgrader/tests/validate.test.ts | 18 +++++++ packages/downgrader/tests/validate.ts | 18 ++++++- 6 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 packages/downgrader/tests/validate.test.ts diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index 091a5d4..e0e71a3 100644 --- a/packages/downgrader/src/shared.ts +++ b/packages/downgrader/src/shared.ts @@ -731,7 +731,7 @@ function isRemovedAlias(ref: string, ctx: Context, convert: Convert): boolean { return end !== undefined && ctx.dangles(end) && convertsToDrop(end, ctx, convert) } -export function skipAliases(ref: string, ctx: Context, follow: (next: string, target: Record) => boolean): string { +function skipAliases(ref: string, ctx: Context, follow: (next: string, target: Record) => boolean): string { const hops = new Set([ref]) let hop = ref for (;;) { @@ -824,6 +824,28 @@ function isPathItemPointer(tokens: readonly string[]): boolean { && (tokens.length === 4 ? first === 'components' : isOperationPointer(tokens.slice(0, -3))) } +// Whether `tokens` points at or into one of `fields`, which converting a Path +// Item drops, so the target is known to be gone before a pass ends. A Path +// Item that is copied rather than converted keeps every field, so the walk +// rejects those: extensions of `paths`, and anything under a Callback Object +// that is a Reference Object or under a value that is not an object. +function isInDroppedPathItemField(root: unknown, tokens: readonly string[] | undefined, fields: readonly string[]): boolean { + if (tokens === undefined || (tokens[0] === 'paths' && !isPath(tokens[1] ?? ''))) { + return false + } + let node = root + for (const [index, token] of tokens.entries()) { + if (!isRecord(node) || (tokens[index - 2] === 'callbacks' && getRef(node) !== undefined)) { + return false + } + if (fields.includes(token) && isPathItemPointer(tokens.slice(0, index))) { + return true + } + node = child(node, token) + } + return false +} + function mergeMissing(out: Record, target: unknown): void { if (isRecord(target)) { for (const [key, item] of Object.entries(target)) { @@ -947,10 +969,20 @@ export function mergeRef(convert: Convert): Finish { } } -export function removedPrefixes(tables: Readonly>): string[] { - return Object.entries(tables).flatMap(([base, fields]) => - [...fields].filter(([, field]) => field === DROP).map(([key]) => `#${base}/${key}/`), - ) +export interface Removed { + readonly pathItemFields: readonly string[] + readonly prefixes: readonly string[] +} + +function droppedKeys(fields: Fields): string[] { + return [...fields].filter(([, field]) => field === DROP).map(([key]) => key) +} + +export function removedParts(tables: Readonly>, pathItem: Fields): Removed { + return { + pathItemFields: droppedKeys(pathItem), + prefixes: Object.entries(tables).flatMap(([base, fields]) => droppedKeys(fields).map(key => `#${base}/${key}/`)), + } } function danglesIn(output: unknown, source: unknown, tokens: readonly string[] | undefined): boolean { @@ -972,7 +1004,7 @@ function danglesIn(output: unknown, source: unknown, tokens: readonly string[] | return to === undefined || (isRecord(to) && PLACEHOLDERS.has(to)) } -export function downgrade(root: unknown, convert: Convert, removed: readonly string[] = []): unknown { +export function downgrade(root: unknown, convert: Convert, removed: Removed = { pathItemFields: [], prefixes: [] }): unknown { const locations = new Map() const locateRef = (ref: string): Location => { let location = locations.get(ref) @@ -1015,7 +1047,13 @@ export function downgrade(root: unknown, convert: Convert, removed: readonly str return (isRecord(target) || typeof target === 'boolean') && aliasEnd(ref) !== undefined } const dangling = new Set() - const isRemovedPart = (ref: string): boolean => removed.some(prefix => ref.startsWith(prefix)) + const isRemovedPart = (ref: string): boolean => removed.prefixes.some(prefix => ref.startsWith(prefix)) + // Without this, a reference into a dropped Path Item field would only be + // found dangling once a pass ends, costing a second pass over everything. + // Links and mappings still go by `isRemovedPart` alone, so one into such a + // field is removed only when its target dangles, as before. + const isKnownGone = (ref: string): boolean => + isRemovedPart(ref) || isInDroppedPathItemField(root, parsePointer(ref), removed.pathItemFields) let previous = root for (;;) { const kept = new Set() @@ -1026,7 +1064,7 @@ export function downgrade(root: unknown, convert: Convert, removed: readonly str copies: new Map(), dangles: (ref) => { if (!dangling.has(ref) && !kept.has(ref)) { - if ((isRemovedPart(ref) || (previous !== root && danglesIn(previous, root, parsePointer(ref)))) && isInlinable(ref)) { + if ((isKnownGone(ref) || (previous !== root && danglesIn(previous, root, parsePointer(ref)))) && isInlinable(ref)) { dangling.add(ref) } else { diff --git a/packages/downgrader/src/v3.1-to-v3.0.ts b/packages/downgrader/src/v3.1-to-v3.0.ts index 5e685f4..8e5aa7a 100644 --- a/packages/downgrader/src/v3.1-to-v3.0.ts +++ b/packages/downgrader/src/v3.1-to-v3.0.ts @@ -30,7 +30,7 @@ import { placeholder, rebasedRef, refOr, - removedPrefixes, + removedParts, resourceOf, setOwn, } from './shared' @@ -192,7 +192,7 @@ const DOCUMENT_FIELDS = defineFields({ webhooks: DROP, }) -const REMOVED = removedPrefixes({ '': DOCUMENT_FIELDS, '/components': COMPONENTS_FIELDS }) +const REMOVED = removedParts({ '': DOCUMENT_FIELDS, '/components': COMPONENTS_FIELDS }, PATH_ITEM_FIELDS) function reference(value: Record): unknown { return { $ref: value.$ref } diff --git a/packages/downgrader/src/v3.2-to-v3.1.ts b/packages/downgrader/src/v3.2-to-v3.1.ts index bf13c56..53a92fa 100644 --- a/packages/downgrader/src/v3.2-to-v3.1.ts +++ b/packages/downgrader/src/v3.2-to-v3.1.ts @@ -27,8 +27,7 @@ import { map, mergeRef, refOr, - removedPrefixes, - skipAliases, + removedParts, } from './shared' const V32_DIALECT_PREFIX = 'https://spec.openapis.org/oas/3.2/dialect/' @@ -181,7 +180,7 @@ const DOCUMENT_FIELDS = defineFields({ webhooks: map(convertPathItem), }) -const REMOVED = removedPrefixes({ '': DOCUMENT_FIELDS, '/components': COMPONENTS_FIELDS }) +const REMOVED = removedParts({ '': DOCUMENT_FIELDS, '/components': COMPONENTS_FIELDS }, PATH_ITEM_FIELDS) function convertServer(value: unknown, ctx: Context): unknown { return convertObject(value, ctx, SERVER_FIELDS) @@ -269,7 +268,11 @@ function convertMediaType(value: unknown, ctx: Context): unknown { function convertContentEntry(value: unknown, ctx: Context): unknown { const ref = getRef(value) - return ref === undefined ? convertMediaType(value, ctx) : inline(skipAliases(ref, ctx, () => true), ctx, convertContentEntry) + if (ref === undefined) { + return convertMediaType(value, ctx) + } + const end = ctx.aliasEnd(ref) + return end === undefined ? DROP : inline(end, ctx, convertMediaType) } function convertRequestBody(value: unknown, ctx: Context): unknown { diff --git a/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts b/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts index e70eb1b..f6cdc7e 100644 --- a/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts +++ b/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts @@ -6,7 +6,7 @@ // - parameter lists that lost `querystring` entries, since the indices of // the entries after a removed one shift -import { cyclicCallbackGraph, dig } from '../../helpers' +import { countReads, cyclicCallbackGraph, dig } from '../../helpers' import { expectValidAs } from '../../validate' import { convertComponent, convertPathItem, convertSpec } from './helpers' @@ -214,6 +214,25 @@ describe('reference Objects', () => { }) }) + // Every Path Item conversion drops `query` and `additionalOperations`, so + // references into them are known to dangle from the input alone. Finding + // that out only when a pass ends would convert the whole document again. + it('handles references into query and additionalOperations in a single pass', () => { + const reads = { count: 0 } + const result = convertSpec({ + components: { + links: { L: { operationRef: '#/paths/~1a/query' } }, + responses: { R: { $ref: '#/paths/~1a/additionalOperations/COPY/responses/200' } }, + }, + paths: { + '/a': { additionalOperations: { COPY: { responses: { 200: { summary: 'Copied' } } } }, query: {} }, + '/b': countReads({ get: { responses: {} } }, 'get', reads), + }, + }) + expect(reads.count).toBe(1) + expect(result.components).toEqual({ links: {}, responses: { R: { description: 'Copied' } } }) + }) + it('follows chains through removed parts and keeps the reference where a chain reaches a surviving part', () => { const result = convertSpec({ components: { @@ -402,6 +421,27 @@ describe('references left as written', () => { }) }) + // Only converted Path Items lose `query`. An extension of `paths`, and a + // Callback Object that is a Reference Object, are copied with all their + // fields, so references into them still resolve. + it('leaves references into a query field that the conversion copies as written', () => { + const responses = { 200: { description: 'kept' } } + const references = { + Callback: { $ref: '#/components/callbacks/C/{$url}/query/responses/200' }, + Extension: { $ref: '#/paths/x-shared/query/responses/200' }, + } + const result = convertSpec({ + components: { + callbacks: { C: { '$ref': '#/components/callbacks/D', '{$url}': { query: { responses } } }, D: {} }, + responses: references, + }, + paths: { 'x-shared': { query: { responses } } }, + }) + expect(dig(result, 'components', 'responses')).toEqual(references) + expect(dig(result, 'components', 'callbacks', 'C', '{$url}', 'query')).toEqual({ responses }) + expect(dig(result, 'paths', 'x-shared', 'query')).toEqual({ responses }) + }) + it('leaves references whose alias chain loops as written', () => { const parameters = { A: { $ref: '#/components/parameters/B' }, diff --git a/packages/downgrader/tests/validate.test.ts b/packages/downgrader/tests/validate.test.ts new file mode 100644 index 0000000..ffe65a4 --- /dev/null +++ b/packages/downgrader/tests/validate.test.ts @@ -0,0 +1,18 @@ +import { expectNoNewDanglingRefs } from './validate' + +// The oracle checks only references that resolved in the input, so a +// reference that wrongly resolved there would be held to the output too. +describe('expectNoNewDanglingRefs', () => { + it('fails when a reference that resolved in the input no longer does', () => { + expect(() => expectNoNewDanglingRefs({ a: [1] }, { r: { $ref: '#/a/0' } })).toThrow() + }) + + // https://www.rfc-editor.org/rfc/rfc6901#section-4 + it.each([ + ['an array property that is not an index', '#/a/length'], + ['an index with a leading zero', '#/a/00'], + ['a plain-name fragment, which names an anchor', '#pet'], + ])('does not resolve %s', (_name, ref) => { + expect(() => expectNoNewDanglingRefs({ a: [1], et: 1 }, { r: { $ref: ref } })).not.toThrow() + }) +}) diff --git a/packages/downgrader/tests/validate.ts b/packages/downgrader/tests/validate.ts index 15c32cb..4add380 100644 --- a/packages/downgrader/tests/validate.ts +++ b/packages/downgrader/tests/validate.ts @@ -10,7 +10,13 @@ const validator = new Validator() /** * Resolves a local `$ref` such as `#/components/schemas/Pet` against `root`. * The fragment is percent-decoded first (RFC 3986) and then split into - * JSON Pointer tokens with `~1` and `~0` unescaped (RFC 6901). + * JSON Pointer tokens with `~1` and `~0` unescaped (RFC 6901). A fragment + * that does not start with `/`, such as the anchor `#pet`, is not a pointer. + * Only an index without leading zeros selects an array element. + * + * This duplicates `parsePointer` and `child` in `src/shared.ts` on purpose. + * The conversion uses them to decide which references dangle, so an oracle + * built on them would agree with any bug in them. */ function resolvePointer(root: unknown, ref: string): unknown { if (!ref.startsWith('#')) { @@ -26,10 +32,18 @@ function resolvePointer(root: unknown, ref: string): unknown { if (pointer === '') { return root } + if (!pointer.startsWith('/')) { + return undefined + } let current = root for (const token of pointer.slice(1).split('/')) { const key = token.replaceAll('~1', '/').replaceAll('~0', '~') - if (typeof current !== 'object' || current === null || !Object.hasOwn(current, key)) { + if ( + typeof current !== 'object' + || current === null + || (Array.isArray(current) && !/^(?:0|[1-9]\d*)$/.test(key)) + || !Object.hasOwn(current, key) + ) { return undefined } current = (current as Record)[key] From c5198109049c0bf21d6e7fd9257fb4f1eb0ee3ed Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 12:58:43 +0000 Subject: [PATCH 2/3] perf(downgrader): only parse references that name a dropped Path Item field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Classifying every distinct reference parsed and walked its pointer, even for 3.1 → 3.0 where no Path Item field is dropped, which cost about 40% on a document with thousands of references. A reference whose text names none of the dropped fields now skips the parse. Also un-export the Removed type and trim repeated comments. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GZa6QbyudtUSAeHt5W2B8A --- packages/downgrader/src/shared.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index e0e71a3..3ba1d89 100644 --- a/packages/downgrader/src/shared.ts +++ b/packages/downgrader/src/shared.ts @@ -825,10 +825,10 @@ function isPathItemPointer(tokens: readonly string[]): boolean { } // Whether `tokens` points at or into one of `fields`, which converting a Path -// Item drops, so the target is known to be gone before a pass ends. A Path -// Item that is copied rather than converted keeps every field, so the walk -// rejects those: extensions of `paths`, and anything under a Callback Object -// that is a Reference Object or under a value that is not an object. +// Item drops. A Path Item that is copied rather than converted keeps every +// field, so the walk rejects those: extensions of `paths`, and anything under +// a Callback Object that is a Reference Object or under a value that is not +// an object. function isInDroppedPathItemField(root: unknown, tokens: readonly string[] | undefined, fields: readonly string[]): boolean { if (tokens === undefined || (tokens[0] === 'paths' && !isPath(tokens[1] ?? ''))) { return false @@ -969,7 +969,7 @@ export function mergeRef(convert: Convert): Finish { } } -export interface Removed { +interface Removed { readonly pathItemFields: readonly string[] readonly prefixes: readonly string[] } @@ -1050,10 +1050,12 @@ export function downgrade(root: unknown, convert: Convert, removed: Removed = { const isRemovedPart = (ref: string): boolean => removed.prefixes.some(prefix => ref.startsWith(prefix)) // Without this, a reference into a dropped Path Item field would only be // found dangling once a pass ends, costing a second pass over everything. + // Most references spell out none of those fields, so they are not parsed. // Links and mappings still go by `isRemovedPart` alone, so one into such a - // field is removed only when its target dangles, as before. + // field is removed only when its target dangles. const isKnownGone = (ref: string): boolean => - isRemovedPart(ref) || isInDroppedPathItemField(root, parsePointer(ref), removed.pathItemFields) + isRemovedPart(ref) + || (removed.pathItemFields.some(field => ref.includes(field)) && isInDroppedPathItemField(root, parsePointer(ref), removed.pathItemFields)) let previous = root for (;;) { const kept = new Set() From b338f9885d69f6703edc6d7b8a0d18d389772d80 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:23:49 +0000 Subject: [PATCH 3/3] test(downgrader): cover every branch of the dropped Path Item field walk The `paths` extension check moves into the walk, where the token is always defined, so the unreachable `?? ''` fallback goes. The look-alike test also covers a component and a file merely named `query`, which reach the walk's two remaining `return false` paths. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GZa6QbyudtUSAeHt5W2B8A --- packages/downgrader/src/shared.ts | 8 ++++++-- .../tests/v3.2-to-v3.1/spec/removed-parts.test.ts | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index 3ba1d89..515b099 100644 --- a/packages/downgrader/src/shared.ts +++ b/packages/downgrader/src/shared.ts @@ -830,12 +830,16 @@ function isPathItemPointer(tokens: readonly string[]): boolean { // a Callback Object that is a Reference Object or under a value that is not // an object. function isInDroppedPathItemField(root: unknown, tokens: readonly string[] | undefined, fields: readonly string[]): boolean { - if (tokens === undefined || (tokens[0] === 'paths' && !isPath(tokens[1] ?? ''))) { + if (tokens === undefined) { return false } let node = root for (const [index, token] of tokens.entries()) { - if (!isRecord(node) || (tokens[index - 2] === 'callbacks' && getRef(node) !== undefined)) { + if ( + !isRecord(node) + || (index === 1 && tokens[0] === 'paths' && !isPath(token)) + || (tokens[index - 2] === 'callbacks' && getRef(node) !== undefined) + ) { return false } if (fields.includes(token) && isPathItemPointer(tokens.slice(0, index))) { diff --git a/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts b/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts index f6cdc7e..3d89d1c 100644 --- a/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts +++ b/packages/downgrader/tests/v3.2-to-v3.1/spec/removed-parts.test.ts @@ -423,12 +423,16 @@ describe('references left as written', () => { // Only converted Path Items lose `query`. An extension of `paths`, and a // Callback Object that is a Reference Object, are copied with all their - // fields, so references into them still resolve. - it('leaves references into a query field that the conversion copies as written', () => { + // fields, and a component or a file may just be named `query`, so + // references to them still resolve. + it('leaves references to a query that the conversion keeps as written', () => { const responses = { 200: { description: 'kept' } } const references = { Callback: { $ref: '#/components/callbacks/C/{$url}/query/responses/200' }, Extension: { $ref: '#/paths/x-shared/query/responses/200' }, + File: { $ref: 'query.yaml' }, + Named: { $ref: '#/components/responses/query' }, + query: { description: 'named' }, } const result = convertSpec({ components: {