From bb4e427196f42fb1cdcefa1d088a328b48278740 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:37:45 +0200 Subject: [PATCH] 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 --- .../upgrade-mutation-guard/action.yaml | 179 ++++++++++++++++++ .github/workflows/ci-merge-queue.yaml | 60 ++++++ .github/workflows/ci-server.yaml | 162 +--------------- 3 files changed, 244 insertions(+), 157 deletions(-) create mode 100644 .github/actions/upgrade-mutation-guard/action.yaml create mode 100644 .github/workflows/ci-merge-queue.yaml diff --git a/.github/actions/upgrade-mutation-guard/action.yaml b/.github/actions/upgrade-mutation-guard/action.yaml new file mode 100644 index 0000000000..295bc86020 --- /dev/null +++ b/.github/actions/upgrade-mutation-guard/action.yaml @@ -0,0 +1,179 @@ +name: Upgrade Mutation Guard +description: > + Validates that added/modified upgrade commands stay in the current version + directory and keep timestamps real and append-only, diffed against a given + base. Shared by the PR-level guard (base = PR base) and the merge-queue guard + (base = merge candidate's base, i.e. the current tip of main). + +inputs: + base_sha: + description: Commit to diff the working tree against. + required: true + allow_previous_version_mutation: + description: Skip the version-directory check (PR label bypass). + required: false + default: 'false' + +runs: + using: composite + steps: + - name: Fetch base commit + shell: bash + env: + BASE_SHA: ${{ inputs.base_sha }} + run: | + set -euo pipefail + if [ -z "$BASE_SHA" ]; then + echo "::error::base_sha input is empty" + exit 1 + fi + git fetch --no-tags --depth=1 origin "$BASE_SHA" + + - name: Check upgrade version commands are in current version only + if: ${{ inputs.allow_previous_version_mutation != 'true' }} + shell: bash + env: + BASE_SHA: ${{ inputs.base_sha }} + COMMANDS_ROOT: packages/twenty-server/src/database/commands/upgrade-version-command + run: | + set -euo pipefail + 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)" + + DIFF=$(git diff --name-status --find-renames "$BASE_SHA" HEAD -- "$COMMANDS_ROOT") + + ADDED_FILES=$(echo "$DIFF" | awk '$1 == "A" { print $2 } $1 ~ /^[RC]/ { print $3 }') + MODIFIED_FILES=$(echo "$DIFF" | awk '$1 == "M" { print $2 }') + + 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" $ADDED_FILES + check_files "modified" $MODIFIED_FILES + + if [ -n "$ADDED_OFFENDERS" ] || [ -n "$MODIFIED_OFFENDERS" ]; then + echo "This change 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 the PR and re-run CI." + echo "Otherwise, 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 + shell: bash + env: + BASE_SHA: ${{ inputs.base_sha }} + COMMANDS_ROOT: packages/twenty-server/src/database/commands/upgrade-version-command + run: | + set -euo pipefail + 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 -e "$FAILED" + echo "::error::Upgrade command timestamps must be real (close to now) and append-only within their version directory." + exit 1 + fi diff --git a/.github/workflows/ci-merge-queue.yaml b/.github/workflows/ci-merge-queue.yaml new file mode 100644 index 0000000000..519544f0aa --- /dev/null +++ b/.github/workflows/ci-merge-queue.yaml @@ -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 }} diff --git a/.github/workflows/ci-server.yaml b/.github/workflows/ci-server.yaml index b5da2d5c48..5332c5d8e4 100644 --- a/.github/workflows/ci-server.yaml +++ b/.github/workflows/ci-server.yaml @@ -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