fix(downgrader): inline pathItems refs, convert cycles, map the 3.2 dialect

packages/downgrader/README.md

@@ -2,7 +2,7 @@
 
 Downgrade [OpenAPI Specification](https://spec.openapis.org/) documents one minor version at a time: 3.2 → 3.1 and 3.1 → 3.0. Each converter works on an entire document or on a single Schema Object.
 
-- **Never throws**: malformed parts are deep-copied through unchanged instead of failing the whole conversion, and cyclic object graphs (e.g. the output of a `$ref` dereferencer) don't recurse forever — a subtree that cycles back into an ancestor is deep-copied with its cycle preserved instead of converted. Only pathologically deep nesting (thousands of levels) can still exhaust the call stack.
+- **Never throws**: malformed parts are deep-copied through unchanged instead of failing the whole conversion, and cyclic object graphs (e.g. the output of a `$ref` dereferencer) don't recurse forever — they are converted with their cycles preserved, a subtree that cycles back into an ancestor pointing at that ancestor's converted form. Only pathologically deep nesting (thousands of levels) can still exhaust the call stack.
 - **Never mutates**: the input document is left untouched.
 - **Extension-preserving, never extension-inventing**: existing `x-` keys and unknown keys always survive, while constructs the target version cannot express are converted where an equivalent exists and removed otherwise.
 
@@ -36,6 +36,7 @@ Converted:
 | 3.2 construct                                                                       | 3.1 result                                                                                                                                                                                                                                                                                                             |
 | ----------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | `openapi: 3.2.x`                                                                    | `openapi: 3.1.2`                                                                                                                                                                                                                                                                                                       |
+| `jsonSchemaDialect` naming a 3.2 OAS dialect                                        | `https://spec.openapis.org/oas/3.1/dialect/base` (the 3.2 dialect only extends the 3.1 base vocabulary); other dialects pass through                                                                                                                                                                                   |
 | `components.mediaTypes` and content-map `$ref`s to them                             | references inlined, the component map removed; content entries whose reference cannot be inlined (external, unknown, or cyclic targets) are removed, as 3.1 content maps cannot hold references — a parameter or header losing its entire `content` that way is removed with it (3.1 requires exactly one entry there) |
 | Media type `itemSchema` without a sibling `schema`                                  | `schema: { type: "array", items: … }` (the 3.2 sequential media type data model)                                                                                                                                                                                                                                       |
 | Response `summary` when no `description` exists                                     | promoted to `description` (required in 3.1, so `""` is synthesized as a last resort)                                                                                                                                                                                                                                   |
@@ -44,7 +45,7 @@ Converted:
 
 Removed (no 3.1 equivalent): `$self`, server `name`, tag `summary`/`parent`/`kind`, the `query` operation and `additionalOperations` of Path Items, `in: "querystring"` parameters (from parameter lists and `components.parameters`, together with references to the removed component entries, following chains of reference aliases), `allowReserved` on non-query parameters, media type `description`, media type / encoding `prefixEncoding`, `itemEncoding`, and nested `encoding`, a media type `itemSchema` beside an existing `schema`, response `summary` beside an existing `description`, OAuth `deviceAuthorization` flows, and security scheme `oauth2MetadataUrl` and `deprecated`.
 
-Known limitations: security requirements using URI keys and `$self`-relative reference resolution are passed through unchanged.
+Known limitations: security requirements using URI keys and `$self`-relative reference resolution are passed through unchanged, and so is a `$schema` keyword inside a Schema Object that names the 3.2 dialect.
 
 ## 3.1 → 3.0
 
@@ -58,7 +59,7 @@ Converted:
 | Reference `summary` / `description` overrides   | removed (3.0 references stand alone)                                   |
 | Security requirement roles on non-OAuth schemes | emptied (`[]`)                                                         |
 
-Removed (no 3.0 equivalent): `webhooks`, `components.pathItems` (local `$ref`s pointing at it are left untouched and will dangle), `jsonSchemaDialect`, `info.summary`, `license.identifier`, and `mutualTLS` security schemes (reference aliases to them included) — their names are stripped from every security requirement, requirements that referenced only such schemes are removed, and a `security` list emptied that way is removed entirely, since an explicit empty list means "no security required" and would make an operation public.
+Removed (no 3.0 equivalent): `webhooks`, `components.pathItems` (Path Item `$ref`s pointing at it, in `paths` and in callbacks, are inlined instead — following chains of references, with the referencing Path Item's own fields winning over inlined ones where both define a field; a reference that cannot be inlined, such as an unknown or cyclic target, is left untouched and will dangle), `jsonSchemaDialect`, `info.summary`, `license.identifier`, and `mutualTLS` security schemes (reference aliases to them included) — their names are stripped from every security requirement, requirements that referenced only such schemes are removed, and a `security` list emptied that way is removed entirely, since an explicit empty list means "no security required" and would make an operation public.
 
 Schema Objects:
 

packages/downgrader/src/shared.test.ts

@@ -9,7 +9,7 @@ import {
   mapArray,
   mapRecord,
   operationFields,
-  setKey,
+  setOwn,
 } from './shared'
 
 function identity<T>(value: T): T {
@@ -205,15 +205,36 @@ describe('convertRecord', () => {
     expect('polluted' in {}).toBe(false)
   })
 
-  it('falls back to a cycle-preserving clone when re-entered for the same object', () => {
+  it('points a cyclic reference at the converted ancestor when re-entered for the same object', () => {
     const node: Record<string, unknown> = { name: 'root' }
     node.self = node
     const result = convertNode(node) as Record<string, unknown>
     expect(result.name).toBe('converted')
-    const inner = result.self as Record<string, unknown>
-    expect(inner).not.toBe(node)
-    expect(inner.name).toBe('root')
-    expect(inner.self).toBe(inner)
+    expect(result.self).toBe(result)
+    expect(node.self).toBe(node)
+  })
+
+  it('converts a cycle that closes several levels down', () => {
+    const grandchild: Record<string, unknown> = { name: 'grandchild' }
+    const child: Record<string, unknown> = { name: 'child', self: grandchild }
+    const root: Record<string, unknown> = { name: 'root', self: child }
+    grandchild.self = child
+    const result = convertNode(root) as Record<string, unknown>
+    const convertedChild = result.self as Record<string, unknown>
+    const convertedGrandchild = convertedChild.self as Record<string, unknown>
+    expect(convertedChild.name).toBe('converted')
+    expect(convertedGrandchild.name).toBe('converted')
+    expect(convertedGrandchild.self).toBe(convertedChild)
+    expect(child.self).toBe(grandchild)
+  })
+
+  it('releases the cycle guard once a conversion finishes', () => {
+    const node: Record<string, unknown> = { name: 'root' }
+    node.self = node
+    const first = convertNode(node) as Record<string, unknown>
+    const second = convertNode(node) as Record<string, unknown>
+    expect(second).not.toBe(first)
+    expect(second.self).toBe(second)
   })
 
   it('converts shared acyclic references at every occurrence', () => {
@@ -343,10 +364,10 @@ describe('getRef', () => {
   })
 })
 
-describe('setKey', () => {
+describe('setOwn', () => {
   it('defines an enumerable, writable, configurable own property', () => {
     const target: Record<string, unknown> = {}
-    setKey(target, 'name', 'value')
+    setOwn(target, 'name', 'value')
     expect(Object.getOwnPropertyDescriptor(target, 'name')).toEqual({
       configurable: true,
       enumerable: true,
@@ -355,9 +376,25 @@ describe('setKey', () => {
     })
   })
 
+  it('shadows Object.prototype members with own data properties', () => {
+    const target: Record<string, unknown> = {}
+    setOwn(target, 'constructor', 1)
+    setOwn(target, 'hasOwnProperty', 2)
+    expect(Object.getOwnPropertyDescriptor(target, 'constructor')?.value).toBe(1)
+    expect(Object.getOwnPropertyDescriptor(target, 'hasOwnProperty')?.value).toBe(2)
+    expect(Object.getPrototypeOf(target)).toBe(Object.prototype)
+  })
+
+  it('redefines a key already present on the target', () => {
+    const target: Record<string, unknown> = {}
+    setOwn(target, 'name', 'first')
+    setOwn(target, 'name', 'second')
+    expect(target).toEqual({ name: 'second' })
+  })
+
   it('sets a __proto__ key as a plain own property without prototype pollution', () => {
     const target: Record<string, unknown> = {}
-    setKey(target, '__proto__', { polluted: true })
+    setOwn(target, '__proto__', { polluted: true })
     const descriptor = Object.getOwnPropertyDescriptor(target, '__proto__')
     expect(descriptor?.value).toEqual({ polluted: true })
     expect(descriptor?.enumerable).toBe(true)

packages/downgrader/src/shared.ts

@@ -53,16 +53,22 @@ export function isRecord(value: unknown): value is Record<string, unknown> {
 }
 
 /**
- * Sets a key on the output record with define-property semantics, so hostile
- * key names like `__proto__` become plain own properties.
+ * Sets a key as an own data property. `__proto__` is defined rather than
+ * assigned, so it becomes a plain property instead of replacing the prototype.
  */
-export function setKey(target: Record<string, unknown>, key: string, value: unknown): void {
-  Object.defineProperty(target, key, {
-    configurable: true,
-    enumerable: true,
-    value,
-    writable: true,
-  })
+export function setOwn(object: object, key: PropertyKey, value: unknown): void {
+  if (key === '__proto__') {
+    Object.defineProperty(object, key, {
+      configurable: true,
+      enumerable: true,
+      value,
+      writable: true,
+    })
+  }
+  else {
+    // SAFETY: only widens the index signature.
+    (object as Record<PropertyKey, unknown>)[key] = value
+  }
 }
 
 function cloneValue(value: unknown, seen: WeakMap<object, unknown>): unknown {
@@ -84,7 +90,7 @@ function cloneValue(value: unknown, seen: WeakMap<object, unknown>): unknown {
   const out: Record<string, unknown> = {}
   seen.set(value, out)
   for (const [key, item] of Object.entries(value)) {
-    setKey(out, key, cloneValue(item, seen))
+    setOwn(out, key, cloneValue(item, seen))
   }
   return out
 }
@@ -104,25 +110,29 @@ export function deepClone<T>(value: T): T {
   return cloneValue(value, new WeakMap()) as T
 }
 
-/** Objects currently being converted somewhere up the call stack. */
-const converting = new WeakSet<object>()
+/** Objects being converted up the call stack, mapped to their output records. */
+const converting = new WeakMap<object, Record<string, unknown>>()
 
 /**
  * Rebuilds a plain object field by field: each key goes through its entry in
  * `fields` (or is deep-cloned when it has none), entries mapped to or
  * returning `DROP` are left out, and `finish` receives the result together
  * with the source for fix-ups that depend on several fields. Non-object input
- * is deep-cloned unchanged, and so is an object already being converted
- * higher up the call stack: a cyclic reference, which would otherwise recurse
- * forever.
+ * is deep-cloned unchanged, and a cyclic reference back to an object still
+ * being converted yields that object's output record, so `finish` should
+ * mutate and return `out` rather than replace it.
  */
 export function convertRecord(value: unknown, fields: FieldTable, finish?: (out: Record<string, unknown>, source: Record<string, unknown>) => unknown): unknown {
-  if (!isRecord(value) || converting.has(value)) {
+  if (!isRecord(value)) {
     return deepClone(value)
   }
-  converting.add(value)
+  const inProgress = converting.get(value)
+  if (inProgress !== undefined) {
+    return inProgress
+  }
+  const out: Record<string, unknown> = {}
+  converting.set(value, out)
   try {
-    const out: Record<string, unknown> = {}
     for (const [key, item] of Object.entries(value)) {
       const convert = Object.hasOwn(fields, key) ? fields[key] : undefined
       if (convert === DROP) {
@@ -131,7 +141,7 @@ export function convertRecord(value: unknown, fields: FieldTable, finish?: (out:
       const converted
         = convert === undefined ? deepClone(item) : convert(item, value)
       if (converted !== DROP) {
-        setKey(out, key, converted)
+        setOwn(out, key, converted)
       }
     }
     return finish === undefined ? out : finish(out, value)
@@ -149,7 +159,7 @@ export function mapRecord(value: unknown, convert: (item: unknown, key: string)
   for (const [key, item] of Object.entries(value)) {
     const converted = convert(item, key)
     if (converted !== DROP) {
-      setKey(out, key, converted)
+      setOwn(out, key, converted)
     }
   }
   return out