Skip to content

Optimize npm package SDK installs - #302

Merged
unbraind merged 2 commits into
mainfrom
fix/extension-sdk-install-link
Jun 19, 2026
Merged

unbraind merged 2 commits into
mainfrom
fix/extension-sdk-install-link

Conversation

@unbraind

@unbraind unbraind commented Jun 19, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Avoid nested @unbrained/pm-cli installs for npm package-backed extensions by linking the installed package back to the active host CLI SDK root.
  • Preserve package runtime metadata after dependency installation and document the SDK-linking behavior for package authors.
  • Update @sentry/node to 10.59.0, regenerate the package-owned CHANGELOG.md, and record pm evidence for pm-oxq2 plus auto-release health verification on pm-nb08.

Verification

  • pnpm build
  • node scripts/run-tests.mjs test -- tests/unit/extensions/extension-command.spec.ts tests/integration/cli.integration.spec.ts
  • node scripts/run-tests.mjs coverage (100/100/100/100)
  • pnpm quality:static
  • pnpm contracts:check
  • pnpm audit --audit-level moderate
  • pnpm security:scan
  • pnpm changelog:pm:check
  • node dist/cli.js package doctor --project --detail deep --json
  • node scripts/release/sentry-telemetry-gate.mjs --json --telemetry-mode best-effort
  • manual temp npm package install smoke

Summary by Sourcery

Optimize npm-backed extension installs by avoiding nested @unbrained/pm-cli copies and wiring SDK imports to the active host CLI.

Bug Fixes:

  • Prevent project-scope extension installs from writing large nested node_modules trees per workspace when using @unbrained/pm-cli as a peer dependency.

Enhancements:

  • Preserve package runtime dependency metadata while installing only non-dev dependencies for extension runtimes.
  • Treat @unbrained/pm-cli as a hosted dependency, linking it from installed packages back to the running CLI SDK root across platforms.

Build:

  • Bump @sentry/node dependency to 10.59.0 and refresh the lockfile to match.

Documentation:

  • Document how package-backed extensions should declare @unbrained/pm-cli dependencies and how the CLI links SDK imports to the running host instead of installing a nested copy.
  • Clarify that external packages should rely on public SDK entry points while the CLI wires @unbrained/pm-cli to the active SDK during npm installs.

Tests:

  • Extend extension runtime tests to cover hosted pm-cli detection, manifest rewriting, symlink behavior, and peer-only versus dependency-only package cases.

Chores:

  • Record evidence and changelog entries for pm-oxq2 and pm-nb08 to track the optimization and release health verification.

Avoid installing a nested @unbrained/pm-cli copy for npm package-backed extensions. Package installs now omit the host CLI package from npm's runtime install step, link @unbrained/pm-cli back to the active host package root, and restore the package runtime manifest so published metadata is preserved.

Document the SDK-linking behavior for package authors, add regression coverage for peer-only and dependency-only package installs, update @sentry/node to 10.59.0, regenerate the package-owned changelog, and record pm evidence for pm-oxq2 plus auto-release health verification on pm-nb08.

Verification: pnpm build; node scripts/run-tests.mjs test -- tests/unit/extensions/extension-command.spec.ts tests/integration/cli.integration.spec.ts; node scripts/run-tests.mjs coverage (100/100/100/100); pnpm quality:static; pnpm contracts:check; pnpm audit --audit-level moderate; pnpm security:scan; pnpm changelog:pm:check; node dist/cli.js package doctor --project --detail deep --json; node scripts/release/sentry-telemetry-gate.mjs --json --telemetry-mode best-effort; manual temp npm package install smoke.
@sourcery-ai

sourcery-ai Bot commented Jun 19, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

Implements host-linked SDK resolution for npm package-backed extensions so that @unbrained/pm-cli is provided by the active CLI instead of being installed per extension, while preserving package runtime metadata, extending tests/docs, and bumping @sentry/node.

