fix(repo): add recovery for release downstream notifications#8027
fix(repo): add recovery for release downstream notifications#8027jacekradko wants to merge 6 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: ce4cb24 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/release.yml (3)
188-190: Optional: UsePromise.allSettledfor more resilient dispatch.If one dispatch fails (e.g., network issue to one repo),
Promise.allrejects immediately and remaining dispatches may not complete. UsingPromise.allSettledensures all dispatches are attempted and failures can be logged individually.♻️ Proposed change
- await Promise.all(dispatches); - core.notice('Recovery dispatch completed successfully'); + const results = await Promise.allSettled(dispatches); + const failures = results.filter(r => r.status === 'rejected'); + if (failures.length > 0) { + failures.forEach((f, i) => core.warning(`Dispatch ${i} failed: ${f.reason}`)); + core.setFailed(`${failures.length} of ${results.length} dispatches failed`); + } else { + core.notice('Recovery dispatch completed successfully'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 188 - 190, Replace the Promise.all usage so all items in the dispatches array are awaited even if some fail: use Promise.allSettled(dispatches) instead of Promise.all(dispatches), then iterate the returned results to log individual failures (use core.error or core.warning for rejected results) and call core.notice('Recovery dispatch completed successfully') only if all settled results succeeded or adjust message to reflect partial failures; update references in this block to the dispatches identifier and the core.notice call accordingly.
167-189: Duplicated dispatch logic — consider extracting to a shared script or composite action.The dispatch array here is identical to lines 91–113 in the normal trigger step. If downstream targets change, both locations need updating. Consider extracting to a reusable composite action or a shared Node script to keep them in sync.
That said, keeping the recovery step self-contained has clarity benefits for incident response.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 167 - 189, The dispatch logic duplicated in the release workflow (the dispatches array built with github.rest.actions.createWorkflowDispatch and awaited via Promise.all(dispatches)) should be extracted to a single reusable implementation; refactor by moving the array construction and dispatch invocation into a shared helper (e.g., a function like buildAndDispatchWorkflows or an external composite action) and replace both occurrences (the normal trigger step and the recovery step) to call that helper so downstream target changes only need to be updated in one place.
142-154: Consider verifying all dispatched packages, not just@clerk/clerk-js.The recovery only confirms
@clerk/clerk-jswas published to npm, but the dispatch sendsclerkUiVersionandnextjsVersionas well. In an edge case where clerk-js published but ui/nextjs didn't, downstream repos would receive potentially unpublished versions.If the publish step is atomic (all-or-nothing), this is fine. Otherwise, consider verifying
@clerk/uiand@clerk/nextjstoo, or document the assumption.♻️ Optional: verify additional packages
+ // Verify all packages that will be dispatched + const packagesToVerify = [ + '@clerk/clerk-js', + '@clerk/ui', + '@clerk/nextjs' + ]; + + for (const pkg of packagesToVerify) { + const localVersion = require(`./packages/${pkg.replace('@clerk/', '')}/package.json`).version; + try { + const npmVer = execSync(`npm view ${pkg}@${localVersion} version`, { encoding: 'utf8' }).trim(); + if (npmVer !== localVersion) { + console.log(`${pkg} version mismatch, skipping recovery`); + return; + } + } catch { + console.log(`${pkg}@${localVersion} not found on npm, no recovery needed`); + return; + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 142 - 154, The recovery logic currently only validates npmVersion for `@clerk/clerk-js` using execSync and clerkjsVersion; update it to also verify `@clerk/ui` and `@clerk/nextjs` (compare their npm view version to clerkUiVersion and nextjsVersion respectively) before returning success, and log which package mismatched (use npmVersionUi, npmVersionNext or similar local vars) so the dispatch only proceeds if all three published versions match; alternatively, if atomic publishing is guaranteed, add a brief comment referencing that assumption next to the existing execSync/check block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 188-190: Replace the Promise.all usage so all items in the
dispatches array are awaited even if some fail: use
Promise.allSettled(dispatches) instead of Promise.all(dispatches), then iterate
the returned results to log individual failures (use core.error or core.warning
for rejected results) and call core.notice('Recovery dispatch completed
successfully') only if all settled results succeeded or adjust message to
reflect partial failures; update references in this block to the dispatches
identifier and the core.notice call accordingly.
- Around line 167-189: The dispatch logic duplicated in the release workflow
(the dispatches array built with github.rest.actions.createWorkflowDispatch and
awaited via Promise.all(dispatches)) should be extracted to a single reusable
implementation; refactor by moving the array construction and dispatch
invocation into a shared helper (e.g., a function like buildAndDispatchWorkflows
or an external composite action) and replace both occurrences (the normal
trigger step and the recovery step) to call that helper so downstream target
changes only need to be updated in one place.
- Around line 142-154: The recovery logic currently only validates npmVersion
for `@clerk/clerk-js` using execSync and clerkjsVersion; update it to also verify
`@clerk/ui` and `@clerk/nextjs` (compare their npm view version to clerkUiVersion
and nextjsVersion respectively) before returning success, and log which package
mismatched (use npmVersionUi, npmVersionNext or similar local vars) so the
dispatch only proceeds if all three published versions match; alternatively, if
atomic publishing is guaranteed, add a brief comment referencing that assumption
next to the existing execSync/check block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: fa16279b-d0e3-4368-a06c-aaebf96222a1
📒 Files selected for processing (2)
.changeset/release-dispatch-recovery.md.github/workflows/release.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
174-195: Consider extracting shared dispatch logic.The dispatch array construction (lines 174-195) is nearly identical to the normal trigger step (lines 91-112). If downstream targets change, both locations must be updated.
This is acceptable for now given the workflow context, but consider extracting to a shared composite action or reusable script if the dispatch logic grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 174 - 195, The dispatches array duplication should be consolidated so updates only occur in one place: extract the shared creation logic around github.rest.actions.createWorkflowDispatch into a reusable helper (e.g., a local function like makeDispatch(owner, repo, workflow_id, ref, inputs) or move into a composite action/reusable script) and replace both the array at lines where dispatches is built and the similar block used for the normal trigger with calls to that helper; ensure the helper accepts owner, repo, workflow_id, ref, and inputs (used for clerkjsVersion, clerkUiVersion, nextjsVersion) and returns the createWorkflowDispatch call so the code uses the single source of truth for all dispatch entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 138-141: The pre-release guard only checks clerkjsVersion; update
the logic around the if block that references clerkjsVersion to also inspect
clerkUiVersion for pre-release markers (e.g., a hyphen) and skip the recovery if
either version is a pre-release; adjust the console.log to indicate which
version(s) caused the skip (use the existing clerkjsVersion and clerkUiVersion
identifiers so the message shows the offending version(s)).
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 174-195: The dispatches array duplication should be consolidated
so updates only occur in one place: extract the shared creation logic around
github.rest.actions.createWorkflowDispatch into a reusable helper (e.g., a local
function like makeDispatch(owner, repo, workflow_id, ref, inputs) or move into a
composite action/reusable script) and replace both the array at lines where
dispatches is built and the similar block used for the normal trigger with calls
to that helper; ensure the helper accepts owner, repo, workflow_id, ref, and
inputs (used for clerkjsVersion, clerkUiVersion, nextjsVersion) and returns the
createWorkflowDispatch call so the code uses the single source of truth for all
dispatch entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: cabe833e-be70-41f9-93aa-1a98310b3cd7
📒 Files selected for processing (1)
.github/workflows/release.yml
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@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: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
139-142:⚠️ Potential issue | 🟡 MinorPre-release check should also validate
clerkUiVersion.The current check only validates
clerkjsVersionfor pre-release markers. IfclerkjsVersionis stable butclerkUiVersionis a pre-release (e.g.,1.0.0-canary.5), the recovery would proceed and dispatch with a pre-release UI version to downstream repos.,
🐛 Proposed fix to check both versions
// Only recover stable releases - if (clerkjsVersion.includes('-')) { - console.log(`Skipping recovery: ${clerkjsVersion} is a pre-release`); + if (clerkjsVersion.includes('-') || clerkUiVersion.includes('-')) { + console.log(`Skipping recovery: clerkjs=${clerkjsVersion}, ui=${clerkUiVersion} contains a pre-release`); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 139 - 142, The current pre-release guard only checks clerkjsVersion and can allow dispatch when clerkUiVersion is a pre-release; update the pre-release check to validate both clerkjsVersion and clerkUiVersion (e.g., test if either string includes '-' or matches a pre-release semver pattern) before proceeding. If either clerkjsVersion or clerkUiVersion is detected as a pre-release, log a clear message naming the offending variable(s) and return to skip recovery/dispatch. Ensure you reference and use the existing variables clerkjsVersion and clerkUiVersion in the updated conditional and log.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
175-197: Consider extracting duplicate dispatch logic into a reusable composite action or script.The dispatch logic at lines 175-196 is nearly identical to lines 91-112. If downstream targets change (new repos, updated workflow names, different inputs), both locations require updates, risking divergence.
A reusable composite action or a shared script could centralize this logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 175 - 197, The dispatch creation logic (the dispatches array built with github.rest.actions.createWorkflowDispatch and awaited via Promise.all(dispatches)) is duplicated; extract it into a single reusable unit (either a composite GitHub Action or a shared script/JS module) that accepts an array of target dispatch descriptors ({owner, repo, workflow_id, ref, inputs}) and performs the createWorkflowDispatch calls; replace both duplicated blocks in release.yml with a single invocation of that composite/action or a step that runs the shared script, passing the specific inputs (clerkjsVersion, clerkUiVersion, nextjsVersion) so behavior and parameter names used by github.rest.actions.createWorkflowDispatch remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 224-226: Replace the deprecated ::set-output usage by writing the
payload to the GitHub Actions output file: capture the node script output into
the payload variable as you already do (payload=$(node scripts/notify.mjs
"$PACKAGES" '${{ github.actor }}')), then append that payload into
$GITHUB_OUTPUT (for example echo "payload=$payload" >> $GITHUB_OUTPUT) instead
of using echo ::set-output; update the run block that defines PACKAGES and
payload to use the $GITHUB_OUTPUT mechanism and ensure multi-line payloads are
handled (heredoc or proper quoting) when invoking scripts/notify.mjs.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 139-142: The current pre-release guard only checks clerkjsVersion
and can allow dispatch when clerkUiVersion is a pre-release; update the
pre-release check to validate both clerkjsVersion and clerkUiVersion (e.g., test
if either string includes '-' or matches a pre-release semver pattern) before
proceeding. If either clerkjsVersion or clerkUiVersion is detected as a
pre-release, log a clear message naming the offending variable(s) and return to
skip recovery/dispatch. Ensure you reference and use the existing variables
clerkjsVersion and clerkUiVersion in the updated conditional and log.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 175-197: The dispatch creation logic (the dispatches array built
with github.rest.actions.createWorkflowDispatch and awaited via
Promise.all(dispatches)) is duplicated; extract it into a single reusable unit
(either a composite GitHub Action or a shared script/JS module) that accepts an
array of target dispatch descriptors ({owner, repo, workflow_id, ref, inputs})
and performs the createWorkflowDispatch calls; replace both duplicated blocks in
release.yml with a single invocation of that composite/action or a step that
runs the shared script, passing the specific inputs (clerkjsVersion,
clerkUiVersion, nextjsVersion) so behavior and parameter names used by
github.rest.actions.createWorkflowDispatch remain identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 2659668d-c8f3-4ae6-9406-179d8f157552
📒 Files selected for processing (1)
.github/workflows/release.yml
jacekradko
force-pushed
the
jacek/release-dispatch-recovery
branch
from
80aab84 to
9a6a63f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
138-141:⚠️ Potential issue | 🟡 MinorPre-release check should also validate
clerkUiVersion.This issue was flagged in a previous review but remains unaddressed. The current check only validates
clerkjsVersionfor pre-release markers. IfclerkjsVersionis stable butclerkUiVersionis a pre-release (e.g.,1.0.0-canary.5), recovery would proceed and dispatch with a pre-release UI version.🐛 Proposed fix to check both versions
// Only recover stable releases - if (clerkjsVersion.includes('-')) { - console.log(`Skipping recovery: ${clerkjsVersion} is a pre-release`); + if (clerkjsVersion.includes('-') || clerkUiVersion.includes('-')) { + console.log(`Skipping recovery: clerkjs=${clerkjsVersion}, ui=${clerkUiVersion} contains a pre-release`); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 138 - 141, The pre-release guard only checks clerkjsVersion and can allow dispatch when clerkUiVersion is a pre-release; update the conditional that references clerkjsVersion to also check clerkUiVersion (both clerkjsVersion.includes('-') || clerkUiVersion.includes('-')) and log which version caused the skip (e.g., include clerkjsVersion and clerkUiVersion in the console message) so recovery is skipped whenever either package is a pre-release.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Line 76: The workflow contains an unused step identifier `id: trigger` that
isn't referenced elsewhere; either remove the `id: trigger` line from the step
definition to avoid dead identifiers or add a brief inline comment next to `id:
trigger` explaining its intended future use for readability and maintainability
so reviewers understand why it remains present.
- Around line 166-170: Move the pre-mode check (variable preMode created via
require("fs").existsSync("./.changeset/pre.json")) to run before any calls to
isPublished() so we skip npm/network queries when in pre-mode; specifically,
relocate the block that sets preMode and calls core.warning("Changeset in
pre-mode, skipping recovery dispatch") and return to execute immediately at the
start of the workflow logic (before any isPublished() checks), ensuring the
early return prevents the subsequent npm publication checks from running.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 138-141: The pre-release guard only checks clerkjsVersion and can
allow dispatch when clerkUiVersion is a pre-release; update the conditional that
references clerkjsVersion to also check clerkUiVersion (both
clerkjsVersion.includes('-') || clerkUiVersion.includes('-')) and log which
version caused the skip (e.g., include clerkjsVersion and clerkUiVersion in the
console message) so recovery is skipped whenever either package is a
pre-release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: a3384b25-b97d-426f-8c3b-f90cdde54127
📒 Files selected for processing (1)
.github/workflows/release.yml
|
Superseded by #8102 which combines this recovery step with the |
1 participant
Summary
changeset publishsucceeds (packages land on npm) but the changesets action step fails afterward (e.g. duringgit push --follow-tags)publishedoutput is never set totrue, so downstream repo notifications are silently skipped — and on retry, nothing is left to publishRoot cause investigation: The
6.1.0stable release on March 9 published to npm but the changesets action failed on attempt 1. Attempt 2 found "No unpublished projects" and also skipped the dispatch. As a result,sdk-infra-workerswas stuck on6.0.0until manually dispatched today.How the recovery step works
if: always() && steps.changesets.conclusion == 'failure')package.jsonversions for@clerk/clerk-jsand@clerk/uiThe dispatches are idempotent —
sdk-infra-workersjust pins the version,dashboardprepares an update,clerk-docsrebuilds typedoc.Test plan
Summary by CodeRabbit