fix: re-pin openapi generator to 3.1-native 7.12.0 and guard codegen in CI#135
Open
Kyzgor wants to merge 1 commit into
Open
fix: re-pin openapi generator to 3.1-native 7.12.0 and guard codegen in CI#135Kyzgor wants to merge 1 commit into
Kyzgor wants to merge 1 commit into
Conversation
…in CI The pinned generator 6.2.1 cannot read the OpenAPI 3.1.0 spec served at api.permit.io: it regenerates an all-`any`, type-erased client (247 of 319 type files) yet exits 0, masked by --skip-validate-spec. Running `yarn generate-openapi-client` today silently replaces the typed client with `any`. Re-pin the generator to 7.12.0, the first 3.1-native line that regenerates a typed client (named properties instead of `any`; 2 known allOf+default residuals remain, allow-listed and tracked for the re-baseline), and add a CI codegen guard that regenerates from a pinned 3.1.0 fixture through the real pipeline and fails if any type collapses to all-`any`. The guard runs in its own Java-provisioned job, off the test suite. The committed client under src/openapi/ is intentionally left unchanged: this restores and guards the toolchain. Re-baselining the generated client against the current spec is a separate, larger follow-up. Fixes permitio#130
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug fix (build tooling). It restores typed regeneration of the OpenAPI client and adds a CI guard so a regression of the generator pin can no longer silently ship an untyped client (a future spec migration past 3.1 is a separately-tracked gap, see "Other information").
Fixes #130.
yarn generate-openapi-clientno longer produces a typed client; it silently emits an all-any, type-erased one and still exits 0.The generator is pinned to
6.2.1(openapitools.json), which bundles swagger-core 2.2.4 and only models OpenAPI 3.0. The served spec athttps://api.permit.io/v2/openapi.jsonis now OpenAPI3.1.0, so 6.2.1 cannot resolve most schemas and degrades them toany. Because the script passes--skip-validate-spec, the run still exits 0. A fresh regeneration today replaces the typed client withany(247 of 319 type files):This was reported and triaged in #130; the chosen direction there is to model the spec natively (a 3.1-capable generator) plus a CI check so it cannot silently regress.
Three changes, scoped to the toolchain. The committed client under
src/openapi/is not regenerated here (see "Other information"):7.12.0(openapitools.json), a 3.1-native generator line that regenerates a typed client (named properties instead ofany). The earlier7.1.0and7.5.0releases abort with aNullPointerExceptionon this spec;7.12.0generates cleanly:Add a CI codegen guard (
scripts/check-codegen.mjsplus a committedsrc/tests/codegen/fixtures/openapi-3.1.0.jsonsnapshot). It regenerates through the same real pipeline asgenerate-openapi-client(the sameopenapi-generator-cliwrapper, the version resolved fromopenapitools.json, and the same--additional-propertiesflags), differing only in that it reads the pinned fixture instead of the live URL and writes to a throwaway directory it then inspects (so no committed file is touched and no prettier pass is needed). It does not rely on the generator's exit code: it reads the emittedtypes/*.tsand fails if any collapses to an all-any[key: string]: any;index signature. Positive canaries assert named scalar properties stay typed, catching the partial case where a property degrades to a bare'prop': any;with no index signature.Wire it as a separate CI job (
codegen-guardin.github/workflows/ci.yaml) withsetup-java(Temurin 11), kept off theyarn testpath. The generator needs Java and a generation run that does not fit ava's per-test budget, so it runs in its own job.yarn generate-openapi-clientoutputanyallOf+defaultresiduals (allow-listed)RoleCreate.keyany(under[key: string]: any)string0(silent)codegen-guardjob goes rednode scripts/check-codegen.mjs(run locally)yarn lint/yarn build/yarn test:unit(48) /yarn test:module-imports(9) (local)src/openapi/client (.tssource)(CI's
test-and-lintjob runs these on Node 18 and 20.yarn testalso includes atest:integrationsuite, and there are separatetest:e2e:*suites; both need live, scoped Permit credentials or a running PDP and are not exercised by a build-tooling change.)The guard is not green-by-construction; I verified locally that it goes red on both regression modes. First, pinning back: reverting
openapitools.jsonto6.2.1(the only file changed) fails with245 type file(s) collapsed to all-\any`(245, not the live-spec 247, because the guard runs against the pinned fixture and excludes the two allow-listedallOf+defaultresiduals). Second, a partial degrade: leaving the pin at7.12.0but degrading one covered property in the fixture (TenantObj.idtoany) fails withcanary tenant-obj.ts: property 'id' degraded to `any`, which exercises the canary path that the tree-wide index-signature scan alone would miss. On the unmodified fixture the guard passes; its full status line iscodegen guard OK - 350 type files fully typed (2 known residual: derived-role-block-edit.ts, derived-role-rule-create.ts; 5 canaries typed)`, i.e. the 348 typed plus 2 allow-listed residuals from the table above.Scope. This PR fixes and guards the toolchain only; it does not regenerate and commit the client.
src/openapi/is left byte-identical, so there are no consumer-facing type changes and this is non-breaking. A full regeneration is a larger, breaking re-baseline, and is deliberately not part of this PR. When that follow-up runs, the current spec removes or renames many generated files and drops a few exported types;granted_totyping on the role types changes as well. That churn is driven by roughly three years of spec drift rather than the generator bump — a fresh 6.2.1 regeneration produces the same removals, so it is not introduced by this re-pin — and the exact re-baseline set is enumerated when that follow-up PR is opened. The re-baseline lands as a separate follow-up PR (a semver-major) so it can be reviewed and released deliberately. This PR is the prerequisite: it makes regeneration produce a typed client again and guards it.Not included, on purpose. The client re-baseline above is one planned follow-up PR that regenerates
src/openapi/and, as part of the same work, resolves theallOf+defaultresiduals the 7.x generator leaves. At least two are currently known (derived-role-block-edit,derived-role-rule-create); the full set is confirmed only when the re-baseline is done, so the guard allow-lists those two rather than asserting a closed count. Live-spec drift detection is also deferred: the guard pins a fixture snapshot, so a spec migration past 3.1 would need the fixture re-armed, and a non-blocking scheduled check for that is better as its own small job.Coordination. #121 (hand-adding
extendsto the six role types) touches the frozensrc/openapi/; it is independent of this PR, and the eventual regeneration folds it in. #131 also edits.github/workflows/ci.yaml; the two are independent (this PR only appends a separatecodegen-guardjob), but whichever merges second needs a trivial rebase of the appended job.Guard exercised in CI, not only locally. The
codegen-guardjob was validated end-to-end on the fork (Kyzgor/permit-node, Temurin 11, GitHub Actions). It is GREEN under the fix (run28370619361:codegen guard OK - 350 type files fully typed) and RED under two negative controls: a pin-revert to6.2.1(run28371064327:245 type file(s) collapsed to all-\any`) and a single covered-property degrade of the fixture (TenantObj.idtoany; run28371066398:canary tenant-obj.ts: property 'id' degraded to `any`). In both failing runs thetest-and-lintjobs (Node 18 and 20) stayed GREEN, isolating the failure tocodegen-guard. The upstreamcodegen-guard` job will run on this PR once opened.CI note.
security/snyk (permit)errors on fork PRs because it needs an org token the fork cannot access. It is not a code issue, and a maintainer can re-run it.