Skip to content

fix(ci): force flatten on build-cache hits so installed POMs resolve (#36947) - #36972

Draft
wezell wants to merge 1 commit into
mainfrom
issue-36947-cache-flatten-fix
Draft

fix(ci): force flatten on build-cache hits so installed POMs resolve (#36947)#36972
wezell wants to merge 1 commit into
mainfrom
issue-36947-cache-flatten-fix

Conversation

@wezell

@wezell wezell commented Aug 8, 2026

Copy link
Copy Markdown
Member

🔴 Fixes an active CI outage on main. Every PR and merge-queue build is currently publishing broken POMs into its own Maven repo.

Symptom

Could not resolve dependencies for project com.dotcms:dotcms-integration:
  Failed to collect dependencies at com.dotcms:dotcms-core -> com.dotcms.core.plugins:com.dotcms.tika-api
  Could not find artifact com.dotcms.core.plugins:dotcms-core-plugins-parent
    :pom:${revision}${sha1}${changelist}

Cause

parent/pom.xml uses Maven CI-friendly versions (<revision>1.0.0</revision>) with flatten-maven-plugin's updatePomFile=true. Flatten rewrites the project POM to the resolved version at process-resources, and install publishes that.

The build cache treats flatten:flatten as a cached execution and skips it on a hit — while alwaysRunPlugins forces install to run anyway. So install faithfully published the raw pom.xml, placeholders and all.

On run 31235284253: 19 modules restored, 19 Skipping plugin execution (cached): flatten:flatten, and the whole integration suite unable to resolve dotcms-core's dependencies.

Why it passed every previous run

The bucket was empty until the first merge_group write on #36960's own merge. A 100% cache miss never exercises this path — so every run before that was legitimately green, including the one that merged the cache. The first run to get real hits was the first to break.

Verified locally against :com.dotcms.tika-api

Scenario Installed POM version flatten skipped
Cold build, no cache 1.0.0-SNAPSHOT 0
Cache hit, previous flags ${revision}${sha1}${changelist} 4
Cache hit, flatten forced 1.0.0-SNAPSHOT 0

Immediate mitigation, independent of this PR

gh variable set BUILD_CACHE_DISABLED --body true --repo dotCMS/core

That kill switch (shipped in #36960) turns the cache off repo-wide with no revert and no deploy. Unset it once this lands.

The shape of this bug is worth naming

alwaysRunPlugins is a hand-curated allowlist of "executions that must not be skipped" — precisely the kind of hand-maintained predicate that the caching proposal argued content hashing should replace. install was identified up front; flatten was not, because nothing fails until a hit actually happens. Any other plugin that mutates state install depends on carries the same exposure, and a miss-only run will never reveal it.

Two follow-ups worth considering, not done here:

  • Keep BUILD_CACHE_DISABLED=true until a merge-queue build demonstrates a green run with hits, rather than trusting this fix on a miss-only PR run.
  • Consider whether <flatten.skip> or narrowing what the cache is allowed to skip is a more durable answer than growing the allowlist.

🤖 Generated with Claude Code

https://claude.ai/code/session_014a2iJy9JXRBSVdKBbmoZ2S

This PR fixes: #36947

…36947)

A cache hit was publishing POMs with unresolved CI-friendly placeholders,
breaking every downstream consumer:

  Could not find artifact com.dotcms.core.plugins:dotcms-core-plugins-parent
    :pom:${revision}${sha1}${changelist}

parent/pom.xml uses Maven CI-friendly versions (<revision>1.0.0</revision>) with
flatten-maven-plugin's updatePomFile=true, so flatten rewrites the project POM to
the resolved version at process-resources and install publishes that. The build
cache treats flatten:flatten as a cached execution and skips it on a hit, while
alwaysRunPlugins forces install to run anyway -- so install faithfully published
the raw pom instead.

Observed on run 31235284253: 19 modules restored, 19 "Skipping plugin execution
(cached): flatten:flatten", and the entire integration suite unable to resolve
dotcms-core's dependencies. This only surfaced once the bucket had been populated
by the first merge_group write, which is why every earlier run was green -- a
100% miss never exercises it.

Reproduced and verified locally against :com.dotcms.tika-api:
  cold build                          -> <version>1.0.0-SNAPSHOT</version>
  cache hit, previous flags           -> <version>${revision}${sha1}${changelist}</version>
  cache hit, flatten forced           -> <version>1.0.0-SNAPSHOT</version>
with flatten skipped 4x in the middle case and 0x after the fix.

Worth naming the shape of this bug: alwaysRunPlugins is a hand-curated allowlist
of "executions that must not be skipped", which is exactly the kind of
hand-maintained predicate the caching proposal argued against. install was
identified up front; flatten was not, because nothing fails until a hit actually
happens. Any other plugin that mutates state install depends on has the same
exposure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014a2iJy9JXRBSVdKBbmoZ2S
@claude

claude Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @wezell's task in 57s —— View job


Code Review

  • Read PR diff and surrounding action.yml context
  • Verify flatten-maven-plugin config in parent/pom.xml
  • Check alwaysRunPlugins identifier format and expansion path
  • Post review

New Issues

No issues found.

The one-line change is correct and well-targeted.

Why the fix is right

  • parent/pom.xml:13 sets <revision>1.0.0</revision> (CI-friendly versions) and parent/pom.xml:264-287 binds flatten-maven-plugin:flatten to process-resources with <updatePomFile>true</updatePomFile>. So install publishes the flattened POM only if flatten:flatten actually ran. Skipping it on a cache hit while forcing install publishes the raw placeholder POM — exactly the reported failure.
  • Adding flatten-maven-plugin:flatten to ALWAYS_RUN restores the invariant the existing comment already asserts for install and docker-maven-plugin:build: any cached execution whose output install/upload depends on must not be skipped.
  • Identifier format is consistent — artifactId:goal (flatten-maven-plugin:flatten), matching the pre-existing maven-install-plugin:install and docker-maven-plugin:build entries. Comma-delimited, no spaces, so unquoted $FINAL_ARGS expansion in maven-job/action.yml:332 stays a single token. ✅

Minor / non-blocking

  • 🟡 Medium: parent/pom.xml:279-285 — there is a second flatten execution, flatten.clean (goal clean, bound to the clean phase). This PR only forces the flatten goal, which is the correct scope for the publish-correctness bug. Just flagging that the clean goal is a distinct cached execution; it isn't relevant here (CI builds aren't running clean on cache-hit paths), so no change needed — noting it for completeness given the PR's own "any other state-mutating plugin carries the same exposure" observation.

On the two follow-ups in the description — both are sound and worth doing, and neither belongs in this PR:

  1. Keeping BUILD_CACHE_DISABLED=true until a merge-queue run demonstrates green-with-hits is the right validation gate, since this PR run is miss-only and won't exercise the fixed path.
  2. The deeper concern is real: alwaysRunPlugins is a hand-maintained allowlist, and flatten being missing until a hit occurred is the second instance of the same class of latent bug. Preferring <flatten.skip>-style guarantees or narrowing what the cache may skip (rather than growing the allowlist) is the more durable direction. Tracking that as a separate issue would be worthwhile.

This is a correct, minimal hotfix for an active main outage. LGTM to merge.

· branch issue-36947-cache-flatten-fix

@wezell
wezell marked this pull request as draft August 10, 2026 00:55
@wezell

wezell commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

Do not merge as-is — this fix trades one break for another. Converting to draft.

The flatten fix does resolve the ${revision} poisoning, but forcing flatten-maven-plugin:flatten breaks the Docker build:

Failed to execute goal io.fabric8:docker-maven-plugin:0.48.0:build (docker-build) on project dotcms-core:
  Configuration error: <image> must have a non-null <name>

Mechanism. flatten-maven-plugin is configured with updatePomFile=true (parent/pom.xml), so it replaces the project POM with the flattened one — and the flattened model drops <build>, including plugin configuration. Forcing flatten and then forcing docker-maven-plugin:build against the rewritten model leaves the Docker plugin with no <image> config.

Evidence is clean:

Run flatten docker-build
31235284253 skipped (cached) ✅ succeeded
31236600023 forced by this PR <image> must have a non-null <name>

The real lesson

alwaysRunPlugins is a hand-curated allowlist of executions that must not be skipped, and it has now produced two distinct correctness bugs:

  1. skip install → jars never reach ~/.m2, every test job fails to resolve
  2. skip flatten → POMs publish with ${revision} unresolved
  3. force flatten → the Docker plugin loses its configuration

Each was invisible until a real cache hit occurred, and each fix surfaced the next. That is the failure mode the caching proposal explicitly warned about — a hand-maintained predicate standing in for "what does a correct build actually require".

The honest read: the Maven Build Cache Extension does not compose cleanly with this repo's CI-friendly-versions (<revision> + flatten updatePomFile=true) setup. Making it safe needs a deliberate design pass, not another entry in the allowlist.

Current state

Options, for a decision rather than a guess

  1. Leave it disabled and treat remote Maven caching as needing redesign. Cheapest, loses ~7m of a 14.2m job that was never the main lever anyway.
  2. Cache only leaf modules that neither flatten nor Docker touch, giving up dotcms-core — most of the benefit disappears with it.
  3. Change the flatten setup (e.g. flattenMode that preserves <build>, or emitting the flattened POM somewhere the cache restores) — real work, touches the release process, needs its own review.

I'd take (1) now and open an issue for (3), rather than keep patching under time pressure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area : CI/CD PR changes GitHub Actions/workflows

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

CI: content-addressed build caching across nx, Maven and Docker

1 participant