Commit Graph

4 Commits

Author SHA1 Message Date
Paul Rastoin a28c3a905a Route pre-2.19 upgrade commands through a legacy validate-build path (#22884)
## Problem

Since the centralized metadata side-effect engine landed in v2.19,
`WorkspaceMigrationValidateBuildAndRunService.validateBuildAndRunWorkspaceMigrationFromRecord`
runs `metadataSideEffectEngineService.expandWithSideEffects(...)` before
building. As a result every historical upgrade command
(`upgrade-version-command/1-21/*` … `2-18/*`), authored before the
engine existed, now flows through it. Their operation matrix is no
longer applied literally: the engine injects/cascades companions (system
fields, `searchVector` field + GIN index, `searchFieldMetadata` rows,
unique backing indexes) and can hard-fail on reserved-identifier
collisions (`RESERVED_SYSTEM_UNIVERSAL_IDENTIFIER`).

Two hazards for already-shipped commands:

1. **Collision → hard failure**: a command declaring a companion the
engine now owns collides with the engine's deterministic
`universalIdentifier`.
2. **Silent drift**: on object/field create/delete the engine
adds/cascades companions the command author never intended, so
workspaces upgraded now differ structurally from those upgraded
incrementally before 2.19.

Suspected real-world impact: a self-hosted user upgrading v2.6.1 →
v2.21.0 hit `duplicate key value violates unique constraint
"IDX_SEARCH_FIELD_METADATA_OBJECT_FIELD_UNIQUE"` in
`upgrade:2-16:backfill-search-field-metadata`, because object-creating
commands now cascade and pre-create the deterministic
`searchFieldMetadata` rows the standalone backfill then re-inserts.

## Changes

- `workspace-migration-validate-build-and-run-service.ts`: extract the
shared compute-and-run tail into a private method, and add
`validateBuildAndRunLegacyWorkspaceMigration` (marked `@deprecated`)
that skips `expandWithSideEffects` and applies the matrix literally. The
existing side-effect entry points are unchanged (the live API and
application manifests depend on them).
- Repoint **all** pre-2.19 upgrade command call sites (1-21 … 2-18,
including `2-10 sync-call-recording-standard-objects`) to the legacy
method. Only the four `2-20/*` commands (target version ≥ 2.19) remain
on the side-effect path.
- `2-16 backfill-search-field-metadata`: recompute
`flatSearchFieldMetadataMaps` from the database before building the
existing-rows dedupe set. The migration runner only invalidates the
flat-maps keys a migration touched, so during a cross-version upgrade
earlier commands can leave this map stale; a stale map breaks the dedupe
and re-inserts rows, tripping
`IDX_SEARCH_FIELD_METADATA_OBJECT_FIELD_UNIQUE`. This is the direct fix
for the reported failure.
- Export `FlatEntityMapsBundle` so the shared tail can be typed.
- Document the side-effect vs legacy path and the selection rule in
`packages/twenty-server/docs/UPGRADE_COMMANDS.md`.

Selection rule: target version **< 2.19** → legacy path; **≥ 2.19** →
side-effect path (default). No exceptions.

## Known gap / merge ordering

The static twenty-standard definition declares all of `callRecording`'s
fields (including the `searchVector` system field) but **not** its
`searchVector` GIN index — every other searchable standard object
declares its GIN index statically. On the legacy path, workspaces
upgrading through `2-10 sync-call-recording-standard-objects` therefore
create the `searchVector` column unindexed (`searchFieldMetadata` rows
are created later in the same pipeline by the 2-16 backfill). The static
GIN index declaration plus a backfill for already-upgraded workspaces
land in a follow-up (twentyhq/core-team-issues#2672), which must ship in
the same release as this PR.

## Out of scope (separate follow-ups)

- `UpgradeMigrationService.getLastAttemptedInstanceCommand()` ordering.
- callRecording `searchVector` GIN index static declaration + backfill
(twentyhq/core-team-issues#2672, same-release dependency, see above).

## Test plan

- `nx typecheck twenty-server` passes.
- `nx lint:diff-with-main twenty-server` (oxlint + oxfmt) clean on
changed files.
- 2-20 command specs (which exercise the unchanged side-effect path)
pass.

---------

Co-authored-by: twenty <noreply@twenty.com>
2026-07-15 14:50:26 +02:00
Paul Rastoin cb95410a51 ci: test twenty-apps install against latest dockerhub and local server + new trigger (#22636)
## Why

App installability can silently regress from two directions, and today
CI only covers one of them:

1. A **server** change (about to merge from the monorepo) breaks the
ability to install the **current public apps** — a
backward-compatibility regression users would hit on upgrade.
2. An **app** change breaks against a server **built from the current
monorepo files** (not just the last published image), so the app and the
upcoming server drift apart before either ships.

Both are compatibility guarantees between the server and the app
catalog. Today they are only tested from the app side, against the
latest published image. This PR makes CI enforce the contract from both
sides:

- Any server PR must keep **every** current public app installable.
- Any app PR is exercised against both the **released** server (its
integration suite — what users run today) and the **upcoming**
(monorepo) server (integration plus deploy + install).

## What

Shared building blocks so both CIs exercise the same paths instead of
duplicating them:

- **`spawn-twenty-server`** (composite action) — returns a running
server (`server-url` + `api-key`) from either the latest published
Docker Hub image or a server built from the monorepo. Both sources
expose the same contract, so callers never branch on how the server came
up.
- **`test-twenty-app`** (composite action) — exercises one app against a
given server, delegating deploy + install to the shared
`deploy-twenty-app` / `install-twenty-app` actions.
- **`discover-apps`** (reusable workflow) — the single source of truth
for the app matrix. Parameterized by `scope` (`public` vs
`internal-and-public`) and `changed-only`, so both CIs derive their
matrix from the filesystem instead of a hand-maintained list. Discovery
stays automatic: a newly added public app is picked up with no CI edit,
which is what keeps the "every public app" guarantee honest.

Wired in:

- **CI Server** gains a `server-apps-install-smoke` matrix that installs
every public app (`discover-apps` with `scope: public, changed-only:
false`) against the about-to-merge server, gated in
`ci-server-status-check` so a regression blocks merge.
- **CI Twenty Apps** discovers changed apps (`scope:
internal-and-public, changed-only: true`) and runs each against both
server sources — the released image and the monorepo build.

## Why the coverage differs per side (not "always everything")

`test-twenty-app` has three explicit modes —
`installation-and-integration-test` (integration + deploy + install),
`integration-test-only` (suite only), `installation-only` (deploy +
install only) — because the useful signal depends on what actually
changed:

- **App PR against the monorepo server →
`installation-and-integration-test`.** The app changed, so run its whole
suite against the upcoming server, install included.
- **App PR against the released server → `integration-test-only`.**
Checks the app's own suite against what users run today; install against
the released image is left to the SDK e2e path.
- **Server PR → `installation-only`, across all apps.** The apps did not
change; the only question is "can each one still be installed." Running
every app's full integration suite on every server PR would be far
slower and largely redundant. Installation-only keeps this broad (the
whole catalog) and cheap enough to always run and block merge.

The tradeoff is deliberate: broad but shallow where nothing in the app
changed, deep where it did.

## Notes / trade-offs

- On app-only PRs the `local` source pays a full server build per app
(the `server-build` cache is only warm on server PRs). Could be
optimized later with a shared warm-up job.
- SDK-local (Verdaccio) install testing stays in
`ci-create-app-e2e-minimal`; this PR's `local` source targets the server
build.

<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22636?utm_source=github"
rel="nofollow noreferrer noopener" target="_blank">``&lt;img alt="Review
in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"&gt;``</a>
2026-07-10 19:18:53 +02:00
Félix Malfait 38fbff465f chore(server): ship the 2.20 standardOverrides drop as a dormant command (#22448)
Follow-up to #22417, per [this
thread](https://github.com/twentyhq/twenty/pull/22417#discussion_r3512187719):
migrate the `2-20/README.md` placeholder into a real command using the
`TWENTY_NEXT_VERSIONS` mechanism.

### What

- Add `DropMetadataStandardOverridesColumnFastInstanceCommand`,
registered against `2.20.0`. It boots (`2.20.0` is in
`TWENTY_ALL_VERSIONS`) but stays **dormant** — the upgrade sequence only
runs `TWENTY_CROSS_UPGRADE_SUPPORTED_VERSIONS` (previous + current), so
it never executes during the 2.19 deploy and activates automatically
when `nx version:bump` promotes 2.20 to current.
- Name constant + unit test (SQL parity, registration against `2.20.0`,
name-constant parity).
- Register it in `instance-commands.constant.ts`.
- Update the `standardOverrides` `@deprecated` comments on object/field
metadata to point at the shipped command.
- Delete `2-20/README.md`.
- Document the "ship a command for a future version" flow in
`docs/UPGRADE_COMMANDS.md` and `.cursor/rules/server-migrations.mdc`
(the mechanism was previously undocumented).

### Note / correction to the README's plan

The old README implied both the command **and** `@WasRemovedInUpgrade`
could be added at 2.20 time. Only the command can ship now: the
decorator's validator runs against the active sequence, so referencing a
still-dormant 2.20 step fails boot with `unknown-step-name`. So the
entity keeps its `WasRemovedInUpgrade<T>` type wrapper for now; the
decorator gets wired (one line, via the name constant) once 2.20 is
current — same deferred-drop shape as `isUIReadOnly`.

### Verification

Could not run `jest`/`typecheck`/`lint` in this environment: `yarn
install` is blocked by egress policy on a git-based transitive dep
(`github.com/electron/node-gyp.git`). Verified by review against the
sibling 2-19 add-column and 2-12 drop commands. **Please let CI run
before merge.**

https://claude.ai/code/session_01KMArJvdEmsX3eAmJLbS1b6

---
_Generated by [Claude
Code](https://claude.ai/code/session_01KMArJvdEmsX3eAmJLbS1b6)_

<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22448?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
2026-07-02 13:18:20 +02:00
Paul Rastoin 847e7124d7 Upgrade command internal doc (#19541)
Open to discussion not sure on where to store such documentation
2026-04-10 09:43:06 +00:00