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).
This commit is contained in:
@@ -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).',
|
||||
|
||||
+71
@@ -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 = [
|
||||
|
||||
+29
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user