File-Level Changes

Change Details Files
Change npm runtime dependency installation for package-backed extensions to avoid installing @unbrained/pm-cli and instead link it to the running host CLI SDK root.
  • Extend installNpmPackageRuntimeDependencies to detect manifests that declare @unbrained/pm-cli and treat that as a reason to run the installer even if there are no other runtime deps.
  • Write a temporary manifest for npm install that strips devDependencies and removes @unbrained/pm-cli from dependency/optional/peer maps, then restore the original runtime manifest after install.
  • Skip @unbrained/pm-cli when building runtimeDependencyInstallSpecs so it is never passed to npm install.
  • Add hasHostedPmCliDependency and removeHostedPmCliDependency helpers to detect and strip the hosted CLI dep fields generically.
  • Add linkHostedPmCliDependency that resolves the host CLI package root and creates a platform-appropriate node_modules/@unbrained/pm-cli symlink in the installed package tree, using resolveDirectorySymlinkType for Windows vs non-Windows behavior.
  • Expose new helpers via _testOnlyInstallSources and import resolvePmPackageRootFromModule to locate the host CLI package root.
src/cli/commands/extension/install-sources.ts
Add unit coverage for new dependency-handling behavior and symlink creation for @unbrained/pm-cli in npm package-backed extensions.
  • Extend extension-command runtime tests to verify runtimeDependencyInstallSpecs skips @unbrained/pm-cli while still installing other deps and peers.
  • Test hasHostedPmCliDependency, removeHostedPmCliDependency, and resolveDirectorySymlinkType in isolation.
  • Add integration-style tests that exercise installNpmPackageRuntimeDependencies for packages with peer-only @unbrained/pm-cli, mixed runtime+peer, and runtime-only deps, asserting correct node_modules layout and absence/presence of the pm-cli link.
tests/unit/extensions/extension-command.spec.ts
Document the new SDK-linking behavior for package-backed extensions and clarify PM_CLI_PACKAGE_ROOT usage.
  • Update EXTENSIONS.md to explain that pm links a package’s @unbrained/pm-cli dependency to the running host CLI to avoid nested CLI installs and encourage declaring it as a peer dependency.
  • Update SDK.md to describe that registry installs get the active SDK via a link from the installed package back to the host CLI rather than a second CLI download.
docs/EXTENSIONS.md
docs/SDK.md
Update observability and changelog metadata for the optimization and telemetry verification.
  • Bump @sentry/node dependency to 10.59.0 and refresh pnpm-lock.yaml.
  • Regenerate CHANGELOG.md entry describing reduced node_modules size from avoiding nested pm-cli installs per workspace.
  • Record pm evidence and auto-release health verification in the pm history and issue tracking artifacts.
package.json
pnpm-lock.yaml
CHANGELOG.md
.agents/pm/history/pm-nb08.jsonl
.agents/pm/history/pm-oxq2.jsonl
.agents/pm/issues/pm-nb08.toon
.agents/pm/issues/pm-oxq2.toon

Possibly linked issues

  • #: PR makes npm-installed extensions link @unbrained/pm-cli to host CLI, fixing missing runtime SDK dependencies described.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@unbraind

Copy link
Copy Markdown
Owner Author

@codex review\n\n/gemini review\n\n@gemini-code-assist review\n\n@coderabbitai full review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@unbraind, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 38 minutes and 16 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 0631b88f-6e10-406d-89b4-cbcb530be6e7

📥 Commits

Reviewing files that changed from the base of the PR and between 0b98220 and 2d0c55e.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • .agents/pm/history/pm-oxq2.jsonl
  • .agents/pm/issues/pm-oxq2.toon
  • package.json
  • src/cli/commands/extension/install-sources.ts
  • tests/unit/extensions/extension-command.spec.ts
📝 Walkthrough

Walkthrough

