feat(reusable-on-code-change): derive the inputs a module should not have to declare - #405
Merged
Merged
Conversation
…have to declare A caller repeated facts the workflow can read from its own context: the triggering ref, the pull request's base branch, the module id as artifact prefix. Others were the same value in every caller. Derive the first kind and retune the defaults of the second, so a module declares only what is genuinely its own.
…n the Sonar fallback Review follow-ups: the three retuned defaults carry no explanation in the only place a migrating author reads; the Sonar fallback chain could resolve to an empty string on an event whose payload has no repository object, which the action turns into a bash syntax error; and should_skip_pagerduty is declared but never forwarded.
This repository does not publish a changelog from .chachalog: its releases take their notes from .github/release.yml, i.e. from pull request titles.
… ignored Review follow-ups: should_use_build_artifacts was described as ignored while both test jobs still forwarded it, and two descriptions claimed more than the workflow enforces - the caller picks the event, and a module can gate audit-ci lower than critical on a maintenance branch too.
delallioui
reviewed
Aug 21, 2026
delallioui
reviewed
Aug 21, 2026
delallioui
reviewed
Aug 21, 2026
delallioui
reviewed
Aug 21, 2026
…ing commit actions/checkout only sets result.commit when ref is empty (input-helper.ts:60-72), so passing refs/pull/N/merge drops the SHA pin and every job re-resolves the merge ref. An empty ref is that action's own 'not specified' branch, which is what the default should be. The forwards to reusable-integration-tests keep the fallback: that workflow declares module_branch with a default of main.
delallioui
approved these changes
Aug 21, 2026
This was referenced Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Cut what a module has to declare to call
reusable-on-code-change.yml. Inputs that repeat a fact the workflow can read from its own context are now derived; inputs that carried the same value in every caller got that value as their default.Jahia/siteSettingsgoes from fourteen inputs to four:That drops two inputs the workflow does not derive, on purpose.
integration_tests_timeout: 45is a cap rather than a target, and one shared timeout is the consistency this change is after.integration_tests_pagerduty_skip_notification: falseis inert on a pull-request run: notifications also requireCURRENT_BRANCHto bemaster,mainor the primary release branch, and it is<n>-merge.Derived instead of declared
module_branch(was required)github.ref— the ref that triggered the callersonar_analysis_primary_release_branch(wasmain)mainintegration_tests_artifact_prefix(wastests)module_idintegration_tests_standalone_executeThe fallbacks are resolved in the jobs (
inputs.x || …), not indefault:, so they stay context-safe.Defaults retuned
static_analysis_auditci_levelmoderatecriticalon-code-change.ymlin the organisation passintegration_tests_should_skip_testrailfalsetrueintegration_tests_jahia_image""ghcr.io/jahia/jahia-ee-dev:8-SNAPSHOTjahia_imageis required downstream, and this is the value callers passintegration_tests_artifact_prefixtestsmodule_idintegration_tests_should_use_build_artifactsis documented as deprecated and no longer forwarded to either test job: the underlying action carriesdeprecationMessage: This input is deprecated. Build artifacts are now downloaded if availableand reads it nowhere, the integration-tests workflow never forwarded it either, and the download runs offbuild_artifactsunconditionally.integration_tests_should_skip_pagerdutygets the same treatment for a different reason — it is declared here but never forwarded to either test job, so setting it has never done anything.Removes a trap rather than fixing a live bug
sonar_analysis_primary_release_branchbecomes-Dsonar.pullrequest.base. A hardcodedmainis the wrong new-code baseline for a pull request opened against a maintenance branch, and 14 repositories in the organisation still declaremaster. No repository experiences that today: both consumers pass the input explicitly, and the maintenance branches of the nearest migration target (Jahia/siteSettings8_7_x,8_12_x) still carry inline jobs, so a pull request against them uses their own workflow file, not this one. The fix is latent-correct — it stops the trap from being sprung by the migrations to come.Untested end to end: no repository in the organisation passes a maintenance branch as
sonar.pullrequest.basetoday, so whether SonarQube accepts one that has never had a branch analysis is unverified. Worth one throw-away pull request into8_7_xafter the migration reaches it.Validated
Jahia/javascript-modules#726 — a throw-away pull request — calls this branch with four inputs instead of eleven. Its run 32485354032 — which resolved this branch at its head — is green, integration tests included, and each dropped input resolved as intended:
module_branchrefs/pull/726/mergesonar_analysis_primary_release_branchsonar.pullrequest.base=main, from the base of the pull requestintegration_tests_standalone_executeintegration_tests_jahia_imageJAHIA_IMAGE=ghcr.io/jahia/jahia-ee-dev:8-SNAPSHOTintegration_tests_artifact_prefixstandalone-javascript-modules-engine-1397, previouslystandalone-tests-<run>integration_tests_should_skip_testrailshould_skip_testrail: trueintegration_tests_should_use_build_artifactsbuild-artifactsstill downloaded, confirming the input is inertstatic_analysis_auditci_levelaudit-ci --skip-dev --criticalThe
integration_tests_cluster_executepath is reasoned, not exercised: no repository runs cluster tests through this workflow today.What a migrating author must watch
criticalhas to keep declaring it.Jahia/jahia-ui-root,Jahia/jahia-user-entriesandJahia/jahia-developer-toolspassauditci_level: highon their default branch, andJahia/siteSettingsdoes the same on8_7_x, so maintenance branches need checking too. Dropping the input there would loosen the gate, and it would do so silently — fewer findings, no error. One repository has nothing to carry over and would loosen anyway:Jahia/jahia-authenticationrunsstatic-analysiswith no level and has a rootpackage.json, so it gates atmoderatetoday and would move tocriticalon migration. 113 of the 144on-code-change.ymlin the organisation passcritical, which is what makes it the right default. The input description says so. Note this leaves the workflow default (critical) and thestatic-analysisaction default (moderate) disagreeing — deliberately: retuning the action would silently loosen the gate for the ~45 repositories that call it directly, which belongs in its own change.integration_tests_standalone_execute: false. With a manifest present and no cluster run requested, the standalone job runs regardless of that flag. A cluster-only setup still works by passingintegration_tests_cluster_execute: true.tests/docker-compose.ymlfalls back to. All 33 repositories that run integration tests fromon-code-change.ymlalready setjahia_imageexplicitly, so no migrating module inherits this default by accident. It also closes a hole:Jahia/javascript-modules'tests/docker-compose.ymlreads a bare${JAHIA_IMAGE}with no fallback, so the previous empty default produced an invalid image reference. Note the flip side — a version-bearing tag as a shared default moves the Jahia-version decision from the module to the harness, and will retarget every caller that dropped the input when9-SNAPSHOTarrives.update_signature_executestill has to be declared. See below.Risk
Five defaults change behaviour for a caller that relied on them; passing the input explicitly keeps the old value.
Both current consumers are safe.
Jahia/toolspasses every retuned and derived input, so nothing changes for it at all.Jahia/javascript-modulespasses all of them exceptintegration_tests_artifact_prefix, so its test artifacts are renamed fromstandalone-tests-<run>tostandalone-javascript-modules-engine-<run>; no workflow file in any non-archived repository of the organisation referencesstandalone-testsorcluster-tests, though anything outside.github/workflowsglobbing the old name would miss.Three migrations are open — Jahia/default#160 and Jahia/module-manager#203 are ready for review, Jahia/siteSettings#286 is draft. All three are immune whichever order things land in: the first two declare no
integration_tests_*input at all and run no integration tests, andsiteSettingspasses each retuned input explicitly. They can be trimmed afterwards. About 45 repositories are still to migrate — retuning a default after they adopt it would be an org-wide event, which is the reason to do it now.Not in this PR
The module signature.
update_signature_executestill defaults tofalse, and theupdate-signatureaction is still a fixer that only acts on a[ci sign]commit, so it cannot fail a pull request whose signature is wrong. Making it verify, and gating the integration tests on it, needs a change to the action and a survey of which repositories would go red — its own PR. Until then a caller that wants the signature job must keep declaringupdate_signature_execute: true.