Skip to content

fix(release): publish the FFI tarballs by path, not GitHub shorthand - #921

Merged
coderdan merged 1 commit into
mainfrom
fix/ffi-publish-tarball-path
Aug 19, 2026
Merged

fix(release): publish the FFI tarballs by path, not GitHub shorthand#921
coderdan merged 1 commit into
mainfrom
fix/ffi-publish-tarball-path

Conversation

@coderdan

Copy link
Copy Markdown
Contributor

What happened

The 2.0.0 release run (#859's merge) failed in Publish FFI packages on the first platform tarball:

npm error command git --no-replace-objects ls-remote ssh://git@github.com/ffi-dist/cipherstash-protect-ffi-darwin-arm64-0.32.0.tgz.git
npm error git@github.com: Permission denied (publickey).

npm publish received the bare relative path ffi-dist/<name>.tgz. npm's spec parser classifies an owner/repo-shaped argument as a GitHub shorthand before it considers it a file, so it tried to git ls-remote a repository named after the tarball. A spec is only treated as a local tarball when it starts with ./, ../, / or file:.

Fix

One line: npm publish --access public --provenance "./$tgz", with a comment explaining why the ./ is load-bearing. Nothing else in the step uses the path as a package spec (the tar reads and npm view probes are unaffected).

What this does and doesn't prove

  • The run died before npm ever talked to the registry, so trusted publishing was never exercised — this fix is necessary for the next attempt but not yet proven sufficient. Worth npm trust list <pkg> on the seven FFI packages before retrying (per AGENTS.md, each configuration must also allow npm publish).
  • ffi-preflight.yml couldn't have caught this: it dry-runs changeset publish, and this custom step has no dry-run path.

Companion to #920 (which re-arms the release as 1.1.0). This must merge before the next Version Packages PR does, or the release fails the same way again.

The first 2.0.0 release attempt failed in `Publish FFI packages` on the
very first tarball:

  npm error command git --no-replace-objects ls-remote
    ssh://git@github.com/ffi-dist/cipherstash-protect-ffi-darwin-arm64-0.32.0.tgz.git

`npm publish` was handed the bare relative path
`ffi-dist/<name>.tgz`, and npm's spec parser classifies an
`owner/repo`-shaped argument as a GitHub shorthand before it considers
it a file — so it tried to clone a repository named after the tarball
and died on SSH auth. A spec is only treated as a local tarball when it
starts with `./`, `../`, `/` or `file:`.

Prefix the path with `./`. Nothing else in the step is affected: the
`tar` reads and `npm view` probes never used the path as a package
spec. Trusted publishing was never exercised by the failure — the run
died before npm talked to the registry — so this fix is necessary for
the next release attempt but not yet proven sufficient; check
`npm trust list` on the seven packages before retrying (AGENTS.md,
"Publishing has moved here").

Note the workflow filename must not change: npm trusted publishing for
all seven FFI packages binds to (repository, workflow filename).
@coderdan
coderdan requested a review from a team as a code owner August 19, 2026 06:29
@changeset-bot

changeset-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 55f0f67

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderdan
coderdan requested a review from freshtonic August 19, 2026 06:31

@freshtonic freshtonic 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.

Approve. Correct diagnosis, minimal fix, and the failure log itself is the proof: npm literally attempted git ls-remote ssh://git@github.com/ffi-dist/cipherstash-protect-ffi-darwin-arm64-0.32.0.tgz.git, which is unambiguously the owner/repo shorthand misparse. The ./ prefix is the documented way npm-package-arg distinguishes a file spec (./, ../, /, file:) from a hosted-git shorthand.

What I verified:

  • $tgz is always the bare misparse-prone shape: it comes from the ffi-dist/*.tgz glob, so every iteration produced exactly the dir/file.tgz form npm classifies as GitHub shorthand. ./$tgz fixes every path through the loop — platform packages and wrapper alike.
  • Nothing else in the step uses the path as a package spec, as the PR body claims: meta()'s tar xzOf "$1" takes a filesystem path, and npm view takes name@version. This is the only npm publish in any workflow (grepped .github/), so there's no second copy of the bug waiting in ffi-preflight.yml or elsewhere.
  • The re-run story holds: the step is idempotent per tarball (npm view skip-guard), so the next release attempt completes the set rather than tripping on anything the failed 2.0.0 run may have left behind — nothing was published, since the run died before npm reached the registry.
  • The comment at the call site is accurate and earns its place — this is precisely the kind of one-character load-bearing detail that gets "cleaned up" later without it.
  • No changeset needed (workflow-only), and the honesty about scope is right: this fix is necessary but unproven-sufficient, since trusted publishing was never exercised. The npm trust list check on all seven packages before the retry — flagged here and in AGENTS.md — remains the other half of de-risking the next attempt.

Agreed on merge order: this must land before the next Version Packages merge, or the release dies identically.

@coderdan
coderdan merged commit 679a432 into main Aug 19, 2026
11 checks passed
@coderdan
coderdan deleted the fix/ffi-publish-tarball-path branch August 19, 2026 06:43
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