dbd2eac69c
Fixes the CI failure on #23520: An upgrade version sequence has to at least contain one instance or one workspace command Workspaces commands do not run for the instance level and aren't triggered automatically Explaining this PR need <img width="1396" height="954" alt="image" src="https://github.com/user-attachments/assets/5455d05b-b286-482a-8914-808f55f3b0bf" /> ``` Upload failed: App requires Twenty server >=2.26.0 but this server is 2.25.0. ``` The server really is 2.26 (`TWENTY_CURRENT_VERSION = '2.26.0'`), but `validateServerCompatibility` resolves the instance version through `UpgradeMigrationService.getInferredVersion()`, which reads the last row in `core.upgradeMigration` with `workspaceId IS NULL AND isInitial = false` and takes the version prefix off its name. Instance commands are the only ones that write a `workspaceId`-null row, and `2-26/` ships none (only three workspace commands), so the highest instance command in the tree is still `2-25-instance-command-fast-1785230296000-add-is-hidden-to-agent-message`. A fully migrated 2.26 server infers 2.25.0, and any app declaring `engines.twenty: ">=2.26.0"` is unpublishable. Two defects, both of which `getWorkspaceCompletedVersion` already avoids: - **Not sequence-aware.** The workspace path walks the registered sequence and only credits a version once the cursor sits on that version's last step. The instance path just reads the cursor's prefix, so a version contributing zero instance commands is unreachable. - **Not status-aware.** `getLastAttemptedInstanceCommand` filters on `attempt = MAX(attempt)` but not on status, so a *failed* 2.25 command still made the server report 2.25.0. ## What changed `UpgradeStatusService` gains `getInstanceCompletedVersion()`, the instance-scope mirror of `getWorkspaceCompletedVersion`. It walks the sequence filtered to instance steps, requires the cursor to sit on the last instance step of its version *and* be `completed`, then advances through any later supported version that declares no instance command at all. The version-skipping rule is the part that unblocks 2.26: a release with no instance-level work has nothing for the cursor to land on, so it is reached as soon as the last version that does have instance commands is done. A version whose instance command exists but has not run still holds the cursor back. - `validateServerCompatibility` calls the new method; `UpgradeMigrationService` is no longer a dependency of `ApplicationVersionValidationService`. - `getInstanceStatus` reports it as `inferredVersion`, so the upgrade gauge metric and `upgrade:status` CLI stop showing 2.25.0 on a 2.26 server. - `getInferredVersion` is deleted. Its one remaining caller passed a command name, which is just `extractVersionFromCommandName`. - Cursor resolution is extracted to `resolve-completed-version-from-cursor.util`, now shared by both scopes; the skip rule lives in `advance-through-versions-without-instance-commands.util`. The asymmetry between the two scopes is intentional and stays: instance commands record a row per workspace as well, so workspace cursors land on both command kinds and never had this gap. ## Testing - `npx nx typecheck twenty-server` clean, `npx nx lint:diff-with-main twenty-server` clean. - 294 unit tests pass across the upgrade and application modules, including 7 new ones for `getInstanceCompletedVersion`. Two pin the boundary: a trailing workspace-only version is reached, a trailing version whose instance command has not run is not. - The fixture in `upgrade-status.service.spec.ts` used `1.21.0`/`1.22.0`/`1.23.0`, which are real entries in `TWENTY_PREVIOUS_VERSIONS`. With the skip rule in place that sequence read as "every version from 2.0 onward has no instance commands" and walked to the end, so the fixture is renumbered to `0.2x.0` to keep those tests on cursor resolution alone. - `failing-app-installation-workspace-version.integration-spec.ts` already carried a comment describing this bug as a hazard it worked around. The workaround still holds, but integration tests were not run here (no DB in this session) — the stale comment is updated. #23520 stays at `>=2.26.0` and unblocks once this lands. --- _Generated by [Claude Code](https://claude.ai/code/session_012SvBG1BB3jTaZs6LA2Wi9R)_ <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/23552?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. -->
188 lines
6.0 KiB
TypeScript
188 lines
6.0 KiB
TypeScript
import { TWENTY_CROSS_UPGRADE_SUPPORTED_VERSIONS } from 'src/engine/core-modules/upgrade/constants/twenty-cross-upgrade-supported-version.constant';
|
|
import { type UpgradeStep } from 'src/engine/core-modules/upgrade/services/upgrade-sequence-reader.service';
|
|
|
|
import {
|
|
type IntegrationTestContext,
|
|
createUpgradeSequenceRunnerIntegrationTestModule,
|
|
DEFAULT_OPTIONS,
|
|
makeVersionedStep,
|
|
resetSeedSequenceCounter,
|
|
restoreUpgradeMigrations,
|
|
seedInstanceMigration,
|
|
setMockActiveWorkspaceIds,
|
|
snapshotUpgradeMigrations,
|
|
WS_1,
|
|
} from 'test/integration/upgrade/utils/upgrade-sequence-runner-integration-test.util';
|
|
|
|
// The sequence only ever covers supported versions, and the skip rule walks
|
|
// that same list, so the fixture has to be built from real ones.
|
|
const [OLDEST_VERSION, MIDDLE_VERSION, NEWEST_VERSION] =
|
|
TWENTY_CROSS_UPGRADE_SUPPORTED_VERSIONS.slice(-3);
|
|
|
|
const OLDEST_INSTANCE_COMMAND = makeVersionedStep('fast-instance', {
|
|
version: OLDEST_VERSION,
|
|
label: 'OldestInstanceCommand',
|
|
});
|
|
const OLDEST_SLOW_INSTANCE_COMMAND = makeVersionedStep('slow-instance', {
|
|
version: OLDEST_VERSION,
|
|
label: 'OldestSlowInstanceCommand',
|
|
});
|
|
const MIDDLE_INSTANCE_COMMAND = makeVersionedStep('fast-instance', {
|
|
version: MIDDLE_VERSION,
|
|
label: 'MiddleInstanceCommand',
|
|
});
|
|
const NEWEST_INSTANCE_COMMAND = makeVersionedStep('fast-instance', {
|
|
version: NEWEST_VERSION,
|
|
label: 'NewestInstanceCommand',
|
|
});
|
|
const NEWEST_WORKSPACE_COMMAND = makeVersionedStep('workspace', {
|
|
version: NEWEST_VERSION,
|
|
label: 'NewestWorkspaceCommand',
|
|
});
|
|
|
|
describe('UpgradeStatusService — instance completed version (integration)', () => {
|
|
let context: IntegrationTestContext;
|
|
let savedUpgradeMigrations: Awaited<
|
|
ReturnType<typeof snapshotUpgradeMigrations>
|
|
>;
|
|
|
|
const mockSequence = (sequence: UpgradeStep[]) => {
|
|
jest
|
|
.spyOn(context.upgradeSequenceReaderService, 'getUpgradeSequence')
|
|
.mockReturnValue(sequence);
|
|
};
|
|
|
|
beforeAll(async () => {
|
|
context = await createUpgradeSequenceRunnerIntegrationTestModule();
|
|
savedUpgradeMigrations = await snapshotUpgradeMigrations(
|
|
context.dataSource,
|
|
);
|
|
}, 30000);
|
|
|
|
afterAll(async () => {
|
|
await restoreUpgradeMigrations(context.dataSource, savedUpgradeMigrations);
|
|
await context.module?.close();
|
|
await context.dataSource?.destroy();
|
|
}, 15000);
|
|
|
|
beforeEach(async () => {
|
|
await context.dataSource.query('DELETE FROM core."upgradeMigration"');
|
|
resetSeedSequenceCounter();
|
|
setMockActiveWorkspaceIds([]);
|
|
jest.restoreAllMocks();
|
|
});
|
|
|
|
it('should return null when no instance command has ever run', async () => {
|
|
mockSequence([OLDEST_INSTANCE_COMMAND, MIDDLE_INSTANCE_COMMAND]);
|
|
|
|
await expect(
|
|
context.upgradeStatusService.getInstanceCompletedVersion(),
|
|
).resolves.toBeNull();
|
|
});
|
|
|
|
it('should reach a trailing version that ships no instance command', async () => {
|
|
mockSequence([MIDDLE_INSTANCE_COMMAND, NEWEST_WORKSPACE_COMMAND]);
|
|
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: MIDDLE_INSTANCE_COMMAND.name,
|
|
status: 'completed',
|
|
});
|
|
|
|
await expect(
|
|
context.upgradeStatusService.getInstanceCompletedVersion(),
|
|
).resolves.toBe(NEWEST_VERSION);
|
|
});
|
|
|
|
it('should hold at the previous version while the trailing instance command has not run', async () => {
|
|
mockSequence([MIDDLE_INSTANCE_COMMAND, NEWEST_INSTANCE_COMMAND]);
|
|
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: MIDDLE_INSTANCE_COMMAND.name,
|
|
status: 'completed',
|
|
});
|
|
|
|
await expect(
|
|
context.upgradeStatusService.getInstanceCompletedVersion(),
|
|
).resolves.toBe(MIDDLE_VERSION);
|
|
});
|
|
|
|
it('should not credit a version whose last instance command failed', async () => {
|
|
mockSequence([MIDDLE_INSTANCE_COMMAND, NEWEST_INSTANCE_COMMAND]);
|
|
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: MIDDLE_INSTANCE_COMMAND.name,
|
|
status: 'completed',
|
|
});
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: NEWEST_INSTANCE_COMMAND.name,
|
|
status: 'failed',
|
|
});
|
|
|
|
await expect(
|
|
context.upgradeStatusService.getInstanceCompletedVersion(),
|
|
).resolves.toBe(MIDDLE_VERSION);
|
|
});
|
|
|
|
it('should credit a version once its failed command is retried successfully', async () => {
|
|
mockSequence([MIDDLE_INSTANCE_COMMAND, NEWEST_INSTANCE_COMMAND]);
|
|
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: MIDDLE_INSTANCE_COMMAND.name,
|
|
status: 'completed',
|
|
});
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: NEWEST_INSTANCE_COMMAND.name,
|
|
status: 'failed',
|
|
});
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: NEWEST_INSTANCE_COMMAND.name,
|
|
status: 'completed',
|
|
attempt: 2,
|
|
});
|
|
|
|
await expect(
|
|
context.upgradeStatusService.getInstanceCompletedVersion(),
|
|
).resolves.toBe(NEWEST_VERSION);
|
|
});
|
|
|
|
it('should not credit a version while earlier instance commands of that version remain', async () => {
|
|
mockSequence([OLDEST_INSTANCE_COMMAND, OLDEST_SLOW_INSTANCE_COMMAND]);
|
|
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: OLDEST_INSTANCE_COMMAND.name,
|
|
status: 'completed',
|
|
});
|
|
|
|
await expect(
|
|
context.upgradeStatusService.getInstanceCompletedVersion(),
|
|
).resolves.toBeNull();
|
|
});
|
|
|
|
it('should report the version reached after the runner executes the whole sequence', async () => {
|
|
const sequence = [
|
|
OLDEST_INSTANCE_COMMAND,
|
|
MIDDLE_INSTANCE_COMMAND,
|
|
NEWEST_WORKSPACE_COMMAND,
|
|
];
|
|
|
|
mockSequence(sequence);
|
|
setMockActiveWorkspaceIds([WS_1]);
|
|
|
|
// The runner resumes from a cursor, so the sequence needs a starting point.
|
|
await seedInstanceMigration(context.dataSource, {
|
|
name: OLDEST_INSTANCE_COMMAND.name,
|
|
status: 'completed',
|
|
workspaceIds: [WS_1],
|
|
});
|
|
|
|
await context.runner.run({
|
|
sequence,
|
|
options: DEFAULT_OPTIONS,
|
|
});
|
|
|
|
await expect(
|
|
context.upgradeStatusService.getInstanceCompletedVersion(),
|
|
).resolves.toBe(NEWEST_VERSION);
|
|
});
|
|
});
|