fix(cli): prefer x-fern-server-name over description in multi-api environment grouping - #17314
fix(cli): prefer x-fern-server-name over description in multi-api environment grouping#17314iamnamananand996 wants to merge 1 commit into
Conversation
…ironment grouping
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
AI Review Summary
Single-line precedence fix in getRawEnvironmentName plus a regression fixture and snapshots. The fix is correct for the reported case, but it silently changes environment names for any existing spec that has both a description and a name/x-fern-server-name on a server — that's a user-visible rename in generated SDKs worth calling out. Fixture coverage could also include the description-only fallback path.
- 🟡 1 warning(s)
- 🔵 2 suggestion(s)
|
|
||
| function getRawEnvironmentName(server: SingleServerInput): string { | ||
| return String(server.description || server.name || server["x-fern-server-name"] || "default").trim(); | ||
| return String(server.name || server["x-fern-server-name"] || server.description || "default").trim(); |
There was a problem hiding this comment.
🟡 warning
This flips precedence for all specs, not just broken ones: any workspace where a server has both a description and a name/x-fern-server-name will now get a different environment key (e.g. CoreUnifiedApi → Production), which is a rename in the generated SDK's environment enum. That's the desired outcome here, but it's technically breaking for existing users relying on the description-derived name. Consider calling this out explicitly in the changelog entry ("environment names may change if...") so downstream consumers aren't surprised by a fix-level bump.
|
|
||
| function getRawEnvironmentName(server: SingleServerInput): string { | ||
| return String(server.description || server.name || server["x-fern-server-name"] || "default").trim(); | ||
| return String(server.name || server["x-fern-server-name"] || server.description || "default").trim(); |
There was a problem hiding this comment.
🔵 suggestion
Worth verifying that the other environment-naming code paths (the per-spec server name resolution in the OpenAPI parser / openapi-ir-to-fern env generation) use the same precedence. If any of them still prefer description, merged env names and per-spec env names can diverge again in a different combination of fields — the same class of bug this PR fixes.
| - url: https://api.stage.example.com | ||
| x-fern-server-name: Staging | ||
| - url: https://api.dev.example.com | ||
| x-fern-server-name: Development |
There was a problem hiding this comment.
🔵 suggestion
The fixture only exercises the new winner (x-fern-server-name beating description). The description-only fallback (servers with no name and no x-fern-server-name, matching across specs by description) is now the last branch and untested here — a second small fixture would lock that behavior in so a future reorder of this chain doesn't silently break description-based grouping.
✅ Verified end-to-end against Twilio's Issue #16 repro workspaceTested by regenerating the Twilio unified SDK (8 specs, Generated TypeScript SDK export interface TwilioApiEnvironmentUrls {
twilio: string;
oauth: string;
}
export const TwilioApiEnvironment = {
Production: { twilio: "https://api.twilio.com", oauth: "https://oauth.twilio.com" },
Staging: { twilio: "https://api.stage-us1.twilio.com", oauth: "https://oauth.stage.twilio.com" },
Development: { twilio: "https://api.dev.twilio.com", oauth: "https://oauth.dev.twilio.com" },
} as const;Exactly matches the "Expected" block in the Twilio issue report; Baseline: bug reproduced with published CLI (fern-api@latest)
i.e. a single-URL enum containing only the oauth spec's URLs — the two specs with an informational Fixed CLI IR check |
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Description
Fixes Twilio pilot Issue #16: with
group-multi-api-environments: true, the CLI fell back to a single-URL environment enum (using only the last spec's oauth URLs) instead of a multi-URL environment, breaking all API calls made viaenvironment:.Root cause: environment-name matching during multi-spec IR merging used
descriptionbefore the explicit server name:Twilio's merged specs carry an informational
servers[0].description(e.g."Twilio Communications Unified API"). Since spec overrides merge arrays element-wise, the override'sx-fern-server-name: Productionlanded on the same server object as the spec's description — and the description won, so environment names no longer matched across specs,detectMultipleBaseUrlsreturned false, and grouping never happened. (It was unrelated to the inlined securitySchemes suspected in the report; only the spec descriptions matter.)Changes Made
x-fern-server-name) overdescriptionwhen matching environments across APIs inopenapi-ir-parser/src/parse.tsmulti-api-environment-grouping-server-description(two specs sharing named environments, one server with an unrelateddescription) + snapshotsTesting
openapi-ir-to-fern-testsfull suite passes (307 tests), new fixture snapshots producemultipleBaseUrlsfern iron Twilio's issue-16 repro workspace; IR now emitsmultipleBaseUrlswithtwilio/oauthURLs per environment (Production/Staging/Development), matching the expected output in their reportLink to Devin session: https://app.devin.ai/sessions/7b10237605394e69a949f45a87bf31c5
Requested by: @iamnamananand996