During pm install npm:<package>, the extension install logic now detects @unbrained/pm-cli in the target package's dependency manifest, skips installing it from npm, and instead symlinks the running host CLI root into the extension's node_modules/@unbrained/pm-cli. Documentation, tests, a changelog entry, and a @sentry/node version bump accompany the change.

Changes

Extension SDK Install-Link Feature

Layer / File(s) Summary
install-sources.ts: pm-cli detection, symlink, and spec filter
src/cli/commands/extension/install-sources.ts
Adds PM_CLI_PACKAGE_NAME constant and required imports; rewrites runtime dependency install flow to detect @unbrained/pm-cli in the manifest (hasHostedPmCliDependency), strip it (removeHostedPmCliDependency), install remaining deps, then symlink the host CLI root (linkHostedPmCliDependency); updates the npm spec filter to always exclude @unbrained/pm-cli; exports new helpers via _testOnlyInstallSources.
Unit tests: helper functions and install scenarios
tests/unit/extensions/extension-command.spec.ts
Adds assertions for hasHostedPmCliDependency, resolveDirectorySymlinkType (win32/linux), removeHostedPmCliDependency across all dependency fields, and augments installNpmPackageRuntimeDependencies tests with post-install package.json rewrite checks, realpath assertions for @unbrained/pm-cli, and peer-only/dependency-only package scenarios.
Docs, changelog, and @sentry/node bump
CHANGELOG.md, docs/EXTENSIONS.md, docs/SDK.md, package.json
Adds a Fixed changelog entry for the ~69MB node_modules write; updates EXTENSIONS.md and SDK.md to document host-CLI linking behavior and the PM_CLI_PACKAGE_ROOT policy for external packages; bumps @sentry/node from ^10.58.0 to ^10.59.0.

Agent PM Tracking Records

Layer / File(s) Summary
pm-oxq2 issue closure and history
.agents/pm/history/pm-oxq2.jsonl, .agents/pm/issues/pm-oxq2.toon
Records the full lifecycle of the pm-oxq2 issue: assignee addition, in-progress transition, accumulation of test/gate evidence, and final closure with a close_reason summarizing the install-link mitigation and verification gates.
pm-nb08 auto-release verification update
.agents/pm/history/pm-nb08.jsonl, .agents/pm/issues/pm-nb08.toon
Appends a second sentry-telemetry-gate verification comment (critical=0, high=0) to the pm-nb08 issue record and advances its updated_at timestamp.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Optimize npm package SDK installs' directly reflects the main objective of the PR: optimizing npm package installations by eliminating nested pm-cli copies and linking to the active host SDK.
Description check ✅ Passed The description is directly related to the changeset, detailing the npm package optimization work, dependency updates, documentation changes, and comprehensive verification steps performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request resolves issue pm-oxq2 by preventing duplicate nested installations of @unbrained/pm-cli and its transitive dependencies when installing package-backed extensions. It filters out @unbrained/pm-cli during the npm install phase and symlinks it directly to the active host CLI package root. Additionally, @sentry/node is updated to ^10.59.0, and corresponding tests and documentation are added. The review feedback recommends wrapping the npm installation and symlinking processes in a try...finally block to ensure package.json is reliably restored to its original state even if the installation fails.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get PR details
gh pr view 302 --repo unbraind/pm-cli --json title,body,baseRefName,headRefName,additions,deletions,changedFiles

echo "---FILES---"
# Get list of changed files
gh pr diff 302 --repo unbraind/pm-cli --name-only

echo "---DIFF---"
# Get the full diff
gh pr diff 302 --repo unbraind/pm-cli

Length of output: 87430



@unbraind: Here is the full review of PR #302.


Summary

The PR cleanly solves the 69 MB nested node_modules problem (pm-oxq2) by stripping @unbrained/pm-cli from the install manifest, running npm install without it, then symlinking the host CLI root in its place, and finally restoring the original manifest (including peerDependencies) to disk. The three-step write→install→symlink→restore pattern is sound and the test coverage is thorough.


