Skip to content

chore(sdk): Bring back 'next' tag publish on sdks - #36722

Merged
KevinDavilaDotCMS merged 6 commits into
mainfrom
36701-reintroduce-next-npm-dist-tag-for-internal-devqa-testing-on-merge-to-main
Jul 25, 2026
Merged

chore(sdk): Bring back 'next' tag publish on sdks#36722
KevinDavilaDotCMS merged 6 commits into
mainfrom
36701-reintroduce-next-npm-dist-tag-for-internal-devqa-testing-on-merge-to-main

Conversation

@KevinDavilaDotCMS

@KevinDavilaDotCMS KevinDavilaDotCMS commented Jul 24, 2026

Copy link
Copy Markdown
Member
  • Updated the action to include an optional input for specifying the npm dist-tag (e.g., latest, next) during SDK package publishing.
  • Modified the action's description to reflect the new functionality.
  • Introduced a new job in the CI workflow to publish SDK packages under the tag for internal development and QA use, ensuring that it only runs when SDK files change.
  • Added Slack notifications for both successful and failed SDK publishes, providing visibility into the publishing process.

This code is already tested in core-workflow-test: https://github.com/dotCMS/core-workflow-test/actions/runs/30069848966

This PR fixes: #36701

This PR fixes: #36701

- Updated the  action to include an optional  input for specifying the npm dist-tag (e.g., latest, next) during SDK package publishing.
- Modified the action's description to reflect the new functionality.
- Introduced a new job in the CI workflow to publish SDK packages under the  tag for internal development and QA use, ensuring that it only runs when SDK files change.
- Added Slack notifications for both successful and failed SDK  publishes, providing visibility into the publishing process.

This change improves the flexibility of SDK versioning and enhances the CI/CD pipeline for internal testing.
@KevinDavilaDotCMS
KevinDavilaDotCMS requested a review from a team as a code owner July 24, 2026 06:00
@KevinDavilaDotCMS KevinDavilaDotCMS changed the title Enhance SDK publishing workflow with npm dist-tag support chore(sdk): Bring back 'next' tag publish on sdks Jul 24, 2026
@github-actions github-actions Bot added the Area : CI/CD PR changes GitHub Actions/workflows label Jul 24, 2026
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @dario-daza's task in 2m 37s —— View job


Code Review — PR #36722

  • Gather context (diff vs origin/main)
  • Verify action published output + finalize-phase job scan
  • Trace continue-on-error / outcome / success() interactions
  • Provide review feedback

The two .yml changes are solid, and the reasoning captured in the PR comments (job-scan blast radius, outcome vs conclusion, semver precedence) is correct. One real gap remains.

