chore: pre-shape repo into pnpm-workspace monorepo layout - #1254
Conversation
20669e5 to
e11da2e
Compare
|
Follow-ups from the PR #1254 review (no behavior change to shipped CLI): - check-patches.ts: harden resolvePackageFile() to derive the package root from the FIRST `<pkgName>/` after the last `node_modules/` boundary, instead of `lastIndexOf('/<pkgName>/')`. A package that vendors a nested copy of itself would otherwise truncate at the wrong inner segment. Also refresh the stale `node-linker=hoisted` comment. - biome.jsonc / packages/cli/.gitignore: reconcile the contradictory comments about which .gitignore Biome reads. Biome honors the workspace-root .gitignore via `vcs.root: "../.."`; the package-level .gitignore is for plain git only. Drop the dead `!docs` ignore (no packages/cli/docs after the move). - build.ts / bundle.ts: fix comments that still claimed deps are "hoisted to the workspace-root node_modules" — under node-linker=isolated they live in the isolated store; the code (module resolution / `pnpm exec`) is unchanged and correct. - packages/cli/AGENTS.md: replace stale Bun-era guidance with the current pnpm/Node/Vitest reality — command reference, "Use Node.js APIs" section, test-runner isolation notes (VITEST_POOL_ID), vitest imports and `vi.mock`, moved docs path (apps/cli-docs), and removed the dangling `.cursor/rules/bun-cli.mdc` reference. Verified: typecheck, lint, all check:* scripts, build (+ binary smoke).
Follow-ups from the PR #1254 review (no behavior change to shipped CLI): - check-patches.ts: harden resolvePackageFile() to derive the package root from the FIRST `<pkgName>/` after the last `node_modules/` boundary, instead of `lastIndexOf('/<pkgName>/')`. A package that vendors a nested copy of itself would otherwise truncate at the wrong inner segment. Also refresh the stale `node-linker=hoisted` comment. - biome.jsonc / packages/cli/.gitignore: reconcile the contradictory comments about which .gitignore Biome reads. Biome honors the workspace-root .gitignore via `vcs.root: "../.."`; the package-level .gitignore is for plain git only. Drop the dead `!docs` ignore (no packages/cli/docs after the move). - build.ts / bundle.ts: fix comments that still claimed deps are "hoisted to the workspace-root node_modules" — under node-linker=isolated they live in the isolated store; the code (module resolution / `pnpm exec`) is unchanged and correct. - packages/cli/AGENTS.md: replace stale Bun-era guidance with the current pnpm/Node/Vitest reality — command reference, "Use Node.js APIs" section, test-runner isolation notes (VITEST_POOL_ID), vitest imports and `vi.mock`, moved docs path (apps/cli-docs), and removed the dangling `.cursor/rules/bun-cli.mdc` reference. Verified: typecheck, lint, all check:* scripts, build (+ binary smoke).
27b35bb to
4bd6a82
Compare
Reshape the CLI repo into a pnpm workspace so it can be cleanly git-subtree-imported into the future getsentry/toolkit monorepo (#1238, part of #1248). Layout: - Move the CLI package into packages/cli/ (src, test, script, patches, plugins, install, configs, package.json). - Move docs/ into apps/cli-docs/. - Keep GitHub/Craft-read files at the repo root (.github/, .craft.yml, LICENSE, CHANGELOG, .gitignore, .nojekyll) so the repo stays fully operational (releases + nightlies) through and after the merge. - Add pnpm-workspace.yaml (packages/* + apps/*) and a thin delegating root package.json that forwards scripts to the sentry package and hoists the pnpm block (patchedDependencies now reference packages/cli/patches/**). - Add a minimal root README; the CLI README moves to packages/cli/. Internal path fixes: - Shared script/paths.ts (DOCS_ROOT etc.) for cross-package doc paths; generate/check scripts updated to use it. - Re-point apps/cli-docs public symlinks (install, skill files) to the new packages/cli/ locations. - build.ts / bundle.ts resolve @sentry/symbolic WASM via module resolution and invoke fossilize via `pnpm exec` (both hoist to the workspace root now). - biome.jsonc + packages/cli/.gitignore so Biome finds an ignore file. - .gitignore path-anchored entries updated to the nested layout. Workflows / release (kept root-relative where GitHub/Craft require it): - ci.yml, docs-preview.yml, eval-skill-fork.yml: paths-filter globs, node_modules cache paths, patches hashFiles glob, artifact paths (packages/cli/dist-bin, apps/cli-docs/dist), docs working-directory, package.json version reads, npm pack dir. - .craft.yml pre/post release commands cd into packages/cli. node-linker=isolated (was hoisted): under the workspace, hoisted mode let an unpatched transitive @sentry/core@10.65.0 (via @spotlightjs/spotlight -> @sentry/node) win the flat hoist over the patched direct @sentry/core@10.63.0, so the CLI bundled the unpatched copy (masked only by esbuild tree-shaking). isolated mode keeps per-package variants so the patched copy is used. check-patches.ts now reads patchedDependencies from the workspace root and resolves the package copy the CLI actually links via require.resolve. Verified: typecheck, lint, all check:* scripts, build (+ binary smoke, patched bundle), and the full unit suite (399 files, 8474 passed).
The check-generated job's skill/docs-section verification steps still referenced pre-move root-relative paths (plugins/, README.md, DEVELOPMENT.md, docs/src/content/docs/*). After the packages/cli + apps/cli-docs move those paths no longer exist, so `git diff --quiet <nonexistent-path>` exits 128 (fatal), failing the "Validate generated files" check even though the committed generated content is current. Repoint all three references (check-skill, check-sections, auto-commit) to: - packages/cli/plugins/sentry-cli/skills/sentry-cli/ - packages/cli/README.md, packages/cli/DEVELOPMENT.md - apps/cli-docs/src/content/docs/{contributing.md,self-hosted.md,getting-started.mdx} Verified: `pnpm --filter sentry run generate:schema && generate:docs` produces no diff against the committed files, and the git diff --quiet commands now exit 0 instead of 128.
Follow-ups from the PR #1254 review (no behavior change to shipped CLI): - check-patches.ts: harden resolvePackageFile() to derive the package root from the FIRST `<pkgName>/` after the last `node_modules/` boundary, instead of `lastIndexOf('/<pkgName>/')`. A package that vendors a nested copy of itself would otherwise truncate at the wrong inner segment. Also refresh the stale `node-linker=hoisted` comment. - biome.jsonc / packages/cli/.gitignore: reconcile the contradictory comments about which .gitignore Biome reads. Biome honors the workspace-root .gitignore via `vcs.root: "../.."`; the package-level .gitignore is for plain git only. Drop the dead `!docs` ignore (no packages/cli/docs after the move). - build.ts / bundle.ts: fix comments that still claimed deps are "hoisted to the workspace-root node_modules" — under node-linker=isolated they live in the isolated store; the code (module resolution / `pnpm exec`) is unchanged and correct. - packages/cli/AGENTS.md: replace stale Bun-era guidance with the current pnpm/Node/Vitest reality — command reference, "Use Node.js APIs" section, test-runner isolation notes (VITEST_POOL_ID), vitest imports and `vi.mock`, moved docs path (apps/cli-docs), and removed the dangling `.cursor/rules/bun-cli.mdc` reference. Verified: typecheck, lint, all check:* scripts, build (+ binary smoke).
After rebasing onto origin/main, the delegating scripts in packages/cli/package.json still pointed at the old script-file paths that pre-shape-era renamed: check-deps.ts → check-no-deps.ts, check-errors.ts → check-error-patterns.ts, check-stale-refs.ts → check-stale-references.ts. check:docs-sections also moved (script/check-docs-sections.ts → script/generate-docs-sections.ts --check). Take main's `scripts` block verbatim so future drift lands via rebase, not by manual sync. Verified: check:patches, check:deps, check:errors, check:stale-refs, check:fragments, typecheck, lint, build (+ binary smoke 0.40.0-dev.0), test:changed all green.
4bd6a82 to
faa626b
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit faa626b. Configure here.
| # create and open the database reliably. The runner's ~/.sentry | ||
| # may not exist under the switched Node 20 runtime. | ||
| SENTRY_CONFIG_DIR: ${{ runner.temp }}/.sentry-smoke | ||
| run: node dist/bin.cjs --help |
There was a problem hiding this comment.
Stale root npm pack step
Medium Severity
The original root npm pack step was left in place after adding a correct pack under packages/cli. The root step packs the private workspace package toolkit-cli-workspace instead of sentry, wastes work on every Node matrix entry, and can fail or produce a misleading tarball before the real pack runs.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit faa626b. Configure here.
| "license": "FSL-1.1-Apache-2.0", | ||
| "engines": { | ||
| "node": ">=18.0" | ||
| }, |
There was a problem hiding this comment.
Missing license in CLI package
Medium Severity
LICENSE.md stayed at the repo root while the publishable package moved to packages/cli/. npm pack from that directory will not include the license text, and the README LICENSE.md link resolves to a missing file under packages/cli/.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit faa626b. Configure here.
When the new test/eval-common/anthropic-client.ts was added on main
during the rebase, git placed it at the root because all subtrees were
renamed in the pre-shape commit. The importing test
(packages/cli/test/script/eval-provider.test.ts) was correctly moved
into the package, so the relative import
`from "../eval-common/anthropic-client.js"`
(./packages/cli/test/script/eval-provider.test.ts) was unable to find
the file at the new location.
Move the file to its correct monorepo location:
test/eval-common/anthropic-client.ts
-> packages/cli/test/eval-common/anthropic-client.ts
Verified: full unit suite (411 files, 8718 tests) passes.
| resolution: {integrity: sha512-haPVm1EkS9pgvHrQ/F3Xy+hgcuMV0Wm9vfIBSiwZ05k+xgb0PkBQpGsAA/oWdDobNaZTH5ppvHtzCFbnSEwHVw==} | ||
| engines: {node: ^18.17.0 || ^20.3.0 || >=21.0.0} | ||
|
|
||
| sharp@0.34.5: |
There was a problem hiding this comment.
High severity vulnerability introduced by a package you're using:
Line 3585 lists a dependency (sharp) with a known High severity vulnerability. Fixing requires upgrading or replacing the dependency.
ℹ️ Why this matters
Affected versions of sharp are vulnerable to Dependency on Vulnerable Third-Party Component. sharp bundles a vulnerable version of the native libvips library, inheriting four memory-safety flaws: an integer overflow leading to a heap-based buffer overflow in the VIPS loader (vipsload, CVE-2026-33327), an integer overflow in the GIF loader (gifload, CVE-2026-33328) causing a denial of service on 32-bit hosts only, a heap-based buffer overflow in the TIFF loader (tiffload, CVE-2026-35591) when handling JPEG or JPEG2000-encoded tiles, and an out-of-bounds read in the EXIF directory decoder (CVE-2026-35590). An attacker who can supply a crafted image can crash the process or corrupt heap memory. Because sharp selects the libvips loader by sniffing the input bytes, no call site can be shown to be safe, and the EXIF flaw is reachable from the JPEG, TIFF, WebP, PNG and HEIF loaders as well. Upgrade to sharp 0.35.0 or later, which bundles libvips 8.18.3. Blocking the affected loaders with sharp.block({ operation: ["VipsForeignLoadNsgif", "VipsForeignLoadTiff", "VipsForeignLoadVips"] }) is only a partial stopgap and does not mitigate the EXIF out-of-bounds read (CVE-2026-35590), for which no workaround exists.
References: GHSA
To resolve this comment:
Upgrade this dependency to at least version 0.35.0 at pnpm-lock.yaml.
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
| @@ -2271,6 +3578,14 @@ packages: | |||
| setprototypeof@1.2.0: | |||
| resolution: {integrity: sha512-E5LDX7Wrp85Kil5bhZv46j8jOeboKq5JMmYM3gVGdGH8xFpPWXUMsNrlODCrkoxMEeNi/XZIwuRvY4XNwYMJpw==} | |||
|
|
|||
| sharp@0.33.5: | |||
There was a problem hiding this comment.
High severity vulnerability introduced by a package you're using:
Line 3581 lists a dependency (sharp) with a known High severity vulnerability. Fixing requires upgrading or replacing the dependency.
ℹ️ Why this matters
Affected versions of sharp are vulnerable to Dependency on Vulnerable Third-Party Component. sharp bundles a vulnerable version of the native libvips library, inheriting four memory-safety flaws: an integer overflow leading to a heap-based buffer overflow in the VIPS loader (vipsload, CVE-2026-33327), an integer overflow in the GIF loader (gifload, CVE-2026-33328) causing a denial of service on 32-bit hosts only, a heap-based buffer overflow in the TIFF loader (tiffload, CVE-2026-35591) when handling JPEG or JPEG2000-encoded tiles, and an out-of-bounds read in the EXIF directory decoder (CVE-2026-35590). An attacker who can supply a crafted image can crash the process or corrupt heap memory. Because sharp selects the libvips loader by sniffing the input bytes, no call site can be shown to be safe, and the EXIF flaw is reachable from the JPEG, TIFF, WebP, PNG and HEIF loaders as well. Upgrade to sharp 0.35.0 or later, which bundles libvips 8.18.3. Blocking the affected loaders with sharp.block({ operation: ["VipsForeignLoadNsgif", "VipsForeignLoadTiff", "VipsForeignLoadVips"] }) is only a partial stopgap and does not mitigate the EXIF out-of-bounds read (CVE-2026-35590), for which no workaround exists.
References: GHSA
To resolve this comment:
Upgrade this dependency to at least version 0.35.0 at pnpm-lock.yaml.
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
| js-yaml@3.15.0: | ||
| resolution: {integrity: sha512-ttBQIIQPDeLjpPOohtUdXuXUVoA2uIB6fEH9HyJ7234s5mBJ5wTx20njxplLZQgLaOfpmPQA7X2t5AX6tIPbog==} | ||
| hasBin: true | ||
|
|
||
| js-yaml@4.2.0: |
There was a problem hiding this comment.
High severity vulnerability may affect your project—review required:
Line 2719 lists a dependency (js-yaml) with a known High severity vulnerability.
ℹ️ Why this matters
Affected versions of js-yaml are vulnerable to Inefficient Algorithmic Complexity / Uncontrolled Resource Consumption. An attacker can supply a YAML document containing a chain of mappings that each merge the previous one via the merge key (<<), causing js-yaml to spend quadratic CPU time while parsing input whose size grows only linearly, resulting in a denial of service.
To resolve this comment:
Check if you are using js-yaml on the CLI.
- If you're affected, upgrade this dependency to at least version 4.3.0 at pnpm-lock.yaml.
- If you're not affected, comment
/fp we don't use this [condition]
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.


Pre-shape the CLI repo into a pnpm-workspace monorepo
Part of the sentry-cli + sentry-mcp →
getsentry/toolkitmerge.Closes #1238 · Master: #1248
This reshapes the repo into a pnpm workspace so it can be cleanly
git subtree-imported into the renamedtoolkitrepo (#1239), while keepingthis repo fully operational (CI, releases, nightlies) throughout the
transition — which the dual-source CLI upgrade (#1244) depends on.
Layout
packages/cli/(src,test,script,patches,plugins,install, configs,package.json)docs/→apps/cli-docs/.github/,.craft.yml,LICENSE,CHANGELOG,.gitignore,.nojekyllpnpm-workspace.yaml(packages/*+apps/*) and a thin delegating rootpackage.jsonthat forwards scripts to thesentrypackage and hoists thepnpmblockREADME; CLI README →packages/cli/README.mdInternal path fixes
script/paths.ts(DOCS_ROOTetc.) for cross-package doc pathsapps/cli-docspublic symlinks (install, skill files) topackages/cli/build.ts/bundle.tsresolve the@sentry/symbolicWASM via module resolution and invoke fossilize viapnpm exec(both hoist to the workspace root)biome.jsonc+packages/cli/.gitignoreso Biome finds an ignore file.gitignorepath-anchored entries updated for the nested layoutWorkflows / release (root-relative where GitHub/Craft require it)
ci.yml,docs-preview.yml,eval-skill-fork.yml: paths-filter globs,node_modulescache paths, patcheshashFilesglob, artifact paths (packages/cli/dist-bin,apps/cli-docs/dist), docsworking-directory, version reads,npm packdir.craft.ymlpre/post-release commandscd packages/clinode-linker=isolated(washoisted)Under the workspace,
hoistedlet an unpatched transitive@sentry/core@10.65.0(via@spotlightjs/spotlight→@sentry/node) win theflat hoist over the patched direct
@sentry/core@10.63.0, so the CLIbundled the unpatched copy (masked only by esbuild tree-shaking).
isolatedmode (pnpm default) keeps per-package variants so the patched copy is used.
check-patches.tsnow readspatchedDependenciesfrom the workspace root andresolves the copy the CLI actually links via
require.resolve.The patch files themselves are unchanged (they apply cleanly);
catalog:-based dedup is deferred to #1240.Verification (local)
typecheck,lint, allcheck:*scriptsbuild(+ Linux binary smoke test, WASM asset loads, patched bundle verified)bundle(npm library)CI on this PR is the authoritative check for the cross-platform binary matrix
and release paths.