fix(repo): use Promise.allSettled for release downstream dispatches#8102
fix(repo): use Promise.allSettled for release downstream dispatches#8102jacekradko wants to merge 5 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 32f1622 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 |
📝 WalkthroughWalkthroughThe release workflow (.github/workflows/release.yml) was updated: two trigger steps now set 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
@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: |
![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 @.github/workflows/release.yml:
- Line 116: The failure log only prints r.reason; update the failed.forEach
callback to include dispatch target identifiers so failures can be triaged
(e.g., include repo/owner, workflow name and ref/inputs). Locate the
failed.forEach(...) expression and augment its core.error message to concatenate
r.reason with fields from the dispatch request object such as r.request
(owner/repo), r.request?.workflow or r.request?.name, and r.request?.ref (or
similar properties used when building the dispatch) so each logged line contains
both the error and the target identifiers.
- Line 77: The workflow currently uses continue-on-error: true which masks job
failures while steps still call core.setFailed; remove the broad
continue-on-error: true and instead change the dispatch-failure handling to call
core.warning(...) rather than core.setFailed(...) so the dispatch error is
treated as a warning while other step failures still fail the job; update the
occurrences of core.setFailed in the dispatch logic to core.warning and delete
the continue-on-error: true entries to preserve proper failure visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: a3f7ef17-b4e2-4a74-b48d-9ab85e8c7015
📒 Files selected for processing (1)
.github/workflows/release.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/release.yml (1)
77-105:⚠️ Potential issue | 🟠 MajorStill avoid pairing
continue-on-errorwithcore.setFailed()here.At Line [77] and Line [194],
continue-on-error: truemasks any exception thrown anywhere in thesegithub-scriptsteps, not just rejected dispatches. Because Line [104] and Line [225] still callcore.setFailed(), the step also emits failure/error annotations instead of the warning-only behavior this PR is trying to introduce. Handle expected dispatch failures withcore.warning(...)and let unexpected script bugs fail the job normally.Suggested fix
- continue-on-error: true @@ - if (failures.length) { - failures.forEach(f => core.error(`Dispatch to ${f.target.repo}/${f.target.workflow_id} failed: ${f.reason?.message ?? f.reason}`)); - core.setFailed(`${failures.length} downstream dispatch(es) failed`); - } + if (failures.length) { + failures.forEach(f => core.warning(`Dispatch to ${f.target.repo}/${f.target.workflow_id} failed: ${f.reason?.message ?? f.reason}`)); + core.warning(`${failures.length} downstream dispatch(es) failed`); + }Apply the same change to both trigger steps.
GitHub Actions documentation: if a step uses `continue-on-error: true` and the script calls `core.setFailed()`, what are the step's `outcome` and `conclusion`, and does `core.warning()` better match warning-only UI annotations?Also applies to: 194-225
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 77 - 105, The github-script steps currently set continue-on-error: true but still call core.setFailed(...) when dispatches fail, which turns expected-dispatch failures into hard failures; change the handler that processes the failures array so it uses core.warning(...) (including the same descriptive message and the failure count) instead of core.setFailed(...), leaving unexpected exceptions unhandled so they will fail the step normally; apply this same replacement in both github-script steps that build the targets/results/failures logic (the code that constructs targets, runs Promise.allSettled, inspects results into failures, and currently calls core.setFailed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 77-105: The github-script steps currently set continue-on-error:
true but still call core.setFailed(...) when dispatches fail, which turns
expected-dispatch failures into hard failures; change the handler that processes
the failures array so it uses core.warning(...) (including the same descriptive
message and the failure count) instead of core.setFailed(...), leaving
unexpected exceptions unhandled so they will fail the step normally; apply this
same replacement in both github-script steps that build the
targets/results/failures logic (the code that constructs targets, runs
Promise.allSettled, inspects results into failures, and currently calls
core.setFailed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6518ee06-f6cf-4be2-a767-d699fccfb653
📒 Files selected for processing (1)
.github/workflows/release.yml
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 @.github/workflows/release.yml:
- Around line 147-153: The catch in isPublished currently swallows all errors;
change it to capture the error (e.g., catch (err)) and log it (for example using
console.warn or core.warning) with context including the package name and
version before returning false so transient registry or network errors are
visible; update the isPublished function around the execSync call to include the
caught error in the log message.
- Around line 171-175: The duplicated targets array (variable name targets
containing entries with repo:'sdk-infra-workers', repo:'dashboard',
repo:'clerk-docs' and workflow_id keys like 'update-pkg-versions.yml',
'prepare-nextjs-sdk-update.yml', 'typedoc.yml') should be deduplicated: extract
it into a single shared source (e.g., a JSON/YAML file or a reusable composite
action) and have this workflow load/reference that single definition, or at
minimum add a clear inline comment in both places pointing to the other location
to prevent drift; update the release.yml to reference the shared definition (or
include the linking comment) and ensure inputs (clerkjsVersion, clerkUiVersion,
version: nextjsVersion) remain intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: cac0549a-5564-41d0-927e-43ff7d996b4e
📒 Files selected for processing (1)
.github/workflows/release.yml
| function isPublished(name, version) { | ||
| try { | ||
| return execSync(`npm view ${name}@${version} version`, { encoding: 'utf8' }).trim() === version; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider logging suppressed errors in isPublished for debuggability.
The catch block silently returns false for any error. While this is reasonable for "check if published" semantics, transient npm registry errors would be indistinguishable from "package not published," potentially skipping recovery when it should have run.
♻️ Proposed improvement for debuggability
function isPublished(name, version) {
try {
return execSync(`npm view ${name}@${version} version`, { encoding: 'utf8' }).trim() === version;
- } catch {
+ } catch (e) {
+ console.log(`npm view ${name}@${version} failed: ${e.message}`);
return false;
}
}📝 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.
| function isPublished(name, version) { | |
| try { | |
| return execSync(`npm view ${name}@${version} version`, { encoding: 'utf8' }).trim() === version; | |
| } catch { | |
| return false; | |
| } | |
| } | |
| function isPublished(name, version) { | |
| try { | |
| return execSync(`npm view ${name}@${version} version`, { encoding: 'utf8' }).trim() === version; | |
| } catch (e) { | |
| console.log(`npm view ${name}@${version} failed: ${e.message}`); | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 147 - 153, The catch in
isPublished currently swallows all errors; change it to capture the error (e.g.,
catch (err)) and log it (for example using console.warn or core.warning) with
context including the package name and version before returning false so
transient registry or network errors are visible; update the isPublished
function around the execSync call to include the caught error in the log
message.
| const targets = [ | ||
| { repo: 'sdk-infra-workers', workflow_id: 'update-pkg-versions.yml', inputs: { clerkjsVersion, clerkUiVersion } }, | ||
| { repo: 'dashboard', workflow_id: 'prepare-nextjs-sdk-update.yml', inputs: { version: nextjsVersion } }, | ||
| { repo: 'clerk-docs', workflow_id: 'typedoc.yml' }, | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract shared targets array to reduce duplication.
The targets array is duplicated verbatim from lines 91-95. If a downstream repository is added or removed, both locations must be updated, risking drift.
Consider extracting the targets definition to a shared location (e.g., a JSON file or a reusable composite action) or at minimum, add a comment linking the two locations to alert future maintainers.
♻️ Alternative: inline comment to link the two locations
+ // NOTE: Keep in sync with targets array in "Trigger workflows on related repos" step above
const targets = [
{ repo: 'sdk-infra-workers', workflow_id: 'update-pkg-versions.yml', inputs: { clerkjsVersion, clerkUiVersion } },
{ repo: 'dashboard', workflow_id: 'prepare-nextjs-sdk-update.yml', inputs: { version: nextjsVersion } },
{ repo: 'clerk-docs', workflow_id: 'typedoc.yml' },
];📝 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.
| const targets = [ | |
| { repo: 'sdk-infra-workers', workflow_id: 'update-pkg-versions.yml', inputs: { clerkjsVersion, clerkUiVersion } }, | |
| { repo: 'dashboard', workflow_id: 'prepare-nextjs-sdk-update.yml', inputs: { version: nextjsVersion } }, | |
| { repo: 'clerk-docs', workflow_id: 'typedoc.yml' }, | |
| ]; | |
| // NOTE: Keep in sync with targets array in "Trigger workflows on related repos" step above | |
| const targets = [ | |
| { repo: 'sdk-infra-workers', workflow_id: 'update-pkg-versions.yml', inputs: { clerkjsVersion, clerkUiVersion } }, | |
| { repo: 'dashboard', workflow_id: 'prepare-nextjs-sdk-update.yml', inputs: { version: nextjsVersion } }, | |
| { repo: 'clerk-docs', workflow_id: 'typedoc.yml' }, | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml around lines 171 - 175, The duplicated targets
array (variable name targets containing entries with repo:'sdk-infra-workers',
repo:'dashboard', repo:'clerk-docs' and workflow_id keys like
'update-pkg-versions.yml', 'prepare-nextjs-sdk-update.yml', 'typedoc.yml')
should be deduplicated: extract it into a single shared source (e.g., a
JSON/YAML file or a reusable composite action) and have this workflow
load/reference that single definition, or at minimum add a clear inline comment
in both places pointing to the other location to prevent drift; update the
release.yml to reference the shared definition (or include the linking comment)
and ensure inputs (clerkjsVersion, clerkUiVersion, version: nextjsVersion)
remain intact.
1 participant
Summary
Improves the reliability of downstream repo notifications in the release workflow. Addresses two failure modes:
1. Isolated dispatch failures shouldn't block other repos
Promise.allwithPromise.allSettledso a failure dispatching to one repo (e.g.clerk-docs403) doesn't prevent notifications to the others (sdk-infra-workers,dashboard)continue-on-error: true+core.setFailed()so failures are visible (red step) but don't block the rest of the release job2. Recovery when changesets fails after publishing
changesets/actionfails (e.g.git push --follow-tagserror)publishedis never set totrue, so downstream notifications are silently skipped — and on retry, nothing is left to publishContext: Failed run — a 403 on
clerk-docscausedPromise.allto reject. The6.1.0stable release on March 9 hit the second issue where changesets failed after publishing to npm.Supersedes #8027.
Test plan
Promise.allSettled: dispatches to unaffected repos succeed even when one dispatch failsSummary by CodeRabbit