Skip to content

MILAB-7081: tell a new version from a new package with npm 10+ - #221

Merged
mike-ainsel merged 1 commit into
v4-betafrom
MILAB-7081_npm-view-new-version
Oct 20, 2026
Merged

mike-ainsel merged 1 commit into
v4-betafrom
MILAB-7081_npm-view-new-version

Conversation

@mike-ainsel

@mike-ainsel mike-ainsel commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

pack-unpublished decides per package whether name@version is published, a new version, or a package npmjs does not have yet (the last is refused before any script runs, because a trusted publisher needs an existing package).

It assumed npm view name@version version exits 0 with no output for a missing version of an existing package. npm 10+ fails with E404 No match found for version instead — the same code as for a missing package — so every pending version was reported as not on npmjs yet. It hit runenv-python-3 (node-matrix-pnpm packs on GitHub-hosted runners with npm 11); block repos pack on hz runners with an older npm and were not affected.

Fix: on E404, run npm view <name> name; the package exists → new version, E404 → new package, anything else → error. npm < 10 keeps working (exit 0, no output → new version).

Tests: the fake registry now answers like npm 10+ by default; added a test for npm < 10 and one for the npmjs case. lib/ rebuilt.

RetriggerView in GreptileConfidence Score: 4/5

The PR appears safe to merge, with a non-blocking test gap for errors from the second lookup.

Fix All in Claude CodeFindings

  1. P2 Second lookup errors go untested ▶
Fix with agent prompt
### Issue 1
actions/node/npm-split-publish/src/registry.ts:72
The existing `failing` option fails the first lookup, so it never tests this new `CommandError` branch. Add a test where the version lookup returns `E404` and the package lookup returns `E500` or `E403`. Check that the second error is thrown with the package lookup command instead of being treated as a missing package.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR distinguishes a missing version from a missing package when npm returns E404.

  • Registry checks distinguish new package versions from new packages.

Important touched types:

  • VersionState: the three possible registry states: published, new-version, and new-package. Its definition is unchanged; the new lookup improves how the state is chosen.
  • Runner: the function type used to run commands. Its definition is unchanged; versionState can now call it twice.
  • RunResult: a command's exit code, standard output, and error output. Its definition is unchanged; the new isNotFound helper checks its output fields.
  • CommandError: an error containing the failed command and its result. Its definition is unchanged; it now also reports failures from the package lookup.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["Look up name@version"] --> B{"Exit code is zero?"}
  B -->|Yes| C{"Output has a version?"}
  C -->|Yes| D["published"]
  C -->|No| E["new-version"]
  B -->|No| F{"E404?"}
  F -->|No| G["Throw CommandError"]
  F -->|Yes| H["Look up package name"]
  H --> I{"Successful, nonempty output?"}
  I -->|Yes| E
  I -->|No| J{"Successful empty output or E404?"}
  J -->|Yes| K["new-package"]
  J -->|No| G
Loading

Reviews (1) · Last reviewed commit: "MILAB-7081: tell a new version from a ne..." · Reviewed by Greptile

npm 10+ answers 'npm view name@version' with E404 both when the version is missing and when the package is missing, so pack treated every pending version of an existing package as a package new to npmjs and refused the release (runenv-python-3 on GitHub-hosted runners). On E404, look up the package itself.
@notion-workspace

Copy link
Copy Markdown

@mike-ainsel
mike-ainsel merged commit 67118c0 into v4-beta Oct 20, 2026
2 checks passed
return 'new-package';
}
throw new CommandError(['npm', ...args].join(' '), result);
throw new CommandError(['npm', ...packageArgs].join(' '), pkg);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Second lookup errors go untested

The existing failing option fails the first lookup, so it never tests this new CommandError branch. Add a test where the version lookup returns E404 and the package lookup returns E500 or E403. Check that the second error is thrown with the package lookup command instead of being treated as a missing package.

Prompt To Fix With AI
This is a comment left during a code review.
Path: actions/node/npm-split-publish/src/registry.ts
Line: 72

Comment:
**Second lookup errors go untested**

The existing `failing` option fails the first lookup, so it never tests this new `CommandError` branch. Add a test where the version lookup returns `E404` and the package lookup returns `E500` or `E403`. Check that the second error is thrown with the package lookup command instead of being treated as a missing package.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

mike-ainsel added a commit that referenced this pull request Oct 20, 2026
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.

1 participant