fix(repo): add recovery for release downstream notifications#8027
fix(repo): add recovery for release downstream notifications#8027jacekradko wants to merge 3 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 9a6a63f 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.
|
📝 WalkthroughWalkthroughAdds 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
| if (clerkjsVersion.includes('-')) { | ||
| console.log(`Skipping recovery: ${clerkjsVersion} is a pre-release`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Pre-release check should also validate clerkUiVersion.
The current check only validates clerkjsVersion for pre-release markers. If clerkjsVersion is stable (e.g., 6.1.0) but clerkUiVersion is 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 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)).
@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
.github/workflows/release.yml
Outdated
| run: | | ||
| PACKAGES='${{ steps.changesets.outputs.publishedPackages || steps.recover.outputs.publishedPackages }}' | ||
| payload=$(node scripts/notify.mjs "$PACKAGES" '${{ github.actor }}') && echo ::set-output name=payload::${payload//$'\n'/'%0A'} |
There was a problem hiding this comment.
Replace deprecated ::set-output workflow command.
The ::set-output command is deprecated. Use >> $GITHUB_OUTPUT instead for setting step outputs.
🔧 Proposed fix
run: |
PACKAGES='${{ steps.changesets.outputs.publishedPackages || steps.recover.outputs.publishedPackages }}'
- payload=$(node scripts/notify.mjs "$PACKAGES" '${{ github.actor }}') && echo ::set-output name=payload::${payload//$'\n'/'%0A'}
+ payload=$(node scripts/notify.mjs "$PACKAGES" '${{ github.actor }}')
+ echo "payload<<EOF" >> $GITHUB_OUTPUT
+ echo "$payload" >> $GITHUB_OUTPUT
+ echo "EOF" >> $GITHUB_OUTPUTNote: The heredoc syntax handles multi-line payloads more reliably than URL-encoding newlines.
📝 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.
| run: | | |
| PACKAGES='${{ steps.changesets.outputs.publishedPackages || steps.recover.outputs.publishedPackages }}' | |
| payload=$(node scripts/notify.mjs "$PACKAGES" '${{ github.actor }}') && echo ::set-output name=payload::${payload//$'\n'/'%0A'} | |
| run: | | |
| PACKAGES='${{ steps.changesets.outputs.publishedPackages || steps.recover.outputs.publishedPackages }}' | |
| payload=$(node scripts/notify.mjs "$PACKAGES" '${{ github.actor }}') | |
| echo "payload<<EOF" >> $GITHUB_OUTPUT | |
| echo "$payload" >> $GITHUB_OUTPUT | |
| echo "EOF" >> $GITHUB_OUTPUT |
🧰 Tools
🪛 actionlint (1.7.11)
[error] 224-224: workflow command "set-output" was deprecated. use echo "{name}={value}" >> $GITHUB_OUTPUT instead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
(deprecated-commands)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 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.
jacekradko
force-pushed
the
jacek/release-dispatch-recovery
branch
from
80aab84 to
9a6a63f
Compare
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