Skip to content

feat(evergreen-tracks): manual-only standard/trailing promotion, drop daily cron - #36652

Merged
sfreudenthaler merged 6 commits into
mainfrom
worktree-issue-36520-manual-promote
Jul 21, 2026
Merged

feat(evergreen-tracks): manual-only standard/trailing promotion, drop daily cron#36652
sfreudenthaler merged 6 commits into
mainfrom
worktree-issue-36520-manual-promote

Conversation

@sfreudenthaler

@sfreudenthaler sfreudenthaler commented Jul 21, 2026

Copy link
Copy Markdown
Member

What

standard/trailing track tags now move only on a manual evergreen-tracks-promote dispatch — there is no cron. A human running the action at a maintenance-window tag update is the cadence gate, so tracks can never re-point off-window. latest is unaffected: it still moves per-release, unattended, via cicd_6-release.yml (--tracks latest).

This reflects the launch decision to hold off on any scheduled promotion until the customer-experience team green-lights automatic (e.g. daily) movement.

Manual dispatch is a two-stage, human-approved flow

  • plan job — always runs a dry-run and prints the intended tag moves.
  • apply job — waits on the evergreen-tracks-apply environment's required-reviewer gate. Nothing moves until a human reviews the plan output and approves the deployment.
  • One-time repo setup: Settings → Environments → evergreen-tracks-apply → Required reviewers. Until reviewers are configured, the apply job runs unattended — the gate is the protection rule, not the YAML.

Why the 2-week planner rework in the issue was NOT built

The original issue scoped a "compute the target as of a 2-week maintenance window" planner rework. Manual invocation makes that unnecessary — the operator is the scheduler — so the planner's age-threshold logic is unchanged. Simplest thing that satisfies the launch decision.

Changes

  • cicd_evergreen-tracks-promote.yml — drop the daily schedule: cron; split into plan (dry-run) → apply (gated on evergreen-tracks-apply environment). Removed the redundant apply input.
  • Regression test — pins --tracks latest to move only latest, never standard/trailing (concrete-scope item 3: a release cut must not roll pods on those tracks).
  • cli.py / cicd_6-release.yml — refreshed stale "daily cron ages standard/trailing" comments.
  • README — documents the new cadence and the approval gate.

Out of scope (tracked separately)

Test

uv run pytest → 65 passed (includes the new isolation test).

Closes: #36520

🤖 Generated with Claude Code

… daily cron

standard/trailing now move only on a manual evergreen-tracks-promote dispatch —
a human running the action at a maintenance-window tag update is the cadence
gate, so tracks never re-point off-window. No cron until the CX team green-lights
automatic promotion. latest keeps moving per-release via --tracks latest.

- Remove the daily schedule from cicd_evergreen-tracks-promote.yml; dispatch is
  dry-run by default, apply=true to move tags.
- Add regression test pinning --tracks latest to move ONLY latest, never
  standard/trailing (concrete-scope item 3).
- Refresh stale "daily cron" comments in cli.py and cicd_6-release.yml.
- Document the cadence in the evergreen-tracks README.

The 2-week maintenance-window planner rework in the original issue scope is
intentionally NOT built: manual invocation replaces it, so the planner's age
thresholds are unchanged.

Closes: #36520

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6UZqGwpjPQrnYyZfYAxiq
@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sfreudenthaler's task in 6m 8s —— View job


Code Review — manual-only standard/trailing promotion

Reviewed the full diff against origin/main. Traced the drift-check shell logic against cli.py/planner.py/executor.py output, and verified the two new regression tests exercise both mutation paths in cmd_promote (the plan() moves and the held-track reconcile loop).

New Issues

  • 🟡 Medium (operational, not code): .github/workflows/cicd_evergreen-tracks-promote.yml:92-97 — the gate job is the entire protection boundary for this feature, and it's config-only. If the evergreen-tracks-apply environment has no required reviewers, gate passes unattended and apply moves tags with zero human review — the "manual approval" guarantee silently evaporates. This is called out in the header comment and PR body, so it's a known trade-off, but it's worth confirming the environment + required-reviewer rule is configured in repo settings before merge, since nothing in the YAML enforces it. Assumption: GitHub environment protection rules are the only available enforcement mechanism (correct — workflow_dispatch can't gate a job otherwise). What to verify: Settings → Environments → evergreen-tracks-apply → Required reviewers is populated.

