ReviewOS

also looking at this

pantry-pm/pantry

fix(publish): an accepted npm upload no longer fails on a slow readback

#259
Merged glennmichael123 wants to merge fix/245-npm-publish-confirm into main
3 files +117 -11
4 days ago

Closes #245.

The problem

pantry publish --npm exited 1 when npm accepted the upload but confirmPublication did not see the version within its window. The version lands anyway, so the only effect was to skip every release step after the publish — Build Release Binaries, Create GitHub Release — for a version that went live a minute later. And because the message says not to upload it again, re-running the workflow was not a safe recovery. (stacksjs/bun-query-builder Releaser run 39184733210, v0.3.14.)

Why the window was so short

npm_publish_timeout_ms (2 min) bounded both the upload socket and the public readback. Its own comment is about the upload — "bounding a socket whose peer accepted bytes and then stopped responding" — so the readback was inheriting a limit chosen for something else.

The batch path had already hit this and been raised to 10 minutes, with a comment recording that npm took longer than two minutes for two of stx 0.2.612's four runtime packages. The immediate readback now gets the same treatment:

  • npm_readback_timeout_ms = 600_000, separate from the upload timeout.
  • --confirm-timeout <seconds> overrides it, for a registry that is slower still — npm's propagation reportedly reaches ~25 minutes on Craft releases. Seconds at the CLI because publishers discuss this in minutes.
  • The exponential backoff asked for in the issue already existed inside awaitPublicationTask (1s → 8s cap); only the window was wrong.

Accepted ⇒ completed

An accepted upload now completes at exit 0 whether or not npm has shown it yet. It prints ✓ Uploaded rather than ✓ Published, and carries the pending line as a warning instead of an Error:. The text is byte-identical: publish_plan.zig documents that release tooling (Pantry's own npm readback, stx's publish retry) recognises a pending upload by that exact string, and a test pins it.

This is deliberately not applied to an upload whose outcome is unknown — a socket that died after the bytes, or a 5xx/redirect over a committed write. Those still fail, because there the version may never appear and a release built on it would reference nothing. The line is: npm told us this run's upload was accepted.

An integrity mismatch is also not "pending": confirmPublication returns error.PublicationIntegrityMismatch rather than false when the public tarball is not the one we sent, and that error propagates to a failure. What reaches confirmed = false is only "npm has not shown it yet".

I did not add the issue's third suggestion, --verify-only. Its purpose is to let a workflow re-run without re-uploading; once a pending publish no longer fails, there is nothing to re-run.

A note on the tests

The two tests added here — one pinning that the readback window is a distinct, larger value than the upload timeout, one asserting a pending publish exits 0 — did not run at first, and the suite totals were identical before and after adding them. lib.zig keeps an explicit list of files whose tests to pull in because refAllDecls does not, and cli/commands/package.zig was never on it: its five existing tests had been dead since they were written. That is also why publish_plan.zig and publish_conflict.zig exist as separate files — testable logic was moved out of package.zig rather than making its tests run. lib.zig now names it, so those five run too.

That last part is the one thing a reviewer should look at closely: five previously-unrun tests are now live, and CI on this PR is the first full run of them.

🤖 Generated with Claude Code

3 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.