Skip to content

feat: add publishing npm package flow - #139

Open
limitofzero wants to merge 76 commits into
mainfrom
feat/implement-npm-publish-flow
Open

limitofzero wants to merge 76 commits into
mainfrom
feat/implement-npm-publish-flow

Conversation

@limitofzero

@limitofzero limitofzero commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a publishing pipeline for @cowprotocol/solana-settlement-client (the TS client generated from the settlement program's IDL) and finishes making its package.json actually publishable — the base branch's version had a non-scoped name, no publishConfig, and main pointing at raw .ts source with no build step producing anything importable.

New workflows

auto-release.yml — triggered on push to main. Diffs Cargo.toml's version against the previous commit; a no-op if unchanged (regular PRs don't touch it). If changed:

  • Fails immediately if programs/settlement/idl/client/js/package.json's version wasn't bumped to match — before creating anything, since an immutable tag pinned to a commit that can never actually publish would be worse than failing loudly here.
  • Creates the GitHub Release, pinned to the triggering commit (--target "$GITHUB_SHA").
  • Explicitly dispatches publish-npm.yml (gh workflow run --ref "$tag"). This has to be explicit: GitHub suppresses the release event for releases created by GITHUB_TOKEN (anti-recursion protection), so the normal release: published trigger would never fire otherwise. Dispatching against the tag (not main) keeps the same commit pinned through to the build.

publish-npm.yml — two jobs. build: checks out the tagged commit, runs just build-js-client (Codama codegen + tsup) and just test-js-client, re-verifies the tag matches the package version, and writes a review summary (npm pack --dry-run --ignore-scripts output, plus a diff of dependencies against the currently-published version). publish: gated behind the npm-publish GitHub Environment's required-reviewer approval, then runs npm publish --provenance --access public --ignore-scripts. Authentication is npm Trusted Publishing (OIDC) — no long-lived npm token is stored or used in the normal flow; NPM_TOKEN is only a bootstrap fallback for the package's very first publish (see setup section below). --ignore-scripts on both steps means no lifecycle script ever runs with credentials in scope — what gets published is exactly the artifact reviewed in build.

package.json / Justfile / ci.yml

  • Renamed the package cow-solana-settlement-client@cowprotocol/solana-settlement-client (scoped under the org, matching @cowprotocol/contracts on the EVM side).
  • Added publishConfig, repository, author; main/types now point at dist/index.js/dist/index.d.ts (the actual build output) instead of raw src/index.ts.
  • Added a build script (tsup) and a tsup devDependency — there was no way to produce a distributable artifact before this.
  • Justfile: added build-js-client (depends on the existing generate-js-client), since nothing built the publishable package before.
  • ci.yml: added a "Build JS client" step to the existing test-js-client job, so a broken build is caught on every PR, not only at release time.
  • .gitignore: added dist/ and .idea/.

