feat(api): make the AIP TypeScript SDK release-ready#4687
Merged
Conversation
Implements the fixes from the @openmeter/client release-readiness review. Correctness: - toWire materializes required-with-default fields, so the minimal documented events.ingest() call carries specversion and is accepted - bigint (int64) request fields map to JSON numbers on the wire (typed UnsafeIntegerError beyond the JSON-safe range) and revive on responses - the package root no longer shadows 18 operation request types with same-named domain models - operations marked x-private/x-internal are omitted from the SDK (they remain in the OpenAPI output) Emitter robustness: - one shared isSuccessStatus resolver (was four divergent copies) - diagnostics on every silent-degradation path (any/unknown fallbacks, ungrouped operations) - generated-file headers on every emitted file - runtime templates are committed files under templates/ instead of base64 blobs generated from an out-of-repo baseline Developer experience: - JSDoc on every generated method/func (summary, description, route) and on Input variant interfaces - named TypeSpec unions (Price, App, Charge, ...) are exported, narrowable types - auto-pagination companions (listAll) for page-number and cursor lists with a shared runtime helper and a hard page cap - HTTPError carries name/invalidParameters/retryAfter; a User-Agent header is sent with the version stamped at publish - README documents transport config, runtime validation, the ./zod export, Result/unwrap, and pagination Packaging and CI: - LICENSE ships in the tarball; engines, caret dependency ranges, sideEffects, files allowlist, and main/types fallbacks added - the SDK conformance suite and emitter checks run in CI and gate the npm publish workflow
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates the TypeSpec JavaScript SDK generator and generated client with pagination companions, union-aware types, bigint/default wire mapping, typed errors, refreshed exports and documentation, plus conformance tests and CI/release gates. ChangesGenerated SDK and runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/spec/packages/aip-client-javascript/src/sdk/meters.ts (1)
150-156: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
queryCsvJSDoc is missing summary and description.The coding guideline requires every emitted facade method to have JSDoc containing summary, differing description, and the HTTP route. The
queryCsvJSDoc only has the route. Since this is generated output, the fix belongs in the emitter's JSDoc generation for CSV variant methods.📄 Suggested JSDoc for queryCsv
- /** POST /openmeter/meters/{meterId}/query */ + /** + * Query meter (CSV) + * + * Query a meter for usage and download the result as a CSV file. + * + * POST /openmeter/meters/{meterId}/query + */ async queryCsv(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/aip-client-javascript/src/sdk/meters.ts` around lines 150 - 156, Update the emitter’s JSDoc generation for CSV variant methods so queryCsv and equivalent emitted facade methods include a summary and distinct description in addition to the existing HTTP route. Preserve the route and ensure the generated documentation satisfies the standard required for all facade methods.Source: Coding guidelines
api/spec/packages/typespec-typescript/src/runtime/wire.ts (1)
11-20: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCall
defaultValue()here.requiredDefault()is returning theZodDefaultcallback itself instead of the resolved value, so required-with-default fields can end up wrong on the wire.api/spec/packages/typespec-typescript/src/runtime/wire.ts:59-81🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/typespec-typescript/src/runtime/wire.ts` around lines 11 - 20, Update the required-default handling in the wire serialization logic around requiredDefault so it invokes the ZodDefault defaultValue callback and uses its resolved value, rather than returning the callback itself. Preserve the existing behavior for fields without defaults.
🧹 Nitpick comments (4)
api/spec/packages/aip-client-javascript/src/funcs/tax.ts (1)
22-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTax operation JSDoc blocks missing differing descriptions across
funcs/tax.tsandsdk/tax.ts. The coding guideline requires every emitted facade method and standalone function to have "JSDoc containing summary, differing description, and the HTTP route." All tax JSDoc blocks include a summary and route but omit the differing description — likely because the TypeSpec source for tax operations lacks@docannotations. Compare with apps operations which include descriptions like "List installed apps." and "Get an installed app." ThelistCodesAllfacade method has the pagination note but is still missing the operation-level description thatapps.listAllincludes.
api/spec/packages/aip-client-javascript/src/funcs/tax.ts#L22-L26: Add@docto thecreateTaxCodeTypeSpec operation so the emitter generates a differing description.api/spec/packages/aip-client-javascript/src/funcs/tax.ts#L49-L53: Same forgetTaxCode.api/spec/packages/aip-client-javascript/src/funcs/tax.ts#L78-L82: Same forlistTaxCodes.api/spec/packages/aip-client-javascript/src/funcs/tax.ts#L112-L116: Same forupsertTaxCode.api/spec/packages/aip-client-javascript/src/funcs/tax.ts#L145-L149: Same fordeleteTaxCode.api/spec/packages/aip-client-javascript/src/sdk/tax.ts#L25-L100: Regeneratesdk/tax.tsafter adding@docannotations so all facade method JSDoc blocks (createCode,getCode,listCodes,listCodesAll,upsertCode,deleteCode) include the operation description.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/aip-client-javascript/src/funcs/tax.ts` around lines 22 - 26, Add distinct `@doc` descriptions to the TypeSpec operations createTaxCode, getTaxCode, listTaxCodes, upsertTaxCode, and deleteTaxCode in api/spec/packages/aip-client-javascript/src/funcs/tax.ts, preserving their existing summaries and routes; then regenerate api/spec/packages/aip-client-javascript/src/sdk/tax.ts so createCode, getCode, listCodes, listCodesAll, upsertCode, and deleteCode include the emitted operation descriptions, with listCodesAll retaining its pagination note.Source: Coding guidelines
api/spec/packages/aip-client-javascript/src/funcs/meters.ts (1)
226-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
queryMeterCsvJSDoc is missing summary and description.Every other standalone function in this file has a JSDoc with a summary, a differing description, and the HTTP route.
queryMeterCsvonly has the route. Per the coding guidelines, every emitted standalone function must have JSDoc containing all three. The fix would be in the emitter's JSDoc template, not in this generated file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/aip-client-javascript/src/funcs/meters.ts` at line 226, The emitter’s JSDoc template for queryMeterCsv must include a summary, description, and HTTP route instead of emitting only the route. Update the template logic generating standalone function documentation, preserving the existing route and using the function’s available metadata for the summary and description.Source: Coding guidelines
api/spec/packages/typespec-typescript/src/readme.ts (1)
154-227: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider documenting the vendored ky-specific option names.
The
configurationsection links to the mainline ky GitHub page and saysSDKOptionsextends ky'sOptions, but the SDK uses a vendored ky with specific option names (totalTimeout,retryOnTimeout) that differ from mainline ky. Users who follow the link to ky's docs may encounter option names that don't work with the vendored version. A brief note about the vendored option names would prevent confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/typespec-typescript/src/readme.ts` around lines 154 - 227, Update the configuration() documentation to note that the SDK uses a vendored ky version with the option names totalTimeout and retryOnTimeout, which may differ from the linked mainline ky documentation. Place the brief clarification near the existing SDKOptions/ky Options description without changing the examples or other configuration guidance.Source: Coding guidelines
api/spec/packages/aip-client-javascript/src/models/types.ts (1)
2664-2682: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBoolean/numeric filters weren't included in the alias consolidation.
Nice consolidation overall —
string,stringExact,ulid, anddatetimefilters all now point at shared named aliases, butbooleanandnumerichere are still inline unions. IfFieldFiltersis meant to mirror the same filter-alias pattern used elsewhere in this file, worth checking whether the emitter should also emitBooleanFieldFilter/NumericFieldFilteraliases for consistency (this file is generated, so any fix would land in the emitter templates rather than here).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/aip-client-javascript/src/models/types.ts` around lines 2664 - 2682, Update the emitter templates that generate FieldFilters so boolean and numeric use shared BooleanFieldFilter and NumericFieldFilter aliases, matching the existing string, stringExact, ulid, and datetime filter aliases; do not edit the generated types.ts output directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/spec/AGENTS.md`:
- Around line 49-54: Update the runtime-helper reference in AGENTS.md to point
from src/wire-runtime.ts to src/runtime/wire.ts, matching the documented source
location while preserving the surrounding guidance.
In `@api/spec/Makefile`:
- Line 56: Update the inline Node version-replacement command in the Makefile so
it verifies that the SDK_VERSION pattern in src/lib/version.ts matched before
writing the file. If no replacement occurs, throw an error and stop the publish
command; otherwise preserve the existing write behavior with the updated
AIP_SDK_RELEASE_VERSION value.
In `@api/spec/packages/aip-client-javascript/src/funcs/customers.ts`:
- Around line 50-54: The generated JSDoc for the listed customer operations
contains only summaries and routes; add distinct `@doc` descriptions in the
TypeSpec source for createCustomer, getCustomer, listCustomers, upsertCustomer,
deleteCustomer, getCustomerBilling, updateCustomerBilling,
updateCustomerBillingAppData, and createCustomerCharges, then regenerate both
funcs/customers.ts and sdk/customers.ts so every corresponding facade method and
standalone function includes a summary, differing description, and HTTP route.
Affected sites are
api/spec/packages/aip-client-javascript/src/funcs/customers.ts lines 50-54,
77-81, 106-110, 141-145, 174-178, 195-199, 224-228, 257-261, and 728-734, plus
api/spec/packages/aip-client-javascript/src/sdk/customers.ts lines 77-81, 89-93,
101-105, 131-135, 143-147, 174-178, 186-191, 198-202, and 504-510; these
generated sites require no direct edits.
---
Outside diff comments:
In `@api/spec/packages/aip-client-javascript/src/sdk/meters.ts`:
- Around line 150-156: Update the emitter’s JSDoc generation for CSV variant
methods so queryCsv and equivalent emitted facade methods include a summary and
distinct description in addition to the existing HTTP route. Preserve the route
and ensure the generated documentation satisfies the standard required for all
facade methods.
In `@api/spec/packages/typespec-typescript/src/runtime/wire.ts`:
- Around line 11-20: Update the required-default handling in the wire
serialization logic around requiredDefault so it invokes the ZodDefault
defaultValue callback and uses its resolved value, rather than returning the
callback itself. Preserve the existing behavior for fields without defaults.
---
Nitpick comments:
In `@api/spec/packages/aip-client-javascript/src/funcs/meters.ts`:
- Line 226: The emitter’s JSDoc template for queryMeterCsv must include a
summary, description, and HTTP route instead of emitting only the route. Update
the template logic generating standalone function documentation, preserving the
existing route and using the function’s available metadata for the summary and
description.
In `@api/spec/packages/aip-client-javascript/src/funcs/tax.ts`:
- Around line 22-26: Add distinct `@doc` descriptions to the TypeSpec operations
createTaxCode, getTaxCode, listTaxCodes, upsertTaxCode, and deleteTaxCode in
api/spec/packages/aip-client-javascript/src/funcs/tax.ts, preserving their
existing summaries and routes; then regenerate
api/spec/packages/aip-client-javascript/src/sdk/tax.ts so createCode, getCode,
listCodes, listCodesAll, upsertCode, and deleteCode include the emitted
operation descriptions, with listCodesAll retaining its pagination note.
In `@api/spec/packages/aip-client-javascript/src/models/types.ts`:
- Around line 2664-2682: Update the emitter templates that generate FieldFilters
so boolean and numeric use shared BooleanFieldFilter and NumericFieldFilter
aliases, matching the existing string, stringExact, ulid, and datetime filter
aliases; do not edit the generated types.ts output directly.
In `@api/spec/packages/typespec-typescript/src/readme.ts`:
- Around line 154-227: Update the configuration() documentation to note that the
SDK uses a vendored ky version with the option names totalTimeout and
retryOnTimeout, which may differ from the linked mainline ky documentation.
Place the brief clarification near the existing SDKOptions/ky Options
description without changing the examples or other configuration guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98756a0d-d633-4445-8a87-39bfe72e255a
⛔ Files ignored due to path filters (1)
api/spec/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (119)
.github/workflows/aip-npm-release.yaml.github/workflows/ci.yamlAGENTS.mdMakefileapi/spec/AGENTS.mdapi/spec/Makefileapi/spec/packages/aip-client-javascript/.npmignoreapi/spec/packages/aip-client-javascript/LICENSEapi/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/package.jsonapi/spec/packages/aip-client-javascript/src/core.tsapi/spec/packages/aip-client-javascript/src/funcs/addons.tsapi/spec/packages/aip-client-javascript/src/funcs/apps.tsapi/spec/packages/aip-client-javascript/src/funcs/billing.tsapi/spec/packages/aip-client-javascript/src/funcs/currencies.tsapi/spec/packages/aip-client-javascript/src/funcs/customers.tsapi/spec/packages/aip-client-javascript/src/funcs/defaults.tsapi/spec/packages/aip-client-javascript/src/funcs/entitlements.tsapi/spec/packages/aip-client-javascript/src/funcs/events.tsapi/spec/packages/aip-client-javascript/src/funcs/features.tsapi/spec/packages/aip-client-javascript/src/funcs/governance.tsapi/spec/packages/aip-client-javascript/src/funcs/index.tsapi/spec/packages/aip-client-javascript/src/funcs/invoices.tsapi/spec/packages/aip-client-javascript/src/funcs/llmCost.tsapi/spec/packages/aip-client-javascript/src/funcs/meters.tsapi/spec/packages/aip-client-javascript/src/funcs/planAddons.tsapi/spec/packages/aip-client-javascript/src/funcs/plans.tsapi/spec/packages/aip-client-javascript/src/funcs/subscriptions.tsapi/spec/packages/aip-client-javascript/src/funcs/tax.tsapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/lib/config.tsapi/spec/packages/aip-client-javascript/src/lib/encodings.tsapi/spec/packages/aip-client-javascript/src/lib/paginate.tsapi/spec/packages/aip-client-javascript/src/lib/request.tsapi/spec/packages/aip-client-javascript/src/lib/to-error.tsapi/spec/packages/aip-client-javascript/src/lib/types.tsapi/spec/packages/aip-client-javascript/src/lib/version.tsapi/spec/packages/aip-client-javascript/src/lib/wire.tsapi/spec/packages/aip-client-javascript/src/models/errors.tsapi/spec/packages/aip-client-javascript/src/models/operations/addons.tsapi/spec/packages/aip-client-javascript/src/models/operations/apps.tsapi/spec/packages/aip-client-javascript/src/models/operations/billing.tsapi/spec/packages/aip-client-javascript/src/models/operations/currencies.tsapi/spec/packages/aip-client-javascript/src/models/operations/customers.tsapi/spec/packages/aip-client-javascript/src/models/operations/defaults.tsapi/spec/packages/aip-client-javascript/src/models/operations/entitlements.tsapi/spec/packages/aip-client-javascript/src/models/operations/events.tsapi/spec/packages/aip-client-javascript/src/models/operations/features.tsapi/spec/packages/aip-client-javascript/src/models/operations/governance.tsapi/spec/packages/aip-client-javascript/src/models/operations/invoices.tsapi/spec/packages/aip-client-javascript/src/models/operations/llmCost.tsapi/spec/packages/aip-client-javascript/src/models/operations/meters.tsapi/spec/packages/aip-client-javascript/src/models/operations/planAddons.tsapi/spec/packages/aip-client-javascript/src/models/operations/plans.tsapi/spec/packages/aip-client-javascript/src/models/operations/subscriptions.tsapi/spec/packages/aip-client-javascript/src/models/operations/tax.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip-client-javascript/src/sdk/addons.tsapi/spec/packages/aip-client-javascript/src/sdk/apps.tsapi/spec/packages/aip-client-javascript/src/sdk/billing.tsapi/spec/packages/aip-client-javascript/src/sdk/currencies.tsapi/spec/packages/aip-client-javascript/src/sdk/customers.tsapi/spec/packages/aip-client-javascript/src/sdk/defaults.tsapi/spec/packages/aip-client-javascript/src/sdk/entitlements.tsapi/spec/packages/aip-client-javascript/src/sdk/events.tsapi/spec/packages/aip-client-javascript/src/sdk/features.tsapi/spec/packages/aip-client-javascript/src/sdk/governance.tsapi/spec/packages/aip-client-javascript/src/sdk/invoices.tsapi/spec/packages/aip-client-javascript/src/sdk/llmCost.tsapi/spec/packages/aip-client-javascript/src/sdk/meters.tsapi/spec/packages/aip-client-javascript/src/sdk/planAddons.tsapi/spec/packages/aip-client-javascript/src/sdk/plans.tsapi/spec/packages/aip-client-javascript/src/sdk/sdk.tsapi/spec/packages/aip-client-javascript/src/sdk/subscriptions.tsapi/spec/packages/aip-client-javascript/src/sdk/tax.tsapi/spec/packages/aip-client-javascript/tests/client.spec.tsapi/spec/packages/aip-client-javascript/tests/errors.spec.tsapi/spec/packages/aip-client-javascript/tests/index-exports.spec.tsapi/spec/packages/aip-client-javascript/tests/meters.spec.tsapi/spec/packages/aip-client-javascript/tests/nesting.spec.tsapi/spec/packages/aip-client-javascript/tests/paginate.spec.tsapi/spec/packages/aip-client-javascript/tests/wire-helpers.tsapi/spec/packages/aip-client-javascript/tests/wire.spec.tsapi/spec/packages/aip-client-javascript/tsconfig.jsonapi/spec/packages/aip-client-javascript/vitest.config.tsapi/spec/packages/aip/tspconfig.yamlapi/spec/packages/typespec-typescript/scripts/gen-runtime-templates.mjsapi/spec/packages/typespec-typescript/src/ZodOperations.tsxapi/spec/packages/typespec-typescript/src/casing-gate.tsapi/spec/packages/typespec-typescript/src/emitter.tsxapi/spec/packages/typespec-typescript/src/http-status.tsapi/spec/packages/typespec-typescript/src/input-variants.tsapi/spec/packages/typespec-typescript/src/interface-types.tsapi/spec/packages/typespec-typescript/src/lib.tsapi/spec/packages/typespec-typescript/src/pagination.tsapi/spec/packages/typespec-typescript/src/readme.tsapi/spec/packages/typespec-typescript/src/runtime-templates.tsapi/spec/packages/typespec-typescript/src/runtime/wire.tsapi/spec/packages/typespec-typescript/src/sdk-files.tsapi/spec/packages/typespec-typescript/src/sdk-operations.tsapi/spec/packages/typespec-typescript/src/ts-types.tsapi/spec/packages/typespec-typescript/src/utils.tsxapi/spec/packages/typespec-typescript/src/visibility.tsapi/spec/packages/typespec-typescript/src/zodBaseSchema.tsxapi/spec/packages/typespec-typescript/templates/runtime/config.tsapi/spec/packages/typespec-typescript/templates/runtime/core.tsapi/spec/packages/typespec-typescript/templates/runtime/encodings.tsapi/spec/packages/typespec-typescript/templates/runtime/errors.tsapi/spec/packages/typespec-typescript/templates/runtime/paginate.tsapi/spec/packages/typespec-typescript/templates/runtime/request.tsapi/spec/packages/typespec-typescript/templates/runtime/to-error.tsapi/spec/packages/typespec-typescript/templates/runtime/types.tsapi/spec/packages/typespec-typescript/templates/runtime/version.tsapi/spec/packages/typespec-typescript/templates/tests/client.spec.tsapi/spec/packages/typespec-typescript/templates/tests/errors.spec.tsapi/spec/packages/typespec-typescript/templates/tests/meters.spec.tsapi/spec/packages/typespec-typescript/templates/tests/nesting.spec.tsapi/spec/packages/typespec-typescript/vitest.config.ts
💤 Files with no reviewable changes (12)
- api/spec/packages/aip-client-javascript/.npmignore
- api/spec/packages/aip-client-javascript/src/sdk/governance.ts
- api/spec/packages/aip-client-javascript/src/funcs/currencies.ts
- api/spec/packages/aip-client-javascript/src/funcs/governance.ts
- api/spec/packages/aip-client-javascript/src/sdk/currencies.ts
- api/spec/packages/aip-client-javascript/src/sdk/invoices.ts
- api/spec/packages/aip-client-javascript/tsconfig.json
- api/spec/packages/typespec-typescript/scripts/gen-runtime-templates.mjs
- api/spec/packages/aip-client-javascript/src/models/operations/invoices.ts
- api/spec/packages/aip-client-javascript/src/funcs/invoices.ts
- api/spec/packages/aip-client-javascript/src/models/operations/currencies.ts
- api/spec/packages/aip-client-javascript/src/models/operations/governance.ts
Review follow-up: the publish-time SDK_VERSION rewrite silently no-opped if the version.ts format drifted, publishing a stale version in the User-Agent header. Also clarify in AGENTS.md that a missing JSDoc description on a generated method is a spec-authoring gap, not an emitter bug.
Contributor
Author
… SDK Operations marked x-internal (without x-private) in the TypeSpec source were omitted from the generated TypeScript SDK entirely. They are now emitted but quarantined under client.internal.<group>.<method>: a new src/sdk/internal.ts holds an Internal aggregate with Internal<Group> facade classes, while internal operations share their group's funcs/ and models/operations/ modules with the public surface. A group whose operations are all internal (currencies) gets no public facade or getter, and the Internal class is not re-exported at the package root (the name belongs to the Internal Server Error model). x-private operations remain fully omitted. The README gains an Internal Operations section, and a fifth generated conformance test (tests/internal.spec.ts) pins the internal routing and the public-surface quarantine. Newly surfaced operations: create-subscription-addon, list-currencies, create-custom-currency, list-cost-bases, create-cost-basis.
Contributor
Author
writeOutput returns a promise the emitter never awaited, so $onEmit resolved before the generated files landed on the host. The tsp CLI masks the race because the process outlives the pending writes, but in-memory compilation (the emitter test harness) observes a partially written output directory.
The emitter previously had no way to test generator behavior before regenerating the real client: the conformance specs run against the committed aip-client-javascript output only. test/emit.ts builds an EmitterTester (createTester from @typespec/compiler/testing) that compiles a fixture spec in-memory, runs the built emitter through the compiler's real emit pipeline, and returns the emitted files for assertion. test/internal-surface.test.ts uses it to pin the customer-visibility partitioning: x-private operations are dropped (alone or combined with x-internal), x-internal operations are quarantined under client.internal.* with pagination companions, entirely-internal groups get no public facade or getter, funcs and operation envelope modules are shared between surfaces, and the README documents the internal section. The package test script now builds first, since the tester resolves the emitter through its package.json entry point (dist/). The harness runs in CI via make -C api/spec test in the aip-npm-release workflow.
Contributor
|
Too many files changed for review. ( Bypass the limit by tagging |
tothandras
enabled auto-merge (squash)
July 13, 2026 08:43
borbelyr-kong
approved these changes
Jul 13, 2026
This was referenced Jul 14, 2026
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
2 participants
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.
Summary
Implements the fixes from the
@openmeter/clientrelease-readiness review: correctness bugs in the generated SDK, emitter robustness, developer experience, packaging, and CI gates.Correctness
toWirematerializes required-with-default fields, so the minimal documentedevents.ingest()call carriesspecversionand is accepted by the server (previously always rejected with 400)UnsafeIntegerErrorbeyond the JSON-safe range) and revive on responses — previouslyJSON.stringifycrashedexport type *)x-private/x-internalare omitted from the published SDK; they remain in the OpenAPI output for internal consumersEmitter robustness
isSuccessStatusresolver (was four divergent copies with different'*'semantics)z.any()/unknownfallbacks, operations dropped by grouping)Code generated … DO NOT EDIT.headers on every emitted filetemplates/instead of base64 blobs generated from an out-of-repo baseline directoryDeveloper experience
…Inputvariant interfacesPrice,App,Charge, …) are exported, discriminator-narrowable typeslistAll) for page-number and cursor lists, backed by one shared runtime helper with a hard page capHTTPErrorcarriesname/invalidParameters/retryAfter; aUser-Agent: openmeter-node/<version>header is sent, version stamped at publish./zodexport,Result/unwrap, and paginationPackaging & CI
engines, caret dependency ranges,sideEffects: false,filesallowlist, andmain/typesfallbacks addedmake test-api-spec) and gate the npm publish workflowDecisions for reviewers
x-private/x-internalturned out to cover whole domains — invoices (all 4 ops), currencies + cost-bases, governance, andcreate-subscription-addon. The spec's own markers say these "should not be exposed to customers"; removing the markers in the spec is the knob if any were meant to be public.expires_atstaysint64/bigint: an earlier iteration modeled it asutcDateTimewith@encode(unixTimestamp)forDateDX, but that changed the public OpenAPI contract (format: int64→format: unixtime); per adversarial review it was reverted to keep the contract byte-identical — the bigint wire mapping makes the field work correctly as-is.Test plan
make -C api/spec test)make lint-api-specgreen (prettier, spec lint, both tsconfig typechecks)make update-openapion this branch produces zero drift (CI generator gates pass)go veton the goverter converter package;api/v3/openapi.yamlandapi/v3/api.gen.goare byte-identical to mainnpm pack --dry-run: LICENSE + README + dist only, no dev artifacts🤖 Generated with Claude Code
Summary by CodeRabbit
listAll/<method>Allasync iteration across paginated resources, plus cursor/page safety viaPaginationLimitExceededError.tax,features,llmCost,plans,addons,planAddons, anddefaults; addedlistAllhelpers across multiple sub-clients.bigintconversion, required-with-default materialization, and richerHTTPErrorfields (includingretryAfter).Greptile Summary
This PR prepares the AIP TypeScript SDK for release. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "feat(api): emit x-internal operations un..." | Re-trigger Greptile
Context used (3)