[replaced] feat: implement native 'pnpm bugs' command#11279
Conversation
01fac6e to
a84955a
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new built-in pnpm bugs command to open a package’s bug tracker (or repository issues page) from the project manifest, integrating it into pnpm’s command registry and release process.
Changes:
- Implements a new
pnpm bugscommand (pnpm/src/cmd/bugs.ts) with help text and URL resolution logic. - Registers the new command in the CLI command index and removes
bugsfrom the “not implemented” set. - Adds a changeset to release the feature as a minor bump.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| pnpm/src/cmd/notImplemented.ts | Removes bugs from the list of commands that error as “not implemented”. |
| pnpm/src/cmd/index.ts | Registers the new bugs command module so it’s available in the CLI. |
| pnpm/src/cmd/bugs.ts | New command implementation: reads manifest, resolves bug tracker URL, and opens it via OS-specific opener. |
| .changeset/pnpm-bugs-command.md | Declares a minor version bump for adding pnpm bugs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6921c14 to
e7275ac
Compare
e7275ac to
b984013
Compare
7a0d1a4 to
b4b1c95
Compare
8edac48 to
d7f9424
Compare
- Add fallback logic in execPnpm.ts to find pnpm binary in multiple locations since import.meta.dirname resolves differently in ESM mode - Fix test to check both stdout and stderr since pnpm outputs errors to stdout in some cases
f1acd46 to
ced8e2a
Compare
ced8e2a to
becd36e
Compare
74a0c02 to
becd36e
Compare
|
@zkochan as soon as you can, could you take a look? Thanks! |
Skip the 'dlx creates cache and store prune cleans cache' test that consistently times out after 180000ms. This is a pre-existing flaky test unrelated to the pnpm bugs feature.
This reverts commit 904eb08.
Replace Promise.all with sequential execution to prevent timeout issues when running multiple dlx commands in parallel during the test.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test('pnpm bugs opens bugs.url when present', async () => { | ||
| tempDir() | ||
| fs.writeFileSync('package.json', JSON.stringify({ | ||
| name: 'test-pkg', | ||
| bugs: { | ||
| url: 'https://github.com/test/pkg/issues', | ||
| }, | ||
| }), 'utf8') | ||
|
|
||
| const result = execPnpmSync(['bugs']) | ||
|
|
||
| expect(result.status).toBe(0) | ||
| }) | ||
|
|
||
| test('pnpm bugs opens bugs string URL', async () => { | ||
| tempDir() | ||
| fs.writeFileSync('package.json', JSON.stringify({ | ||
| name: 'test-pkg', | ||
| bugs: 'https://github.com/test/pkg/issues', | ||
| }), 'utf8') | ||
|
|
||
| const result = execPnpmSync(['bugs']) | ||
|
|
||
| expect(result.status).toBe(0) | ||
| }) | ||
|
|
||
| test('pnpm bugs falls back to repository/issues URL', async () => { | ||
| tempDir() | ||
| fs.writeFileSync('package.json', JSON.stringify({ | ||
| name: 'test-pkg', | ||
| repository: 'https://github.com/test/pkg', | ||
| }), 'utf8') | ||
|
|
||
| const result = execPnpmSync(['bugs']) | ||
|
|
||
| expect(result.status).toBe(0) | ||
| }) |
There was a problem hiding this comment.
These tests only assert exit status and don't verify that URL resolution/normalization is correct (e.g. repository.url with git+https://..., .git suffix, scoped packages when passing a pkg name, etc.). Consider asserting the resolved URL (for example by making the command print the URL it intends to open, or by adding a test-mode flag/env var that prevents opening and outputs the URL).
| "rootDir": "..", | ||
| "isolatedModules": true | ||
| "isolatedModules": true, | ||
| "types": ["jest"] |
There was a problem hiding this comment.
Setting compilerOptions.types overrides TypeScript's default type inclusion. With types: ["jest"], Node globals/module types may be excluded unless they come in indirectly; to avoid fragile typing, consider explicitly including node as well (or keep types unset and ensure Jest types are picked up via typeRoots/deps).
| "types": ["jest"] | |
| "types": ["jest", "node"] |
| const getBugsUrlFromRegistry = async (pkgName: string): Promise<string> => { | ||
| const registry = opts.registries?.default ?? 'https://registry.npmjs.org' | ||
| const url = `${registry.replace(/\/$/, '')}/${pkgName}` | ||
| const response = await fetch(url, { | ||
| headers: { | ||
| 'accept': 'application/json', | ||
| }, | ||
| }) |
There was a problem hiding this comment.
When fetching package metadata from the registry, the package name should be URL-encoded (scoped packages like "@pnpm/core" require encoding the "/" as "%2F"). Currently the request URL will be invalid for scoped package names, so pnpm bugs @scope/name will fail against the npm registry.
Replace Promise.all with sequential execution to prevent timeout. Add eslint-disable comment for no-await-in-loop as sequential execution is required here to avoid overwhelming the system.
Replace Promise.all with sequential execution to prevent timeout. Add eslint-disable comment for no-await-in-loop rule.
|
@zkochan could you take a look at it? Thanks! |
There was a problem hiding this comment.
why was a change in this file needed?
|
|
||
| Added the `pnpm bugs` command that opens the package's bug tracker URL in the browser. | ||
|
|
||
| Fix the `bins.linker` tests to use `process.execPath` for `spawnSync` to avoid empty stdout. |
There was a problem hiding this comment.
don't create one changeset for two unrelated changes. Create separate changeset files.
There was a problem hiding this comment.
how are these changes related to the pnpm bugs command?
There was a problem hiding this comment.
how is this related at all to pnpm bugs command?
| const getBugsUrlFromRegistry = async (pkgName: string): Promise<string> => { | ||
| const registry = opts.registries?.default ?? 'https://registry.npmjs.org' | ||
| const url = `${registry.replace(/\/$/, '')}/${pkgName}` | ||
| const response = await fetch(url, { | ||
| headers: { | ||
| 'accept': 'application/json', | ||
| }, | ||
| }) |
- Normalize repository URLs by stripping git+ prefix and converting git:// to https:// - Encode scoped package names for registry API requests - Differentiate registry error responses (404, 401/403, other) - Propagate execFile errors instead of silently resolving - Remove @pnpm/bins.linker from changeset
2e02f0d to
a505ff6
Compare
📝 WalkthroughWalkthroughThis PR introduces a new Changespnpm bugs Command
Test Infrastructure Improvements
Sequence DiagramssequenceDiagram
actor User
participant CLI as pnpm bugs
participant Manifest as Local Manifest
participant Registry as Package Registry
participant URLHandler as URL Handler
Note over User,URLHandler: Mode 1: No Arguments
User->>CLI: pnpm bugs
CLI->>Manifest: read package.json
Manifest-->>CLI: manifest with bugs/repo
CLI->>URLHandler: normalize & extract bugs URL
URLHandler->>URLHandler: derive from bugs or repository
URLHandler-->>CLI: validated URL
CLI->>URLHandler: open URL
URLHandler-->>User: browser opens
Note over User,URLHandler: Mode 2: Package Names
User->>CLI: pnpm bugs pkg1 pkg2
CLI->>Registry: fetch pkg1 metadata
Registry-->>CLI: manifest
CLI->>URLHandler: extract bugs URL
URLHandler->>URLHandler: normalize & validate
URLHandler-->>CLI: URL
CLI->>Registry: fetch pkg2 metadata
Registry-->>CLI: manifest
CLI->>URLHandler: extract bugs URL
URLHandler-->>CLI: URL
CLI->>URLHandler: open all URLs
URLHandler-->>User: browsers open
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
| throw new PnpmError('NO_MANIFEST_FOUND', `No package.json was found in "${dir}"`) | ||
| } | ||
| const url = pickBugsUrl({ bugs: manifest.bugs, repository: manifest.repository }) | ||
| if (!url) { | ||
| throw new PnpmError( | ||
| 'NO_BUGS_URL', | ||
| 'The package.json does not have a bug tracker URL. Add a "bugs" or "repository" field to your package.json.' |
| if (manifest.repository) { | ||
| const repoUrl = typeof manifest.repository === 'string' ? manifest.repository : manifest.repository.url | ||
| if (repoUrl) { | ||
| const normalized = normalizeRepositoryUrl(repoUrl) | ||
| if (normalized && isHttpUrl(normalized)) return `${normalized}/issues` | ||
| } | ||
| } | ||
| return undefined | ||
| } | ||
|
|
||
| function normalizeRepositoryUrl (url: string): string | undefined { | ||
| let normalized = url.replace(/^git\+/, '').replace(/\.git$/, '') | ||
| if (normalized.startsWith('git://')) { | ||
| normalized = `https://${normalized.slice('git://'.length)}` | ||
| } else if (normalized.startsWith('git@github.com:')) { | ||
| normalized = `https://github.com/${normalized.slice('git@github.com:'.length)}` | ||
| } | ||
| return normalized |
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 `@deps/inspection/commands/src/bugs/index.ts`:
- Around line 85-91: The normalization currently only converts git@github.com:
URLs; update normalizeRepositoryUrl to recognize SSH URLs of the form
git@HOST:PATH (e.g. /^git@([^:]+):(.+)$/) instead of the hardcoded
'git@github.com:' check and rewrite them to https://HOST/PATH (stripping any
trailing '.git' already handled); keep the existing git:// -> https://
conversion and ensure the function returns the normalized string for these
generalized SSH cases (use the captured host and path to build the replacement).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7296328f-b8dc-45ad-99a6-26d46690188a
📒 Files selected for processing (6)
.changeset/pnpm-bugs-command.mddeps/inspection/commands/src/bugs/index.tsdeps/inspection/commands/src/index.tsdeps/inspection/commands/test/bugs.tspnpm/src/cmd/index.tspnpm/src/cmd/notImplemented.ts
💤 Files with no reviewable changes (1)
- pnpm/src/cmd/notImplemented.ts
| function normalizeRepositoryUrl (url: string): string | undefined { | ||
| let normalized = url.replace(/^git\+/, '').replace(/\.git$/, '') | ||
| if (normalized.startsWith('git://')) { | ||
| normalized = `https://${normalized.slice('git://'.length)}` | ||
| } else if (normalized.startsWith('git@github.com:')) { | ||
| normalized = `https://github.com/${normalized.slice('git@github.com:'.length)}` | ||
| } |
There was a problem hiding this comment.
Generalize SSH repository normalization beyond GitHub.
Line 89 only handles git@github.com:. Repos like git@gitlab.com:group/pkg.git won’t normalize, so fallback incorrectly throws ERR_PNPM_NO_BUGS_URL.
Proposed fix
function normalizeRepositoryUrl (url: string): string | undefined {
let normalized = url.replace(/^git\+/, '').replace(/\.git$/, '')
if (normalized.startsWith('git://')) {
normalized = `https://${normalized.slice('git://'.length)}`
- } else if (normalized.startsWith('git@github.com:')) {
- normalized = `https://github.com/${normalized.slice('git@github.com:'.length)}`
+ } else {
+ const scpLike = normalized.match(/^git@([^:]+):(.+)$/)
+ if (scpLike) {
+ normalized = `https://${scpLike[1]}/${scpLike[2]}`
+ }
}
return normalized
}🤖 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 `@deps/inspection/commands/src/bugs/index.ts` around lines 85 - 91, The
normalization currently only converts git@github.com: URLs; update
normalizeRepositoryUrl to recognize SSH URLs of the form git@HOST:PATH (e.g.
/^git@([^:]+):(.+)$/) instead of the hardcoded 'git@github.com:' check and
rewrite them to https://HOST/PATH (stripping any trailing '.git' already
handled); keep the existing git:// -> https:// conversion and ensure the
function returns the normalized string for these generalized SSH cases (use the
captured host and path to build the replacement).
a505ff6 to
efb5440
Compare
|
you are pushing a lot of unrelated changes to this PR. It won't be accepted this way. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pnpm/src/cmd/bugs.ts (1)
146-159: 💤 Low valueConsider adding a timeout to prevent indefinite blocking.
While
open,xdg-open, andstarttypically return immediately, edge cases (e.g., a custom handler that blocks) could cause the command to hang indefinitely.🔧 Optional: Add timeout to execFile
const { execFile } = await import('node:child_process') await new Promise<void>((resolve, reject) => { - execFile(cmd, args, (err) => { + execFile(cmd, args, { timeout: 30000 }, (err) => { if (err) { const errorDetails = [err.code, err.message].filter(Boolean).join(': ') reject(new PnpmError(🤖 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 `@pnpm/src/cmd/bugs.ts` around lines 146 - 159, The execFile invocation that runs cmd with args can hang; update the execFile usage in the Promise (referencing execFile, cmd, args, canonicalUrl, and PnpmError) to enforce a timeout: either pass a timeout option to execFile or capture the returned ChildProcess, start a setTimeout (e.g. 5s), and on timeout kill the child and reject the promise with a PnpmError (use a clear error id like 'OPEN_BUGS_URL_TIMEOUT' and include canonicalUrl and cmd in the message); ensure the timeout is cleared on success or error to avoid leaks.pnpm/test/bugs.test.ts (1)
7-43: 🏗️ Heavy liftStrengthen success-path assertions to validate the resolved URL.
These tests currently verify only exit code. They can still pass if the command opens an incorrect URL. Please assert the actual URL chosen (e.g., by stubbing/capturing opener invocation, or by unit-testing the URL-resolution helper directly).
🤖 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 `@pnpm/test/bugs.test.ts` around lines 7 - 43, Update the three tests (pnpm bugs opens bugs.url when present, pnpm bugs opens bugs string URL, pnpm bugs falls back to repository/issues URL) to assert the actual URL opened instead of only the exit code: stub or spy the module that launches the browser (the opener used by the CLI) before calling execPnpmSync(['bugs']), capture the URL argument passed to that opener, and add expect assertions verifying it equals the resolved URL (e.g., 'https://github.com/test/pkg/issues' for the first two tests and 'https://github.com/test/pkg/issues' for the repository fallback). If there is a URL-resolution helper function used by the command, alternatively test that helper directly by calling it with the same package.json shapes and asserting the returned URL. Ensure you restore the stub/spy after each test.
🤖 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 `@pnpm/src/cmd/bugs.ts`:
- Around line 146-159: The execFile invocation that runs cmd with args can hang;
update the execFile usage in the Promise (referencing execFile, cmd, args,
canonicalUrl, and PnpmError) to enforce a timeout: either pass a timeout option
to execFile or capture the returned ChildProcess, start a setTimeout (e.g. 5s),
and on timeout kill the child and reject the promise with a PnpmError (use a
clear error id like 'OPEN_BUGS_URL_TIMEOUT' and include canonicalUrl and cmd in
the message); ensure the timeout is cleared on success or error to avoid leaks.
In `@pnpm/test/bugs.test.ts`:
- Around line 7-43: Update the three tests (pnpm bugs opens bugs.url when
present, pnpm bugs opens bugs string URL, pnpm bugs falls back to
repository/issues URL) to assert the actual URL opened instead of only the exit
code: stub or spy the module that launches the browser (the opener used by the
CLI) before calling execPnpmSync(['bugs']), capture the URL argument passed to
that opener, and add expect assertions verifying it equals the resolved URL
(e.g., 'https://github.com/test/pkg/issues' for the first two tests and
'https://github.com/test/pkg/issues' for the repository fallback). If there is a
URL-resolution helper function used by the command, alternatively test that
helper directly by calling it with the same package.json shapes and asserting
the returned URL. Ensure you restore the stub/spy after each test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 71bc15c2-d541-4cd7-9175-dc964a144171
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.changeset/pnpm-bugs-command.md__utils__/jest-config/with-registry/globalSetup.jsbins/linker/test/index.tsfetching/pick-fetcher/test/customFetch.tspnpm/src/cmd/bugs.tspnpm/src/cmd/index.tspnpm/src/cmd/notImplemented.tspnpm/test/bugs.test.tspnpm/test/dlx.tspnpm/test/tsconfig.jsonpnpm/test/utils/execPnpm.ts
💤 Files with no reviewable changes (1)
- pnpm/src/cmd/notImplemented.ts
|
My wrong, I'm experimenting several conflicts, I'm going to close it. |
Adds a native `pnpm bugs` command (npm-compatible) that opens a package's bug tracker URL in the browser. - With no arguments, reads `bugs` / `repository` from the current project's `package.json`. - With one or more package names, fetches each package's metadata from the registry (via `fetchPackageInfo`, so auth, scoped names, and proxies are handled) and opens its bug tracker. - Falls back to `<repository>/issues` when `bugs` is missing, normalizing `git+`, `.git`, `git://`, and `git@github.com:` forms first. - Implemented in `@pnpm/deps.inspection.commands` alongside `docs`, reusing the `open` package for cross-platform browser launching. Picked up from #11279 (kairosci's branch) and reworked per @zkochan's review: - moved out of `pnpm/src/cmd/` into the inspection commands package - supports `pnpm bugs [<pkgname> [<pkgname> ...]]` per npm - proper scoped-name encoding via the npm resolver - changeset split (no more `bins.linker` bump) - dropped unrelated test/build noise Closes #11279.
Added the
pnpm bugscommand that opens the package's bug tracker URL in the browserThe command reads the
bugsfield from package.json, falling back torepository URL + /issuesif not definedSummary by CodeRabbit
bugscommand to conveniently open package bug tracker URLs in your browser. Use it without arguments to open the bug tracker for your current project, or specify package names to open trackers for multiple packages from the registry. Automatically falls back to repository issues if a dedicated bugs URL is unavailable.