Manual setup required before this can run for real (repo admin, one-time)

  • Create the npm-publish GitHub Environment with required reviewers.
  • Trusted Publisher configuration lives on the package's own npmjs.com settings page, which doesn't exist until the package has been published at least once — so bootstrap it:
    1. Generate an npm access token (short-lived is fine, it's only needed once) and add it as the NPM_TOKEN secret on the npm-publish environment.
    2. Let one release run through the normal flow end-to-end (bump PR → auto-release.ymlpublish-npm.yml → approve) — this publish uses NPM_TOKEN.
    3. On npmjs.com, open the now-existing package's settings and add a Trusted Publisher for this repo, workflow publish-npm.yml, and environment npm-publish.
    4. Delete the NPM_TOKEN secret. Every publish after this point authenticates via OIDC automatically — no workflow changes needed.
  • Confirm Settings → Actions → General grants the default GITHUB_TOKEN read/write permissions (needed for auto-release.yml to create releases and dispatch workflows).

kaze-cow and others added 30 commits July 28, 2026 17:19
using newer version of litesvm nodejs lib, and settlement seed now has
to be imported dynamically from the IDL because of version bumping
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
* add comments for settlement instruction and validate match
* simplify superfluous comments in the IDL in general
* switch to using `LazyLock` and update call sites
it increases the amount of code overall, but it puts us in the right
trajectory to be effectively generating parts of the IDL from rust.
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
Co-authored-by: Federico Giacon <58218759+fedgiac@users.noreply.github.com>
@limitofzero
limitofzero marked this pull request as ready for review September 2, 2026 20:02
@limitofzero
limitofzero requested a review from a team as a code owner September 2, 2026 20:02

@kaze-cow kaze-cow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its difficult to review this PR in completeness because I wasn't able to run a test release myself and its mostly CI. If there is some way we could have a call or something to review the release process that would be great.

considering the rest of our releases in this repo don't currently use an automated release, I am questioning whether we should be going through all the effort to have it for node.js

Comment thread README.md Outdated
Comment on lines +132 to +134
Publishing itself requires a manual approval in the `npm-publish` GitHub Environment. Before approving, check the job summary the workflow posts: it lists the exact tarball contents about to be published and a dependency diff against the previously published version. Approve only if both look as expected for the changes in this release.

Authentication to npm uses [Trusted Publishing](https://docs.npmjs.com/trusted-publishers) (OIDC) — no long-lived npm token is stored. This requires a one-time setup on npmjs.com *after* the package's first publish (a Trusted Publisher is configured on the package's own settings page, so it can't be set up before the package exists): add a Trusted Publisher for this exact repo, `publish-npm.yml`, and the `npm-publish` environment, then delete the `NPM_TOKEN` secret — it's a bootstrap-only fallback for that first publish.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not an expert on the latest patterns and strategies in npm publishing. I actually find myself to be more confused after reading these instructions because

For instructions relating to steps that need to be completed one time (ex. after package first publish with the trusted publisher setup), this doesn't need to be documented.

I don't know what a "GitHub Environment" is, and its not explained . While I see its explained on the linked trusted publishing document, maybe just best to simplify this second paragraph to generally say "Authentication to npm uses trusted publishing. Follow the instructions there to set up after first release" or so

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair, I only did half of this. Simplified the github env bit but left the one time bootstrap steps in place since I wasn't sure where else they should live. Where would you rather they go, just drop them entirely and rely on the linked Trusted Publishing docs plus whoever does the bootstrap figuring it out from there or move them somewhere that isn't the permanent readme, like the PR description or a comment in the workflow file? I think it could be fully removed since everything will be configured manually for the first release

Comment thread README.md Outdated
Comment thread programs/settlement/idl/client/js/pnpm-workspace.yaml
],
"peerDependencies": {
"@solana/kit": "^6.10.0"
"@solana/kit": "^8.0.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for our reference, what are your thoughts on peer dependencies? we have a conversation on the original thread on whether we should have them or not (the version bump here is actually being applied there as well)

cc @fedgiac

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good question, and I don't have a strong opinion here either. The case for peer is if a consumer also uses @solana/kit directly, you want one shared instance so types like Address/TransactionSigner line up. The case against, is peer deps have caused real pain before and version drift mostly shows up as install failures for consumers.
right now nothing actually consumes this package yet, cow-sdk/cowswap owns Solana code is still on classic web3.js, not kit, so the shared instance problem doesn't exist in practice today. Given that I'm leaning toward just making it a regular dependency unless either of you sees a concrete reason to keep it as peer.

@fedgiac what do you think, still feel the same way as on #73?

Comment thread programs/settlement/idl/client/js/package.json Outdated
Comment thread .github/workflows/publish-npm.yml Outdated
Comment thread .github/workflows/publish-npm.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/auto-release.yml Outdated
Comment thread .github/workflows/auto-release.yml Outdated
Comment thread programs/settlement/idl/client/js/README.md Outdated
Comment thread programs/settlement/idl/client/js/README.md
Base automatically changed from kaze/sc-255-write-idl-and-generate-corresponding-libraries-for to main September 7, 2026 11:03
@limitofzero
limitofzero requested a review from fedgiac September 7, 2026 18:09

@fedgiac fedgiac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot going on in this PR and I'm not sure I really understood the flow in detail.
I think it would have been better to split it: the workflow with the NPM publishing by itself, then separately the workflow that uses it to do autoreleases on main. (And possibly even all the rest to another PR to avoid distractions!)

Splitting would have also helped the comment prose: right now, there are a lot of cross-referential comments: "see details in the readme" -> "see details in the workflow", "this file is used in this other file." It feels like I need to load everything at the same into my working memory to be able to understand this PR.
Also, the comments are, to me, very difficult to understand. I think I'd have understood the PR more quickly with all comments removed.

Not sure what's Kaze's opinion on this, but would you be able to split this PR and explain each of the two workflows in isolation?
I'm basically unfamiliar with every tool used in this PR (Trusted Publishing?) so limiting the scope of the discussion would help.

Comment thread README.md Outdated
- [Bump the crate version](#bumping-the-crate-version) by a patch version.
- Commit the code changes resulting from the changes above.
- Create a PR with the changes and wait for approval.
- Create a PR with the changes and wait for approval, then merge it. Merging automatically creates a GitHub release (tag `v$VERSION`, e.g. `v0.42.1`) via [`auto-release.yml`](.github/workflows/auto-release.yml), which in turn triggers the npm package publish workflow — see [Publishing the npm package](#publishing-the-npm-package).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, tmi.

Suggested change
- Create a PR with the changes and wait for approval, then merge it. Merging automatically creates a GitHub release (tag `v$VERSION`, e.g. `v0.42.1`) via [`auto-release.yml`](.github/workflows/auto-release.yml), which in turn triggers the npm package publish workflow — see [Publishing the npm package](#publishing-the-npm-package).
- Create a PR with the changes and wait for approval, then merge it. Merging automatically creates a GitHub release (tag `v$VERSION`, e.g. `v0.42.1`) and [publishes the npm package](#publishing-the-npm-package).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread README.md Outdated
Comment on lines +130 to +134
The TS/JS client (`@cowprotocol/solana-settlement-client`, generated from `programs/settlement/idl/cow_settlement.json` via Codama) is published automatically by [`publish-npm.yml`](.github/workflows/publish-npm.yml) whenever a GitHub release is cut — its version must already match the release tag (see [Bumping the crate version](#bumping-the-crate-version), which bumps it alongside the crates). The release itself is also created automatically, by [`auto-release.yml`](.github/workflows/auto-release.yml), as soon as a version-bump PR merges into `main` — see the [Breaking change](#breaking-change) and [Patch update](#patch-update) flows above. Merging the bump PR is the only manual step left before a release goes out; npm publishing still needs manual approval (below).

Publishing requires manual approval: `publish-npm.yml`'s publish step runs under a [GitHub Environment](https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment) named `npm-publish`, configured in repo Settings → Environments with required reviewers. Before approving, check the job summary the workflow posts: it lists the exact tarball contents about to be published and a dependency diff against the previously published version. Approve only if both look as expected for the changes in this release.

Authentication to npm uses [Trusted Publishing](https://docs.npmjs.com/trusted-publishers) — no stored npm token. One-time setup after the package's *first* publish (it can't be configured before the package exists): on npmjs.com, add a Trusted Publisher for this repo, `publish-npm.yml`, and the `npm-publish` environment, then delete the `NPM_TOKEN` secret.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this way too much for a readme? What I need to know here:

Suggested change
The TS/JS client (`@cowprotocol/solana-settlement-client`, generated from `programs/settlement/idl/cow_settlement.json` via Codama) is published automatically by [`publish-npm.yml`](.github/workflows/publish-npm.yml) whenever a GitHub release is cut — its version must already match the release tag (see [Bumping the crate version](#bumping-the-crate-version), which bumps it alongside the crates). The release itself is also created automatically, by [`auto-release.yml`](.github/workflows/auto-release.yml), as soon as a version-bump PR merges into `main` — see the [Breaking change](#breaking-change) and [Patch update](#patch-update) flows above. Merging the bump PR is the only manual step left before a release goes out; npm publishing still needs manual approval (below).
Publishing requires manual approval: `publish-npm.yml`'s publish step runs under a [GitHub Environment](https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment) named `npm-publish`, configured in repo Settings → Environments with required reviewers. Before approving, check the job summary the workflow posts: it lists the exact tarball contents about to be published and a dependency diff against the previously published version. Approve only if both look as expected for the changes in this release.
Authentication to npm uses [Trusted Publishing](https://docs.npmjs.com/trusted-publishers) — no stored npm token. One-time setup after the package's *first* publish (it can't be configured before the package exists): on npmjs.com, add a Trusted Publisher for this repo, `publish-npm.yml`, and the `npm-publish` environment, then delete the `NPM_TOKEN` secret.
The TS/JS client ([`@cowprotocol/solana-settlement-client`](programs/settlement/idl/client/js/README.md)) is published automatically to NPM when a GitHub release is cut.

Authentication to npm uses Trusted Publishing — no stored npm token. One-time setup after the package's first publish (it can't be configured before the package exists): on npmjs.com, add a Trusted Publisher for this repo, publish-npm.yml, and the npm-publish environment, then delete the NPM_TOKEN secret.

This is nice information but belongs to the action's yaml file, not the readme.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cut down to roughly your suggestion, and the Trusted Publishing paragraph moved into publish-npm.yml as you asked. I put it on the npm publish step itself rather than the file header, since it's really explaining the NODE_AUTH_TOKEN right below it. Two things I kept in the readme line beyond your version: "with a manual approval step", since that's worth knowing before you cut a release and there's nothing in the readme otherwise saying the publish pauses, and links to the two workflows.

Comment thread .gitignore Outdated
Comment thread .gitignore Outdated

Building a `createOrder` instruction:

```typescript

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to do in this PR, just linking this to #141, we can implement that by just moving this code into its own folder.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, that would work well for #141. Worth noting for whoever picks it up: I did typecheck this snippet against the built package while writing it, by dropping it in as a temp file and running tsc. That's how I caught that resolveOrderPda wasn't exported from the entry point at all

- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
with:
node-version: "24"
registry-url: https://registry.npmjs.org

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to specify the registry URL here and not in other places where we used the action?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's only about token auth, not OIDC, and the publish job is the only one that authenticates.
registry-url`is what makes setup-node write an .npmrc binding NODE_AUTH_TOKEN to that registry. The other three call sites only install and build, so there's nothing to authenticate and no reason to write one. Because it's only about token auth, not OIDC, and the publish job is the only one that authenticates
Your question caught a real error though: the comment I had on that line claimed the url was needed for OIDC, which is wrong. Fixed

Comment thread .github/workflows/auto-release.yml Outdated
Comment thread .github/workflows/publish-npm.yml Outdated
Comment thread .github/workflows/publish-npm.yml Outdated
Comment on lines +109 to +112
# A workflow_dispatch with an empty `tag` is documented as a build-only dry
# run (skips the version check above) — it must not be able to reach an
# actual `npm publish` just because someone approves the environment gate.
if: github.event_name == 'release' || github.event.inputs.tag != ''

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't exactly say way but this was very difficult to understand.

I think something that makes this file hard to understand is that there are three flows woven together, each overlapping, but not so much that it's clearly visible, and each flow has its own quirks. Since they're so important in the design, it's something that would be helpful to explicitly state at the start, give them a name, and then in the comments actually reference to the specific flow.
The flows are:

  1. Tagged workflow_dispatch
  2. Manual GitHub release
  3. Dry run (through workflow_dispatch)

This underpins a lot of random details in this file and should be made clear and ideally referenced to when needed for brevity. It would be very good if flows 1 and 2 could be merged but I doubt this is possible.

What this comment is trying to say: don't publish in the dry-run flow only. But instead it's reexplaining what the dry run is and talks about version checks and approvals that are irrelevant here: there is no version to check and there's no approval in a dry run because it's an empty workflow dispatch that can only be triggered by hand.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made the flows part of the code rather than comments,which I think answers this better.

if [ "$GITHUB_EVENT_NAME" = release ]; then
  flow=manual-release
  ...
elif [ -n "$INPUT_TAG" ]; then
  flow=tagged-dispatch
  ...
else
  flow=dry-run

Comment thread .github/workflows/publish-npm.yml Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants