Update CI CD to ensure SDK publishing occurs

[KevinDavilaDotCMS]

Proposed Changes

• change 1
• change 2

Checklist

• Tests
• Translations
• Security Implications Contemplated (add notes if applicable)

Additional Info

** any additional useful context or info **

Screenshots

Original	Updated
** original screenshot **	** updated screenshot **

This PR fixes: #36701

This PR fixes: #36701

[claude]

Claude finished @KevinDavilaDotCMS's task in 3m 6s —— View job

---

PR Review: Update CI CD to ensure SDK publishing occurs

Scope of the diff is a single change to the publish-sdk-next job's if: gate in .github/workflows/cicd_3-trunk.yml: replacing needs.test.result == 'success' with the always() && !failure() && !cancelled() idiom used by the sibling jobs.

New Issues

• 🟡 Medium: .github/workflows/cicd_3-trunk.yml:140-152 — The comment's core premise is worth verifying against a real run. It states that because the test matrix is empty on a normal push, "test always resolves to skipped," so the old success gate blocked the job. But the reusable cicd_comp_test-phase.yml has a setup-matrix job with no if: condition (cicd_comp_test-phase.yml:88), so it always runs and succeeds even when has-tests == 'false' and every matrix suite is skipped. A caller job that uses: a reusable workflow generally resolves to success when the workflow completes with at least one successful (and no failed) job — which suggests needs.test.result was actually 'success', not 'skipped', on ordinary pushes.

  • Assumption: GitHub resolves the caller test job to success (not skipped) when setup-matrix succeeds and the downstream matrix jobs are all skipped.
  • What to verify: Look at a recent -3 Trunk Workflow run on main where sdk_libs changed and confirm what needs.test.result actually was and whether publish-sdk-next was skipped under the old gate. If test was in fact success, the original gate would have worked and this change, while harmless, addresses a different root cause than the comment describes — the comment should be corrected to reflect the real reason.

• 🟡 Medium: .github/workflows/cicd_3-trunk.yml:150 — Behavioral loosening: !failure() && !cancelled() treats a skipped test as acceptable, whereas == 'success' did not. If the upstream build job ever fails (not merely skipped-because-artifacts-reused), test becomes skipped (its own !failure() gate fails), and this job would still publish to the public npm registry. The SDK build is self-contained (deploy-javascript-sdk re-checks out and rebuilds from source), and this matches the deployment sibling's idiom, so it's likely acceptable — but confirm that publishing a next SDK build on top of a failed trunk build is intended.

  • What to verify: Is publishing @dotcms/*@next desirable when the core build phase has failed? If not, consider needs.test.result != 'failure' && needs.build.result != 'failure' or keeping build out of the cascade explicitly.

No syntax, security, or secret-handling issues in the changed lines. The github.event_name != 'workflow_dispatch' and sdk_libs == 'true' conditions are preserved, so manual dispatches and non-SDK pushes remain excluded as before.

Both findings are non-blocking (🟡). The change is functionally correct and consistent with sibling jobs; the main ask is to confirm the comment's rationale matches observed CI behavior.

[KevinDavilaDotCMS]

Went through both findings from the Claude Code review — both are 🟡 Medium, but worth settling since they question the core rationale of this fix.

1. Was needs.test.result really 'skipped', or would it actually resolve to 'success'?

Good question to ask, and it's already answered by real data, not just theory: this fix was written specifically because of a real production run — run 30474411639 — where a push that genuinely touched core-web/libs/sdk/client/... (so sdk_libs was true, and the event was a plain push, not workflow_dispatch) still showed:

Publish SDK (next) | completed | skipped

With sdk_libs == 'true' and github.event_name != 'workflow_dispatch' both satisfied, the only way the old gate (needs.test.result == 'success') could still skip the job is if test's result genuinely wasn't 'success'. So the diagnosis holds up against a real run, not just against reading the YAML. Confirmed root cause: cicd_3-trunk.yml never wires any per-suite flag (jvm_unit_test, cli, frontend, postman, karate) into the reusable test phase for a normal push — only run-all-tests, which is only ever true on a manual workflow_dispatch. So the test matrix is always empty on an ordinary push, and the caller-side test job resolves to skipped, not success.

2. Would this still publish next if build genuinely failed (not just got skipped for artifact reuse)?

Checked this against how failure() actually behaves here: it evaluates the whole transitive dependency chain, not just a job's direct needs. publish-sdk-next needs [initialize, test], and test itself needs build — so if build truly fails, that failure propagates through test and is still visible to publish-sdk-next's !failure() check, which correctly skips it too. This is the exact same idiom already relied on by the sibling deployment job, whose direct needs also don't list build, and which is already trusted to correctly refuse to deploy when build fails. No gap here.

Both findings check out — no changes needed. Thanks for flagging them; happy to add the run-30474411639 reference as a comment in the YAML if that'd help future readers, but didn't want to touch the diff without checking first.

[mergify]

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

• Queue this pull request