Feature/add socket#253
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3c3d55a The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request standardizes dependency management across the workspace by implementing pnpm catalogs and YAML anchors for shared versions. It also adds JSR and socket.dev badges to all package READMEs, introduces an automation script for badge management, and updates the turbo generator templates. Review feedback highlights a typo in a URL, potential performance issues with broad workspace patterns, a bug in the badge script's index handling, and a variable mismatch in the README template.
Comment on lines
+31
to
+39
| const indexOfNpmVersion = lines.findIndex((line) => | ||
| line.startsWith("[![NPM Version]"), | ||
| ); | ||
|
|
||
| const body = [ | ||
| ...lines.slice(0, indexOfNpmVersion), | ||
| currentTemplate, | ||
| ...lines.slice(indexOfNpmVersion), | ||
| ].join("\n"); |
There was a problem hiding this comment.
If the NPM Version badge is not found in the README, indexOfNpmVersion will be -1. The subsequent slice operations will then incorrectly insert the new badge before the last line of the file. You should check if the index is valid before proceeding. Additionally, the template string contains leading and trailing newlines which, when joined with \n, will create unwanted empty lines in the README.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds JSR and socket.dev badges across many core package READMEs, adds an automation script ( Changes
Sequence Diagram(s)sequenceDiagram
participant Script as scripts/add-socket.ts
participant Workspace as PNPM Workspace (getPackages)
participant FS as FileSystem (README.md)
rect rgba(100,150,240,0.5)
Workspace->>Script: return package list (name, dir, relativeDir)
end
Script->>Script: filter packages where relativeDir startsWith "core"
loop per package
Script->>FS: read README.md
FS-->>Script: README content
Script->>Script: build socket.dev badge markdown
alt snippet exists
Script->>Script: skip package
else not present
Script->>FS: write updated README.md (insert badge before NPM Version badge)
FS-->>Script: write success
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/catppuccin-opml/README.md`:
- Around line 6-7: The README badge order is inconsistent with other package
READMEs; in core/catppuccin-opml/README.md swap the two badge lines so the
socket.dev badge
(https://badge.socket.dev/npm/package/@stephansama/catppuccin-opml) appears
before the JSR badge (https://jsr.io/badges/@stephansama/catppuccin-opml) to
match core/catppuccin-xsl/README.md and keep badge ordering uniform across
packages.
In `@core/github-env/README.md`:
- Line 7: Fix the typo in the README badge URL: update the markdown link where
the badge is defined (the
"[](https://scket.dev/npm/package/@stephansama/github-env/overview)"
entry) by changing the target URL host from "scket.dev" to "socket.dev" so the
link becomes
"(https://socket.dev/npm/package/@stephansama/github-env/overview)"; ensure both
the badge image URL and the clickable link use "socket.dev" consistently.
In `@package.json`:
- Line 30: The package.json script "scripts:add-socket" invokes the CLI "tsx"
but the project does not declare "tsx" in dependencies/devDependencies; add
"tsx": "catalog:" to the root package.json (prefer devDependencies for a dev
tool) so running pnpm run scripts:add-socket works in clean environments, then
run pnpm install to update the lockfile and verify pnpm run scripts:add-socket
executes without "tsx: command not found".
In `@pnpm-workspace.yaml`:
- Around line 86-87: Update the pnpm-workspace.yaml to use the existing
__versions anchor pattern: add a &prettier entry with value "3.8.1" to the
__versions block, then replace the hardcoded prettier: 3.8.1 line with prettier:
*prettier so the catalog uses the anchor; look for the __versions block and the
prettier key in the catalog to make these changes.
In `@scripts/add-socket.ts`:
- Around line 21-23: The code assumes README.md exists and that the anchor index
is found, causing crashes or malformed output; update the logic around
readmePath/readmeFile and currentTemplate to: check for README existence before
calling fs.promises.readFile (or catch the read error) and create a sensible
default README content if missing; after computing anchorIndex (the index used
by the insertion logic around template.replaceAll/currentTemplate), handle
indexOf returning -1 by choosing a safe insertion point (e.g., append at the end
or after the last non-empty line) instead of using -1 which writes before the
last line; ensure write logic uses these guarded values so writing never
produces malformed output.
- Around line 8-13: The script name add-socket.ts is misleading because the
current const template only inserts a JSR badge (const template) while a socket
badge template is commented out; either implement insertion of the socket.dev
badge or rename the script/command to reflect it only adds JSR. To fix: locate
the const template in scripts/add-socket.ts and either (A) restore and use the
commented socket template (or add a flag/option to choose between `socket` and
`jsr`) so both badges can be inserted, ensuring the chosen template variable is
used in the insertion routine, or (B) rename the file/CLI command and update any
references/documentation to add-jsr (and remove the commented socket template)
so behavior matches name. Ensure references to `template` and the commented
socket template are handled and update README/usage strings accordingly.
In `@turbo/generators/templates/common/README.md`:
- Line 7: The README badge URL uses an undefined template variable
`{{package}}`; update the href to use the existing `{{name}}` variable so both
the badge image and link match (replace `{{package}}` with `{{name}}` in the
badge link on the README line that contains [![socket.dev]...]); ensure you only
change the URL token and keep the rest of the badge markup intact.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5977854f-1d99-4f17-8a7b-62d5d71cd76a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
.config/.cspell.json.prettierrccore/ai-commit-msg/README.mdcore/alfred-kaomoji/README.mdcore/astro-iconify-svgmap/README.mdcore/auto-readme/README.mdcore/catppuccin-jsonresume-theme/README.mdcore/catppuccin-opml/README.mdcore/catppuccin-rss/README.mdcore/catppuccin-typedoc/README.mdcore/catppuccin-xsl/README.mdcore/example/README.mdcore/find-makefile-targets/README.mdcore/github-env/README.mdcore/multipublish/README.mdcore/prettier-plugin-handlebars/README.mdcore/remark-asciinema/README.mdcore/svelte-social-share-links/README.mdcore/typed-env/README.mdcore/typed-events/README.mdcore/typed-nocodb-api/README.mdcore/typed-templates/README.mdcore/types-github-action-env/README.mdcore/types-lhci/README.mdpackage.jsonpnpm-workspace.yamlscripts/add-socket.tsscripts/package.jsonscripts/tsconfig.jsonturbo/generators/templates/common/README.md
Comment on lines
+6
to
+7
| [](https://jsr.io/@stephansama/catppuccin-opml) | ||
| [](https://socket.dev/npm/package/@stephansama/catppuccin-opml/overview) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent badge ordering across READMEs.
The badge order here is JSR → socket.dev, but in core/catppuccin-xsl/README.md it's socket.dev → JSR. Consider maintaining a consistent badge order across all package READMEs for uniformity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/catppuccin-opml/README.md` around lines 6 - 7, The README badge order is
inconsistent with other package READMEs; in core/catppuccin-opml/README.md swap
the two badge lines so the socket.dev badge
(https://badge.socket.dev/npm/package/@stephansama/catppuccin-opml) appears
before the JSR badge (https://jsr.io/badges/@stephansama/catppuccin-opml) to
match core/catppuccin-xsl/README.md and keep badge ordering uniform across
packages.
More templates
@stephansama/ai-commit-msg
@stephansama/alfred-kaomoji
@stephansama/astro-iconify-svgmap
@stephansama/auto-readme
@stephansama/catppuccin-jsonresume-theme
@stephansama/catppuccin-opml
@stephansama/catppuccin-rss
@stephansama/catppuccin-typedoc
@stephansama/catppuccin-xsl
create-stephansama-example
@stephansama/find-makefile-targets
@stephansama/github-env
@stephansama/multipublish
@stephansama/prettier-plugin-handlebars
@stephansama/remark-asciinema
@stephansama/svelte-social-share-links
@stephansama/typed-env
@stephansama/typed-events
@stephansama/typed-nocodb-api
@stephansama/typed-templates
@stephansama/types-github-action-env
@stephansama/types-lhci
commit: |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/add-socket.ts (1)
17-35:⚠️ Potential issue | 🟠 MajorGuard missing README and missing insertion anchor before write.
Line 18 can throw when
README.mdis absent, andindexOfNpmVersion === -1leads to incorrect insertion slices. Handle both cases before constructingbody.Proposed fix
const readmePath = path.resolve(pkg.dir, "README.md"); -const readmeFile = await fs.promises.readFile(readmePath, "utf8"); +const hasReadme = await fs.promises + .access(readmePath) + .then(() => true) + .catch(() => false); +if (!hasReadme) continue; + +const readmeFile = await fs.promises.readFile(readmePath, "utf8"); @@ const indexOfNpmVersion = lines.findIndex((line) => line.startsWith("[![NPM Version]"), ); +if (indexOfNpmVersion === -1) { + console.warn(`[add-socket] Skipping ${readmePath}: NPM badge line not found`); + continue; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/add-socket.ts` around lines 17 - 35, Guard against missing README and missing anchor before constructing body: when resolving/reading README (readmePath/readmeFile) wrap the read in a try/catch or check existence first and treat a missing README as an empty string; compute lines = readmeFile.split("\n") accordingly. After computing indexOfNpmVersion, handle the -1 case by choosing a safe insertion index (e.g., lines.length to append or 0 to prepend) instead of slicing with -1; then build body from lines.slice(0, insertIndex), currentTemplate, and lines.slice(insertIndex) so insertion is correct even when the anchor is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/add-socket.ts`:
- Around line 17-35: Guard against missing README and missing anchor before
constructing body: when resolving/reading README (readmePath/readmeFile) wrap
the read in a try/catch or check existence first and treat a missing README as
an empty string; compute lines = readmeFile.split("\n") accordingly. After
computing indexOfNpmVersion, handle the -1 case by choosing a safe insertion
index (e.g., lines.length to append or 0 to prepend) instead of slicing with -1;
then build body from lines.slice(0, insertIndex), currentTemplate, and
lines.slice(insertIndex) so insertion is correct even when the anchor is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 927334a3-730f-4365-a41e-8e1920e52614
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/sad-bats-battle.mdcore/github-env/README.mdcore/prettier-plugin-handlebars/package.jsonpnpm-workspace.yamlscripts/README.mdscripts/add-socket.tsturbo/generators/templates/common/README.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/multipublish/src/index.ts`:
- Around line 32-37: The current guard in index.ts checks pkg.packageJson.name
for a scope and skips publishing based on the npm package name; instead, change
the check to detect JSR-specific configuration/name scope: remove the
pkg.packageJson.name.includes("/") branch and replace it with a guard that
verifies the presence of the JSR config for the package (use the
loader/validator in jsr.ts) and then checks the JSR config's name field for a
scope (e.g., ensure jsrConfig exists and jsrConfig.name.includes("/"));
reference the existing jsr.ts helpers (the functions that parse/validate the JSR
config and the jsrConfig.name usage at lines ~39-49 and ~85) to locate how to
read the JSR config, and only skip when there is no JSR config or the JSR name
is not scoped.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: bda02ee6-2fc8-49b5-b5ec-b2304a639201
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
core/multipublish/src/index.tspackage.jsonpnpm-workspace.yaml
Comment on lines
+32
to
+37
| if (!pkg.packageJson.name.includes("/")) { | ||
| console.warn( | ||
| `attempting to publish a non scoped package skipping`, | ||
| ); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json, re
from pathlib import Path
skip_dirs = {".git", "node_modules", "dist", "build", ".next", ".turbo"}
def should_skip(path: Path):
return any(part in skip_dirs for part in path.parts)
def extract_name(path: Path):
txt = path.read_text(encoding="utf-8", errors="ignore")
m = re.search(r'"name"\s*:\s*"([^"]+)"', txt)
return m.group(1) if m else None
for pkg_json in Path(".").rglob("package.json"):
if should_skip(pkg_json):
continue
try:
pkg = json.loads(pkg_json.read_text(encoding="utf-8"))
except Exception:
continue
npm_name = pkg.get("name")
if not isinstance(npm_name, str) or "/" in npm_name:
continue
pdir = pkg_json.parent
for cfg_name in ("jsr.json", "deno.json", "jsr.jsonc", "deno.jsonc"):
cfg = pdir / cfg_name
if not cfg.exists():
continue
jsr_name = extract_name(cfg)
if jsr_name and "/" in jsr_name:
print(f"{pdir}\tnpm={npm_name}\tjsr={jsr_name}\tconfig={cfg_name}")
PYRepository: stephansama/packages
Length of output: 46
🏁 Script executed:
# First, let's examine the index.ts file around lines 32-37
cat -n core/multipublish/src/index.ts | head -50Repository: stephansama/packages
Length of output: 1655
🏁 Script executed:
# Also examine the jsr.ts file to understand JSR config loading and validation
cat -n core/multipublish/src/jsr.tsRepository: stephansama/packages
Length of output: 3811
🏁 Script executed:
# Check if there are any JSR/deno config files in the repo
find . -type f \( -name "jsr.json" -o -name "deno.json" -o -name "jsr.jsonc" -o -name "deno.jsonc" \) ! -path "*/node_modules/*" ! -path "*/.git/*" | head -20Repository: stephansama/packages
Length of output: 46
Guard checks npm package name scope instead of JSR config presence or JSR name scope.
Line 32 validates pkg.packageJson.name (npm package name), but JSR versioning should be gated by JSR config presence or the name field within the JSR config itself (lines 39-49, 85 in jsr.ts). These are independent—a package can have an unscoped npm name but scoped JSR name in jsr.json/deno.json, which would be incorrectly skipped by the current guard. The verification confirms no such packages currently exist in this repository, but the architectural issue remains.
Suggested fix
-import { updateJsrConfigVersion } from "./jsr";
+import { loadConfig as loadJsrConfig, updateJsrConfigVersion } from "./jsr";
for (const pkg of releasedPackages) {
if (args.versionJsr) {
- if (!pkg.packageJson.name.includes("/")) {
+ const { config: jsrConfig } = await loadJsrConfig(pkg.dir);
+ if (!jsrConfig?.name.includes("/")) {
console.warn(
- `attempting to publish a non scoped package skipping`,
+ `skipping JSR version update for package "${pkg.packageJson.name}" because JSR name is not scoped`,
);
continue;
}
await updateJsrConfigVersion(pkg);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/multipublish/src/index.ts` around lines 32 - 37, The current guard in
index.ts checks pkg.packageJson.name for a scope and skips publishing based on
the npm package name; instead, change the check to detect JSR-specific
configuration/name scope: remove the pkg.packageJson.name.includes("/") branch
and replace it with a guard that verifies the presence of the JSR config for the
package (use the loader/validator in jsr.ts) and then checks the JSR config's
name field for a scope (e.g., ensure jsrConfig exists and
jsrConfig.name.includes("/")); reference the existing jsr.ts helpers (the
functions that parse/validate the JSR config and the jsrConfig.name usage at
lines ~39-49 and ~85) to locate how to read the JSR config, and only skip when
there is no JSR config or the JSR name is not scoped.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/multipublish/src/index.test.ts (1)
61-78:⚠️ Potential issue | 🟡 MinorAssert exact publish call count to prevent false positives.
The two
toHaveBeenCalledWithchecks don’t fail on extra publish invocations. Add an exact count assertion for stronger regression protection.✅ Suggested test hardening
expect(mocks.publishPlatform).toHaveBeenCalledWith( expect.objectContaining({ packageJson: expect.objectContaining({ name: "pkg1", version: "1.0.0", }), }), "npm", ); expect(mocks.publishPlatform).toHaveBeenCalledWith( expect.objectContaining({ packageJson: expect.objectContaining({ name: "@stephansama/pkg2", version: "1.0.0", }), }), "npm", ); + expect(mocks.publishPlatform).toHaveBeenCalledTimes(2);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/multipublish/src/index.test.ts` around lines 61 - 78, The test currently uses two expect(mocks.publishPlatform).toHaveBeenCalledWith(...) assertions but doesn't assert the total number of publish calls; add an exact call count assertion (e.g., expect(mocks.publishPlatform).toHaveBeenCalledTimes(2)) in the test in core/multipublish/src/index.test.ts to ensure no extra publishes occur alongside the two specific toHaveBeenCalledWith checks for publishPlatform and the packageJson objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@core/multipublish/src/index.test.ts`:
- Around line 61-78: The test currently uses two
expect(mocks.publishPlatform).toHaveBeenCalledWith(...) assertions but doesn't
assert the total number of publish calls; add an exact call count assertion
(e.g., expect(mocks.publishPlatform).toHaveBeenCalledTimes(2)) in the test in
core/multipublish/src/index.test.ts to ensure no extra publishes occur alongside
the two specific toHaveBeenCalledWith checks for publishPlatform and the
packageJson objects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8329ad88-69c8-4120-b8c4-3b2b3a0918a3
📒 Files selected for processing (3)
.husky/commit-msg.husky/pre-commitcore/multipublish/src/index.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
mainhave been mergedmain