Skip to content

fix(ci): select the nightly build ref along main's first-parent line - #37342

Open
sfreudenthaler wants to merge 1 commit into
mainfrom
issue-37202-nightly-first-parent
Open

fix(ci): select the nightly build ref along main's first-parent line#37342
sfreudenthaler wants to merge 1 commit into
mainfrom
issue-37202-nightly-first-parent

Conversation

@sfreudenthaler

Copy link
Copy Markdown
Member

Why

.github/workflows/cicd_4-nightly.yml picks the nightly build ref with:

BUILD_REF=$(git log --before="${MIDNIGHT}" --format="%H" -1)

git log walks the whole reachable graph in commit-date order, not main's mainline. Under squash merging that was harmless — one commit per PR, dated at merge time. Squash merging is now disabled on this repo (allow_squash_merge: false), so every feature-branch commit lands on main verbatim, carrying the commit date it had on the branch. A commit authored days ago but merged after the nightly ran can win the -1 — and it was never main's tip.

Reproduced on real history

All three most recent nightly windows select a non-mainline commit today:

window without --first-parent with --first-parent
2026-08-29 dfadb3e180 Merge branch 'main' into issue-36855-deterministic-id-... 7a7708ee35 feat(users): API Tokens tab …
2026-08-31 e379884d35 Merge branch 'main' into nicobytes/issue-36850-node-24-... 31928163af fix(clustering): propagate system table set() …
2026-09-01 f7294b1ac0 test(block-editor): cover codeBlock … be94fd0a7f fix(evergreen-tracks): make dry-run unmissable …

Two of the three are Merge branch 'main' into <feature-branch> commits — the nightly would build a feature branch, not main.

What actually shipped

The scheduled 03:30 runs got the right answer, but only by timing. Confirmed from the run logs:

  • run 33354100873 (Aug 31) → 31928163afd5345a59898ce4dbd1a4c1c8d698f9
  • run 33466553920 (Sep 1) → be94fd0a7fed542c6977aeaff6857653a09268c2

e379884d35 is dated 2026-08-30T14:59Z but did not enter main until 2026-08-31T21:35Z, so the 03:31 run could not yet see it.

The exposure is the repeatability guarantee, which is the documented reason this branch of the script exists:

This ensures repeatability — manually re-running later in the day to investigate a failure gives the same build.

Re-running the 2026-08-31 nightly today builds e379884d35, a feature branch. So the investigate-a-failure path is broken right now, and the scheduled path is exposed any time a PR merges between midnight and 03:30 carrying commits dated before midnight — which is routine.

What changed

--first-parent on both git log calls: the midnight selection and the empty-result fallback. Ten-line diff, no logic restructuring.

Also refreshes the commits/day figure the fetch-depth: 1000 bound is justified against — the comment cited "~19 commits/day" from the squash era; it is ~40/day now (118 commits across v26.08.28-01...v26.08.31-01), making 1000 commits ~25 days rather than ~52. Still far past the midnight boundary, so the bound holds. --first-parent only narrows the walk, so it cannot make the shallow depth less safe.

Testing

No unit-test harness exists for this workflow. Verified by running both forms of the command against real origin/main history for three consecutive nightly windows (table above), and by confirming the actual shipped BUILD_REF from the two most recent run logs. The --ancestry-path check confirming e379884d35 entered main after the Aug 31 run is in the commit message.

Closes: #37202

Part of dotCMS/private-issues#673 — item 4 of 4. Item 1 (#37203) is closed, item 2 (#37200) landed in #37219, item 3 (#37201) is #37213.

🤖 Generated with Claude Code

`git log --before=midnight -1` walks the whole reachable graph in commit-date
order, not main's mainline. With squash merging disabled every feature-branch
commit lands on main verbatim, carrying the commit date it had on the branch --
so a commit authored days ago but merged after the nightly ran can win the
`-1`, and it was never main's tip.

Reproduced against real history (all three most recent nightly windows pick a
non-mainline commit today):

  2026-08-31  without: e379884 Merge branch 'main' into nicobytes/issue-36850-...
              with:    3192816 fix(clustering): propagate system table set() ...
  2026-09-01  without: f7294b1 test(block-editor): cover codeBlock ...
              with:    be94fd0 fix(evergreen-tracks): make dry-run unmissable ...