src/cli/commands/extension/install-sources.ts

Medium: Missing try/finally for manifest restoration

The function writes installManifest to disk (with @unbrained/pm-cli removed), then runs npm install, then runs linkHostedPmCliDependency, and only then restores runtimeOnlyManifest. If linkHostedPmCliDependency (or an earlier step) throws, the manifest on disk is left in the stripped-down installManifest state — @unbrained/pm-cli is gone from all dependency fields and devDependencies is gone too.

await fs.writeFile(packageJsonPath, `${JSON.stringify(installManifest, null, 2)}\n`, "utf8");
// ... npm install ...
// ... linkHostedPmCliDependency (can throw) ...
// This line never runs if anything above throws:
await fs.writeFile(packageJsonPath, `${JSON.stringify(runtimeOnlyManifest, null, 2)}\n`, "utf8");

Suggested guard:

await fs.writeFile(packageJsonPath, `${JSON.stringify(installManifest, null, 2)}\n`, "utf8");
try {
  if (dependencySpecs.length > 0) {
    await runNpmCommand([...], packageRoot);
  }
  if (shouldLinkHostedPmCli) {
    await linkHostedPmCliDependency(packageRoot);
  }
} finally {
  await fs.writeFile(packageJsonPath, `${JSON.stringify(runtimeOnlyManifest, null, 2)}\n`, "utf8");
}

Minor: Hardcoded "@Unbrained" in linkHostedPmCliDependency

The scopedDirectory path hard-codes the scope string while the constant PM_CLI_PACKAGE_NAME already has the full @unbrained/pm-cli string. Consider deriving it:

const [scope, pkg] = PM_CLI_PACKAGE_NAME.split("/");
const scopedDirectory = path.join(packageRoot, "node_modules", scope);
const linkPath = path.join(scopedDirectory, pkg);

This is a low-risk cosmetic issue but prevents the two strings from drifting if the constant ever changes.

Observation: --omit=peer and explicit peer specs interact correctly

The addition of --omit=peer is correct and intentional. runtimeDependencyInstallSpecs still explicitly lists the extension's own peers (other than @unbrained/pm-cli) in the install args, so those still get installed. --omit=peer only suppresses auto-resolved transitive peers of the installed packages, which is the desired behavior (cutting Sentry/OTel from being pulled in as transitive peers).


tests/unit/extensions/extension-command.spec.ts

Minor: Test assertion relies on process.cwd() for symlink target

await expect(realpath(path.join(packageRoot, "node_modules", "`@unbrained`", "pm-cli"))).resolves.toBe(
  await realpath(process.cwd()),
);

The implementation uses resolvePmPackageRootFromModule(import.meta.url, ["../../../.."]) (four levels up from dist/cli/commands/extension/), while the test compares against process.cwd(). These are equivalent when tests run from the repo root (as they do), but if the test runner ever changes directory the assertion would silently pass to the wrong target. A more robust assertion would resolve the expected path the same way:

import { resolvePmPackageRootFromModule } from "../../../src/core/packages/root.js";
// ...
const expectedRoot = await realpath(resolvePmPackageRootFromModule(import.meta.url, ["../../../.."]));
await expect(realpath(path.join(packageRoot, "node_modules", "`@unbrained`", "pm-cli"))).resolves.toBe(expectedRoot);

Well-done: peer-only and dependency-only sub-cases

The addition of peerOnlyPackageRoot and dependencyOnlyPackageRoot sub-cases within the same test is good; they cover the two important edge cases (no non-pm-cli deps to install at all, and pm-cli absent entirely).


pnpm-lock.yaml / package.json

