ci: run upgrade mutation guard in the merge queue against main (#23216)
Follow-up to #23215 (merged). Rebased on `main`. ## Why #23215 fixes an instance of a class of bug: an upgrade command whose version is chosen at `generate:instance-command` time from `TWENTY_CURRENT_VERSION`, then left behind when `main` bumps the version before the PR merges (base-drift). The command ships one minor early and instances already on the newer version skip it forever. The existing `server-previous-version-upgrade-mutation-guard` in `ci-server.yaml` runs on `pull_request`, so it validates against the PR's base. When the base is stale (main moved after the branch was cut), the guard reads the branch's own `TWENTY_CURRENT_VERSION` and the check passes even though the command is now a version behind main. That is exactly how the original bug slipped through. ## What The version-directory and append-only-timestamp validation is extracted into a shared composite action, `.github/actions/upgrade-mutation-guard`, diffed against a caller-supplied `base_sha`. It is called from two places: - **`ci-server.yaml`** (PR-level guard, `base = pull_request.base.sha`) for fast feedback. The job keeps its existing name/check. This replaces ~290 lines of inline shell. - **`ci-merge-queue.yaml`** (new, `merge_group`-triggered, `base = merge_group.base_sha`). GitHub builds each merge-queue candidate on top of the current tip of `main`, so the checks read `TWENTY_CURRENT_VERSION` and the existing per-directory timestamps from main's real state at merge time. Because the candidate is rebased onto main, base-drift is caught by construction: the same validation simply runs where the base is guaranteed current. No origin/main comparison hack; the logic now lives in one place. ## Bypass semantics The guard has two independent checks, and they are treated differently on purpose: - **Version-directory check** keeps its `ci:allow-previous-version-upgrade-mutation` bypass, a deliberate, reviewed escape hatch for legitimately touching a previous-version directory. The PR-level guard reads the label directly; the merge-queue guard resolves it from the queued PR (the `merge_group` event carries no labels) and passes it to the composite action, which skips only the version-directory step. - **Timestamp / append-only check has no bypass.** The old `ci:allow-upgrade-command-timestamp-exception` label is removed. A fake or out-of-order timestamp rewinds the upgrade cursor and re-hides already-applied columns, so there is no "allowed" version of it: the timestamp just has to be configured correctly (real epoch millis, strictly greater than every existing command in the same version directory). If a blocking existing max is itself a fabricated future timestamp, re-slot that command to its real merge epoch rather than reaching for a bypass. Preventing previous-version mutation is the guard's primary purpose. In the merge queue the guard job always runs and skips only the version-directory step when the bypass label is set, so it reports a real success/failure (the required check never resolves to a skipped state, and a label-lookup failure fails closed) and the timestamp check always runs. ## Requires a settings change (not in this diff) Enabling the merge queue and marking the check required are branch-protection settings, not file changes. After merge, an admin needs to: 1. Enable the merge queue for `main` in branch protection. 2. Add `CI - Merge Queue / upgrade-mutation-guard` to the merge queue's required checks. ## Notes - Composite action, not a `workflow_call` reusable workflow, deliberately: converting the `ci-server.yaml` job to a reusable-workflow call would rename its status check to `server-previous-version-upgrade-mutation-guard / ` and break that required-check mapping in branch protection. A composite action dedups the logic while keeping both callers' check names intact. --------- Co-authored-by: Paul Rastoin <paul.rastoin@gmail.com>
This commit is contained in:
Paul Rastoin
@@ -0,0 +1,60 @@
|
||||
name: CI - Merge Queue
|
||||
|
||||
# Runs when a PR is queued to merge. GitHub builds the merge candidate on top of
|
||||
# the current tip of main, so these checks validate against main's real state at
|
||||
# merge time rather than the PR's (possibly stale) base. This is what closes the
|
||||
# upgrade-command base-drift race: a command generated against an old version,
|
||||
# then left behind when main bumps, is caught here even if the PR-level guard
|
||||
# passed on a stale base.
|
||||
|
||||
on:
|
||||
merge_group:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
pull-requests: read
|
||||
|
||||
jobs:
|
||||
# merge_group carries no PR labels, so resolve them from the queued PR (its
|
||||
# number is in the merge-queue ref) and pass the version-mutation bypass to the
|
||||
# guard as an input, which skips only the version-directory step. The job
|
||||
# always runs and reports a real success/failure, so the required check never
|
||||
# resolves to a skipped state and a label-lookup failure fails closed. The
|
||||
# timestamp / append-only check has no bypass and always runs: violating it
|
||||
# rewinds the upgrade cursor.
|
||||
upgrade-mutation-guard:
|
||||
timeout-minutes: 5
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- name: Checkout merge candidate
|
||||
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
|
||||
with:
|
||||
fetch-depth: 2
|
||||
- name: Resolve version-mutation bypass label from the queued PR
|
||||
id: labels
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
HEAD_REF: ${{ github.event.merge_group.head_ref }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
PR_NUMBER=$(echo "$HEAD_REF" | sed -n -E 's|.*/pr-([0-9]+)-[0-9a-f]+$|\1|p')
|
||||
|
||||
skip=false
|
||||
if [ -n "$PR_NUMBER" ]; then
|
||||
LABELS=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json labels --jq '.labels[].name')
|
||||
echo "Queued PR #$PR_NUMBER labels:"
|
||||
echo "$LABELS"
|
||||
|
||||
if echo "$LABELS" | grep -qxF 'ci:allow-previous-version-upgrade-mutation'; then
|
||||
skip=true
|
||||
fi
|
||||
else
|
||||
echo "Could not parse a PR number from '$HEAD_REF'; enforcing strictly."
|
||||
fi
|
||||
|
||||
echo "skip=$skip" >> "$GITHUB_OUTPUT"
|
||||
- name: Validate upgrade command mutations against main
|
||||
uses: ./.github/actions/upgrade-mutation-guard
|
||||
with:
|
||||
base_sha: ${{ github.event.merge_group.base_sha }}
|
||||
allow_previous_version_mutation: ${{ steps.labels.outputs.skip }}
|
||||
@@ -96,164 +96,12 @@ jobs:
|
||||
- name: Fetch custom Github Actions and base branch history
|
||||
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
|
||||
with:
|
||||
fetch-depth: 10
|
||||
- name: Get changed upgrade-version-command files
|
||||
id: changed-files
|
||||
uses: tj-actions/changed-files@48d8f15b2aaa3d255ca5af3eba4870f807ce6b3c # v45.0.9
|
||||
fetch-depth: 2
|
||||
- name: Validate upgrade command mutations
|
||||
uses: ./.github/actions/upgrade-mutation-guard
|
||||
with:
|
||||
files: |
|
||||
packages/twenty-server/src/database/commands/upgrade-version-command/**
|
||||
- name: Check upgrade version commands are in current version only
|
||||
if: >
|
||||
steps.changed-files.outputs.any_changed == 'true' &&
|
||||
!contains(github.event.pull_request.labels.*.name, 'ci:allow-previous-version-upgrade-mutation')
|
||||
run: |
|
||||
VERSION_CONSTANT_FILE="packages/twenty-server/src/engine/core-modules/upgrade/constants/twenty-current-version.constant.ts"
|
||||
|
||||
CURRENT_VERSION=$(sed -n "s/.*TWENTY_CURRENT_VERSION = '\([0-9.]*\)'.*/\1/p" "$VERSION_CONSTANT_FILE")
|
||||
|
||||
if [ -z "$CURRENT_VERSION" ]; then
|
||||
echo "::error::Could not extract TWENTY_CURRENT_VERSION from $VERSION_CONSTANT_FILE"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
CURRENT_DIR=$(echo "$CURRENT_VERSION" | sed -E 's/^([0-9]+)\.([0-9]+)\..*/\1-\2/')
|
||||
|
||||
echo "Current version: $CURRENT_VERSION (directory: $CURRENT_DIR)"
|
||||
|
||||
ADDED_OFFENDERS=""
|
||||
MODIFIED_OFFENDERS=""
|
||||
|
||||
check_files() {
|
||||
local category="$1"
|
||||
shift
|
||||
for file in "$@"; do
|
||||
VERSION_DIR=$(echo "$file" | sed -n 's|.*upgrade-version-command/\([0-9]*-[0-9]*\)/.*|\1|p')
|
||||
|
||||
if [ -n "$VERSION_DIR" ] && [ "$VERSION_DIR" != "$CURRENT_DIR" ]; then
|
||||
if [ "$category" = "added" ]; then
|
||||
ADDED_OFFENDERS="$ADDED_OFFENDERS\n - $file (version directory: $VERSION_DIR)"
|
||||
else
|
||||
MODIFIED_OFFENDERS="$MODIFIED_OFFENDERS\n - $file (version directory: $VERSION_DIR)"
|
||||
fi
|
||||
fi
|
||||
done
|
||||
}
|
||||
|
||||
check_files "added" ${{ steps.changed-files.outputs.added_files }}
|
||||
check_files "modified" ${{ steps.changed-files.outputs.modified_files }}
|
||||
|
||||
if [ -n "$ADDED_OFFENDERS" ] || [ -n "$MODIFIED_OFFENDERS" ]; then
|
||||
echo "This PR touches upgrade command files outside the current version directory ($CURRENT_DIR / $CURRENT_VERSION)."
|
||||
|
||||
if [ -n "$ADDED_OFFENDERS" ]; then
|
||||
echo ""
|
||||
echo "New files added to non-current version directories:"
|
||||
echo -e "$ADDED_OFFENDERS"
|
||||
fi
|
||||
|
||||
if [ -n "$MODIFIED_OFFENDERS" ]; then
|
||||
echo ""
|
||||
echo "Existing files modified in non-current version directories:"
|
||||
echo -e "$MODIFIED_OFFENDERS"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo "If this is intentional, add the label 'ci:allow-previous-version-upgrade-mutation' to this PR and re-run CI."
|
||||
echo "Otherwise, please move your changes to the current version directory ($CURRENT_DIR)."
|
||||
|
||||
echo "::error::Upgrade commands were added or modified in non-current version directories."
|
||||
exit 1
|
||||
fi
|
||||
- name: Check upgrade command timestamps are real and keep the sequence append-only
|
||||
if: >
|
||||
steps.changed-files.outputs.any_changed == 'true' &&
|
||||
!contains(github.event.pull_request.labels.*.name, 'ci:allow-upgrade-command-timestamp-exception')
|
||||
run: |
|
||||
BASE_SHA="${{ github.event.pull_request.base.sha }}"
|
||||
|
||||
if [ -z "$BASE_SHA" ]; then
|
||||
echo "Not a pull request context, skipping timestamp checks."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
git fetch --depth=1 origin "$BASE_SHA"
|
||||
|
||||
COMMANDS_ROOT="packages/twenty-server/src/database/commands/upgrade-version-command"
|
||||
|
||||
extract_ts() {
|
||||
basename "$1" | sed -n -E 's/.*-(instance-command-(fast|slow)|workspace-command)-([0-9]{13})-.*/\3/p'
|
||||
}
|
||||
|
||||
DIFF=$(git diff --name-status --find-renames "$BASE_SHA" HEAD -- "$COMMANDS_ROOT")
|
||||
|
||||
NEW_FILES=$(echo "$DIFF" | awk '$1 == "A" { print $2 } $1 ~ /^[RC]/ { print $3 }')
|
||||
GONE_FILES=$(echo "$DIFF" | awk '$1 == "D" { print $2 } $1 ~ /^R/ { print $2 }')
|
||||
|
||||
NOW_MS=$(($(date +%s) * 1000))
|
||||
MIN_TS=$((NOW_MS - 60 * 24 * 3600 * 1000))
|
||||
MAX_TS=$((NOW_MS + 2 * 24 * 3600 * 1000))
|
||||
|
||||
FAILED=""
|
||||
|
||||
for file in $NEW_FILES; do
|
||||
case "$file" in *.spec.ts) continue ;; esac
|
||||
|
||||
TS=$(extract_ts "$file")
|
||||
|
||||
if [ -z "$TS" ]; then
|
||||
continue
|
||||
fi
|
||||
|
||||
if [ "$TS" -lt "$MIN_TS" ] || [ "$TS" -gt "$MAX_TS" ]; then
|
||||
FAILED="$FAILED\n - $file (timestamp $TS is outside [now - 60 days, now + 2 days])"
|
||||
continue
|
||||
fi
|
||||
|
||||
VERSION_DIR=$(dirname "$file")
|
||||
|
||||
MAX_BASE_TS=0
|
||||
for base_file in $(git ls-tree -r --name-only "$BASE_SHA" -- "$VERSION_DIR"); do
|
||||
case "$base_file" in *.spec.ts) continue ;; esac
|
||||
|
||||
if echo "$GONE_FILES" | grep -qxF "$base_file"; then
|
||||
continue
|
||||
fi
|
||||
|
||||
BASE_TS=$(extract_ts "$base_file")
|
||||
|
||||
if [ -n "$BASE_TS" ] && [ "$BASE_TS" -gt "$MAX_BASE_TS" ]; then
|
||||
MAX_BASE_TS=$BASE_TS
|
||||
fi
|
||||
done
|
||||
|
||||
if [ "$MAX_BASE_TS" -gt 0 ] && [ "$TS" -le "$MAX_BASE_TS" ]; then
|
||||
FAILED="$FAILED\n - $file (timestamp $TS <= existing max $MAX_BASE_TS)"
|
||||
fi
|
||||
done
|
||||
|
||||
if [ -n "$FAILED" ]; then
|
||||
echo "Upgrade command timestamps must be the real epoch millis of when the command"
|
||||
echo "is authored (within 60 days before / 2 days after now), and strictly greater"
|
||||
echo "than every existing command in the same version directory."
|
||||
echo ""
|
||||
echo "The upgrade cursor tracks a single last-applied position: a command that sorts"
|
||||
echo "before already-applied commands moves the cursor backwards when it runs,"
|
||||
echo "re-hiding columns gated by @WasIntroducedInUpgrade on instances that already"
|
||||
echo "applied them. Fabricated future timestamps poison the sequence for every"
|
||||
echo "command added after them."
|
||||
echo ""
|
||||
echo "If the blocking existing max is itself a fabricated future timestamp, re-slot"
|
||||
echo "that command to its real merge epoch (rename the file and the timestamp in the"
|
||||
echo "decorator/constant, and make its up() idempotent since it will re-run under"
|
||||
echo "the new name) instead of inflating the new command's timestamp."
|
||||
echo ""
|
||||
echo "To bypass this check for a deliberate exception, add the label"
|
||||
echo "'ci:allow-upgrade-command-timestamp-exception' to this PR and re-run CI."
|
||||
echo -e "$FAILED"
|
||||
echo "::error::Upgrade command timestamps must be real (close to now) and append-only within their version directory."
|
||||
exit 1
|
||||
fi
|
||||
base_sha: ${{ github.event.pull_request.base.sha }}
|
||||
allow_previous_version_mutation: ${{ contains(github.event.pull_request.labels.*.name, 'ci:allow-previous-version-upgrade-mutation') }}
|
||||
|
||||
server-validation:
|
||||
needs: server-build
|
||||
|
||||
Reference in New Issue
Block a user