Add npm publish preflight to releases#1619
Merged
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
lucid-agents | 1efa97a | Commit Preview URL Branch Preview URL |
Mar 22 2026, 01:16 AM |
Greptile SummaryThis PR adds an npm publish preflight check to both release workflows ( Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CI as GitHub Actions
participant Script as changeset-publish.ts
participant npm as npm CLI
CI->>Script: bun run scripts/changeset-publish.ts --preflight-only
Script->>Script: sanitiseManifest (writes/restores package.json)
Script->>npm: npm whoami
alt auth fails (E401)
npm-->>Script: exit ≠ 0
Script-->>CI: ❌ "NPM_TOKEN is missing or invalid"
else auth succeeds
npm-->>Script: exit 0 (username)
end
loop for each npm scope
Script->>npm: npm view <pkg> version --json
alt package found
npm-->>Script: exit 0
Script->>npm: npm access list collaborators <pkg> --json
alt access denied (E403/E404)
npm-->>Script: exit ≠ 0
Script-->>CI: ❌ "lacks collaborator or publish access"
else access granted
npm-->>Script: exit 0
end
else no published package found
Script-->>Script: warn and skip scope
end
end
Script-->>CI: ✅ "npm publish preflight succeeded."
Note over CI,Script: If not --preflight-only, continues to<br/>bun x changeset publish
Prompt To Fix All With AIThis is a comment left during a code review.
Path: scripts/changeset-publish.ts
Line: 307-333
Comment:
**Output buffering breaks real-time CI streaming**
The `exec` function was changed from `stdout: "inherit"` / `stderr: "inherit"` to `stdout: "pipe"` / `stderr: "pipe"`, with output flushed only after the subprocess exits. For quick commands like `npm whoami` this is harmless, but `bun x changeset publish` publishes every package in the monorepo sequentially — potentially running for several minutes — and CI logs will show nothing until the entire command completes.
If the publish stalls, hangs, or is killed by the 30-minute job timeout, there will be zero diagnostic output in the log. Consider streaming output in real time while still capturing it for error reporting, e.g. via a `PassThrough` tee or by writing to the log in chunks as each line arrives.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: scripts/changeset-publish.ts
Line: 251-278
Comment:
**Package sanitization runs unnecessarily during `--preflight-only`**
`runPublish` writes (and then restores) modified `package.json` files before calling `verifyNpmPublishAccess()`. When `--preflight-only` is passed, this disk I/O is entirely unnecessary — the sanitization step exists to prepare manifests for publishing, not to check credentials.
More importantly, if `sanitiseManifest` throws (e.g. an unresolved workspace dependency), the preflight step will fail with a manifest error instead of an npm auth error, which is confusing.
Consider skipping the sanitization block when `parsedArgs.preflightOnly` is true:
```typescript
async function runPublish() {
const parsedArgs = partitionPublishArgs(process.argv.slice(2));
const backups: Backup[] = [];
const sanitisedPackages: string[] = [];
if (!parsedArgs.preflightOnly) {
for (const pkg of packages) {
// … sanitisation …
}
}
// …
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: .github/workflows/release-bot.yml
Line: 55-59
Comment:
**Preflight fires unconditionally on every push to `master`**
Unlike `release.yml` where the preflight is gated by `steps.metadata.outputs.should_publish == 'true'`, this step has no condition. It will run on every push to `master` even when there are no pending changesets and the `changesets/action` step would not publish anything. This adds two npm API calls (`npm whoami` + `npm access list collaborators`) on every merge to `master`.
More practically, repositories forked without `NPM_TOKEN` configured will see this step fail on every push, which is noisy. Consider adding a guard or at least documenting the intentional unconditional behaviour.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: "fix release npm publ..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+307
to
333
| async function exec( | ||
| argv: string[], | ||
| opts: { allowFailure?: boolean } = {} | ||
| ): Promise<ExecResult> { | ||
| const proc = Bun.spawn(argv, { | ||
| cwd: repoRoot, | ||
| stdin: "inherit", | ||
| stdout: "inherit", | ||
| stderr: "inherit", | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const code = await proc.exited; | ||
| if (code !== 0) { | ||
|
|
||
| const [stdout, stderr, code] = await Promise.all([ | ||
| proc.stdout ? new Response(proc.stdout).text() : "", | ||
| proc.stderr ? new Response(proc.stderr).text() : "", | ||
| proc.exited, | ||
| ]); | ||
|
|
||
| if (stdout) process.stdout.write(stdout); | ||
| if (stderr) process.stderr.write(stderr); | ||
|
|
||
| const output = [stdout, stderr].filter(Boolean).join("\n"); | ||
| if (code !== 0 && !opts.allowFailure) { | ||
| throw new Error(`${argv.join(" ")} exited with code ${code}`); | ||
| } | ||
|
|
||
| return { code, output }; | ||
| } |
There was a problem hiding this comment.
The exec function was changed from stdout: "inherit" / stderr: "inherit" to stdout: "pipe" / stderr: "pipe", with output flushed only after the subprocess exits. For quick commands like npm whoami this is harmless, but bun x changeset publish publishes every package in the monorepo sequentially — potentially running for several minutes — and CI logs will show nothing until the entire command completes.
If the publish stalls, hangs, or is killed by the 30-minute job timeout, there will be zero diagnostic output in the log. Consider streaming output in real time while still capturing it for error reporting, e.g. via a PassThrough tee or by writing to the log in chunks as each line arrives.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/changeset-publish.ts
Line: 307-333
Comment:
**Output buffering breaks real-time CI streaming**
The `exec` function was changed from `stdout: "inherit"` / `stderr: "inherit"` to `stdout: "pipe"` / `stderr: "pipe"`, with output flushed only after the subprocess exits. For quick commands like `npm whoami` this is harmless, but `bun x changeset publish` publishes every package in the monorepo sequentially — potentially running for several minutes — and CI logs will show nothing until the entire command completes.
If the publish stalls, hangs, or is killed by the 30-minute job timeout, there will be zero diagnostic output in the log. Consider streaming output in real time while still capturing it for error reporting, e.g. via a `PassThrough` tee or by writing to the log in chunks as each line arrives.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
251
to
+278
| @@ -191,8 +272,28 @@ async function runPublish() { | |||
| } | |||
|
|
|||
| try { | |||
| const extraArgs = process.argv.slice(2); | |||
| await exec(["bun", "x", "changeset", "publish", ...extraArgs]); | |||
| await verifyNpmPublishAccess(); | |||
| if (parsedArgs.preflightOnly) { | |||
| console.log("npm publish preflight succeeded."); | |||
| return; | |||
There was a problem hiding this comment.
--preflight-only
runPublish writes (and then restores) modified package.json files before calling verifyNpmPublishAccess(). When --preflight-only is passed, this disk I/O is entirely unnecessary — the sanitization step exists to prepare manifests for publishing, not to check credentials.
More importantly, if sanitiseManifest throws (e.g. an unresolved workspace dependency), the preflight step will fail with a manifest error instead of an npm auth error, which is confusing.
Consider skipping the sanitization block when parsedArgs.preflightOnly is true:
async function runPublish() {
const parsedArgs = partitionPublishArgs(process.argv.slice(2));
const backups: Backup[] = [];
const sanitisedPackages: string[] = [];
if (!parsedArgs.preflightOnly) {
for (const pkg of packages) {
// … sanitisation …
}
}
// …
}Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/changeset-publish.ts
Line: 251-278
Comment:
**Package sanitization runs unnecessarily during `--preflight-only`**
`runPublish` writes (and then restores) modified `package.json` files before calling `verifyNpmPublishAccess()`. When `--preflight-only` is passed, this disk I/O is entirely unnecessary — the sanitization step exists to prepare manifests for publishing, not to check credentials.
More importantly, if `sanitiseManifest` throws (e.g. an unresolved workspace dependency), the preflight step will fail with a manifest error instead of an npm auth error, which is confusing.
Consider skipping the sanitization block when `parsedArgs.preflightOnly` is true:
```typescript
async function runPublish() {
const parsedArgs = partitionPublishArgs(process.argv.slice(2));
const backups: Backup[] = [];
const sanitisedPackages: string[] = [];
if (!parsedArgs.preflightOnly) {
for (const pkg of packages) {
// … sanitisation …
}
}
// …
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+55
to
+59
| - name: Preflight npm publish access | ||
| env: | ||
| NPM_TOKEN: ${{ secrets.NPM_TOKEN }} | ||
| NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} | ||
| run: bun run scripts/changeset-publish.ts --preflight-only |
There was a problem hiding this comment.
master
Unlike release.yml where the preflight is gated by steps.metadata.outputs.should_publish == 'true', this step has no condition. It will run on every push to master even when there are no pending changesets and the changesets/action step would not publish anything. This adds two npm API calls (npm whoami + npm access list collaborators) on every merge to master.
More practically, repositories forked without NPM_TOKEN configured will see this step fail on every push, which is noisy. Consider adding a guard or at least documenting the intentional unconditional behaviour.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/release-bot.yml
Line: 55-59
Comment:
**Preflight fires unconditionally on every push to `master`**
Unlike `release.yml` where the preflight is gated by `steps.metadata.outputs.should_publish == 'true'`, this step has no condition. It will run on every push to `master` even when there are no pending changesets and the `changesets/action` step would not publish anything. This adds two npm API calls (`npm whoami` + `npm access list collaborators`) on every merge to `master`.
More practically, repositories forked without `NPM_TOKEN` configured will see this step fail on every push, which is noisy. Consider adding a guard or at least documenting the intentional unconditional behaviour.
How can I resolve this? If you propose a fix, please make it concise.
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
1 participant
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.
Summary
Add an npm publish preflight to the release workflows so bad npm credentials fail before versioning or publish steps run.
Refactor scripts/changeset-publish.ts to support --preflight-only, probe scope access with npm, and translate common npm auth and permission failures into clearer errors.
Add unit coverage for the new publish-argument parsing and npm failure classification helpers.
Verification
bun test scripts/tests/changeset-publish-utils.test.ts
LUCID_SKIP_NPM_PUBLISH_PREFLIGHT=1 bun run scripts/changeset-publish.ts --preflight-only