chore(release): gate SDK tests in CI and fix protocol publish target - #163
Merged
Conversation
Two release-hygiene gaps found while auditing #161. CI never ran the SDK regression suites. @cossistant/{react,core,types,next, browser} had no `test` script, so `turbo run test` skipped them and the suites only ever ran locally. #161 adds the `test` scripts; this adds the CI step that consumes them, so the provider-lifecycle, data-hook and embed regressions gate merges instead of relying on someone running `bun test` by hand. The step is a no-op until #161 lands (no `test` task to run, so turbo executes nothing and passes), and becomes meaningful the moment it does — 473 tests across the five packages. @cossistant/protocol's pub:* scripts used bare `npm publish`, which ignores publishConfig.directory. With `files: ["dist"]` the tarball root package.json still pointed main/exports at ./src/*.ts, which is excluded from the tarball, so all 12 entry points resolved to missing files. #161 fixed this for react, next and browser; #161's follow-up commit fixed core, types and tiny-markdown. protocol is the last one. `changeset publish` (the CI release path) was never affected: it honours publishConfig.directory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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.
Two release-hygiene gaps found while auditing #161. Both are small and independent of that PR's review.
1. CI never ran the SDK regression suites
@cossistant/{react,core,types,next,browser}have notestscript, soturbo run testskipped them entirely — the suites only ever ran locally, by hand.#161 adds the
testscripts. This adds the CI step that consumes them, so the provider-lifecycle, data-hook and embed regressions gate merges.Ordering is safe either way:
testtask exists yet, so turbo executes nothing and the step passes. Verified green against currentmain.The step is deliberately not in #161: that branch's
ci.ymlpredates the protocol steps from #162, so editing it there produces a merge conflict and would cost #161 its clean merge status.2.
@cossistant/protocolpublishes a broken tarballpub:*used barenpm publish, which ignorespublishConfig.directory. Combined withfiles: ["dist"], the tarball rootpackage.jsonstill pointedmain/exportsat./src/*.ts— excluded from the tarball — so all 12 entry points resolved to missing files.#161 fixed this for
react/next/browser; its follow-up commit fixedcore/types/tiny-markdown.protocolis the last one.To be clear about blast radius:
changeset publish— the CI release path — was never affected. It resolvespublishConfig.directoryand callsnpm publish <dir>. This only ever broke a manualbun run pub:release.Verification
All 7 CI gates pass on this branch against current
main:check-types,check:openapi, protocol tests, the new SDK test step, the REST OpenAPI contract test, browser embed build, example app build.Follow-up not included here
Re-enabling the browser embed size gate. It's currently disabled, and
mainis +65.2 KB gzip over the recorded baseline. #161 brings the widget back to +3.3 KB, so the gate becomes enforceable again — but only once #161 has landed, since the baselines have to reflect the post-merge build. Tracked separately.🤖 Generated with Claude Code