Resolved

  • .github/workflows/cicd_evergreen-tracks-promote.yml:113-118 — prior 🟠 High (approval wait held the shared evergreen-tracks-registry lock, stalling unattended latest). The lock now lives at job level on apply only; the lock-free gate job absorbs the human wait, so apply enters concurrency evaluation only post-approval. cicd_6-release.yml's promote-latest can move latest while an approval sits pending. Correctly fixed.
  • .github/workflows/cicd_evergreen-tracks-promote.yml:135-156 — prior 🟡 Medium (approved plan ≠ what runs). apply now re-derives the plan from live registry state under the lock and fails on any mismatch, with an empty-plan guard so a broken stdout capture can't silently pass. Enforced, not advisory.
  • .github/workflows/cicd_evergreen-tracks-promote.yml:69-77,139-143 — prior 🟡 Medium (drift check tripped by unattended latest churn). All three invocations scope to --tracks standard,trailing, so a mid-approval latest move by the release pipeline no longer trips the diff. Also more correct — this workflow never owned latest.

Notes (non-blocking, verified correct)

  • cli.py:172 — moving logging.basicConfig to stream=sys.stdout is what makes the drift diff work (plan text is the command's stdout; 2>/dev/null drops uv chatter). Because point_tag's DRY-RUN would point… lines log to the same stdout in both the plan capture and the apply re-derive, they're symmetric and cancel in the diff. Good.
  • Drift diff safety: GitHub's default shell (bash -eo pipefail) means a non-zero exit from uv run inside CURRENT_PLAN=$(…) aborts the step, so a failed re-derive fails closed rather than applying a stale plan. Confirmed the sort handles the non-deterministic held-track line ordering (comes from a set).
  • test_cli.py:199-201 — the held-standard test correctly pins the second mutation path: with --tracks latest, held & wanted empties the reconcile set, so a drifted held standard stays put. Meaningful guard, not a tautology.

No blocking issues. The three prior findings are genuinely resolved; the only open item is the operational prerequisite (required-reviewer config), which is a repo-settings action rather than a code change.

worktree-issue-36520-manual-promote

…environment

Manual dispatch now runs plan (dry-run, prints the moves) then apply, where the
apply job waits on the evergreen-tracks-apply environment's required-reviewer
gate. Nothing moves until a human reviews the plan and approves. Drops the now-
redundant apply input. latest still moves unattended via the release pipeline.

One-time repo setup: Settings > Environments > evergreen-tracks-apply >
Required reviewers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6UZqGwpjPQrnYyZfYAxiq
@alwaysmeticulous

alwaysmeticulous Bot commented Jul 21, 2026

Copy link
Copy Markdown

🤖 No test run has been triggered as your Meticulous project has been deactivated (since you haven't viewed any test results in a while). Click here to reactivate.

Last updated for commit 2328ea4. This comment will update as new commits are pushed.

…iation path

cmd_promote has two tag-mutation paths: plan() moves (scoped by filtering the
tracks list) and the held-track reconciliation loop (scoped only by
`held = held & wanted`). The existing isolation test only exercised the former.
Add a test where standard is held and drifted, asserting a release-style
`--tracks latest` run never reconciles it — so the release job stays latest-only
even with a hold marker present. Verified: deleting `held = held & wanted` makes
this test fail.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6UZqGwpjPQrnYyZfYAxiq
…ing the wait

Review finding #36652: the workflow-level `evergreen-tracks-registry`
concurrency lock was held for the ENTIRE promote dispatch — including the
minutes-to-days the approval gate sits waiting. cicd_6-release.yml's
promote-latest shares that group (cancel-in-progress: false), so a GA shipping
while a standard/trailing approval was pending would queue `latest` behind the
human, breaking the "latest moves the instant a GA ships" invariant.

A job waiting on an environment required-reviewer gate acquires its concurrency
slot BEFORE the gate evaluates (community discussion #17401), so job-level
concurrency on `apply` alone is insufficient. Split the gate out: a no-lock
`gate` job carries the environment approval; `apply` (needs [plan, gate]) holds
the registry lock at job level and only enters concurrency evaluation after
approval, so the lock covers just the fast mutation — never the human wait.

Also documents (Medium finding) that `apply` re-derives its plan from live
registry state: the approval authorizes the operation, not the exact digest set
the plan job printed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6UZqGwpjPQrnYyZfYAxiq
@sfreudenthaler

Copy link
Copy Markdown
Member Author

Addressed the review findings (commit b8fde8e)

🟠 High — approval wait held the shared registry lock, could stall unattended latest. Confirmed the root cause: a job waiting on an environment required-reviewer gate acquires its concurrency slot before the gate evaluates (community discussion #17401), so moving the lock to job-level on apply alone would not have helped — the waiting job would still hold evergreen-tracks-registry.

Fix: split the approval into its own lock-free gate job. apply now needs: [plan, gate] and carries the job-level evergreen-tracks-registry lock, so it only enters concurrency evaluation after approval — the lock covers just the fast mutation, never the human wait. A GA release's promote-latest can now move latest immediately even while a standard/trailing approval sits pending. Workflow-level concurrency removed.

🟡 Medium — approved plan ≠ what runs. Documented in the workflow + README that apply re-derives its plan from live registry state, so the approval authorizes the operation (repo + thresholds), not the exact digest set the plan printed. Given standard/trailing age slowly and the operator controls timing, a documented re-derive is the right cost — no plan-pinning machinery added.

Verified end-to-end against dotcms/dotcms-test (run 29798854833): plangate waits for approval → approved → apply moved the tags. 66 unit tests green.

Enforces the previously-advisory Medium finding: apply now re-derives the plan
from live registry state and fails if it no longer matches the approved plan
(a GA landed, a hold/taint changed, a digest was re-tagged), so it can never
move tags nobody reviewed. On drift it prints both plans and exits non-zero;
re-dispatch to review the new plan.

- cli.py: log to stdout (not stderr) so the workflow can capture the plan text
  cleanly (2>/dev/null drops uv's chatter).
- plan job: capture the dry-run and expose it as a job output.
- apply job: re-derive, compare (sorted — held-track lines come from a set),
  fail on mismatch; also fail on an empty plan so a broken capture can't
  silently pass.

Verified locally: stdout capture yields clean plan lines, and the comparison
passes on identical/reordered/no-move plans and fails on drift/empty.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6UZqGwpjPQrnYyZfYAxiq
@sfreudenthaler

Copy link
Copy Markdown
Member Author

🟡 Medium now enforced, not advisory (commit 2328ea4)

Per follow-up: apply no longer trusts that the approved plan still holds — it re-derives the plan from live registry state and fails if it differs from what was approved (a GA landed, a hold/taint changed, a digest was re-tagged). On drift it prints both plans and exits non-zero; the operator re-dispatches to review the new plan. So a stale approval can never move tags nobody reviewed.

Implementation (kept minimal):

  • cli.py logs to stdout instead of stderr, so the workflow captures the plan text cleanly (2>/dev/null drops uv's chatter).
  • plan job exposes its dry-run plan as a job output; apply re-derives, compares (sorted — held-track lines come from a set), and fails on mismatch. Also fails on an empty plan so a broken capture can't silently pass.

Verified: comparison logic passes on identical/reordered/no-move plans and fails on drift/empty (local); and an end-to-end dispatch (run 29799379094) logged plan unchanged since approval; applying before moving the tags. 66 unit tests green.

…atest churn can't fail the drift check

Review finding: the manual dispatch planned all tracks (no --tracks), so the
drift check diffed the full plan including `latest`. But `latest` is owned by
the release pipeline and moves unattended during the approval wait — exactly
what the separate gate job was built to allow. A GA shipping mid-approval would
change the re-derived plan's `latest ->` line and fail the apply, forcing a
re-dispatch even though nothing about standard/trailing changed, undoing the
concurrency decoupling at the plan-diff layer.

Scope all three promote invocations (plan capture, apply re-derive, apply) to
`--tracks standard,trailing`. This workflow never moves `latest` anyway, so the
scoping is also more correct — it matches the workflow's documented purpose and
keeps the "one engine, two triggers" model intact. Verified: the scoped plan
emits no `latest` line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6UZqGwpjPQrnYyZfYAxiq
@sfreudenthaler

Copy link
Copy Markdown
Member Author

🟡 Scoped manual promote to standard,trailing (commit 3174160)

Good catch — the drift check was diffing the full plan including latest, but latest moves unattended via the release pipeline during the approval wait (the exact thing the separate gate job enables). A GA shipping mid-approval would have changed the re-derived plan's latest -> line and failed apply, forcing a needless re-dispatch even though standard/trailing were untouched — undoing the concurrency decoupling at the plan-diff layer.

Fix: all three promote invocations (plan capture, apply re-derive, apply) now pass --tracks standard,trailing. This workflow never moves latest anyway (the release pipeline owns it), so scoping is also just more correct and keeps the "one engine, two triggers" model intact. Verified the scoped dry-run emits no latest line. 66 tests green.

@mergify

mergify Bot commented Jul 21, 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

@sfreudenthaler
sfreudenthaler added this pull request to the merge queue Jul 21, 2026
Merged via the queue into main with commit 5a3a1c5 Jul 21, 2026
40 checks passed
@sfreudenthaler
sfreudenthaler deleted the worktree-issue-36520-manual-promote branch July 21, 2026 15:06
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 QA : Passed

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Rebuild track promotion engine to honor 2-week maintenance-window cadence

2 participants