chore(repo): Build and publish native passkeys binaries on production releases#8955
chore(repo): Build and publish native passkeys binaries on production releases#8955wobsoriano wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 3fcfb27 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a native binary CI pipeline for ChangesElectron Passkeys Native Binary Pipeline
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
wobsoriano
changed the title
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
| "fixed": [ | ||
| [ | ||
| "@clerk/electron-passkeys", | ||
| "@clerk/electron-passkeys-darwin-arm64", | ||
| "@clerk/electron-passkeys-darwin-x64", | ||
| "@clerk/electron-passkeys-win32-arm64-msvc", | ||
| "@clerk/electron-passkeys-win32-x64-msvc" | ||
| ] | ||
| ], |
There was a problem hiding this comment.
all five packages must publish at the same version number. The four platform packages are optionalDependencies of the wrapper at an exact version, and napi-rs's native loader matches the .node binary by the package version it was built with
There was a problem hiding this comment.
this workflow builds the native Rust addon for @clerk/electron-passkeys, then assembles the per-platform npm packages and uploads them as a single artifact. It's called by release.yml when a version bump to @clerk/electron-passkeys is detected
| - name: Download electron-passkeys native binaries | ||
| if: ${{ needs.detect-native.outputs.should-build == 'true' }} | ||
| uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 | ||
| with: | ||
| name: electron-passkeys-npm | ||
| path: packages/electron-passkeys/npm |
There was a problem hiding this comment.
Binaries land here before changesets/action runs so npm publish picks themup
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/electron-passkeys.yml:
- Around line 11-20: The workflow file has a duplicate permissions key defined
in the YAML structure, which causes a parsing error. Remove the second
permissions block that only contains contents: read and keep only the first
permissions block that includes both contents: read and actions: read. This will
resolve the YAML syntax error and allow the workflow to parse correctly.
In @.github/workflows/release.yml:
- Around line 46-52: Navigate to the `.github/workflows/electron-passkeys.yml`
reusable workflow file and locate the duplicate `permissions` key that is
causing the YAML parsing error. Remove one of the duplicate `permissions` blocks
so that only a single `permissions` definition remains in the file. This will
resolve the syntax error and allow the workflow call in the build-native job to
execute successfully.
In `@scripts/check-electron-passkeys-binaries.mjs`:
- Around line 27-45: The main function lacks error handling for the
findMissingBinaries call, which means any thrown errors will result in unhandled
rejections instead of GitHub annotations. Wrap the await
findMissingBinaries(npmDir) call in a try-catch block and use console.error with
the ::error:: format to report caught exceptions. Additionally, the error
message template is inaccurate as it always says "empty package" regardless of
the count value; update the message logic to correctly indicate that the issue
is having an incorrect number of binaries (not exactly 1), adjusting the text
appropriately when count is greater than 1 versus when count is 0.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 935b7c82-ea09-45c8-a2d9-eae66f8d7961
📒 Files selected for processing (10)
.changeset/config.json.changeset/electron-passkeys-native-binaries.md.github/workflows/electron-passkeys.yml.github/workflows/release.ymlscripts/canary.mjsscripts/check-electron-passkeys-binaries.mjsscripts/check-electron-passkeys-binaries.test.mjsscripts/common.mjsscripts/detect-electron-passkeys-publish.mjsscripts/snapshot.mjs
| permissions: | ||
| contents: read | ||
| actions: read | ||
|
|
||
| concurrency: | ||
| group: electron-passkeys-${{ github.head_ref || github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Duplicate permissions key causes YAML syntax error.
The workflow defines permissions twice (lines 11-12 and 19-20), which violates YAML syntax and will cause the workflow to fail parsing.
🐛 Proposed fix
Remove the duplicate permissions block:
permissions:
contents: read
actions: read
concurrency:
group: electron-passkeys-${{ github.head_ref || github.ref }}
cancel-in-progress: true
-permissions:
- contents: read
-
jobs:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: read | |
| actions: read | |
| concurrency: | |
| group: electron-passkeys-${{ github.head_ref || github.ref }} | |
| cancel-in-progress: true | |
| permissions: | |
| contents: read | |
| permissions: | |
| contents: read | |
| actions: read | |
| concurrency: | |
| group: electron-passkeys-${{ github.head_ref || github.ref }} | |
| cancel-in-progress: true | |
| jobs: |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 19-19: key "permissions" is duplicated in "workflow" section. previously defined at line:11,col:1
(syntax-check)
🪛 YAMLlint (1.37.1)
[error] 19-19: duplication of key "permissions" in mapping
(key-duplicates)
🤖 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 @.github/workflows/electron-passkeys.yml around lines 11 - 20, The workflow
file has a duplicate permissions key defined in the YAML structure, which causes
a parsing error. Remove the second permissions block that only contains
contents: read and keep only the first permissions block that includes both
contents: read and actions: read. This will resolve the YAML syntax error and
allow the workflow to parse correctly.
Source: Linters/SAST tools
| build-native: | ||
| name: Build electron-passkeys native binaries | ||
| needs: detect-native | ||
| if: ${{ needs.detect-native.outputs.should-build == 'true' }} | ||
| permissions: | ||
| contents: read | ||
| uses: ./.github/workflows/electron-passkeys.yml |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Workflow call will fail due to syntax error in electron-passkeys.yml.
The reusable workflow .github/workflows/electron-passkeys.yml contains a duplicate permissions key (already flagged in that file), which will cause this workflow call to fail during YAML parsing.
This will be resolved once the duplicate permissions block is removed from electron-passkeys.yml.
🧰 Tools
🪛 actionlint (1.7.12)
[error] 52-52: error while parsing reusable workflow "./.github/workflows/electron-passkeys.yml": yaml: unmarshal errors: line 19: mapping key "permissions" already defined at line 11
(workflow-call)
🤖 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 @.github/workflows/release.yml around lines 46 - 52, Navigate to the
`.github/workflows/electron-passkeys.yml` reusable workflow file and locate the
duplicate `permissions` key that is causing the YAML parsing error. Remove one
of the duplicate `permissions` blocks so that only a single `permissions`
definition remains in the file. This will resolve the syntax error and allow the
workflow call in the build-native job to execute successfully.
Source: Linters/SAST tools
| async function main() { | ||
| const npmDir = process.argv[2] || DEFAULT_NPM_DIR; | ||
| const missing = await findMissingBinaries(npmDir); | ||
|
|
||
| if (missing.length > 0) { | ||
| for (const { dir, count } of missing) { | ||
| console.error( | ||
| `::error::${dir} has ${count} .node binaries (expected exactly 1); publishing it would ship an empty package`, | ||
| ); | ||
| } | ||
|
|
||
| process.exit(1); | ||
| } | ||
|
|
||
| console.log('All electron-passkeys platform packages contain exactly one .node binary.'); | ||
| } | ||
|
|
||
| if (process.argv[1] && fileURLToPath(import.meta.url) === resolve(process.argv[1])) { | ||
| await main(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle CLI read errors explicitly and fix inaccurate failure text.
If findMissingBinaries() throws (e.g., missing/unreadable directory), the script currently fails with an unhandled rejection instead of a GitHub annotation. Also, Line 34 always says “empty package,” which is incorrect when count > 1.
Suggested patch
async function main() {
- const npmDir = process.argv[2] || DEFAULT_NPM_DIR;
- const missing = await findMissingBinaries(npmDir);
+ const npmDir = process.argv[2] || DEFAULT_NPM_DIR;
+ let missing;
+ try {
+ missing = await findMissingBinaries(npmDir);
+ } catch (error) {
+ const message = error instanceof Error ? error.message : String(error);
+ console.error(`::error::Failed to scan ${npmDir}: ${message}`);
+ process.exit(1);
+ }
if (missing.length > 0) {
for (const { dir, count } of missing) {
console.error(
- `::error::${dir} has ${count} .node binaries (expected exactly 1); publishing it would ship an empty package`,
+ count === 0
+ ? `::error::${dir} has 0 .node binaries (expected exactly 1); publishing it would ship an empty package`
+ : `::error::${dir} has ${count} .node binaries (expected exactly 1); publishing it would ship an invalid package`,
);
}
process.exit(1);
}🤖 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 `@scripts/check-electron-passkeys-binaries.mjs` around lines 27 - 45, The main
function lacks error handling for the findMissingBinaries call, which means any
thrown errors will result in unhandled rejections instead of GitHub annotations.
Wrap the await findMissingBinaries(npmDir) call in a try-catch block and use
console.error with the ::error:: format to report caught exceptions.
Additionally, the error message template is inaccurate as it always says "empty
package" regardless of the count value; update the message logic to correctly
indicate that the issue is having an incorrect number of binaries (not exactly
1), adjusting the text appropriately when count is greater than 1 versus when
count is 0.
2 participants
Description
Follow-up to #8786 (the Electron SDK scaffold). That PR added
@clerk/electron-passkeysto the monorepo but deliberately shipped no native binaries. This PR adds the release pipeline that builds and publishes them.On a production release where
@clerk/electron-passkeysis bumped, the native.nodebinaries (macOS arm64/x64, Windows arm64/x64) are built fresh on native runners, assembled into the per-platform packages, and published in lockstep with the wrapper.Canary and snapshot releases exclude these packages
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit