Conversation
Add an npm publish prerequisite check for @ever-co scope access, make the publish workflow fail early with a clearer auth/scope error, rewrite the README around the current Rust/native packaging flow, and replace the stale CircleCI release config with a build-validation pipeline.
Summary by CodeRabbit
WalkthroughCircleCI moved from Node-based v2 jobs to a Rust-centric single Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
Greptile SummaryThis PR introduces a
Confidence Score: 4/5Safe to merge after addressing the undefined-return path in loadPlatformPackages. One P1 finding remains: the implicit undefined return from loadPlatformPackages() after fail() could cause a TypeError in non-standard environments. The rest of the changes are well-structured and the workflow gate logic is sound. P2 style findings do not block merge. scripts/check-npm-publish-prereqs.js — defensive return after fail() call in catch block
|
| Filename | Overview |
|---|---|
| scripts/check-npm-publish-prereqs.js | New preflight script that verifies npm auth and scope access before publishing; has a potential undefined-return path if process.exit is intercepted and uses const where let is preferred by convention. |
| .github/workflows/publish-npm-packages.yml | Adds a preflight-publish job that gates all publish jobs on npm auth/scope verification; dependency chain looks correct. |
| package.json | Adds check:publish-prereqs script entry; missing a format script covering all asset types per project convention. |
| .circleci/config.yml | Replaces the old TypeScript test+deploy workflow with a Rust-oriented build/syntax-check job; deploy responsibility now lives entirely in GitHub Actions. |
| README.md | Updated to reflect Rust-based architecture, revised examples, and trimmed stale badges and legacy sections. |
Sequence Diagram
sequenceDiagram
participant WD as workflow_dispatch
participant PF as preflight-publish job
participant NPM as npm registry
participant PP as publish-platform jobs (7x)
participant PM as publish-main job
WD->>PF: trigger
PF->>NPM: npm whoami (NODE_AUTH_TOKEN)
NPM-->>PF: username
PF->>NPM: npm access list packages @ever-co --json
NPM-->>PF: pkg read-write or read-only map
PF->>PF: loadPlatformPackages() scan npm/cli-*/package.json
PF->>PF: fail if any pkg is read-only
PF-->>PP: needs: preflight-publish (gate)
PP->>PP: cargo build --release --target matrix.target
PP->>PP: prepare-platform-package.js
PP->>NPM: npm publish dist/artifact --access public
PP-->>PM: needs: publish-platform (gate)
PM->>NPM: npm publish --access public (ever-cli root)
Comments Outside Diff (1)
-
package.json, line 14-23 (link)Missing
formatscript for consistent asset formattingPer project convention, the
scriptssection should include aformatscript that covers.js,.jsx,.json,.css, and.mdfiles for consistent formatting across all asset types. Example:"format": "prettier --write \"**/*.{js,jsx,json,css,md}\""
Rule Used: Expand the format script in package.json to includ... (source)
Learnt From
ever-co/ever-teams#3858Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "fix(release): use documented npm access ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check-npm-publish-prereqs.js`:
- Around line 28-48: The current discovery in loadPlatformPackages is ad-hoc and
diverges from sync-package-versions.js; extract the directory-discovery logic
into a shared helper (e.g., discoverPlatformPackageDirs(repoRoot) in
scripts/lib/platform-packages.js) that returns a sorted list of npm/cli-*
package directory paths (or names), then update loadPlatformPackages to call
discoverPlatformPackageDirs and map those dirs to package.json -> package names,
and update sync-package-versions.js to use discoverPlatformPackageDirs instead
of its hardcoded list so both scripts share the same discovery source.
- Around line 74-87: The validation currently filters platformPackages into
writablePackages and readOnlyPackages using accessiblePackages but misses
packages that aren't present in accessiblePackages (i.e., unpublished/not
visible); update the logic around writablePackages/readOnlyPackages to also
compute a missingPackages list by selecting pkg where
!Object.hasOwn(accessiblePackages, pkg), and if missingPackages.length > 0 call
fail with a clear message (similar to the existing fail for read-only) listing
missingPackages.join(', ') so the PR check reports packages not yet published
and avoids surprising publish-time failures; reference the existing symbols
writablePackages, readOnlyPackages, accessiblePackages, platformPackages and the
fail() call when implementing this change.
🪄 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: 7ab305bb-2f6e-4e2d-b64a-4100b9ffd742
📒 Files selected for processing (5)
.circleci/config.yml.github/workflows/publish-npm-packages.ymlREADME.mdpackage.jsonscripts/check-npm-publish-prereqs.js
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/check-npm-publish-prereqs.js (2)
28-44: 🛠️ Refactor suggestion | 🟠 MajorUnify package discovery with the version-sync script to avoid drift.
loadPlatformPackages()implements localnpm/cli-*discovery here. This should come from a shared helper used by both publish preflight and version-sync flows.#!/bin/bash set -euo pipefail echo "Locating sync-package-versions script..." files="$(fd -i 'sync-package-versions.js$' -t f || true)" if [ -z "${files}" ]; then echo "sync-package-versions.js not found" exit 0 fi echo "${files}" echo echo "Checking for hardcoded platform package discovery patterns:" echo "${files}" | xargs -I{} rg -n -C3 "cli-|npm/|readdirSync|package.json" "{}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-npm-publish-prereqs.js` around lines 28 - 44, loadPlatformPackages currently performs an independent local discovery of npm/cli-* packages (using repoRoot, npmDir and readdirSync) which duplicates the logic in the version-sync flow; replace this inline discovery by importing and invoking the shared helper used by sync-package-versions (the version-sync script or its exported function) so both flows use the same implementation, remove the readdirSync/JSON parsing block from loadPlatformPackages, and update any exports/require paths so loadPlatformPackages delegates to that shared helper (preserve the function name loadPlatformPackages as the caller-facing API).
95-101:⚠️ Potential issue | 🟠 MajorFail preflight when platform packages are not visible in scope.
Lines 95–100 only log unseen packages. This allows CI preflight to pass while publish can still fail later on create-permission checks.
Proposed fix
-if (unpublishedPackages.length > 0) { - console.log( - `Packages not yet visible in the scope: ${unpublishedPackages.join( - ', ', - )}. This can be valid for a first publish, but publish will still require organization-level permission to create packages under the scope.`, - ); -} else if (writablePackages.length === 0) { +if (unpublishedPackages.length > 0) { + fail( + `Packages not yet visible in the scope: ${unpublishedPackages.join( + ', ', + )}. Preflight cannot verify create permission for these packages.`, + ); +} else if (writablePackages.length === 0) { console.log('No writable platform packages are currently visible in the scope.'); }Is there an npm CLI command that reliably verifies org-level create permission for unpublished scoped packages before first publish?
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check-npm-publish-prereqs.js`:
- Around line 63-81: The code assumes npm JSON output yields an object and
assigns accessiblePackages directly from JSON.parse(output); add a defensive
type/shape guard after parsing to ensure accessiblePackages is a plain object
mapping package names to access levels before using it in
writablePackages/readOnlyPackages/unpublishedPackages. Specifically, after the
run('npm', ...) and JSON.parse(output) call (where accessiblePackages is set),
validate that typeof accessiblePackages === 'object' && accessiblePackages !==
null && !Array.isArray(accessiblePackages); if the check fails, set
accessiblePackages = {} and call fail(...) or log a clear error (similar to the
existing fail used in the catch) so subsequent filters (writablePackages,
readOnlyPackages, unpublishedPackages) that rely on
Object.hasOwn(accessiblePackages, ...) are safe; keep the rest of the flow
(loadPlatformPackages(), filters) unchanged.
---
Duplicate comments:
In `@scripts/check-npm-publish-prereqs.js`:
- Around line 28-44: loadPlatformPackages currently performs an independent
local discovery of npm/cli-* packages (using repoRoot, npmDir and readdirSync)
which duplicates the logic in the version-sync flow; replace this inline
discovery by importing and invoking the shared helper used by
sync-package-versions (the version-sync script or its exported function) so both
flows use the same implementation, remove the readdirSync/JSON parsing block
from loadPlatformPackages, and update any exports/require paths so
loadPlatformPackages delegates to that shared helper (preserve the function name
loadPlatformPackages as the caller-facing API).
🪄 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: 357f5e2d-6f0d-45fb-857c-a30612297ca5
📒 Files selected for processing (1)
scripts/check-npm-publish-prereqs.js
2 participants
No description provided.