New Issues

  • 🟠 High: .github/workflows/cicd_3-trunk.yml:56 — The Compute next version step has no continue-on-error, unlike the publish step below it. If npm view @dotcms/client version fails transiently (registry hiccup / rate limit), that step fails unmasked → the whole publish-sdk-next job fails → finalize's prepare-report-data scans every job's conclusion (cicd_comp_finalize-phase.yml:110) and marks the entire trunk run red. This is precisely the "don't sink the run over an internal dev/QA publish" outcome the publish-step guard was added to avoid — the guard just doesn't cover the step that's arguably more likely to hit npm flakiness (it's the first network call).

    Second-order effect: because an unguarded step failure short-circuits the remaining steps and the failure-Slack step is gated only on if: steps.deploy-javascript-sdk.outcome == 'failure' (no always()/failure()), that step is skipped when compute-version fails. So the || 'unknown — the version-compute step itself failed' fallback added in Fix 2 is unreachable — no Slack message fires for the very case it was written for, and the run still goes red silently.

    Fix options (any one):

    • Add continue-on-error: true to the Compute next version step, and change the failure-Slack guard to if: always() && (steps.deploy-javascript-sdk.outcome == 'failure' || steps.next-version.outcome == 'failure') so it fires (and the fallback text renders) on either failure.
    • Or move the version computation into the composite action so the single guarded step covers it.

    Fix this →

Notes (non-blocking)

  • The success() / outcome / continue-on-error interplay on the publish step and the two Slack steps is correct: on a masked publish failure, success() is still true but steps.deploy-javascript-sdk.outputs.published is empty (the action's deployment_status step is if: success() and won't run), so the announcement is skipped and the failure notification fires on outcome == 'failure'. Good.
  • next versions like <latest>-next.<run> are -next.N-suffixed, so idempotency's "already published" skip never trips (run number is unique per run) and semver keeps them below latest. As you noted, fine for explicit @next installs.

Re: reviewer comments on PersonaAPITest.java

The await/flaky-test change is not in this PR's diff against origin/main — commit 15e931bb "remove flaky test fix" already reverted it. The only changed files now are the two workflow/action .yml files. So nollymar's and dario-daza's concerns appear already addressed; no action needed here.

@KevinDavilaDotCMS

Copy link
Copy Markdown
Member Author

Applied two fixes from the Claude Code review, plus a note on why a third finding is being accepted as-is rather than code-fixed.

Fix 1 — continue-on-error: true on the next publish step

cicd_comp_finalize-phase.yml doesn't only look at its own needs: — its prepare-report-data job independently queries GET /repos/{repo}/actions/runs/{run_id}/jobs and scans every job's conclusion in the run, excluding only names matching ^(Finalize|Final Status|.*[Ff]inalize.*). "Publish SDK (next)" doesn't match that exclusion, so without this fix a transient npm/registry hiccup on this internal dev/QA publish would mark the entire trunk workflow red. Added continue-on-error: true to the publish step so it can't sink the run.

One subtlety this surfaced: simply adding continue-on-error: true would have silently broken the existing "Slack Notification (SDK next failure)" step. continue-on-error masks a step's conclusion to success, and the job-level failure()/success() context functions key off conclusion, not outcome — so if: failure() would never fire again once the step above it has continue-on-error: true. Fixed by checking the step's own outcome directly instead: if: steps.deploy-javascript-sdk.outcome == 'failure'. outcome reflects the real pre-continue-on-error result, so the failure Slack message still fires correctly.

Fix 2 — fallback text for the failure Slack message

If "Compute next version" itself fails (e.g. npm view fails to resolve the current latest), steps.next-version.outputs.version is empty, and the failure message would render publish version with nothing after it. Added a fallback: ${{ steps.next-version.outputs.version || 'unknown — the version-compute step itself failed' }}.

Finding #3 — semver precedence / caret-range risk for next versions — accepted as a known limitation, not code-fixed here

next versions look like <latest>-next.<run_number> (e.g. 26.7.14-1-next.42), which per semver precedence sorts below 26.7.14-1 itself. This is expected and fine for the intended use — devs/QA install it explicitly via @dotcms/client@next, never via a bare version range. The real risk is only if something resolves it through a caret range (^26.7.14-1), which would never match a prerelease tag like this per semver rules anyway (so in practice this mostly self-protects) — but this is the same class of caret-pinning fragility already tracked as follow-up debt for examples/* in the base date-lockstep work (#36587). Not scoping a fix into this PR since nothing in this repo currently consumes next versions via a range, but flagging it explicitly so it isn't re-discovered as a surprise later.

- Added  to the SDK publishing step to prevent transient npm registry failures from marking the entire workflow as failed.
- Updated the Slack notification step to check the specific outcome of the SDK publish step, ensuring accurate failure reporting.
- Improved the failure message to handle cases where the version computation step fails, providing clearer feedback in notifications.

These changes improve the robustness of the CI/CD pipeline for SDK publishing and enhance visibility into potential issues during the process.
…ookup issue

- Marked the  as ignored to prevent test failures caused by a  during unit tests. This issue is unrelated to recent changes and is being tracked for a separate fix.
@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code and removed AI: Safe To Rollback labels Jul 24, 2026

@nollymar nollymar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR should not require an IT fix and an await might not be the right approach

@github-actions github-actions Bot removed AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code labels Jul 24, 2026
@mergify

mergify Bot commented Jul 24, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@KevinDavilaDotCMS
KevinDavilaDotCMS added this pull request to the merge queue Jul 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 24, 2026
@KevinDavilaDotCMS
KevinDavilaDotCMS added this pull request to the merge queue Jul 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 25, 2026
@KevinDavilaDotCMS
KevinDavilaDotCMS added this pull request to the merge queue Jul 25, 2026
Merged via the queue into main with commit 66cdc38 Jul 25, 2026
64 checks passed
@KevinDavilaDotCMS
KevinDavilaDotCMS deleted the 36701-reintroduce-next-npm-dist-tag-for-internal-devqa-testing-on-merge-to-main branch July 25, 2026 04:05
KevinDavilaDotCMS pushed a commit to dotCMS/core-workflow-test that referenced this pull request Jul 25, 2026
…fallback

Mirrors the fixes already verified and pushed on dotCMS/core PR dotCMS#36722:
continue-on-error on the SDK next-tag publish step (with the failure Slack
step checking step outcome directly, not job-level failure()), and a
fallback for the version string when the compute-version step itself fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : CI/CD PR changes GitHub Actions/workflows

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Reintroduce next npm dist-tag for internal dev/QA testing on merge to main

4 participants