fix(cli): support multi-spec on-prem generation and preflight SDK migration - #17782
ATechAdventurer wants to merge 9 commits into
Conversation
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Two changes bundled here: a Docker-label capability probe that lets multi-spec workspaces reach capable adapter images, and a prepare-then-write reordering in fern sdk migrate so validation failures no longer leave partial output. The migration reordering looks correct and is well covered by the new tests. The capability probe has rough edges around cancellation during the preflight pull and the uniqueness/consistency of the new spec id.
- 🔵 2 suggestion(s)
Review Scope
- Mode: Full pull request
- Range:
Full pull request through 10f7132d09010a8f7500f1798dfa1f73266c7e82 - Current range coverage: Complete
- Cumulative pull request coverage: Complete
This review is complete for the current scope. Addressing all critical and warning findings above satisfies the review; suggestions are optional. Request another review only if subsequent changes introduce new behavior or materially expand the pull request's scope.
| if (result.exitCode !== 0) { | ||
| // Execution normally pulls missing images, but this gate runs before execution. | ||
| await ensureDockerHubOatLogin({ imageName, runner, logger }); | ||
| await pullImage(imageName, runner); |
There was a problem hiding this comment.
🔵 suggestion
pullImage is invoked without an AbortSignal, and imageSupportsMultiSpec doesn't accept one either. A pull of a large adapter image during this preflight probe therefore isn't cancellable, so Ctrl-C here waits for the whole pull to finish. Consider threading the run's signal from ContainerExecutionEnvironment/runGenerator into imageSupportsMultiSpec and passing it to both loggingExeca and pullImage.
|
|
||
| const specs = entries.map( | ||
| (entry): SourceSpec => ({ | ||
| ...(entries.length > 1 ? { id: basename(entry.specPath) } : {}), |
There was a problem hiding this comment.
🔵 suggestion
id is only set when entries.length > 1 and is derived from basename(specPath). Two things worth reconsidering:
- Single-spec runs get no
idat all, so spec identity is inconsistent across run shapes — if the adapter surfacesidin diagnostics, single-spec output silently loses it. basenameisn't unique:specs/v1/openapi.ymlandspecs/v2/openapi.ymlboth collapse toopenapi.yml, making composition diagnostics ambiguous in exactly the multi-spec case this is meant to serve. Folding in the namespace (or the manifest index) would disambiguate.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
1 participant
Summary
Address the on-prem QA report where a Python SDK workspace contains OpenAPI and AsyncAPI and local generation refuses the two sources.
com.postman.sdk-gen.adapter.multi-spec="true".sdk-config.ymlor any docs file. Unsupported custom settings and malformed later version files now fail without leaving validation-induced partial output.Verification
pnpm fern-dev:buildpassed, including 95 compilation/build tasks.USE_FERN_RC=true FERN_RC_NAMESPACE=multi-spec-qa FERN_GENERATOR_NETWORK=none fern-dev generate --group python-sdk --localgenerated and formatted Python from OpenAPI + AsyncAPI using a locally built capable adapter. Imported the generated SDK and accessed REST and WebSocket methods.sdk migratewith unsupported docs import settings failed while leavingsdk-config.ymlabsent. Successful migration, help output, and invalid group handling were also exercised.Rollout and limits
Companion runtime PR: https://github.com/postman-eng/sdk-gen-core/pull/2527
Requires the companion sdk-gen-core change and newly published adapter images carrying the capability label. This PR does not relabel existing images or assume a generator version has the capability.
The exact QA migration error was not supplied; the migration fix covers reproduced validation/read/parse ordering failures, not all possible errors. It does not make multi-file filesystem writes transactional: a later write failure can still leave partial output.
The actual ElevenLabs specs were unavailable; QA used repository Petstore/WebSocket fixtures. Docker was exercised live; Podman was covered through runner-selection tests. Fern paths lie outside this session's LSP root, so the successful full TypeScript build is the diagnostic evidence for those files.