feat(sdk): generate and publish @formbricks/hub from this repo (ENG-2092) - #127
Open
xernobyl wants to merge 5 commits into
Open
feat(sdk): generate and publish @formbricks/hub from this repo (ENG-2092)#127xernobyl wants to merge 5 commits into
xernobyl wants to merge 5 commits into
Conversation
Replaces the Stainless-generated SDK, whose platform shuts down on 1 Sep 2026.
See ENG-2092 for why Hey API: of six generators tested against this spec, it was
the only free one that cleared every requirement, and Stainless was the only one
that got our repeated array filters wrong.
The SDK is a build artifact: the generator config is committed, its output is
not. src/generated/ and dist/ are gitignored, produced in CI, and published to
npm on release. That is defensible here because the generator is deterministic
(verified byte-identical across runs), npm provenance names the source commit so
anyone can regenerate and compare, and the tarball ships readable src/.
- 31 flat functions, one per operationId in the spec — no class, so consumers can
tree-shake. tsc --strict clean, dual ESM/CJS with .d.ts for both.
- Zero runtime dependencies. The zod validators live behind a separate
"@formbricks/hub/schemas" entry so importing the client pulls nothing in, and
zod stays an optional peer.
- createHubClient({ apiKey, baseUrl }) as sugar over createClient(createConfig()).
baseUrl is required, not defaulted: the Hub is self-hostable.
- files is an allowlist, not Stainless's "**/*" — generation and packing happen in
a directory that can hold .env and build scratch. Verified: 30 files, nothing
outside dist/src/README/LICENSE.
- Generator version pinned exactly, so a bump is a reviewable change.
tests/wire.test.mjs pins the regression that motivated the migration: array
filters must go out as repeated parameters, never comma-joined. It runs against
the built package rather than src/, so it also covers the exports map. Confirmed
it fails on the old comma-joined output rather than passing regardless.
Version starts at 0.13.0. npm caret ranges pin the minor on 0.x, so `^0.12.0`
consumers stay on 0.12.x and are not dragged into the method renames.
Refs ENG-2092
…on PRs Two workflows, deliberately separate files. publish-sdk.yml runs on a stable Hub release (prereleases skipped, matching the Docker builds) and is split into two jobs so the publish credential never coexists with third-party code execution: - build: installs with --ignore-scripts, generates, type-checks, runs the wire tests, packs, and compares against the published tarball. Holds no id-token. - publish: downloads only that tarball and runs npm publish. No checkout, no install, nothing from the repo or its dependencies executes here. That split matters because the build job runs the generator and its transitive dependencies; a single compromised dev dependency in a job holding id-token could publish arbitrary code as @formbricks/hub with genuine provenance. The compare step is the artifact model's core, not a nicety: with nothing committed, the published tarball is the only baseline. Identical output skips the publish so a Go-only release does not burn a version; changed output against an already-published version fails loudly rather than overwriting. Auth is npm trusted publishing (OIDC) — no token, and no token fallback, unlike the script this replaces. Provenance is automatic for a public package from a public repo. The publish job runs in an npm-publish environment because npm's trusted-publisher binding covers repo and workflow filename but NOT the ref, so the environment's branch protection is what stops any branch publishing. sdk-preview.yml diffs the exported surface against npm on PRs touching the spec or sdks/**. It generates PR-controlled code, so it is pull_request (never pull_request_target), read-only, secret-free, and writes to the job summary rather than a PR comment — no write token anywhere. Commenting would need the artifact + workflow_run pattern, and a careless version of that trips Scorecard's Dangerous-Workflow check, so the summary is the default. No path-filter changes needed: sdks/** matches none of the Go workflows' patterns (and sdks/typescript/tests/ does not match their root-anchored tests/**). Refs ENG-2092
- Deletes .github/workflows/stainless-action.yml. It built the SDK on the Stainless platform on every PR and merge; that platform shuts down on 1 Sep 2026 and publish-sdk.yml replaces it. - Removes the three x-stainless-model extensions from openapi.yaml along with the vendor-specific comments explaining them. Verified they were doing nothing for us: after removal the generated client still has all 31 operations, the recursive TaxonomyNodeData.children survives, and TaxonomyRunData and EnrichmentTypeStatus are still single shared types — named from the spec's own component names, which is what the extensions were overriding. Spectral clean. Refs ENG-2092
#126 (ENG-2369) appends its docs bullets after the same two lines, so both PRs adding there conflicted regardless of the sections being far apart. Anchors them against different neighbours so the two land in either order without a conflict. Refs ENG-2092
… tarball Two problems found reviewing my own workflow. The prerelease guard was a step that ran `exit 1`, which marks every prerelease as a FAILED workflow run rather than skipping it. hub-release.yml sets an output and gates downstream jobs instead; this now gates the job itself, so a prerelease is genuinely skipped. The publish job took a pre-packed .tgz. npm's own docs are inconsistent about whether provenance is generated when publishing a tarball rather than a package directory — the trusted-publishing page says no flag is needed, the provenance page says to pass --provenance and describes the directory flow. A release is not the moment to discover which is right, so the build job now hands over the publishable directory (dist, src, package.json, README, LICENSE) and the publish job runs `npm publish` from it, which is the documented path. --provenance is passed explicitly for the same reason: harmless if already implied, silent if not. Smoke-tested locally: npm publish --dry-run from a reconstructed artifact directory produces the same 30-file, 148 kB tarball as packing in place. The compare step's three branches were exercised against the real registry — differs+new version publishes, differs+published version exits 1, and identical input skips. Refs ENG-2092
xernobyl
marked this pull request as ready for review
August 26, 2026 10:02
|
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
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 |
Contributor
Author
|
Marked ready so CodeRabbit actually reviews it — it skips drafts, so its green tick until now meant "didn't look", not "looks fine". Please don't merge this before the npm trusted-publisher change lands. Nothing here breaks on merge, but the first Hub release after it would try to publish and fail authentication, which is a confusing way to find out. The four values needed are in the description; @mattinannt is doing it since maintainers are Ordering that works: npm repointed → this merges → next Hub release publishes |
Contributor
Author
|
@coderabbitai review |
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
1 participant
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.
What does this PR do?
Replaces the Stainless-generated
@formbricks/hubSDK with one generated here fromopenapi.yamlby@hey-api/openapi-ts, published to npm on a stable Hub release.Linear: https://linear.app/formbricks/issue/ENG-2092 · Decision doc: Hub SDK after Stainless — why Hey API
Why. Stainless shuts down 1 September 2026. Of six generators tested against this spec, Hey API was the only free one that cleared every requirement — and Stainless was the only one of the six that got our repeated array filters wrong, which is why
formbricks/formbrickscarries ~440 lines correcting it.The SDK is a build artifact. The generator config is committed; the output is not.
src/generated/anddist/are gitignored, produced in CI, and published on release. That is defensible here for three specific reasons, and stops being defensible if any one stops holding:src/(verified: 30 files, 144 KB packed).Publishing is credential-free. npm trusted publishing (OIDC) — no token, and unlike the script this replaces, no token fallback. Provenance is automatic for a public package from a public repo.
Two jobs, on purpose. The build job runs the generator and its transitive dependencies and holds no
id-token; a separate publish job downloads only the packed tarball and runsnpm publish. Without that split, one compromised dev dependency could publish arbitrary code as@formbricks/hub— to ~10k downloads/month, carrying genuine provenance, which makes it harder to spot rather than easier.Version starts at 0.13.0 and the SDK version line stays independent of the Hub's (they have never matched — npm was at 0.12.0 while the Hub was at 0.8.5). npm caret ranges pin the minor on
0.x, so^0.12.0consumers stay on 0.12.x and are not dragged into the method renames.Also in here:
stainless-action.ymldeleted, and the threex-stainless-modelextensions removed fromopenapi.yaml— verified they were doing nothing for us (see the commit for what was checked).The rename, for consumers
Resource methods become one flat function per operation, so bundlers can tree-shake:
createHubClient({ apiKey, baseUrl })is provided as sugar;baseUrlis required rather than defaulted because the Hub is self-hostable. Zod validators for every shape live behind@formbricks/hub/schemasso the default entry pulls in zero dependencies andzodstays an optional peer.The full 15-method → 31-function mapping is in the job summary of the
sdk-previewworkflow, and is what ENG-2351 will migrate against.Prerequisite: npm trusted publishing must be repointed (needs @mattinannt)
@formbricks/hub@0.12.0was published via OIDC bound toformbricks/hub-typescript+publish-npm.yml. npm allows one trusted publisher per package, matched on repository and workflow filename, case-sensitive. Until it is repointed, the publish job fails with "Unable to authenticate".On npmjs.com →
@formbricks/hub→ Settings → Trusted Publisher:formbricks/hubpublish-sdk.ymlnpm-publishnpm publishnpm-publishwith a deployment-branch rule (tags / default branch) is what closes that. The workflow already references it.hub-typescript's ability to publish@formbricks/hub, so do it at cutover.@formbricks/hub-mcpis unaffected (separate config, token-published).Maintainers are
formbricks-comandmatthiasnannt; I don't have access.How should this be tested?
All of this runs locally from
sdks/typescript, with no secrets and no database:operationIdinopenapi.yaml, as flat functions with no class.tsc --noEmitreports 0 errors.tests/wire.test.mjsdrives the built package (so it covers the exports map) against a local echo server and asserts{ source_type: ["survey","review"] }goes out as?source_type=survey&source_type=review. I confirmed it fails on the old comma-joined output rather than passing regardless — replay?source_type=survey%2Creviewthrough its assertions and all three reject it.pnpm generatetwice anddiff -r src/generated— must be byte-identical. Everything else depends on this.npm pack --dry-run --json— 30 files, nothing outside thefilesallowlist (dist,src,README.md,LICENSE), and readablesrc/present.x-stainless-modelextensions, still 31 operations,children?: Array<TaxonomyNodeData>intact,TaxonomyRunDataandEnrichmentTypeStatusstill single shared types.npx @stoplight/spectral-cli@6 lint openapi.yaml→ no errors.publish-sdk.ymlvia workflow_dispatch withdry_run: true. The build job should run end to end and the publish job should be skipped.versionback to0.12.0with a changed surface — the build must fail with "version 0.12.0 is already on npm. Bump the version".sdk-previewon this PR should print the surface diff to its job summary and request no write permissions.Not testable before the npm change: the real publish. Treat the 0.9.0 Hub release as the cutover and watch the job.
Checklist
Required
make build— no Go code changedmake tests— not applicable; the SDK has its own suite (pnpm test), which passes 3/3make fmtandmake lint; no new warnings — no Go changes; Prettier run over the new TS/JSON/MDorigin/main, 0 behindAppreciated
openapi.yamlchanged only by removing vendor extensions; Spectral clean, no behaviour changedocs/if changes were necessary —AGENTS.mdgains a TypeScript SDK section; the npm-facing README issdks/typescript/README.mdmake tests-coverage— no Go logic changedNote on conflicts: #126 (ENG-2369, docs off Stainless) also edits
AGENTS.md. I placed this section higher in the file to avoid a conflict, but whichever lands second may still need a trivial rebase.