Compare commits

...
Author SHA1 Message Date
sonarly-bot bff10a04a1 fix(server): align workspace safety check with cursor semantics
https://sonarly.com/issue/38314?type=bug

Fresh self-host installs can enter a restart loop after creating the first workspace because startup migrations fail a workspace-version safety check that expects a row that is never written for new workspaces.

Fix: I updated the workspace safety check in `run-instance-commands` to align with cursor semantics instead of exact row existence:

1. Replaced `areAllWorkspacesAtCommand({ commandName })` usage with per-workspace cursor validation:
   - Fetch each active/suspended workspace cursor via `getWorkspaceLastAttemptedCommandNameOrThrow`.
   - Validate each cursor has reached or passed the required previous-version tail command using sequence order.

2. Added `UpgradeSequenceReaderService.isStepCompletedOrPassed(...)`:
   - Returns true if cursor is after required step.
   - Returns true on same step only when status is `completed`.
   - Returns false when cursor is before required step.

3. Updated safety-check error wording from “not completed” to “not reached” to match cursor/order-based semantics.

4. Added unit tests in `upgrade-sequence-reader.service.spec.ts` covering:
   - same-step completed (true),
   - same-step failed (false),
   - cursor after step (true),
   - cursor before step (false).

This preserves the intent of the safety gate (prevent unsafe upgrades) while removing false failures for fresh workspaces initialized with a valid later cursor.

Authored by Sonarly by autonomous analysis (run 43722).
2026-05-18 09:11:22 +00:00
3 changed files with 120 additions and 6 deletions
@@ -135,15 +135,29 @@ export class RunInstanceCommandsCommand extends CommandRunner {
return;
}
const allAtPreviousVersion =
await this.upgradeMigrationService.areAllWorkspacesAtCommand({
commandName: lastWorkspaceCommand.name,
workspaceIds: activeOrSuspendedWorkspaceIds,
});
const workspaceCursors =
await this.upgradeMigrationService.getWorkspaceLastAttemptedCommandNameOrThrow(
activeOrSuspendedWorkspaceIds,
);
const allAtPreviousVersion = activeOrSuspendedWorkspaceIds.every(
(workspaceId) => {
const cursor = workspaceCursors.get(workspaceId);
if (!cursor) {
return false;
}
return this.upgradeSequenceReaderService.isStepCompletedOrPassed({
cursor,
stepName: lastWorkspaceCommand.name,
});
},
);
if (!allAtPreviousVersion) {
throw new Error(
'Unable to run instance commands. Some workspace(s) have not completed ' +
'Unable to run instance commands. Some workspace(s) have not reached ' +
`the last workspace command for ${previousVersion} ("${lastWorkspaceCommand.name}").\n` +
'Please ensure all workspaces are upgraded to at least the previous version before running migrations.\n' +
'Use --force to bypass this check (not recommended).',
@@ -74,6 +74,77 @@ const makeFastInstance = (name: string) => makeStep('fast-instance', name);
const makeWorkspace = (name: string) => makeStep('workspace', name);
describe('UpgradeSequenceReaderService', () => {
describe('isStepCompletedOrPassed', () => {
it('should return true when cursor is on required step with completed status', async () => {
const sequence = [
makeFastInstance('Ic0'),
makeWorkspace('Wc0'),
makeWorkspace('Wc1'),
];
const service = await buildServiceWithMockedSequence(sequence);
const result = service.isStepCompletedOrPassed({
cursor: { name: 'Wc1', status: 'completed' },
stepName: 'Wc1',
});
expect(result).toBe(true);
});
it('should return false when cursor is on required step with failed status', async () => {
const sequence = [
makeFastInstance('Ic0'),
makeWorkspace('Wc0'),
makeWorkspace('Wc1'),
];
const service = await buildServiceWithMockedSequence(sequence);
const result = service.isStepCompletedOrPassed({
cursor: { name: 'Wc1', status: 'failed' },
stepName: 'Wc1',
});
expect(result).toBe(false);
});
it('should return true when cursor is after required step', async () => {
const sequence = [
makeFastInstance('Ic0'),
makeWorkspace('Wc0'),
makeWorkspace('Wc1'),
makeFastInstance('Ic1'),
];
const service = await buildServiceWithMockedSequence(sequence);
const result = service.isStepCompletedOrPassed({
cursor: { name: 'Ic1', status: 'failed' },
stepName: 'Wc1',
});
expect(result).toBe(true);
});
it('should return false when cursor is before required step', async () => {
const sequence = [
makeFastInstance('Ic0'),
makeWorkspace('Wc0'),
makeWorkspace('Wc1'),
];
const service = await buildServiceWithMockedSequence(sequence);
const result = service.isStepCompletedOrPassed({
cursor: { name: 'Wc0', status: 'completed' },
stepName: 'Wc1',
});
expect(result).toBe(false);
});
});
describe('getInitialCursorForNewWorkspace', () => {
it('should return last workspace command of segment following completed instance command', async () => {
const sequence = [
@@ -162,6 +162,35 @@ export class UpgradeSequenceReaderService {
: workspaceCommands.slice(cursorIndex);
}
isStepCompletedOrPassed({
cursor,
stepName,
}: {
cursor: { name: string; status: UpgradeMigrationStatus };
stepName: string;
}): boolean {
const sequence = this.getUpgradeSequence();
const cursorIndex = this.locateStepInSequenceOrThrow({
sequence,
stepName: cursor.name,
});
const stepIndex = this.locateStepInSequenceOrThrow({
sequence,
stepName,
});
if (cursorIndex > stepIndex) {
return true;
}
if (cursorIndex < stepIndex) {
return false;
}
return cursor.status === 'completed';
}
getInitialCursorForNewWorkspace(lastAttemptedInstanceCommand: {
name: string;
status: UpgradeMigrationStatus;