The scheduled 03:30 runs got the right answer only by timing: e379884 is
dated 2026-08-30T14:59Z but did not enter main until 2026-08-31T21:35Z, so the
03:31 run could not see it. That breaks the repeatability guarantee this branch
of the script exists to provide -- re-running the 2026-08-31 nightly today to
investigate a failure now builds a feature branch instead of main.

--first-parent on both the midnight selection and the empty-result fallback.
Also refreshes the commits/day figure the fetch-depth: 1000 bound is justified
against: ~19/day under squash, ~40/day now (118 commits across
v26.08.28-01...v26.08.31-01), so 1000 is ~25 days rather than ~52.

Closes: #37202

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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


Code Review

I read the diff against the checked-out workflow and traced the git log selection logic.

Verdict: correct fix, no blocking issues. The change is sound and the reasoning in the PR body / commit message holds up against the code.

Key checks I ran:

  • --first-parent is anchored correctly. The checkout uses ref: main (cicd_4-nightly.yml:102), so HEAD is main's tip. git log --first-parent from there walks exactly main's mainline — precisely the set the PR wants. The fix would be wrong only if HEAD were a detached feature ref, which it isn't here.
  • --before ... -1 still returns the right commit. Along the first-parent line, committer dates are effectively monotonic (merge commits are dated at merge time, direct pushes in push order), so -1 correctly picks the newest first-parent commit at/before midnight. --before filters on committer date, which is the merge-time date — consistent with "what was on main."
  • Merge-commit tree is the tested tree. With squash disabled, the first-parent merge commit represents main's post-merge state (what the merge queue validated), so building it is correct.
  • Shallow depth is still safe. --first-parent only narrows the walk, so fetch-depth: 1000 can't become less safe. The refreshed ~40 commits/day figure keeps 1000 commits (~25 days) far past the midnight boundary.
  • Fallback branch (:127) is consistent — same --first-parent flag, and on the near-impossible empty case it returns main's tip, which is on the first-parent line anyway.

Notes (non-blocking)

  • 🟡 Minor: :96 — the ~40 commits/day figure is an informal estimate (118 across four tags). It's only justifying that the bound comfortably clears midnight, so precision doesn't matter, but it will drift as merge volume grows. Not worth churning the comment over.

I can't push changes to this file regardless — the GitHub App can't modify .github/workflows/. But nothing here needs changing.

No issues found.

· issue-37202-nightly-first-parent

@sfreudenthaler sfreudenthaler added the PR : dotbot review Trigger dotbot AI code review on this PR label Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

dotbot code review:

  • Reviewer: deepseek/deepseek-v4-pro-0813 (medium)
  • Overall: patch is correct
  • New findings this run: 0
  • Prior unresolved dotbot findings still relevant: 0
  • Active findings total: 0

The change adds --first-parent to both git log invocations in the nightly workflow, correctly restricting ref selection to main's first-parent mainline. Since the checkout pins HEAD to main, the walk starts from main's tip and can no longer descend into merged feature branches to select a non-mainline commit. The fallback branch is consistent, and the refreshed shallow-depth comment is accurate and has no functional impact.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

reviewed by dotbot · deepseek/deepseek-v4-pro-0813 · medium

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

dotbot code review:

  • Reviewer: google/gemini-3.8-flash (medium)
  • Overall: patch is correct
  • New findings this run: 0
  • Prior unresolved dotbot findings still relevant: 0
  • Active findings total: 0

The change correctly adds --first-parent to both git log commands in cicd_4-nightly.yml, ensuring that nightly ref resolution strictly follows the mainline commits of main instead of selecting earlier commits from merged branches.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

reviewed by dotbot · google/gemini-3.8-flash · medium

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

dotbot code review:

  • Reviewer: ~z-ai/glm-latest (medium)
  • Overall: patch is correct
  • New findings this run: 0
  • Prior unresolved dotbot findings still relevant: 0
  • Active findings total: 0

The change adds --first-parent to both git log invocations in the nightly workflow. Since the checkout pins HEAD to main, the walk starts at main's tip and is correctly restricted to main's first-parent mainline, preventing selection of commits that were never main's tip. The fallback branch is consistent and the comment update is accurate.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

reviewed by dotbot · ~z-ai/glm-latest · medium

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 PR : dotbot review Trigger dotbot AI code review on this PR

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Nightly build ref: add --first-parent to midnight commit selection

1 participant