Skip to content

Conversation

rc-glean
Copy link
Contributor

@rc-glean rc-glean commented Oct 9, 2025

Description

This PR fixes a bug where the REMOVE_X_INTERNAL=true normalizer fails to remove the x-internal extension from inline object properties within schemas.

Problem

When REMOVE_X_INTERNAL=true is set, the normalizer correctly removes x-internal from top-level schemas in components/schemas, but fails to process inline object properties.

This causes issues when:

  1. A schema is imported cross-file (e.g., admin.yaml imports from chat.yaml)
  2. That schema has an inline object property with x-internal: true
  3. The inline property has type: object with nested properties

Result: TypeScript generator creates a type reference but no interface definition, causing compilation errors.

Solution

Apply the same x-internal removal logic to normalizeProperties() that already exists in normalizeComponentsSchemas(), ensuring inline properties are handled consistently.

Bug Trigger Conditions

  1. ✅ Cross-file import (e.g., admin.yaml imports from chat.yaml)
  2. ✅ Schema has an inline object property
  3. ✅ That inline property has x-internal: true

Testing

  • ✅ Tested with OpenAPI Generator v7.13.0 (all 3,375 tests pass)
  • ✅ Verified fix resolves the issue on actual production schema (Glean admin API)
  • ✅ No regressions in existing tests

Example

Before (broken):

export interface AgentConfig {
    clientCapabilities?: AgentConfigClientCapabilities; // ❌ Type referenced but not defined
}
// AgentConfigClientCapabilities interface missing!

After (fixed):

export interface AgentConfig {
    clientCapabilities?: AgentConfigClientCapabilities; // ✅ Type referenced
}

export interface AgentConfigClientCapabilities { // ✅ Type IS defined!
    canRenderImages?: boolean;
    artifacts?: ArtifactsClientCapability;
}

When REMOVE_X_INTERNAL=true is set, the normalizer removes the x-internal
extension from top-level schemas in components/schemas but fails to remove
it from inline object properties within those schemas.

This causes issues when:
1. A schema is imported cross-file (e.g., admin.yaml imports from chat.yaml)
2. That schema has an inline object property with x-internal: true
3. The inline property has type: object with nested properties

Result: TypeScript generator creates a type reference but no interface
definition, causing compilation errors.

This fix applies the same x-internal removal logic to normalizeProperties()
that already exists in normalizeComponentsSchemas(), ensuring inline
properties are handled consistently.

Fixes behavior for inline schemas with x-internal in cross-file imports.
@Copilot Copilot AI review requested due to automatic review settings October 9, 2025 16:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where the REMOVE_X_INTERNAL=true normalizer fails to remove the x-internal extension from inline object properties within schemas, causing TypeScript generation issues when schemas are imported cross-file.

  • Applies consistent x-internal removal logic to inline properties
  • Prevents TypeScript compilation errors from missing interface definitions
  • Ensures cross-file schema imports work correctly with inline object properties

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

…en/OpenAPINormalizer.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@rc-glean rc-glean closed this Oct 9, 2025
@rc-glean rc-glean reopened this Oct 9, 2025
@rc-glean rc-glean changed the title fix: Apply REMOVE_X_INTERNAL normalizer to nested inline properties [fix] Apply REMOVE_X_INTERNAL normalizer to nested inline properties Oct 9, 2025
rc-glean and others added 2 commits October 9, 2025 10:16
Adds test to verify that REMOVE_X_INTERNAL normalizer correctly removes
x-internal extension from inline object properties, not just top-level schemas.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@wing328
Copy link
Member

wing328 commented Oct 10, 2025

@rc-glean thanks for the PR

can you please review the build failure when you've time?

@wing328 wing328 added Issue: Bug OpenAPI Normalizer Normalize the spec for easier processing labels Oct 10, 2025
@wing328 wing328 added this to the 7.17.0 milestone Oct 10, 2025
@rc-glean
Copy link
Contributor Author

@wing328 this was the first time copilot suggestions in PR recommended a broken fix for me lol c899e9e

Can you retrigger the 4 workflows awaiting approval please?

@wing328
Copy link
Member

wing328 commented Oct 11, 2025

That's ok.

Did you test the fix locally with the last commit in this branch to confirm it works for your use cases?

@rc-glean
Copy link
Contributor Author

Thank you. Confirmed the latest commit in this PR: 60cf734, fixes the issue.

Ran the following verification:

## ✅ Fix Verified Against Production Schema

I've tested this fix against Glean's production OpenAPI schema where we encountered this exact bug.

### Test Setup
1. **Schema**: Glean Admin API with cross-file import (`admin.yaml``chat.yaml`)
2. **Trigger Condition**: Inline object property with `x-internal: true` inside a cross-file imported schema
3. **Generator Config**: `typescript-fetch` with `--openapi-normalizer REMOVE_X_INTERNAL=true`

### Reproduction
In `chat.yaml`, `AgentConfig` has an inline `clientCapabilities` property marked `x-internal: true`:

```yaml
AgentConfig:
  properties:
    clientCapabilities:
      x-internal: true
      type: object
      properties:
        canRenderImages:
          type: boolean
        artifacts:
          $ref: "#/components/schemas/ArtifactsClientCapability"

When admin.yaml imports this schema, TypeScript generation fails.

Results

❌ Before (v7.16.0 stable)

// AgentConfig references the type...
clientCapabilities?: AgentConfigClientCapabilities;

// ...but the interface is NEVER generated (compilation error)

✅ After (this PR - commit 60cf734)

// Interface is properly generated as AgentConfig_clientCapabilities
export interface AgentConfigClientCapabilities {
    canRenderImages?: boolean;
    artifacts?: ArtifactsClientCapability;
    paper?: PaperConfig;
}

Generator logs confirm proper handling:

[main] INFO o.o.codegen.InlineModelResolver - Inline schema created as AgentConfig_clientCapabilities

Impact

This bug forced us to extract inline schemas into separate top-level schemas as a workaround. With this fix, we can use natural inline object definitions with x-internal flags.

@wing328
Copy link
Member

wing328 commented Oct 14, 2025

👌

let's give it a try

i think ideally the logic should be done in normalizeSchema. i'll try to do the refactoring later if I can find the time.

thanks again for the contribution.

@wing328 wing328 merged commit b199901 into OpenAPITools:master Oct 14, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue: Bug OpenAPI Normalizer Normalize the spec for easier processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants