feat: add pnpm stage command#11863
Conversation
|
💖 Thanks for opening this pull request! 💖 |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
f68a3ff to
8c11b4d
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new ChangesStaged Publishing Feature
Sequence Diagram(s)The sequence diagrams visualizing CLI wiring and stage publish flow are embedded in the hidden review artifact above. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds support for npm’s staged publishing workflow to pnpm by introducing a new pnpm stage command (publish/list/view/approve/reject/download) and wiring staged publishing into the existing pack/publish pipeline.
Changes:
- Introduces a new
stagecommand in@pnpm/releasing.commandsand registers it in the pnpm CLI/help output. - Extends the publish flow to support staged publishing (including returning
stageIdfrom the registry when available). - Updates dependencies/lockfiles (notably
libnpmpublish→^11.2.0) and adds Jest coverage for the new command.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| releasing/commands/src/stage.ts | Implements pnpm stage subcommands and registry interactions (including tarball download + summarization). |
| releasing/commands/test/stage.test.ts | Adds integration-style tests covering stage publish/list/view/approve/reject/download behaviors. |
| releasing/commands/src/publish/publishPackedPkg.ts | Adds stage option support, sets staged publish headers/options, and captures stageId in summaries. |
| releasing/commands/src/publish/recursivePublish.ts | Adjusts recursive publish argv to reflect stage publish when staging recursively. |
| releasing/commands/src/index.ts | Exports the new stage command module. |
| releasing/commands/package.json | Adds workspace dependency needed for auth header computation. |
| pnpm/src/cmd/index.ts | Registers stage as a top-level pnpm command. |
| pnpm/src/cmd/help.ts | Adds stage to the generated help command list. |
| pnpm-workspace.yaml | Bumps libnpmpublish to ^11.2.0. |
| pnpm-lock.yaml | Updates lockfile for libnpmpublish bump and new workspace dependency. |
| .changeset/stage-publish.md | Declares a minor release for pnpm and @pnpm/releasing.commands due to the new command. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@releasing/commands/src/stage.ts`:
- Around line 522-525: The JSON parsing in the stream.on('end') handler can
throw synchronously and escape the promise rejection path; wrap the manifest =
JSON.parse(Buffer.concat(chunks).toString()) call in a try/catch inside the
stream.on('end') callback (the block handling header.name ===
'package/package.json') and call reject(err) with the parse error (or emit an
error) so malformed package.json leads to the promise being rejected instead of
throwing.
In `@releasing/commands/test/stage.test.ts`:
- Around line 205-215: The server callback that calls handler(request) must
guard against rejections so responses are always closed: in the async
createServer callback (the function that reads the body into request and then
calls handler(request)), wrap the await handler(request) call in try/catch; on
catch call writeResponse(res, { status: 500, body: String(err) }) (or a minimal
error response) and ensure the response is ended so the test doesn't stall, then
rethrow or log as needed to make the test fail fast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 82f445dc-86ef-472f-8893-9a480eb25068
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/stage-publish.mdpnpm-workspace.yamlpnpm/src/cmd/help.tspnpm/src/cmd/index.tsreleasing/commands/package.jsonreleasing/commands/src/index.tsreleasing/commands/src/publish/publishPackedPkg.tsreleasing/commands/src/publish/recursivePublish.tsreleasing/commands/src/stage.tsreleasing/commands/test/stage.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
pnpm/src/cmd/help.tspnpm/src/cmd/index.tsreleasing/commands/src/publish/recursivePublish.tsreleasing/commands/src/index.tsreleasing/commands/src/publish/publishPackedPkg.tsreleasing/commands/src/stage.tsreleasing/commands/test/stage.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use instanceof Error for error type checking in Jest tests; use util.types.isNativeError() instead in TypeScript
Files:
releasing/commands/test/stage.test.ts
🧠 Learnings (2)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
pnpm/src/cmd/help.tspnpm/src/cmd/index.tsreleasing/commands/src/publish/recursivePublish.tsreleasing/commands/src/index.tsreleasing/commands/src/publish/publishPackedPkg.tsreleasing/commands/src/stage.tsreleasing/commands/test/stage.test.ts
📚 Learning: 2026-05-05T23:03:04.286Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11479
File: __utils__/scripts/package.json:6-9
Timestamp: 2026-05-05T23:03:04.286Z
Learning: The pattern cross-env NODE_OPTIONS="$NODE_OPTIONS ..." in package.json scripts is an established convention in the pnpm/pnpm repository and is used across many packages (e.g., fs/hard-link-dir, worker, __utils__/scripts). Do not flag this as a cross-platform issue in individual files; if a change is needed, apply it as a repo-wide change in a separate PR. Scope this guidance to all package.json files in the repo; use the minimatch pattern '**/package.json' to identify relevant files and review changes at the repository level rather than per-file.
Applied to files:
releasing/commands/package.json
🪛 OpenGrep (1.21.0)
releasing/commands/src/stage.ts
[ERROR] 513-513: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (8)
.changeset/stage-publish.md (1)
1-7: LGTM!pnpm-workspace.yaml (1)
223-223: LGTM!pnpm/src/cmd/help.ts (1)
247-250: LGTM!pnpm/src/cmd/index.ts (1)
22-22: LGTM!Also applies to: 170-170
releasing/commands/package.json (1)
56-56: LGTM!releasing/commands/src/index.ts (1)
4-4: LGTM!releasing/commands/src/publish/publishPackedPkg.ts (1)
19-19: LGTM!Also applies to: 50-50, 78-79, 91-91, 109-109, 113-126, 158-165, 195-195, 198-198, 201-201, 224-224, 230-233
releasing/commands/src/publish/recursivePublish.ts (1)
116-116: LGTM!Also applies to: 132-132
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.34632760892,
"stddev": 0.12467107940163871,
"median": 2.32572393792,
"user": 2.77029644,
"system": 3.45115922,
"min": 2.21027521542,
"max": 2.62886738542,
"times": [
2.31601781342,
2.33543006242,
2.21027521542,
2.62886738542,
2.47413014142,
2.2434375044199997,
2.36410399042,
2.24673936642,
2.28908081742,
2.3551937924199997
]
},
{
"command": "pacquet@main",
"mean": 2.3584218758199995,
"stddev": 0.07505238960390247,
"median": 2.35054293292,
"user": 2.74734154,
"system": 3.45497102,
"min": 2.21893259242,
"max": 2.45845517742,
"times": [
2.34877093842,
2.39555470442,
2.3505328944199997,
2.35055297142,
2.2773870444199997,
2.4460219844199997,
2.31815619242,
2.41985425842,
2.21893259242,
2.45845517742
]
},
{
"command": "pnpm",
"mean": 4.586416998719999,
"stddev": 0.06651876924969248,
"median": 4.55849862492,
"user": 7.73859044,
"system": 4.004200320000001,
"min": 4.50713754542,
"max": 4.68165852142,
"times": [
4.50713754542,
4.67069628042,
4.54944307142,
4.55099878242,
4.51491857942,
4.65008052942,
4.5358256274199995,
4.56599846742,
4.63741258242,
4.68165852142
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6834689917400001,
"stddev": 0.04215535469085949,
"median": 0.6710384172400001,
"user": 0.37682859999999996,
"system": 1.46836914,
"min": 0.6571289197400001,
"max": 0.80086996074,
"times": [
0.80086996074,
0.6906054557400001,
0.66779901474,
0.6571289197400001,
0.6711350947400001,
0.67327586874,
0.6651990157400001,
0.67094173974,
0.67372750274,
0.66400734474
]
},
{
"command": "pacquet@main",
"mean": 0.71966333734,
"stddev": 0.07674080257156707,
"median": 0.6705865562400001,
"user": 0.3796667,
"system": 1.4665477399999998,
"min": 0.6574542427400001,
"max": 0.8581749537400001,
"times": [
0.82095047574,
0.6688616027400001,
0.78165365074,
0.65872084174,
0.6596287337400001,
0.6723115097400001,
0.8581749537400001,
0.7512405587400001,
0.6574542427400001,
0.66763680374
]
},
{
"command": "pnpm",
"mean": 2.43741995024,
"stddev": 0.13496512892093623,
"median": 2.43106602674,
"user": 2.9883540999999996,
"system": 2.1754937400000003,
"min": 2.3016253507399997,
"max": 2.76096254374,
"times": [
2.45256325274,
2.3016253507399997,
2.3419058757399998,
2.76096254374,
2.44309885374,
2.4632299297399998,
2.41903319974,
2.30520294074,
2.52354253674,
2.3630350187399998
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.9572168311999985,
"stddev": 0.08905605159296778,
"median": 4.953987027599999,
"user": 6.66269576,
"system": 3.5060043399999996,
"min": 4.7784364425999994,
"max": 5.0627103086,
"times": [
5.0048889895999995,
4.9300923716,
4.916477297599999,
5.0612300686,
4.8753279186,
4.950904222599999,
5.0627103086,
5.0350308596,
4.7784364425999994,
4.957069832599999
]
},
{
"command": "pacquet@main",
"mean": 4.908264446099999,
"stddev": 0.08621498153805772,
"median": 4.894483470099999,
"user": 6.65164056,
"system": 3.48834294,
"min": 4.761247742599999,
"max": 5.058843449599999,
"times": [
4.761247742599999,
4.9873054906,
4.954674294599999,
4.9680369876,
4.903364250599999,
4.8478955116,
4.883983885599999,
5.058843449599999,
4.8856026896,
4.8316901586
]
},
{
"command": "pnpm",
"mean": 6.4037400266999995,
"stddev": 0.10666186957213439,
"median": 6.4347946946,
"user": 10.283634760000002,
"system": 4.37835534,
"min": 6.205159064599999,
"max": 6.5298714875999995,
"times": [
6.2403401056,
6.3650332056,
6.205159064599999,
6.409753010599999,
6.5147670826,
6.450027308599999,
6.4358669666,
6.452859612599999,
6.5298714875999995,
6.4337224226
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 3.9936052031200004,
"stddev": 0.1474983433733279,
"median": 3.97796665072,
"user": 4.3003547399999995,
"system": 2.2231088,
"min": 3.82039515022,
"max": 4.327191943220001,
"times": [
4.034314220220001,
3.90861067322,
4.327191943220001,
4.083571254220001,
3.9291066572199997,
4.04809333522,
4.026826644220001,
3.91789525022,
3.8400469032199998,
3.82039515022
]
},
{
"command": "pacquet@main",
"mean": 3.95815218142,
"stddev": 0.12478903631253288,
"median": 3.95481471172,
"user": 4.28708624,
"system": 2.2235854999999995,
"min": 3.8112523172199997,
"max": 4.11234773622,
"times": [
3.88000103622,
3.85990417622,
4.058871609220001,
3.84290075822,
4.07830347522,
3.8112523172199997,
4.029628387220001,
4.11234773622,
4.0883765182200005,
3.81993580022
]
},
{
"command": "pnpm",
"mean": 4.23083682812,
"stddev": 0.0945740783956454,
"median": 4.21113651872,
"user": 5.24638144,
"system": 2.6317564000000004,
"min": 4.11295795722,
"max": 4.45664392022,
"times": [
4.21601483422,
4.1621256112200005,
4.18480532822,
4.26414166022,
4.11295795722,
4.45664392022,
4.275152016220001,
4.26195276522,
4.1683159852200005,
4.20625820322
]
}
]
} |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
releasing/commands/src/stage.ts (1)
412-438: 💤 Low valueConsider consolidating
stageRequestparameters into an options object.This function has 6 parameters. Per coding guidelines, functions should have no more than two or three arguments; use a single options object instead for multiple parameters.
♻️ Suggested refactor
+interface StageRequestOptions { + opts: StageOptions + registry: string + url: string + init: StageRequestInit + action: string + otp?: string +} + async function stageRequest ( - opts: StageOptions, - registry: string, - url: string, - init: StageRequestInit, - action: string, - otp?: string + { opts, registry, url, init, action, otp }: StageRequestOptions ): Promise<Response> {As per coding guidelines: "Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@releasing/commands/src/stage.ts` around lines 412 - 438, The function stageRequest currently takes six positional parameters; refactor it to accept a single options object (e.g., StageRequestParams) that contains { opts: StageOptions, registry: string, url: string, init: StageRequestInit, action: string, otp?: string }, update the function signature and body to destructure those fields, and adjust all call sites accordingly; ensure types are exported/declared, preserve existing logic that uses createFetchFromRegistry, getAuthHeader, throwIfOtpRequired, and throwStageRegistryError, and keep the same request construction (headers, method, timeout) and error behavior after migration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@releasing/commands/src/stage.ts`:
- Around line 412-438: The function stageRequest currently takes six positional
parameters; refactor it to accept a single options object (e.g.,
StageRequestParams) that contains { opts: StageOptions, registry: string, url:
string, init: StageRequestInit, action: string, otp?: string }, update the
function signature and body to destructure those fields, and adjust all call
sites accordingly; ensure types are exported/declared, preserve existing logic
that uses createFetchFromRegistry, getAuthHeader, throwIfOtpRequired, and
throwStageRegistryError, and keep the same request construction (headers,
method, timeout) and error behavior after migration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 54818116-586d-4cfe-9907-3384b04f47fc
📒 Files selected for processing (2)
releasing/commands/src/stage.tsreleasing/commands/test/stage.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Agent
- GitHub Check: Compile & Lint
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
releasing/commands/test/stage.test.tsreleasing/commands/src/stage.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use instanceof Error for error type checking in Jest tests; use util.types.isNativeError() instead in TypeScript
Files:
releasing/commands/test/stage.test.ts
🧠 Learnings (1)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
releasing/commands/test/stage.test.tsreleasing/commands/src/stage.ts
🔇 Additional comments (26)
releasing/commands/src/stage.ts (15)
1-27: LGTM!
29-52: LGTM!
54-73: LGTM!Also applies to: 74-153
155-184: LGTM!
186-215: LGTM!
217-251: LGTM!
253-290: LGTM!
292-310: LGTM!
312-356: LGTM!
358-373: LGTM!
374-410: LGTM!
440-511: LGTM!
540-545: LGTM!
513-539: LGTM!Also applies to: 546-576
578-635: LGTM!releasing/commands/test/stage.test.ts (11)
1-28: LGTM!
29-60: LGTM!
62-112: LGTM!
114-117: LGTM!
119-164: LGTM!
166-207: LGTM!
209-230: LGTM!
233-243: LGTM!
256-261: LGTM!
245-255: LGTM!Also applies to: 262-271
273-300: LGTM!
7e961c5 to
2197e2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@releasing/commands/src/publish/publishPackedPkg.ts`:
- Around line 112-120: The staged-publish path calls context.publish() directly
and thus bypasses the web-auth/doneUrl wrapper in publishWithOtpHandling(),
causing staged publishes to miss browser-based auth; change the logic so that
when isStage is true you still call publishWithOtpHandling({ context, manifest:
publishedManifest, publishOptions, tarballData, /* new flag */ skipOtpRetries:
true }) (or add an equivalent parameter to publishWithOtpHandling) so OTP
retry/prompt behavior is suppressed but the web-auth flow is preserved; update
publishWithOtpHandling signature and StagePublishResponse typing as needed to
accept and propagate the skipOtpRetries flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 21f12109-21fd-422c-bf39-46a8bc95970f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.changeset/stage-publish.mdpnpm-workspace.yamlpnpm/src/cmd/help.tspnpm/src/cmd/index.tsreleasing/commands/package.jsonreleasing/commands/src/index.tsreleasing/commands/src/publish/publishPackedPkg.tsreleasing/commands/src/publish/recursivePublish.tsreleasing/commands/src/stage.tsreleasing/commands/test/stage.test.tsreleasing/commands/tsconfig.json
✅ Files skipped from review due to trivial changes (2)
- .changeset/stage-publish.md
- releasing/commands/tsconfig.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
releasing/commands/src/index.tsreleasing/commands/src/publish/recursivePublish.tspnpm/src/cmd/index.tspnpm/src/cmd/help.tsreleasing/commands/test/stage.test.tsreleasing/commands/src/publish/publishPackedPkg.tsreleasing/commands/src/stage.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use instanceof Error for error type checking in Jest tests; use util.types.isNativeError() instead in TypeScript
Files:
releasing/commands/test/stage.test.ts
🧠 Learnings (2)
📚 Learning: 2026-05-05T23:03:04.286Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11479
File: __utils__/scripts/package.json:6-9
Timestamp: 2026-05-05T23:03:04.286Z
Learning: The pattern cross-env NODE_OPTIONS="$NODE_OPTIONS ..." in package.json scripts is an established convention in the pnpm/pnpm repository and is used across many packages (e.g., fs/hard-link-dir, worker, __utils__/scripts). Do not flag this as a cross-platform issue in individual files; if a change is needed, apply it as a repo-wide change in a separate PR. Scope this guidance to all package.json files in the repo; use the minimatch pattern '**/package.json' to identify relevant files and review changes at the repository level rather than per-file.
Applied to files:
releasing/commands/package.json
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
releasing/commands/src/index.tsreleasing/commands/src/publish/recursivePublish.tspnpm/src/cmd/index.tspnpm/src/cmd/help.tsreleasing/commands/test/stage.test.tsreleasing/commands/src/publish/publishPackedPkg.tsreleasing/commands/src/stage.ts
🪛 OpenGrep (1.21.0)
releasing/commands/src/stage.ts
[ERROR] 529-529: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (8)
pnpm-workspace.yaml (1)
223-223: LGTM!Also applies to: 395-395, 436-436
pnpm/src/cmd/help.ts (1)
247-250: LGTM!pnpm/src/cmd/index.ts (1)
22-22: LGTM!Also applies to: 169-170
releasing/commands/package.json (1)
56-56: LGTM!releasing/commands/src/index.ts (1)
4-4: LGTM!releasing/commands/src/publish/recursivePublish.ts (1)
116-116: LGTM!Also applies to: 132-132
releasing/commands/src/stage.ts (1)
63-65: Resolve:--registryis accepted forpnpm stagevia forwarded publish CLI option types
releasing/commands/src/stage.tsforwardscliOptionsTypes()topublishCommand.cliOptionsTypes(), andpublish.ts’scliOptionsTypes()is built fromrcOptionsTypes();publish.tsalready includes theregistryoption in its option types.releasing/commands/test/stage.test.ts (1)
1-300: LGTM!
- Type stageId via OtpPublishResponse so publishPackedPkg no longer needs a cast. - Hoist fetchFromRegistry + auth header into a per-subcommand StageContext. - Send npm-auth-type: web on all stage requests, not just approve/reject. - Consolidate stageRequest / stageRequestWithOtp / stageJsonRequest into (context, params) form.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
releasing/commands/src/publish/publishPackedPkg.ts (1)
113-120:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the web-auth wrapper on the staged publish path.
This branch still skips
publishWithOtpHandling(), so staged publishes bypass thedoneUrl/browser-auth flow thatcreatePublishContext()is explicitly wired to support. Users without a preconfigured token can still failpnpm stage publishinstead of being taken through web auth.Suggested shape
- const response = isStage - ? await context.publish(publishedManifest, tarballData, publishOptions) - : await publishWithOtpHandling({ - context, - manifest: publishedManifest, - publishOptions, - tarballData, - }) + const response = await publishWithOtpHandling({ + context, + manifest: publishedManifest, + publishOptions, + tarballData, + skipOtpRetries: isStage, + })Then keep OTP prompting/retries disabled inside
publishWithOtpHandling()whenskipOtpRetriesis set, but still run the existing web-auth flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@releasing/commands/src/publish/publishPackedPkg.ts` around lines 113 - 120, The staged publish path currently calls context.publish directly and bypasses the web-auth wrapper; change the isStage branch so it calls publishWithOtpHandling(...) as well, passing the same context, publishedManifest, tarballData and publishOptions but set skipOtpRetries: true for staged publishes so OTP prompting/retries stay disabled while still running the createPublishContext/browser-auth/doneUrl flow; ensure publishWithOtpHandling is invoked for both staged and non-staged publishes (only toggle skipOtpRetries when isStage).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@releasing/commands/src/publish/publishPackedPkg.ts`:
- Around line 113-120: The staged publish path currently calls context.publish
directly and bypasses the web-auth wrapper; change the isStage branch so it
calls publishWithOtpHandling(...) as well, passing the same context,
publishedManifest, tarballData and publishOptions but set skipOtpRetries: true
for staged publishes so OTP prompting/retries stay disabled while still running
the createPublishContext/browser-auth/doneUrl flow; ensure
publishWithOtpHandling is invoked for both staged and non-staged publishes (only
toggle skipOtpRetries when isStage).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d3d3ecc-5530-425f-8097-d758e4ed9806
📒 Files selected for processing (3)
releasing/commands/src/publish/otp.tsreleasing/commands/src/publish/publishPackedPkg.tsreleasing/commands/src/stage.ts
✅ Files skipped from review due to trivial changes (1)
- releasing/commands/src/publish/otp.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run benchmark on ubuntu-latest
- GitHub Check: Analyze (javascript)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use trailing commas in TypeScript code
Prefer functions over classes in TypeScript code
Declare functions after they are used, relying on function hoisting in TypeScript
Functions should have no more than two or three arguments; use a single options object instead for multiple parameters in TypeScript
Follow import order in TypeScript: 1) standard libraries, 2) external dependencies (alphabetically sorted), 3) relative imports
Write self-explanatory code in TypeScript; avoid comments that restate what the code already says. Use comments only for non-obvious reasons, hidden invariants, or workarounds
Use JSDoc for function contracts (preconditions, postconditions, edge cases), not for re-narrating the function body in TypeScript
Files:
releasing/commands/src/stage.tsreleasing/commands/src/publish/publishPackedPkg.ts
🧠 Learnings (1)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
releasing/commands/src/stage.tsreleasing/commands/src/publish/publishPackedPkg.ts
🔇 Additional comments (1)
releasing/commands/src/stage.ts (1)
227-238: LGTM!Also applies to: 255-259, 265-270, 276-282, 288-293, 350-429
|
Bencher Report| Branch | pr/11863 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | milliseconds (ms) |
|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 4,957.22 ms |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 3,993.61 ms |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,346.33 ms |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 683.47 ms |
The registry responds to stage approve/reject with 401 and a body of
`{ authUrl, doneUrl }` when the user must complete a browser-based
authentication, but `www-authenticate` does not contain "otp" in that
case. The previous check missed this and surfaced the response as a
generic STAGE_REGISTRY_ERROR. Detect web-auth responses by body shape so
withOtpHandling can drive the polling flow.
Add tests that lock in the OTP-trigger detection on the stage command:
- 401 with `{authUrl, doneUrl}` enters the web-auth flow, exercised here
via the full polling-completion path (registry returns a token, the
retry request carries it as npm-otp).
- 401 with web-auth body but no TTY surfaces as OTP_NON_INTERACTIVE.
- 401 without any OTP signals stays a STAGE_REGISTRY_ERROR so we don't
over-trigger the OTP flow on unrelated unauthorized responses.
Calling `context.publish()` directly for staged publishes bypassed `publishWithOtpHandling`, so users without a preconfigured token had no path through the browser-based authentication flow on `pnpm stage publish`. Route the staged publish through the same wrapper as the regular publish; `OtpPublishResponse.stageId` carries the registry's identifier when set.
The 631-line `stage.ts` carried six subcommands, tarball parsing,
auth/request plumbing, error class, OTP detection, and rendering helpers
in one file. Reorganized to match the existing `publish/` folder layout:
- `stage/{index,help,publish,list,view,approve,reject,download,context,
request,parsing,rendering,errors,types}.ts` — one concept per file.
- `tarball/{publishSummary,summarizeTarball}.ts` — shared between
`publish` and `stage` instead of duplicated. `PublishSummary` and
`extractBundledDependencies` now live with the tarball helper rather
than inside the publish subfolder, so other commands can reuse them
without reaching into `publish/`.
Behavior unchanged. Also dropped `StageRegistryError`'s redundant
`statusCode` field (was identical to `status`) to bring it in line with
`FailedToPublishError`.
|
Thank you so much for helping get this over the line, lmk if I can do anything. |
|
Congrats on merging your first pull request! 🎉🎉🎉 |
`pnpm stage publish` ships natively in pnpm v11.3.0 (May 2026, PR pnpm/pnpm#11863) with full subcommand parity to `npm stage`. The project is already pinned to `pnpm@11.3.0` in `packageManager`, so the feature is available without a version bump. * Replace the four `npm stage publish` invocations with their `pnpm` equivalents — flags (`--no-git-checks`, `--tag next|beta|alpha`) are identical. * Drop the two-step `npm install -g npm@~11.10.0 && npm install -g npm@11.15.0` workaround for npm/cli#9151; only pnpm is needed in the publish job now. * Single package manager across the workflow; `pnpm stage publish` also rewrites `workspace:*` correctly should the project ever move to a workspace layout.
* chore: enable staged publishing Also bumps all the actions using a sha rather than a mutable tag. * chore: pr feedback * ci(release): use native pnpm stage publish `pnpm stage publish` ships natively in pnpm v11.3.0 (May 2026, PR pnpm/pnpm#11863) with full subcommand parity to `npm stage`. The project is already pinned to `pnpm@11.3.0` in `packageManager`, so the feature is available without a version bump. * Replace the four `npm stage publish` invocations with their `pnpm` equivalents — flags (`--no-git-checks`, `--tag next|beta|alpha`) are identical. * Drop the two-step `npm install -g npm@~11.10.0 && npm install -g npm@11.15.0` workaround for npm/cli#9151; only pnpm is needed in the publish job now. * Single package manager across the workflow; `pnpm stage publish` also rewrites `workspace:*` correctly should the project ever move to a workspace layout. --------- Co-authored-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
Note
This is largely authored by GPT5.5 based on the
npm/cliPR's, afterwards it needed some guidance to better adhere to the repository standards.Resolves #11796
Summary
Implements npm's newly released staged publishing flow in pnpm, following npm/cli#9379 and including the
stage download --jsonpackage-name keying fix from npm/cli#9380/#9381.I ran into some issues where
workspace:*was kept around when usingnpm stage publishreplacing my regularpnpm publish. Hence why I wanted to try and contribute this.Adds
pnpm stagewith these subcommands:pnpm stage publish [<tarball>|<dir>]— stage a package for publishing without OTP promptingpnpm stage list [<package-spec>]— list staged package versionspnpm stage view <stage-id>— inspect one staged packagepnpm stage approve <stage-id>— approve and publish a staged package, with OTP handlingpnpm stage reject <stage-id>— reject a staged package, with OTP handlingpnpm stage download <stage-id>— download the staged tarball for inspectionImplementation notes
stage publishand passesstage: truethrough tolibnpmpublish.libnpmpublishto^11.2.0, which contains the npm staged-publish endpoint support.stage publishfree of OTP handling; OTP remains deferred to approve/reject.stageIdinstage publish --jsonwhen the registry returns it.stage download --jsonoutput by package name, matching the npm follow-up fixes and avoiding anundefinedtop-level key.@pnpm/releasing.commandsandpnpm.Summary by CodeRabbit
New Features
pnpm stagecommand with subcommands:publish,list,view,approve,reject,downloadTests
stagepublish (dry-run and JSON), list, view, approve/reject, and download behaviorsChores