fix(upgrade): re-slot all 2-19 upgrade commands to their real merge epochs and guard timestamps in CI (#22498)
# Fix broken 2-19 upgrade command sequence (dev incident: `lastStreamError` / `workspaceDiscoverability`) ## Incident The dev environment (tracking main) throws: - `Property "lastStreamError" was not found in "AgentChatThreadEntity"` - `Cannot return null for non-nullable field Workspace.workspaceDiscoverability` ## Root cause The 2-19 upgrade commands were committed with **fabricated future timestamps** (year-2027 epochs like `1820000000000`). The upgrade cursor (`upgrade-aware-entity-metadata.adapter.ts`) tracks a **single** most-recent applied step: it looks up the latest `core."upgradeMigration"` row's name in the sequence (sorted by timestamp within kind) and hides every `@WasIntroducedInUpgrade` column at or past that index. Two ways this breaks, and both were live on main: 1. **Cursor regression**: a command merged *later* with a *smaller* timestamp (e.g. `pendingQuestion` at `1811…` after `metadata-overrides` at `1820…` had run) sorts *before* already-applied steps. When migrate runs, the "latest" row now points earlier in the sequence, re-hiding columns that were already applied. 2. **Migrate not running at all** (Felix's hypothesis): if the deploy pipeline skipped `database:migrate:prod`, none of the 2-19 rows exist and every 2-19-gated column is hidden. Both hypotheses have the same fix path; the discriminating query is in the verification section below. ## Fix **Real timestamps** (per maintainer direction — no more fabricated epochs): | Command | Old (fabricated) | New (real merge epoch) | Introduced by | |---|---|---|---| | workspace: backfill-workspace-custom-application-registration | `1820000000000` | `1782853718000` | #22378 | | fast: add-metadata-overrides-column | `1820000100000` | `1782986475000` | #22417 | | slow: backfill-metadata-overrides | `1820000110000` | `1782986476000` (+1s to order after its fast pair) | #22417 | | fast: add-last-stream-error-to-agent-chat-thread | `1821000000000` | `1782996657000` | #22434 | | fast: add-pending-question-to-agent-chat-thread | `1811000000000` | `1782999138000` | #22346 | | fast: add-workspace-discoverability-to-workspace | `1820000001000` | `1783004140000` | #22423 | Each value is the committer epoch of the squash-merge commit that introduced the command on main (verified via `git log --diff-filter=A`). Sorted by real time, the fast sequence is strictly increasing, so the cursor can no longer regress. **Idempotency**: renaming a command changes its step name, so every one of these re-runs on any instance that already applied it under the old name (dev cluster, edge self-hosters — 2.19 is unreleased, so tagged releases are unaffected). All six are now safe to re-run: - `lastStreamError`, `pendingQuestion`, `metadata-overrides` fast: `ADD COLUMN IF NOT EXISTS` (already were) - `metadata-overrides` slow backfill: `WHERE … IS NULL` guard (already was) - workspace command: skips when `applicationRegistrationId` is already set (already did) - `workspaceDiscoverability`: **made idempotent in this PR** — `CREATE TYPE` wrapped in a `duplicate_object` handler, `ADD COLUMN IF NOT EXISTS` **CI guard** (replaces the append-only check added earlier on this branch): - Timestamps must be **real**: within `[now − 60 days, now + 2 days]`. This is the check that would have prevented the original sin — 2027 epochs can never pass. - Still **append-only** within the version directory, but computed from `git diff --name-status --find-renames` so renamed/copied files are checked too (cubic's P2), and files the PR deletes/renames away no longer count toward the existing max (otherwise a re-slotting PR like this one could never pass its own guard). - Covers `workspace-command-<ts>-` filenames, not just `instance-command-fast|slow-<ts>-`; skips `.spec.ts` files. - Failure message documents the escape path (re-slot the fabricated blocker to its real epoch + make it idempotent) and a bypass label `ci:allow-upgrade-command-timestamp-exception` for deliberate exceptions. ## Deploy sequencing (important) After this merges and deploys, `database:migrate:prod` **must run** before the API pods are relied on: the old step names no longer exist in the sequence, so until the renamed commands run once, the cursor resolves to 0 and *every* gated column is hidden. The commands are idempotent, so the re-run is harmless. Running API/worker pods only compute the cursor at boot — restart them after migrate. ## Verification / diagnosis on dev ```sql SELECT name, status, "createdAt" FROM core."upgradeMigration" WHERE "workspaceId" IS NULL ORDER BY "createdAt" DESC LIMIT 15; ``` - Latest rows named `…_182xxxxxxxxxx` (fabricated) and completed → migrate ran, cursor regressed (hypothesis 1). - No 2-19 rows at all → migrate never ran for 2-19 (hypothesis 2). - After the fix: latest row should be `2.19.0_AddWorkspaceDiscoverabilityToWorkspaceFastInstanceCommand_1783004140000`, status `completed`. https://claude.ai/code/session_01Lyi6zTema2FMVVh8MD6c38
This commit is contained in:
Félix Malfait
@@ -165,6 +165,95 @@ jobs:
|
||||
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
|
||||
|
||||
server-validation:
|
||||
needs: server-build
|
||||
|
||||
Reference in New Issue
Block a user