Info: Sentry 10.59.0 introduces new @apm-js-collab/* transitive deps

@sentry/server-utils@10.59.0 now pulls in @apm-js-collab/code-transformer, @apm-js-collab/code-transformer-bundler-plugins, and @apm-js-collab/tracing-hooks. These appear to be Sentry's new internal code-transformation packages for framework detection. The pnpm audit --audit-level moderate gate in the PR checklist covers this. vite is listed as an optional peer of @sentry/server-utils but the lockfile correctly resolves it from the existing dev dep.


Documentation

The documentation additions are functional but terse — a single appended sentence in each file. If extension authors are the target audience, a dedicated callout or note block in docs/EXTENSIONS.md (especially near the peerDependencies recommendation) would make the behavior more discoverable. Not blocking.


Overall

The implementation is correct, well-tested, and directly addresses the root cause (pm-oxq2). The only actionable change worth considering before merge is the try/finally guard to ensure the manifest is always restored even when linkHostedPmCliDependency throws.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request resolves issue pm-oxq2 by skipping nested host CLI installs for npm package extensions and instead linking @unbrained/pm-cli to the active host package root, significantly reducing disk usage. It also updates @sentry/node to version 10.59.0. Feedback on the changes points out a robustness issue where package.json could be left in a temporary, modified state if the installation fails, and suggests wrapping the installation logic in a try...finally block to ensure the manifest is always restored.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/cli/commands/extension/install-sources.ts (1)

555-556: ⚡ Quick win

Derive symlink path from PM_CLI_PACKAGE_NAME to avoid name-drift.

linkHostedPmCliDependency hardcodes scope/name instead of reusing PM_CLI_PACKAGE_NAME, so future rename updates can diverge.

♻️ Suggested refactor
 async function linkHostedPmCliDependency(packageRoot: string): Promise<void> {
   const hostPackageRoot = resolvePmPackageRootFromModule(import.meta.url, ["../../../.."]);
-  const scopedDirectory = path.join(packageRoot, "node_modules", "`@unbrained`");
-  const linkPath = path.join(scopedDirectory, "pm-cli");
+  const [scope, packageName] = PM_CLI_PACKAGE_NAME.split("/");
+  const scopedDirectory = path.join(packageRoot, "node_modules", scope);
+  const linkPath = path.join(scopedDirectory, packageName);
   await fs.mkdir(scopedDirectory, { recursive: true });
   await fs.rm(linkPath, { recursive: true, force: true });
   await fs.symlink(hostPackageRoot, linkPath, resolveDirectorySymlinkType(process.platform));
 }
🤖 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 `@src/cli/commands/extension/install-sources.ts` around lines 555 - 556, The
symlink path construction in the `linkHostedPmCliDependency` function hardcodes
the scope "`@unbrained`" and package name "pm-cli" instead of deriving them from
the PM_CLI_PACKAGE_NAME constant. Replace the hardcoded values with a derived
version by parsing PM_CLI_PACKAGE_NAME (which should be in the format
"`@scope/package-name`") to extract the scope and package name components, then
construct scopedDirectory and linkPath using these parsed values instead of the
literal strings.
🤖 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 `@src/cli/commands/extension/install-sources.ts`:
- Around line 555-556: The symlink path construction in the
`linkHostedPmCliDependency` function hardcodes the scope "`@unbrained`" and
package name "pm-cli" instead of deriving them from the PM_CLI_PACKAGE_NAME
constant. Replace the hardcoded values with a derived version by parsing
PM_CLI_PACKAGE_NAME (which should be in the format "`@scope/package-name`") to
extract the scope and package name components, then construct scopedDirectory
and linkPath using these parsed values instead of the literal strings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 44e0dd78-9128-45b1-b91d-a6c105d3d1cc

📥 Commits

Reviewing files that changed from the base of the PR and between 106aae0 and 0b98220.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (10)
  • .agents/pm/history/pm-nb08.jsonl
  • .agents/pm/history/pm-oxq2.jsonl
  • .agents/pm/issues/pm-nb08.toon
  • .agents/pm/issues/pm-oxq2.toon
  • CHANGELOG.md
  • docs/EXTENSIONS.md
  • docs/SDK.md
  • package.json
  • src/cli/commands/extension/install-sources.ts
  • tests/unit/extensions/extension-command.spec.ts

Restore stripped npm package manifests in a finally block when runtime dependency installation or hosted pm-cli linking fails.

Add regression coverage for link-failure restoration and derive the hosted @unbrained/pm-cli link path from the package constant.

Override @sentry/cli's transitive undici to 6.27.0 so pnpm audit is clean while @sentry/cli remains on its latest published version.

Verification: pnpm build; node scripts/run-tests.mjs test -- tests/unit/extensions/extension-command.spec.ts; node scripts/run-tests.mjs coverage (100/100/100/100, 4655 tests); pnpm quality:static; pnpm contracts:check; pnpm changelog:pm:check; pnpm security:scan; pnpm audit --audit-level moderate; node dist/cli.js package doctor --project --detail deep --json; node scripts/release/sentry-telemetry-gate.mjs --json --telemetry-mode best-effort.
@unbraind

Copy link
Copy Markdown
Owner Author

Review agents: new head 2d0c55e addresses manifest-restore feedback and the undici audit finding. Please review the current head again.

@codex review
/gemini review
@gemini-code-assist review
@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get current PR details and diff at new head
gh pr view 302 --repo unbraind/pm-cli --json title,headRefOid,additions,deletions,changedFiles

echo "---DIFF---"
gh pr diff 302 --repo unbraind/pm-cli

Length of output: 90055



@unbraind: Clean follow-up. Both actionable items from the previous review are addressed.


What changed

try/finally manifest restoration — ✅ Resolved. The finally block now unconditionally writes runtimeOnlyManifest back to disk regardless of whether runNpmCommand or linkHostedPmCliDependency throws.

Hardcoded "@Unbrained" scope — ✅ Resolved. linkHostedPmCliDependency now derives both scope and package name from PM_CLI_PACKAGE_NAME.split("/"), so the two strings can't drift independently.

undici advisory — ✅ Resolved. The undici: 6.27.0 override is present in both package.json and pnpm-lock.yaml, and @sentry/cli now picks it up.

Regression coverage for the finally path — ✅ Well done. The failingLinkPackageRoot sub-case places a plain file at node_modules/@unbrained to force fs.mkdir(scopedDirectory) to throw, then asserts that the manifest is restored with peerDependencies intact and devDependencies absent. That directly covers the failure path being protected.


Remaining minor (non-blocking)

The process.cwd() symlink-target assertion flagged in the earlier review is still in place. It works correctly as long as the test runner's working directory is the repo root (which it is), so it doesn't need to block this PR — but worth tracking if the test infrastructure ever relocates.


pnpm-lock.yaml note

@sentry/server-utils@10.59.0 now pulls in three new @apm-js-collab/* packages (code-transformer, code-transformer-bundler-plugins, tracing-hooks) and optionally resolves vite (correctly satisfied from the existing dev dep). The undici 6.27.0 override covers the previously flagged advisory. Audit gates in the PR checklist cover both.


Overall the implementation is correct and the test coverage is thorough. No new blocking issues.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request resolves issue pm-oxq2 by skipping nested installs of the host CLI package (@unbrained/pm-cli) when installing npm package extensions. Instead, it symlinks (or junctions on Windows) the extension's dependency back to the active host package root, preserving the package manifest in a finally block if the installation or linking fails. The changes also include updating @sentry/node to ^10.59.0, locking undici to 6.27.0 to resolve security advisories, updating documentation, and adding comprehensive unit tests to cover the new linking and restoration behaviors. There are no review comments, so no additional feedback is provided.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@unbraind
unbraind merged commit ca0e0bb into main Jun 19, 2026
15 checks passed
@unbraind
unbraind deleted the fix/extension-sdk-install-link branch June 19, 2026 15:19
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