From ace6ca973a8e4890860de28b2ec5ab014d985d40 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Thu, 18 Sep 2025 19:21:53 +0000 Subject: [PATCH 01/12] =?UTF-8?q?improve=20error=20popover=20readability?= =?UTF-8?q?=20in=20=C2=ABDeploymentStatus=C2=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhance the error popover by adding a «panelStyle» property with word-wrapping. This ensures long text breaks properly, improving readability and preventing layout issues. --- public/dashboard-assistant/components/deployment-status.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/public/dashboard-assistant/components/deployment-status.tsx b/public/dashboard-assistant/components/deployment-status.tsx index f5351be5..0307003b 100644 --- a/public/dashboard-assistant/components/deployment-status.tsx +++ b/public/dashboard-assistant/components/deployment-status.tsx @@ -96,6 +96,7 @@ export const DeploymentStatus = ({
{uiStatus === StepStatus.ERROR && step?.error ? ( setOpenErrorPopoverKey(null)} anchorPosition="rightCenter" From 0ce7d613d0b36ab78016f07d17a5abcce68bfbd0 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Thu, 18 Sep 2025 19:53:51 +0000 Subject: [PATCH 02/12] improve error handling and enrich contextual information MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhance error propagation and contextual data in HTTP client operations by introducing a detailed «HttpError» utility. Update test cases to validate enriched error messages, including status text and request details. Refactor installation steps to incorporate structured error handling using «StepError», providing detailed context and possible causes for failures. Improve diagnostic messaging during installation, including step-specific details and enriched error descriptions. Add utility functions to extract status codes and derive potential causes for issues, improving debugging capabilities. Update related unit tests to ensure comprehensive coverage of new behaviors. --- .../common/http/infrastructure/http-error.ts | 154 ++++++++++++++++++ .../window-fetch-http-client.test.ts | 35 +++- .../window-fetch-http-client.ts | 4 +- .../installation-progress-manager.test.ts | 2 +- .../infrastructure/installation-manager.ts | 6 +- .../infrastructure/steps/create-agent-step.ts | 27 ++- .../steps/create-connector-step.ts | 23 ++- .../infrastructure/steps/create-model-step.ts | 24 ++- .../steps/register-agent-step.ts | 21 ++- .../steps/test-model-connection-step.test.ts | 21 ++- .../steps/test-model-connection-step.ts | 90 +++++++++- .../steps/update-ml-commons-settings-step.ts | 19 ++- .../infrastructure/utils/step-error.ts | 136 ++++++++++++++++ 13 files changed, 535 insertions(+), 27 deletions(-) create mode 100644 public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts create mode 100644 public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts diff --git a/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts b/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts new file mode 100644 index 00000000..a7788809 --- /dev/null +++ b/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts @@ -0,0 +1,154 @@ +/* + * Copyright Wazuh Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +const SENSITIVE_KEY_PATTERN = /(key|token|secret|password|authorization)$/i; +const MAX_BODY_LENGTH = 1000; + +export class HttpError extends Error { + public response?: Response; + public status?: number; + public statusText?: string; + public request?: { method: string; url: string; body?: string }; + public responseBodyOmitted?: boolean; + + private constructor(message: string) { + super(message); + this.name = 'HttpError'; + } + + public static async create( + response: Response, + options: RequestInit & { method: string }, + url: string, + ): Promise { + const method = options.method.toUpperCase(); + const sanitizedUrl = this.sanitizeUrl(url); + const hasResponseBody = await this.hasMeaningfulBody(response); + + const statusText = response.statusText?.trim(); + const statusSummary = statusText + ? `${response.status} ${statusText}` + : `${response.status}`; + + const message = this.buildFailureMessage({ + method, + url: sanitizedUrl, + statusSummary, + hasResponseBody, + }); + + const error = new HttpError(message); + error.response = response; + error.status = response.status; + error.statusText = response.statusText; + error.request = { + method, + url: sanitizedUrl, + body: this.sanitizeRequestBody(options.body ?? undefined), + }; + if (hasResponseBody) { + error.responseBodyOmitted = true; + } + + return error; + } + + private static buildFailureMessage(params: { + method: string; + url: string; + statusSummary: string; + hasResponseBody: boolean; + }): string { + const { method, url, statusSummary, hasResponseBody } = params; + + const messageParts = [ + `HTTP ${method} ${url} failed with status ${statusSummary}`, + hasResponseBody + ? 'Response body omitted due to security policy' + : undefined, + ].filter(Boolean); + + return messageParts.join('. '); + } + + private static sanitizeUrl(url: string): string { + try { + const base = + typeof window !== 'undefined' && window.location?.origin + ? window.location.origin + : 'http://localhost'; + const parsed = new URL(url, base); + parsed.searchParams.forEach((value, key) => { + if (SENSITIVE_KEY_PATTERN.test(key)) { + parsed.searchParams.delete(key); + } + }); + + if (!url.startsWith('http')) { + return ( + `${parsed.pathname}${parsed.search}${parsed.hash}` || parsed.pathname + ); + } + + return parsed.toString(); + } catch (error) { + return url; + } + } + + private static async hasMeaningfulBody(response: Response): Promise { + try { + const body = await response.clone().text(); + return Boolean(body.trim()); + } catch (error) { + return false; + } + } + + private static sanitizeStructuredValue(value: unknown): unknown { + if (value == null) { + return value; + } + + if (Array.isArray(value)) { + return value.map(item => this.sanitizeStructuredValue(item)); + } + + if (typeof value === 'object') { + return Object.entries(value as Record).reduce< + Record + >((acc, [key, entryValue]) => { + if (typeof entryValue !== 'string' || !SENSITIVE_KEY_PATTERN.test(key)) { + acc[key] = this.sanitizeStructuredValue(entryValue); + } + return acc; + }, {}); + } + + return value; + } + + private static sanitizeRequestBody( + body?: BodyInit | null, + ): string | undefined { + if (!body) { + return undefined; + } + + if (typeof body === 'string') { + try { + const parsed = JSON.parse(body); + const sanitized = this.sanitizeStructuredValue(parsed); + return JSON.stringify(sanitized); + } catch (error) { + return body.length > MAX_BODY_LENGTH + ? `${body.slice(0, MAX_BODY_LENGTH)}...` + : body; + } + } + + return ''; + } +} diff --git a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts index a2e94306..3d104629 100644 --- a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts +++ b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts @@ -8,7 +8,9 @@ import { WindowFetchHttpClient } from './window-fetch-http-client'; interface JsonResponse { ok: boolean; status: number; + statusText?: string; json: () => Promise>; + clone: () => { text: () => Promise }; } describe('WindowFetchHttpClient', () => { @@ -36,6 +38,9 @@ describe('WindowFetchHttpClient', () => { ok: true, status: 200, json: async () => payload, + clone: () => ({ + text: async () => JSON.stringify(payload), + }), }; const fetchSpy = jest.fn().mockResolvedValue(mockResponse); // @ts-ignore @@ -58,6 +63,9 @@ describe('WindowFetchHttpClient', () => { ok: true, status: 200, json: async () => ({}), + clone: () => ({ + text: async () => JSON.stringify({}), + }), }; const fetchSpy = jest.fn().mockResolvedValue(mockResponse); // @ts-ignore @@ -93,19 +101,34 @@ describe('WindowFetchHttpClient', () => { ); }); - it('propagates HTTP errors with response attached', async () => { + it('propagates HTTP errors with contextual information attached', async () => { + expect.assertions(5); + const errorPayload = { message: 'boom' }; const mockResponse: JsonResponse = { ok: false, status: 500, - json: async () => ({ message: 'boom' }), + statusText: 'Internal Server Error', + json: async () => errorPayload, + clone: () => ({ + text: async () => JSON.stringify(errorPayload), + }), }; const fetchSpy = jest.fn().mockResolvedValue(mockResponse); // @ts-ignore window.fetch = fetchSpy; const client = new WindowFetchHttpClient(); - await expect(client.delete('/x')).rejects.toMatchObject({ - message: 'HTTP error! status: 500', - response: mockResponse, - }); + + try { + await client.delete('/x'); + throw new Error('Expected HTTP error to be thrown'); + } catch (error) { + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain( + 'HTTP DELETE /x failed with status 500 Internal Server Error' + ); + expect((error as Error).message).toContain('Response body: {"message":"boom"}'); + expect((error as { response?: JsonResponse }).response).toBe(mockResponse); + expect((error as { request?: { method: string } }).request?.method).toBe('DELETE'); + } }); }); diff --git a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts index 65cff6f6..50b7eff6 100644 --- a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts +++ b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts @@ -4,6 +4,7 @@ */ import type { HttpClient } from '../domain/entities/http-client'; +import { HttpError } from './http-error'; export class WindowFetchHttpClient implements HttpClient { private defaultHeaders = { 'osd-xsrf': 'kibana' }; @@ -28,8 +29,7 @@ export class WindowFetchHttpClient implements HttpClient { return window.fetch(url, options).then(async (response) => { if (!response.ok) { - const error = new Error(`HTTP error! status: ${response.status}`); - (error as any).response = response; + const error = await HttpError.create(response, options as RequestInit & { method: string }, url); throw error; } return response.json(); diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts index 72e2238a..84db9b33 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts @@ -65,7 +65,7 @@ describe('InstallationProgressManager', () => { const p = mgr.getProgress(); expect(p.getSteps()[0].state).toBe(ExecutionState.FAILED); - expect(p.getSteps()[0].message).toBe('boom-msg'); + expect(p.getSteps()[0].message).toBe('boom-msg Additional details: boom'); expect(p.getSteps()[0].error).toBeInstanceOf(Error); expect(p.hasFailedSteps()).toBe(true); expect(p.getFailedSteps().length).toBe(1); diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts index 685d0205..cf92f200 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts @@ -48,9 +48,13 @@ export class InstallationManager implements IInstallationManager { } catch (error) { const progress = progressManager.getProgress(); const failedSteps = progress.getFailedSteps(); + const firstFailedStep = failedSteps[0]; + const stepContext = firstFailedStep ? `during step "${firstFailedStep.stepName}"` : 'at an unknown step'; + const errorMessage = error instanceof Error ? error.message : String(error); + return { success: false, - message: `Installation failed: ${error}`, + message: `Installation failed ${stepContext}: ${errorMessage}`, progress, data: context.toObject(), errors: failedSteps.map((step) => ({ diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts index 12497ded..813ab9e6 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts @@ -13,6 +13,7 @@ import { InstallationAIAssistantStep, InstallAIDashboardAssistantDto, } from '../../domain'; +import { StepError } from '../utils/step-error'; export class CreateAgentStep extends InstallationAIAssistantStep { constructor() { @@ -62,10 +63,28 @@ export class CreateAgentStep extends InstallationAIAssistantStep { request: InstallAIDashboardAssistantDto, context: InstallationContext ): Promise { - const agent = await getUseCases().createAgent( - this.createAgentDto(request, context.get('modelId')) - ); - context.set('agentId', agent.id); + const details: Record = { + provider: request.selected_provider, + }; + + if (context.has('modelId')) { + details.modelId = context.get('modelId'); + } + + try { + const dto = this.createAgentDto(request, context.get('modelId')); + details.agentName = dto.name; + const agent = await getUseCases().createAgent(dto); + details.agentId = agent?.id; + context.set('agentId', agent.id); + } catch (error) { + throw StepError.create({ + stepName: this.getName(), + action: 'creating the assistant agent', + cause: error, + details, + }); + } } public getSuccessMessage(): string { diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts index ff0e4b07..8f068505 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts @@ -11,6 +11,7 @@ import { InstallAIDashboardAssistantDto, } from '../../domain'; import { getUseCases } from '../../../../services/ml-use-cases.service'; +import { StepError } from '../utils/step-error'; export class CreateConnectorStep extends InstallationAIAssistantStep { constructor() { @@ -41,8 +42,26 @@ export class CreateConnectorStep extends InstallationAIAssistantStep { request: InstallAIDashboardAssistantDto, context: InstallationContext ): Promise { - const connector = await getUseCases().createConnector(this.buildDto(request)); - context.set('connectorId', connector.id); + const details: Record = { + provider: request.selected_provider, + endpoint: request.api_url, + modelId: request.model_id, + }; + + try { + const dto = this.buildDto(request); + details.connectorName = dto.name; + const connector = await getUseCases().createConnector(dto); + details.connectorId = connector?.id; + context.set('connectorId', connector.id); + } catch (error) { + throw StepError.create({ + stepName: this.getName(), + action: 'creating the ML Commons connector', + cause: error, + details, + }); + } } getSuccessMessage(): string { diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts index 1a340b0e..a6688765 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts @@ -10,6 +10,7 @@ import { InstallationAIAssistantStep, InstallAIDashboardAssistantDto, } from '../../domain'; +import { StepError } from '../utils/step-error'; export class CreateModelStep extends InstallationAIAssistantStep { constructor() { @@ -31,8 +32,27 @@ export class CreateModelStep extends InstallationAIAssistantStep { request: InstallAIDashboardAssistantDto, context: InstallationContext ): Promise { - const model = await getUseCases().createModel(this.buildDto(request, context)); - context.set('modelId', model.id); + const details: Record = { + provider: request.selected_provider, + }; + + if (context.has('connectorId')) { + details.connectorId = context.get('connectorId'); + } + + try { + const dto = this.buildDto(request, context); + const model = await getUseCases().createModel(dto); + details.modelId = model?.id; + context.set('modelId', model.id); + } catch (error) { + throw StepError.create({ + stepName: this.getName(), + action: 'creating the ML Commons model', + cause: error, + details, + }); + } } getSuccessMessage(): string { diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts index 351a0f1c..c7c1c010 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts @@ -9,6 +9,7 @@ import { InstallationAIAssistantStep, InstallAIDashboardAssistantDto, } from '../../domain'; +import { StepError } from '../utils/step-error'; export class RegisterAgentStep extends InstallationAIAssistantStep { constructor() { @@ -19,8 +20,24 @@ export class RegisterAgentStep extends InstallationAIAssistantStep { request: InstallAIDashboardAssistantDto, context: InstallationContext ): Promise { - const agentId = context.get('agentId'); - await getUseCases().useAgent(agentId); + const details: Record = {}; + + if (context.has('agentId')) { + details.agentId = context.get('agentId'); + } + + try { + const agentId = context.get('agentId'); + details.agentId = agentId; + await getUseCases().useAgent(agentId); + } catch (error) { + throw StepError.create({ + stepName: this.getName(), + action: 'registering the assistant agent for usage', + cause: error, + details, + }); + } } public getSuccessMessage(): string { diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts index 85cdd460..08b442bd 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts @@ -44,7 +44,26 @@ describe('TestModelConnectionStep', () => { const step = new TestModelConnectionStep(); const ctx = new InstallationContext(); ctx.set('modelId', 'm-1'); - await expect(step.execute(req, ctx)).rejects.toThrow('Failed to connect to model'); + await expect(step.execute(req, ctx)).rejects.toThrow(/Test Model Connection failed while validating the model connection/); expect(step.getFailureMessage()).toMatch(/Failed to test model connection/); }); + + it('enriches errors with possible causes when API key is invalid', async () => { + const apiError = new Error('Unauthorized'); + (apiError as { status?: number }).status = 401; + + ((global as unknown) as { + __mockUseCases: import('../../../../services/__mocks__').MockUseCases; + }).__mockUseCases = { + validateModelConnection: jest.fn().mockRejectedValue(apiError), + }; + + const step = new TestModelConnectionStep(); + const ctx = new InstallationContext(); + ctx.set('modelId', 'm-1'); + + await expect(step.execute(req, ctx)).rejects.toThrow( + /Possible causes: Verify the API key; it may be incorrect or lack permissions\./ + ); + }); }); diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts index 22fa8dba..c9f94aa8 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts @@ -9,6 +9,69 @@ import { InstallationAIAssistantStep, InstallAIDashboardAssistantDto, } from '../../domain'; +import { StepError } from '../utils/step-error'; + +const extractStatusCode = (error: unknown): number | undefined => { + if (!error || typeof error !== 'object') { + return undefined; + } + + const possibleStatus = (error as { status?: unknown }).status; + const responseStatus = (error as { response?: { status?: unknown } }).response?.status; + + const parseStatus = (value: unknown): number | undefined => { + if (typeof value === 'number') { + return value; + } + if (typeof value === 'string') { + const parsed = Number.parseInt(value, 10); + if (!Number.isNaN(parsed)) { + return parsed; + } + } + return undefined; + }; + + const directStatus = parseStatus(possibleStatus); + if (typeof directStatus === 'number') { + return directStatus; + } + + const nestedStatus = parseStatus(responseStatus); + if (typeof nestedStatus === 'number') { + return nestedStatus; + } + + return undefined; +}; + +const derivePossibleCauses = (error: unknown, modelId?: string): string[] => { + const causes = new Set(); + const status = extractStatusCode(error); + const message = error instanceof Error ? error.message.toLowerCase() : String(error).toLowerCase(); + + if (status === 401 || status === 403) { + causes.add('Verify the API key; it may be incorrect or lack permissions.'); + } + + if (status === 404) { + causes.add( + modelId + ? `Confirm that model \`${modelId}\` exists and is deployed.` + : 'Confirm that the selected model exists and is deployed.' + ); + } + + const indicatesNetworkIssue = + !status && + (error instanceof TypeError || message.includes('failed to fetch') || message.includes('network')); + + if (indicatesNetworkIssue) { + causes.add('Check the API URL for typos and ensure the endpoint is reachable.'); + } + + return Array.from(causes); +}; export class TestModelConnectionStep extends InstallationAIAssistantStep { constructor() { @@ -19,10 +82,29 @@ export class TestModelConnectionStep extends InstallationAIAssistantStep { request: InstallAIDashboardAssistantDto, context: InstallationContext ): Promise { - // Simulate testing model connection - const isConnected = await getUseCases().validateModelConnection(context.get('modelId')); - if (!isConnected) { - throw new Error('Failed to connect to model'); + const details: Record = {}; + const modelId: string | undefined = context.has('modelId') ? context.get('modelId') : undefined; + + if (modelId) { + details.modelId = modelId; + } + + try { + if (!modelId) { + throw new Error('Model identifier not found in installation context'); + } + const isConnected = await getUseCases().validateModelConnection(modelId); + if (!isConnected) { + throw new Error('Model connection validation returned false'); + } + } catch (error) { + throw StepError.create({ + stepName: this.getName(), + action: 'validating the model connection', + cause: error, + details, + possibleCauses: derivePossibleCauses(error, modelId), + }); } } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts index c03ba9be..1f66a346 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts @@ -11,6 +11,7 @@ import { InstallAIDashboardAssistantDto, } from '../../domain'; import { getUseCases } from '../../../../services/ml-use-cases.service'; +import { StepError } from '../utils/step-error'; export class UpdateMlCommonsSettingsStep extends InstallationAIAssistantStep { constructor() { @@ -32,8 +33,22 @@ export class UpdateMlCommonsSettingsStep extends InstallationAIAssistantStep { request: InstallAIDashboardAssistantDto, context: InstallationContext ): Promise { - const dto = this.buildDto(request); - await getUseCases().persistMlCommonsSettings(dto); + const details: Record = { + provider: request.selected_provider, + }; + + try { + const dto = this.buildDto(request); + details.endpointsRegex = dto.endpoints_regex; + await getUseCases().persistMlCommonsSettings(dto); + } catch (error) { + throw StepError.create({ + stepName: this.getName(), + action: 'updating ML Commons settings in the cluster', + cause: error, + details, + }); + } } getSuccessMessage(): string { diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts new file mode 100644 index 00000000..266aa9d5 --- /dev/null +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts @@ -0,0 +1,136 @@ +/* + * Copyright Wazuh Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +export interface StepErrorOptions { + stepName: string; + action?: string; + cause: unknown; + details?: Record; + possibleCauses?: string[]; +} + +const SENSITIVE_KEY_PATTERN = /(key|token|secret|password|authorization)$/i; +const MAX_DETAIL_VALUE_LENGTH = 500; + +export class StepError extends Error { + public details?: Record; + public cause?: Error; + + private constructor(message: string, cause: Error, details?: Record) { + super(message); + this.name = 'StepError'; + this.details = details; + this.cause = cause; + + if (cause.stack) { + this.stack = cause.stack; + } + } + + public static create(options: StepErrorOptions): StepError { + const cause = options.cause instanceof Error ? options.cause : new Error(String(options.cause)); + const sanitizedDetails = this.sanitizeDetails(options.details); + const detailsText = this.formatDetails(sanitizedDetails); + const possibleCausesText = this.formatPossibleCauses(options.possibleCauses); + + const message = this.buildMessage({ + stepName: options.stepName, + action: options.action, + detailsText, + possibleCausesText, + causeMessage: cause.message, + }); + + return new StepError(message, cause, sanitizedDetails); + } + + private static buildMessage(params: { + stepName: string; + action?: string; + detailsText?: string; + possibleCausesText?: string; + causeMessage: string; + }): string { + const { stepName, action, detailsText, possibleCausesText, causeMessage } = params; + + const messageParts = [ + `"${stepName}" step failed${action ? ` while ${action}` : ''}`, + detailsText ? `Details: ${detailsText}` : undefined, + possibleCausesText ? `Possible causes: ${possibleCausesText}` : undefined, + `Cause: ${causeMessage}`, + ].filter(Boolean); + + return messageParts.join('. '); + } + + private static sanitizeDetailValue(key: string, value: unknown): unknown { + if (value == null) { + return value; + } + + if (typeof value === 'string') { + if (SENSITIVE_KEY_PATTERN.test(key)) { + return ''; + } + + return value.length > MAX_DETAIL_VALUE_LENGTH + ? `${value.slice(0, MAX_DETAIL_VALUE_LENGTH)}...` + : value; + } + + if (typeof value === 'number' || typeof value === 'boolean') { + return value; + } + + try { + const serialized = JSON.stringify(value); + if (!serialized) { + return value; + } + return serialized.length > MAX_DETAIL_VALUE_LENGTH + ? `${serialized.slice(0, MAX_DETAIL_VALUE_LENGTH)}...` + : serialized; + } catch (error) { + return value; + } + } + + private static sanitizeDetails(details?: Record): Record | undefined { + if (!details) { + return undefined; + } + + return Object.entries(details).reduce>((acc, [key, value]) => { + acc[key] = this.sanitizeDetailValue(key, value); + return acc; + }, {}); + } + + private static formatDetails(details?: Record): string | undefined { + if (!details || Object.keys(details).length === 0) { + return undefined; + } + + return Object.entries(details) + .map(([key, value]) => `${key}=${value}`) + .join(', '); + } + + private static formatPossibleCauses(possibleCauses?: string[]): string | undefined { + if (!possibleCauses || possibleCauses.length === 0) { + return undefined; + } + + const trimmed = possibleCauses + .map((cause) => cause.trim()) + .filter((cause) => cause.length > 0); + + if (trimmed.length === 0) { + return undefined; + } + + return trimmed.join('; '); + } +} From e1afdf174a4e478cb472fef01b998e1caa2577d8 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Thu, 18 Sep 2025 20:40:31 +0000 Subject: [PATCH 03/12] add rollback support to installation steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce rollback functionality to the installation process, enabling the reversal of completed steps upon failure. Extend «InstallationAIAssistantStep» with a new «rollback» method and update «InstallationProgressManager» to manage rollback execution. Add rollback handling for agent, connector, and model creation steps, ensuring created resources are cleaned up if subsequent steps fail. Enhance integration tests to validate rollback behavior and update the «InstallationResult» structure to include «rollbackErrors» for improved error reporting. --- .../ports/__mocks__/agent-repository.ts | 1 + .../application/ports/agent-repository.ts | 6 +- .../application/use-cases/delete-agent.ts | 13 +++ .../application/use-cases/delete-connector.ts | 13 +++ .../installation-ai-assistant-step.ts | 5 ++ .../domain/entities/installation-context.ts | 4 + .../installation-progress-manager.test.ts | 42 ++++++--- .../entities/installation-progress-manager.ts | 48 ++++++++++ .../domain/types/installation-result.ts | 4 + .../installation-manager.int.test.ts | 90 ++++++++++++++++++- .../infrastructure/installation-manager.ts | 3 +- .../infrastructure/steps/create-agent-step.ts | 22 +++++ .../steps/create-connector-step.ts | 22 +++++ .../infrastructure/steps/create-model-step.ts | 22 +++++ .../steps/register-agent-step.ts | 8 ++ .../steps/test-model-connection-step.ts | 8 ++ .../steps/update-ml-commons-settings-step.ts | 8 ++ .../application/use-cases/delete-model.ts | 13 +++ .../services/__mocks__/index.ts | 3 + .../services/ml-use-cases.service.ts | 6 ++ 20 files changed, 326 insertions(+), 15 deletions(-) create mode 100644 public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts create mode 100644 public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts create mode 100644 public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts diff --git a/public/dashboard-assistant/modules/agent/application/ports/__mocks__/agent-repository.ts b/public/dashboard-assistant/modules/agent/application/ports/__mocks__/agent-repository.ts index de9397a3..912156b8 100644 --- a/public/dashboard-assistant/modules/agent/application/ports/__mocks__/agent-repository.ts +++ b/public/dashboard-assistant/modules/agent/application/ports/__mocks__/agent-repository.ts @@ -8,6 +8,7 @@ import type { AgentRepository } from '../agent-repository'; export function createAgentRepositoryMock(): jest.Mocked { return { create: jest.fn(), + delete: jest.fn(), execute: jest.fn(), getActive: jest.fn(), register: jest.fn(), diff --git a/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts b/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts index cc113fd4..72769da9 100644 --- a/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts +++ b/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts @@ -3,11 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { CreateRepository } from '../../../common/domain/entities/repository'; +import { CreateRepository, DeleteRepository } from '../../../common/domain/entities/repository'; import { Agent } from '../../domain/entities/agent'; import { CreateAgentDto } from '../dtos/create-agent-dto'; -export interface AgentRepository extends CreateRepository { +export interface AgentRepository + extends CreateRepository, + DeleteRepository { execute(id: string, parameters: any): Promise; getActive(): Promise; register(agentId: string): Promise; diff --git a/public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts b/public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts new file mode 100644 index 00000000..1270fb13 --- /dev/null +++ b/public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts @@ -0,0 +1,13 @@ +/* + * Copyright Wazuh Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { AgentRepository } from '../ports/agent-repository'; + +export const deleteAgentUseCase = (agentRepository: AgentRepository) => async ( + agentId: string +): Promise => { + await agentRepository.delete(agentId); +}; + diff --git a/public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts b/public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts new file mode 100644 index 00000000..68bd0c1c --- /dev/null +++ b/public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts @@ -0,0 +1,13 @@ +/* + * Copyright Wazuh Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { ConnectorRepository } from '../ports/connector-repository'; + +export const deleteConnectorUseCase = ( + connectorRepository: ConnectorRepository +) => async (connectorId: string): Promise => { + await connectorRepository.delete(connectorId); +}; + diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-ai-assistant-step.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-ai-assistant-step.ts index 7cbac313..078400db 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-ai-assistant-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-ai-assistant-step.ts @@ -22,4 +22,9 @@ export abstract class InstallationAIAssistantStep { ): Promise; abstract getSuccessMessage(): string; abstract getFailureMessage(): string; + abstract rollback( + request: InstallAIDashboardAssistantDto, + context: InstallationContext, + error: Error + ): Promise; } diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-context.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-context.ts index 3ee1a4bd..261a9fe1 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-context.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-context.ts @@ -21,6 +21,10 @@ export class InstallationContext { return this.context.has(key); } + public delete(key: string): void { + this.context.delete(key); + } + public clear(): void { this.context.clear(); } diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts index 84db9b33..a048d1d6 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts @@ -5,7 +5,9 @@ import { InstallationProgressManager } from './installation-progress-manager'; import { InstallationAIAssistantStep } from './installation-ai-assistant-step'; -import { ExecutionState, StepResultState } from '../enums'; +import { ExecutionState } from '../enums'; +import type { InstallAIDashboardAssistantDto } from '../types/install-ai-dashboard-assistant-dto'; +import { InstallationContext } from './installation-context'; class TestStep extends InstallationAIAssistantStep { constructor( @@ -24,9 +26,24 @@ class TestStep extends InstallationAIAssistantStep { getFailureMessage(): string { return this.failureMsg; } + async rollback( + _request: InstallAIDashboardAssistantDto, + _context: InstallationContext, + _error: Error + ): Promise { + // no-op for tests + } } describe('InstallationProgressManager', () => { + const request: InstallAIDashboardAssistantDto = { + selected_provider: 'test-provider', + model_id: 'test-model', + api_url: 'https://example.org', + api_key: 'secret', + }; + const createContext = () => new InstallationContext(); + it('initializes progress with provided steps in PENDING state', () => { const steps = [new TestStep('Step 1'), new TestStep('Step 2')]; const mgr = new InstallationProgressManager(steps); @@ -42,7 +59,7 @@ describe('InstallationProgressManager', () => { const onProgressChange = jest.fn(); const mgr = new InstallationProgressManager(steps, onProgressChange); - await mgr.runStep(steps[0], async () => { + await mgr.runStep(steps[0], request, createContext(), async () => { /* success */ }); @@ -58,7 +75,7 @@ describe('InstallationProgressManager', () => { const mgr = new InstallationProgressManager(steps); await expect( - mgr.runStep(steps[0], async () => { + mgr.runStep(steps[0], request, createContext(), async () => { throw new Error('boom'); }) ).rejects.toThrow('boom'); @@ -76,9 +93,14 @@ describe('InstallationProgressManager', () => { const mgr = new InstallationProgressManager(steps); let resolveExec!: () => void; - const running = mgr.runStep(steps[0], () => new Promise((res) => (resolveExec = res))); + const running = mgr.runStep( + steps[0], + request, + createContext(), + () => new Promise((res) => (resolveExec = res)) + ); - await expect(mgr.runStep(steps[0], async () => {})).rejects.toThrow(); + await expect(mgr.runStep(steps[0], request, createContext(), async () => {})).rejects.toThrow(); resolveExec(); await running; @@ -88,13 +110,13 @@ describe('InstallationProgressManager', () => { const steps = [new TestStep('S1'), new TestStep('S2')]; const mgr = new InstallationProgressManager(steps); - await mgr.runStep(steps[0], async () => {}); - await mgr.runStep(steps[1], async () => {}); + await mgr.runStep(steps[0], request, createContext(), async () => {}); + await mgr.runStep(steps[1], request, createContext(), async () => {}); const p = mgr.getProgress(); expect(p.isFinished()).toBe(true); - await expect(mgr.runStep(steps[0], async () => {})).rejects.toThrow(); + await expect(mgr.runStep(steps[0], request, createContext(), async () => {})).rejects.toThrow(); }); it('reset returns all steps to PENDING and clears result/message/error', async () => { @@ -103,9 +125,9 @@ describe('InstallationProgressManager', () => { const mgr = new InstallationProgressManager(steps, onProgressChange); // Make one success and one failure - await mgr.runStep(steps[0], async () => {}); + await mgr.runStep(steps[0], request, createContext(), async () => {}); await expect( - mgr.runStep(steps[1], async () => { + mgr.runStep(steps[1], request, createContext(), async () => { throw new Error('x'); }) ).rejects.toThrow('x'); diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts index c4c3a3ec..b3890616 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts @@ -6,11 +6,20 @@ import { ExecutionState, StepResultState } from '../enums'; import { InstallationAIAssistantStep } from './installation-ai-assistant-step'; import { InstallationProgress } from './installation-progress'; +import type { InstallAIDashboardAssistantDto } from '../types/install-ai-dashboard-assistant-dto'; +import type { InstallationContext } from './installation-context'; + +type RollbackError = { + step: string; + message: string; +}; export class InstallationProgressManager { private readonly progress: InstallationProgress; // Prevent concurrent executions private inProgress = false; + private completedSteps: InstallationAIAssistantStep[] = []; + private rollbackErrors: RollbackError[] = []; constructor( steps: InstallationAIAssistantStep[], @@ -39,6 +48,8 @@ export class InstallationProgressManager { public async runStep( step: InstallationAIAssistantStep, + request: InstallAIDashboardAssistantDto, + context: InstallationContext, executor: () => Promise ): Promise { if (this.inProgress) { @@ -51,11 +62,14 @@ export class InstallationProgressManager { this.inProgress = true; this.progress.startStep(i); + this.rollbackErrors = []; try { await executor(); + this.completedSteps.push(step); this.succeedStep(i, step); } catch (err) { const error = err instanceof Error ? err : new Error(String(err)); + await this.rollbackSteps(step, request, context, error); this.failStep(i, step, error); throw error; } finally { @@ -63,6 +77,10 @@ export class InstallationProgressManager { } } + public getRollbackErrors(): RollbackError[] | undefined { + return this.rollbackErrors.length > 0 ? [...this.rollbackErrors] : undefined; + } + private succeedStep(stepIndex: number, step: InstallationAIAssistantStep): void { this.progress.completeStep(stepIndex, StepResultState.SUCCESS, step.getSuccessMessage()); } @@ -76,4 +94,34 @@ export class InstallationProgressManager { this.onProgressChange(this.getProgress()); } } + + private async rollbackSteps( + failedStep: InstallationAIAssistantStep, + request: InstallAIDashboardAssistantDto, + context: InstallationContext, + failure: Error + ): Promise { + const stepsToRollback = [...this.completedSteps].reverse(); + + await this.invokeRollback(failedStep, request, context, failure); + for (const step of stepsToRollback) { + await this.invokeRollback(step, request, context, failure); + } + + this.completedSteps = []; + } + + private async invokeRollback( + step: InstallationAIAssistantStep, + request: InstallAIDashboardAssistantDto, + context: InstallationContext, + failure: Error + ): Promise { + try { + await step.rollback(request, context, failure); + } catch (error) { + const normalizedError = error instanceof Error ? error : new Error(String(error)); + this.rollbackErrors.push({ step: step.getName(), message: normalizedError.message }); + } + } } diff --git a/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts b/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts index e09545bf..2be24c48 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts @@ -18,4 +18,8 @@ export interface InstallationResult { [key: string]: any; }; errors?: InstallationError[]; + rollbackErrors?: Array<{ + step: string; + message: string; + }>; } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.int.test.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.int.test.ts index ac5382cb..f1c9d81f 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.int.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.int.test.ts @@ -38,6 +38,9 @@ describe('InstallationManager (integration)', () => { validateModelConnection: jest.fn().mockResolvedValue(true), createAgent: jest.fn().mockResolvedValue({ id: 'agent-1' }), useAgent: jest.fn().mockResolvedValue(undefined), + deleteConnector: jest.fn(), + deleteModel: jest.fn(), + deleteAgent: jest.fn(), }; const progressUpdates: unknown[] = []; @@ -70,6 +73,9 @@ describe('InstallationManager (integration)', () => { validateModelConnection: jest.fn(), createAgent: jest.fn(), useAgent: jest.fn(), + deleteConnector: jest.fn().mockResolvedValue(undefined), + deleteModel: jest.fn(), + deleteAgent: jest.fn(), }; const manager = new InstallationManager(); @@ -77,7 +83,87 @@ describe('InstallationManager (integration)', () => { expect(result.success).toBe(false); expect(result.errors).toBeDefined(); expect(result.errors![0]).toMatchObject({ step: 'Create Model' }); - // Data should contain only the connectorId - expect(result.data).toEqual({ connectorId: 'conn-1' }); + // Data should not contain identifiers for successfully rolled-back steps + expect(result.data).toEqual({}); + expect(result.rollbackErrors).toBeUndefined(); + expect( + ((global as unknown) as { + __mockUseCases: import('../../../services/__mocks__').MockUseCases; + }).__mockUseCases!.deleteConnector + ).toHaveBeenCalledWith('conn-1'); + expect( + ((global as unknown) as { + __mockUseCases: import('../../../services/__mocks__').MockUseCases; + }).__mockUseCases!.deleteModel + ).not.toHaveBeenCalled(); + expect( + ((global as unknown) as { + __mockUseCases: import('../../../services/__mocks__').MockUseCases; + }).__mockUseCases!.deleteAgent + ).not.toHaveBeenCalled(); + }); + + it('rolls back created resources when a later step fails', async () => { + ((global as unknown) as { + __mockUseCases: import('../../../services/__mocks__').MockUseCases; + }).__mockUseCases = { + persistMlCommonsSettings: jest.fn().mockResolvedValue(undefined), + createConnector: jest.fn().mockResolvedValue({ id: 'conn-1' }), + createModel: jest.fn().mockResolvedValue({ id: 'model-1' }), + validateModelConnection: jest.fn().mockResolvedValue(true), + createAgent: jest.fn().mockResolvedValue({ id: 'agent-1' }), + useAgent: jest.fn().mockRejectedValue(new Error('register failed')), + deleteConnector: jest.fn().mockResolvedValue(undefined), + deleteModel: jest.fn().mockResolvedValue(undefined), + deleteAgent: jest.fn().mockResolvedValue(undefined), + }; + + const manager = new InstallationManager(); + const result = await manager.execute(request); + + expect(result.success).toBe(false); + expect(result.errors![0]).toMatchObject({ step: 'Register Agent' }); + expect(result.data).toEqual({}); + expect(result.rollbackErrors).toBeUndefined(); + + const mockUseCases = ((global as unknown) as { + __mockUseCases: import('../../../services/__mocks__').MockUseCases; + }).__mockUseCases!; + + expect(mockUseCases.deleteAgent).toHaveBeenCalledWith('agent-1'); + expect(mockUseCases.deleteModel).toHaveBeenCalledWith('model-1'); + expect(mockUseCases.deleteConnector).toHaveBeenCalledWith('conn-1'); + }); + + it('rolls back created resources when model connection validation fails', async () => { + ((global as unknown) as { + __mockUseCases: import('../../../services/__mocks__').MockUseCases; + }).__mockUseCases = { + persistMlCommonsSettings: jest.fn().mockResolvedValue(undefined), + createConnector: jest.fn().mockResolvedValue({ id: 'conn-1' }), + createModel: jest.fn().mockResolvedValue({ id: 'model-1' }), + validateModelConnection: jest.fn().mockRejectedValue(new Error('validation failed')), + createAgent: jest.fn(), + useAgent: jest.fn(), + deleteConnector: jest.fn().mockResolvedValue(undefined), + deleteModel: jest.fn().mockResolvedValue(undefined), + deleteAgent: jest.fn().mockResolvedValue(undefined), + }; + + const manager = new InstallationManager(); + const result = await manager.execute(request); + + expect(result.success).toBe(false); + expect(result.errors![0]).toMatchObject({ step: 'Test Model Connection' }); + expect(result.data).toEqual({}); + expect(result.rollbackErrors).toBeUndefined(); + + const mockUseCases = ((global as unknown) as { + __mockUseCases: import('../../../services/__mocks__').MockUseCases; + }).__mockUseCases!; + + expect(mockUseCases.deleteModel).toHaveBeenCalledWith('model-1'); + expect(mockUseCases.deleteConnector).toHaveBeenCalledWith('conn-1'); + expect(mockUseCases.deleteAgent).not.toHaveBeenCalled(); }); }); diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts index cf92f200..47ae4dea 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts @@ -36,7 +36,7 @@ export class InstallationManager implements IInstallationManager { const context = new InstallationContext(); try { for (const step of steps) { - await progressManager.runStep(step, () => step.execute(request, context)); + await progressManager.runStep(step, request, context, () => step.execute(request, context)); } return { @@ -62,6 +62,7 @@ export class InstallationManager implements IInstallationManager { message: step.message || 'Unknown error', details: step.error, })), + rollbackErrors: progressManager.getRollbackErrors(), }; } } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts index 813ab9e6..714742dc 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts @@ -94,4 +94,26 @@ export class CreateAgentStep extends InstallationAIAssistantStep { public getFailureMessage(): string { return 'Failed to create agent. Please check the configuration and try again.'; } + + public override async rollback( + _request: InstallAIDashboardAssistantDto, + context: InstallationContext, + _error: Error + ): Promise { + if (!context.has('agentId')) { + return; + } + + const agentId = context.get('agentId'); + try { + await getUseCases().deleteAgent(agentId); + context.delete('agentId'); + } catch (error) { + throw new Error( + `Failed to rollback agent creation for agentId="${agentId}": ${ + error instanceof Error ? error.message : String(error) + }` + ); + } + } } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts index 8f068505..e9432060 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts @@ -71,4 +71,26 @@ export class CreateConnectorStep extends InstallationAIAssistantStep { getFailureMessage(): string { return 'Failed to create connector. Please check the configuration and try again.'; } + + public override async rollback( + _request: InstallAIDashboardAssistantDto, + context: InstallationContext, + _error: Error + ): Promise { + if (!context.has('connectorId')) { + return; + } + + const connectorId = context.get('connectorId'); + try { + await getUseCases().deleteConnector(connectorId); + context.delete('connectorId'); + } catch (error) { + throw new Error( + `Failed to rollback connector creation for connectorId="${connectorId}": ${ + error instanceof Error ? error.message : String(error) + }` + ); + } + } } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts index a6688765..24f165b8 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts @@ -62,4 +62,26 @@ export class CreateModelStep extends InstallationAIAssistantStep { getFailureMessage(): string { return 'Failed to create model. Please check the configuration and try again.'; } + + public override async rollback( + _request: InstallAIDashboardAssistantDto, + context: InstallationContext, + _error: Error + ): Promise { + if (!context.has('modelId')) { + return; + } + + const modelId = context.get('modelId'); + try { + await getUseCases().deleteModel(modelId); + context.delete('modelId'); + } catch (error) { + throw new Error( + `Failed to rollback model creation for modelId="${modelId}": ${ + error instanceof Error ? error.message : String(error) + }` + ); + } + } } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts index c7c1c010..84824075 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts @@ -47,4 +47,12 @@ export class RegisterAgentStep extends InstallationAIAssistantStep { public getFailureMessage(): string { return 'Failed to register agent. Please check the configuration and try again.'; } + + public override async rollback( + _request: InstallAIDashboardAssistantDto, + _context: InstallationContext, + _error: Error + ): Promise { + // Registration step does not persist reversible data beyond targeting the active agent. + } } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts index c9f94aa8..88e6bca2 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts @@ -115,4 +115,12 @@ export class TestModelConnectionStep extends InstallationAIAssistantStep { public getFailureMessage(): string { return 'Failed to test model connection. Please check the model configuration and try again.'; } + + public override async rollback( + _request: InstallAIDashboardAssistantDto, + _context: InstallationContext, + _error: Error + ): Promise { + // This step only validates connectivity and does not create resources. + } } diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts index 1f66a346..3989dd96 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts @@ -58,4 +58,12 @@ export class UpdateMlCommonsSettingsStep extends InstallationAIAssistantStep { getFailureMessage(): string { return 'Failed to update ML Commons settings. Please check the configuration and try again.'; } + + public override async rollback( + _request: InstallAIDashboardAssistantDto, + _context: InstallationContext, + _error: Error + ): Promise { + // Current implementation does not persist auxiliary data to revert; nothing to rollback. + } } diff --git a/public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts b/public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts new file mode 100644 index 00000000..0e742fd5 --- /dev/null +++ b/public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts @@ -0,0 +1,13 @@ +/* + * Copyright Wazuh Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { ModelRepository } from '../ports/model-repository'; + +export const deleteModelUseCase = (modelRepository: ModelRepository) => async ( + modelId: string +): Promise => { + await modelRepository.delete(modelId); +}; + diff --git a/public/dashboard-assistant/services/__mocks__/index.ts b/public/dashboard-assistant/services/__mocks__/index.ts index f841d1fb..01a73e3c 100644 --- a/public/dashboard-assistant/services/__mocks__/index.ts +++ b/public/dashboard-assistant/services/__mocks__/index.ts @@ -6,9 +6,12 @@ export interface MockUseCases { persistMlCommonsSettings?: jest.Mock, [params: { endpoints_regex: string[] }]>; createConnector?: jest.Mock, [params: Record]>; + deleteConnector?: jest.Mock, [connectorId: string]>; createModel?: jest.Mock, [params: Record]>; + deleteModel?: jest.Mock, [modelId: string]>; validateModelConnection?: jest.Mock, [modelId: string]>; createAgent?: jest.Mock, [params: Record]>; + deleteAgent?: jest.Mock, [agentId: string]>; useAgent?: jest.Mock, [agentId: string]>; } diff --git a/public/dashboard-assistant/services/ml-use-cases.service.ts b/public/dashboard-assistant/services/ml-use-cases.service.ts index 31bf2800..c8e18e3d 100644 --- a/public/dashboard-assistant/services/ml-use-cases.service.ts +++ b/public/dashboard-assistant/services/ml-use-cases.service.ts @@ -6,15 +6,18 @@ import { createAgentUseCase } from '../modules/agent/application/use-cases/create-agent'; import { registerAgentUseCase } from '../modules/agent/application/use-cases/register-agent'; import { createConnectorUseCase } from '../modules/connector/application/use-cases/create-connector'; +import { deleteConnectorUseCase } from '../modules/connector/application/use-cases/delete-connector'; import { triggerAIAssistantInstaller } from '../modules/installation-manager/application/use-cases'; import type { InstallationProgress } from '../modules/installation-manager/domain'; import { InstallationManager } from '../modules/installation-manager/infrastructure/installation-manager'; import { persistMLCommonsSettingsUseCase } from '../modules/ml-commons-settings/application/use-cases/update-ml-commons-settings'; import { createModelUseCase } from '../modules/model/application/use-cases/create-model'; +import { deleteModelUseCase } from '../modules/model/application/use-cases/delete-model'; import { deleteModelWithRelatedEntitiesUseCase } from '../modules/model/application/use-cases/delete-model-with-related-entities'; import { getModelsUseCase } from '../modules/model/application/use-cases/get-models'; import { composeModelsWithAgentDataUseCase } from '../modules/model/application/use-cases/compose-models-with-agent-data'; import { validateModelConnectionUseCase } from '../modules/model/application/use-cases/validate-model-connection-use-case'; +import { deleteAgentUseCase } from '../modules/agent/application/use-cases/delete-agent'; import { getHttpClient, getProxyHttpClient } from './common'; import { getRepositories } from './repositories.service'; @@ -25,8 +28,11 @@ class MLUseCases { this.repos.mlCommonsSettingsRepository ); createConnector = createConnectorUseCase(this.repos.connectorRepository); + deleteConnector = deleteConnectorUseCase(this.repos.connectorRepository); createModel = createModelUseCase(this.repos.modelRepository); + deleteModel = deleteModelUseCase(this.repos.modelRepository); createAgent = createAgentUseCase(this.repos.agentRepository); + deleteAgent = deleteAgentUseCase(this.repos.agentRepository); useAgent = registerAgentUseCase(this.repos.agentRepository); getModels = getModelsUseCase(this.repos.modelRepository); getModelsWithAgentData = composeModelsWithAgentDataUseCase( From 5e2d8c33a56f91f7a2918afc8a0f24d5154499d8 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Thu, 18 Sep 2025 20:47:40 +0000 Subject: [PATCH 04/12] =?UTF-8?q?update=20error=20message=20in=20=C2=ABTes?= =?UTF-8?q?tModelConnectionStep=C2=BB=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refine the error message in the «TestModelConnectionStep» test to improve clarity and match the updated phrasing. This ensures the test provides more precise feedback when the step validation fails. --- .../infrastructure/steps/test-model-connection-step.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts index 08b442bd..e55502df 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts @@ -44,7 +44,7 @@ describe('TestModelConnectionStep', () => { const step = new TestModelConnectionStep(); const ctx = new InstallationContext(); ctx.set('modelId', 'm-1'); - await expect(step.execute(req, ctx)).rejects.toThrow(/Test Model Connection failed while validating the model connection/); + await expect(step.execute(req, ctx)).rejects.toThrow(/"Test Model Connection" step failed while validating the model connection/); expect(step.getFailureMessage()).toMatch(/Failed to test model connection/); }); From aa9d23c415289e1189688b1a5192b57d4c60947b Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Thu, 18 Sep 2025 20:54:33 +0000 Subject: [PATCH 05/12] =?UTF-8?q?remove=20handling=20of=20response=20body?= =?UTF-8?q?=20in=20=C2=ABHttpError=C2=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify the «HttpError» implementation by removing all logic related to determining and handling the presence of a response body. Eliminate the «responseBodyOmitted» property, the «hasMeaningfulBody» function, and associated references. This change reduces complexity and aligns with a security-focused approach by avoiding reliance on response body content. --- .../common/http/infrastructure/http-error.ts | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts b/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts index a7788809..5d6aa970 100644 --- a/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts +++ b/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts @@ -11,7 +11,6 @@ export class HttpError extends Error { public status?: number; public statusText?: string; public request?: { method: string; url: string; body?: string }; - public responseBodyOmitted?: boolean; private constructor(message: string) { super(message); @@ -25,7 +24,6 @@ export class HttpError extends Error { ): Promise { const method = options.method.toUpperCase(); const sanitizedUrl = this.sanitizeUrl(url); - const hasResponseBody = await this.hasMeaningfulBody(response); const statusText = response.statusText?.trim(); const statusSummary = statusText @@ -36,7 +34,6 @@ export class HttpError extends Error { method, url: sanitizedUrl, statusSummary, - hasResponseBody, }); const error = new HttpError(message); @@ -48,9 +45,6 @@ export class HttpError extends Error { url: sanitizedUrl, body: this.sanitizeRequestBody(options.body ?? undefined), }; - if (hasResponseBody) { - error.responseBodyOmitted = true; - } return error; } @@ -59,15 +53,11 @@ export class HttpError extends Error { method: string; url: string; statusSummary: string; - hasResponseBody: boolean; }): string { - const { method, url, statusSummary, hasResponseBody } = params; + const { method, url, statusSummary } = params; const messageParts = [ `HTTP ${method} ${url} failed with status ${statusSummary}`, - hasResponseBody - ? 'Response body omitted due to security policy' - : undefined, ].filter(Boolean); return messageParts.join('. '); @@ -98,15 +88,6 @@ export class HttpError extends Error { } } - private static async hasMeaningfulBody(response: Response): Promise { - try { - const body = await response.clone().text(); - return Boolean(body.trim()); - } catch (error) { - return false; - } - } - private static sanitizeStructuredValue(value: unknown): unknown { if (value == null) { return value; From a08736631e7335355b1e921c9254a87f67cf73fb Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Thu, 18 Sep 2025 20:58:04 +0000 Subject: [PATCH 06/12] update tests to validate additional error details MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhance the test for «WindowFetchHttpClient» by adding assertions for «status» and «statusText» in propagated HTTP errors, ensuring more comprehensive error validation. Simplify the failure message validation in «InstallationProgressManager» tests by removing redundant details, improving clarity and focus on the core error message. --- .../http/infrastructure/window-fetch-http-client.test.ts | 5 +++-- .../domain/entities/installation-progress-manager.test.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts index 3d104629..692d5bd8 100644 --- a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts +++ b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.test.ts @@ -102,7 +102,7 @@ describe('WindowFetchHttpClient', () => { }); it('propagates HTTP errors with contextual information attached', async () => { - expect.assertions(5); + expect.assertions(6); const errorPayload = { message: 'boom' }; const mockResponse: JsonResponse = { ok: false, @@ -126,9 +126,10 @@ describe('WindowFetchHttpClient', () => { expect((error as Error).message).toContain( 'HTTP DELETE /x failed with status 500 Internal Server Error' ); - expect((error as Error).message).toContain('Response body: {"message":"boom"}'); expect((error as { response?: JsonResponse }).response).toBe(mockResponse); expect((error as { request?: { method: string } }).request?.method).toBe('DELETE'); + expect((error as { status?: number }).status).toBe(500); + expect((error as { statusText?: string }).statusText).toBe('Internal Server Error'); } }); }); diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts index a048d1d6..0dc88ddf 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts @@ -82,7 +82,7 @@ describe('InstallationProgressManager', () => { const p = mgr.getProgress(); expect(p.getSteps()[0].state).toBe(ExecutionState.FAILED); - expect(p.getSteps()[0].message).toBe('boom-msg Additional details: boom'); + expect(p.getSteps()[0].message).toBe('boom-msg'); expect(p.getSteps()[0].error).toBeInstanceOf(Error); expect(p.hasFailedSteps()).toBe(true); expect(p.getFailedSteps().length).toBe(1); From 20a9f5d1f5a7ed1813e6939ac790be5633564984 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Thu, 18 Sep 2025 21:27:53 +0000 Subject: [PATCH 07/12] refactor formatting and interface definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify code structure by normalizing formatting for better readability across files. Replace «RollbackError» type with an «interface» for consistency. Update multiline constructs to align with style guidelines. These changes improve code maintainability and adhere to standardized practices. --- .../application/ports/agent-repository.ts | 4 +- .../application/use-cases/delete-agent.ts | 1 - .../common/http/infrastructure/http-error.ts | 41 ++++++++----------- .../window-fetch-http-client.ts | 6 ++- .../application/use-cases/delete-connector.ts | 7 ++-- .../entities/installation-progress-manager.ts | 4 +- .../infrastructure/installation-manager.ts | 4 +- .../steps/test-model-connection-step.test.ts | 4 +- .../infrastructure/utils/step-error.ts | 8 ++-- .../application/use-cases/delete-model.ts | 1 - 10 files changed, 38 insertions(+), 42 deletions(-) diff --git a/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts b/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts index 72769da9..0ff77c95 100644 --- a/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts +++ b/public/dashboard-assistant/modules/agent/application/ports/agent-repository.ts @@ -7,9 +7,7 @@ import { CreateRepository, DeleteRepository } from '../../../common/domain/entit import { Agent } from '../../domain/entities/agent'; import { CreateAgentDto } from '../dtos/create-agent-dto'; -export interface AgentRepository - extends CreateRepository, - DeleteRepository { +export interface AgentRepository extends CreateRepository, DeleteRepository { execute(id: string, parameters: any): Promise; getActive(): Promise; register(agentId: string): Promise; diff --git a/public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts b/public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts index 1270fb13..82545b9f 100644 --- a/public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts +++ b/public/dashboard-assistant/modules/agent/application/use-cases/delete-agent.ts @@ -10,4 +10,3 @@ export const deleteAgentUseCase = (agentRepository: AgentRepository) => async ( ): Promise => { await agentRepository.delete(agentId); }; - diff --git a/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts b/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts index 5d6aa970..be75526a 100644 --- a/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts +++ b/public/dashboard-assistant/modules/common/http/infrastructure/http-error.ts @@ -20,15 +20,13 @@ export class HttpError extends Error { public static async create( response: Response, options: RequestInit & { method: string }, - url: string, + url: string ): Promise { const method = options.method.toUpperCase(); const sanitizedUrl = this.sanitizeUrl(url); const statusText = response.statusText?.trim(); - const statusSummary = statusText - ? `${response.status} ${statusText}` - : `${response.status}`; + const statusSummary = statusText ? `${response.status} ${statusText}` : `${response.status}`; const message = this.buildFailureMessage({ method, @@ -56,9 +54,9 @@ export class HttpError extends Error { }): string { const { method, url, statusSummary } = params; - const messageParts = [ - `HTTP ${method} ${url} failed with status ${statusSummary}`, - ].filter(Boolean); + const messageParts = [`HTTP ${method} ${url} failed with status ${statusSummary}`].filter( + Boolean + ); return messageParts.join('. '); } @@ -77,9 +75,7 @@ export class HttpError extends Error { }); if (!url.startsWith('http')) { - return ( - `${parsed.pathname}${parsed.search}${parsed.hash}` || parsed.pathname - ); + return `${parsed.pathname}${parsed.search}${parsed.hash}` || parsed.pathname; } return parsed.toString(); @@ -94,26 +90,25 @@ export class HttpError extends Error { } if (Array.isArray(value)) { - return value.map(item => this.sanitizeStructuredValue(item)); + return value.map((item) => this.sanitizeStructuredValue(item)); } if (typeof value === 'object') { - return Object.entries(value as Record).reduce< - Record - >((acc, [key, entryValue]) => { - if (typeof entryValue !== 'string' || !SENSITIVE_KEY_PATTERN.test(key)) { + return Object.entries(value as Record).reduce>( + (acc, [key, entryValue]) => { + if (typeof entryValue !== 'string' || !SENSITIVE_KEY_PATTERN.test(key)) { acc[key] = this.sanitizeStructuredValue(entryValue); - } - return acc; - }, {}); + } + return acc; + }, + {} + ); } return value; } - private static sanitizeRequestBody( - body?: BodyInit | null, - ): string | undefined { + private static sanitizeRequestBody(body?: BodyInit | null): string | undefined { if (!body) { return undefined; } @@ -124,9 +119,7 @@ export class HttpError extends Error { const sanitized = this.sanitizeStructuredValue(parsed); return JSON.stringify(sanitized); } catch (error) { - return body.length > MAX_BODY_LENGTH - ? `${body.slice(0, MAX_BODY_LENGTH)}...` - : body; + return body.length > MAX_BODY_LENGTH ? `${body.slice(0, MAX_BODY_LENGTH)}...` : body; } } diff --git a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts index 50b7eff6..46c75e4a 100644 --- a/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts +++ b/public/dashboard-assistant/modules/common/http/infrastructure/window-fetch-http-client.ts @@ -29,7 +29,11 @@ export class WindowFetchHttpClient implements HttpClient { return window.fetch(url, options).then(async (response) => { if (!response.ok) { - const error = await HttpError.create(response, options as RequestInit & { method: string }, url); + const error = await HttpError.create( + response, + options as RequestInit & { method: string }, + url + ); throw error; } return response.json(); diff --git a/public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts b/public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts index 68bd0c1c..7d6d1219 100644 --- a/public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts +++ b/public/dashboard-assistant/modules/connector/application/use-cases/delete-connector.ts @@ -5,9 +5,8 @@ import type { ConnectorRepository } from '../ports/connector-repository'; -export const deleteConnectorUseCase = ( - connectorRepository: ConnectorRepository -) => async (connectorId: string): Promise => { +export const deleteConnectorUseCase = (connectorRepository: ConnectorRepository) => async ( + connectorId: string +): Promise => { await connectorRepository.delete(connectorId); }; - diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts index b3890616..f860d56c 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts @@ -9,10 +9,10 @@ import { InstallationProgress } from './installation-progress'; import type { InstallAIDashboardAssistantDto } from '../types/install-ai-dashboard-assistant-dto'; import type { InstallationContext } from './installation-context'; -type RollbackError = { +interface RollbackError { step: string; message: string; -}; +} export class InstallationProgressManager { private readonly progress: InstallationProgress; diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts index 47ae4dea..e08aab27 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts @@ -49,7 +49,9 @@ export class InstallationManager implements IInstallationManager { const progress = progressManager.getProgress(); const failedSteps = progress.getFailedSteps(); const firstFailedStep = failedSteps[0]; - const stepContext = firstFailedStep ? `during step "${firstFailedStep.stepName}"` : 'at an unknown step'; + const stepContext = firstFailedStep + ? `during step "${firstFailedStep.stepName}"` + : 'at an unknown step'; const errorMessage = error instanceof Error ? error.message : String(error); return { diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts index e55502df..b3f4e5e5 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.test.ts @@ -44,7 +44,9 @@ describe('TestModelConnectionStep', () => { const step = new TestModelConnectionStep(); const ctx = new InstallationContext(); ctx.set('modelId', 'm-1'); - await expect(step.execute(req, ctx)).rejects.toThrow(/"Test Model Connection" step failed while validating the model connection/); + await expect(step.execute(req, ctx)).rejects.toThrow( + /"Test Model Connection" step failed while validating the model connection/ + ); expect(step.getFailureMessage()).toMatch(/Failed to test model connection/); }); diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts index 266aa9d5..4d9a8621 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/utils/step-error.ts @@ -97,7 +97,9 @@ export class StepError extends Error { } } - private static sanitizeDetails(details?: Record): Record | undefined { + private static sanitizeDetails( + details?: Record + ): Record | undefined { if (!details) { return undefined; } @@ -123,9 +125,7 @@ export class StepError extends Error { return undefined; } - const trimmed = possibleCauses - .map((cause) => cause.trim()) - .filter((cause) => cause.length > 0); + const trimmed = possibleCauses.map((cause) => cause.trim()).filter((cause) => cause.length > 0); if (trimmed.length === 0) { return undefined; diff --git a/public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts b/public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts index 0e742fd5..f57e180b 100644 --- a/public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts +++ b/public/dashboard-assistant/modules/model/application/use-cases/delete-model.ts @@ -10,4 +10,3 @@ export const deleteModelUseCase = (modelRepository: ModelRepository) => async ( ): Promise => { await modelRepository.delete(modelId); }; - From b3e833da62e8334587140613baaf70c7d6e42046 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Fri, 19 Sep 2025 18:31:23 +0000 Subject: [PATCH 08/12] add rollback handling and error collection in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce an optional «onRollback» callback to the «TestStep» class, enabling custom rollback logic during test execution. Update the «rollback» method to invoke this callback if provided. Add a new test case to verify proper rollback execution order for failed and completed steps. Ensure rollback errors are collected and validated. Enhances test coverage and ensures consistent behavior during rollback scenarios. --- .../installation-progress-manager.test.ts | 52 +++++++++++++++++-- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts index 0dc88ddf..1fbfe541 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts @@ -13,7 +13,12 @@ class TestStep extends InstallationAIAssistantStep { constructor( name: string, private readonly successMsg: string = 'ok', - private readonly failureMsg: string = 'fail' + private readonly failureMsg: string = 'fail', + private readonly onRollback?: ( + request: InstallAIDashboardAssistantDto, + context: InstallationContext, + error: Error + ) => Promise | void ) { super({ name }); } @@ -27,11 +32,13 @@ class TestStep extends InstallationAIAssistantStep { return this.failureMsg; } async rollback( - _request: InstallAIDashboardAssistantDto, - _context: InstallationContext, - _error: Error + request: InstallAIDashboardAssistantDto, + context: InstallationContext, + error: Error ): Promise { - // no-op for tests + if (this.onRollback) { + await this.onRollback(request, context, error); + } } } @@ -88,6 +95,41 @@ describe('InstallationProgressManager', () => { expect(p.getFailedSteps().length).toBe(1); }); + it('invokes rollbacks for failed and completed steps and collects rollback errors', async () => { + const rollbackOrder: string[] = []; + const steps = [ + new TestStep('S1', 'ok', 'fail', async () => { + rollbackOrder.push('S1'); + }), + new TestStep('S2', 'ok', 'fail', async () => { + rollbackOrder.push('S2'); + throw new Error('S2 rollback failed'); + }), + new TestStep('S3', 'ok', 'boom', async () => { + rollbackOrder.push('S3'); + }), + ]; + const mgr = new InstallationProgressManager(steps); + + await mgr.runStep(steps[0], request, createContext(), async () => { + /* success */ + }); + await mgr.runStep(steps[1], request, createContext(), async () => { + /* success */ + }); + + await expect( + mgr.runStep(steps[2], request, createContext(), async () => { + throw new Error('boom'); + }) + ).rejects.toThrow('boom'); + + expect(rollbackOrder).toEqual(['S3', 'S2', 'S1']); + expect(mgr.getRollbackErrors()).toEqual([ + { step: 'S2', message: 'S2 rollback failed' }, + ]); + }); + it('prevents concurrent step execution', async () => { const steps = [new TestStep('S1')]; const mgr = new InstallationProgressManager(steps); From f617af54335df4db21930d6eb4d379bde2031715 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Fri, 19 Sep 2025 18:46:37 +0000 Subject: [PATCH 09/12] =?UTF-8?q?add=20unit=20test=20for=20=C2=ABModelRegi?= =?UTF-8?q?ster=C2=BB=20component?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a unit test to verify the behavior of the «ModelRegister» component during installation failure. Mock dependencies like «ModelForm» and «useAssistantInstallation» to simulate scenarios, ensuring proper handling of error states and absence of success messages. --- .../__tests__/model-register.test.tsx | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 public/dashboard-assistant/components/__tests__/model-register.test.tsx diff --git a/public/dashboard-assistant/components/__tests__/model-register.test.tsx b/public/dashboard-assistant/components/__tests__/model-register.test.tsx new file mode 100644 index 00000000..22172da3 --- /dev/null +++ b/public/dashboard-assistant/components/__tests__/model-register.test.tsx @@ -0,0 +1,101 @@ +/* + * Copyright Wazuh Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import userEvent from '@testing-library/user-event'; +import { render, screen, waitFor } from '../../../../test/test_utils'; + +jest.mock('../model-form', () => { + const React = require('react'); + + const MockModelForm = ({ onChange, onValidationChange }: any) => { + React.useEffect(() => { + onChange?.({ + modelProvider: 'OpenAI', + model: 'gpt-4o', + apiUrl: 'https://api.openai.com/v1/chat/completions', + apiKey: 'mock-key', + }); + onValidationChange?.(true); + }, [onChange, onValidationChange]); + + return
; + }; + + return { + __esModule: true, + ModelForm: MockModelForm, + }; +}); + +jest.mock('../../modules/installation-manager/hooks/use-assistant-installation', () => { + const React = require('react'); + const { ExecutionState } = require('../../modules/installation-manager/domain'); + + const failedStep = { + stepName: 'Create Model', + state: ExecutionState.FAILED, + error: new Error('Model step failed'), + }; + + const progress = { + getSteps: () => [ + { + stepName: 'Persist ML Commons settings', + state: ExecutionState.FINISHED_SUCCESSFULLY, + }, + failedStep, + ], + getFailedSteps: () => [failedStep], + isFinished: () => false, + isFinishedSuccessfully: () => false, + isFinishedWithWarnings: () => false, + hasFailed: () => true, + }; + + return { + __esModule: true, + useAssistantInstallation: () => { + const [error, setError] = React.useState(undefined); + + const install = React.useCallback(async () => { + setError('Steps: "Create Model" has failed'); + }, []); + + return { + install, + setModel: jest.fn(), + reset: jest.fn(), + isLoading: false, + error, + result: { success: false }, + modelData: undefined, + progress, + isSuccess: false, + }; + }, + }; +}); + +import { ModelRegister } from '../model-register'; + +describe('ModelRegister', () => { + it('does not show success toast when installation fails', async () => { + const user = userEvent.setup(); + + render(); + + const deployButton = await screen.findByRole('button', { name: /Deploy/i }); + expect(deployButton).toBeEnabled(); + + await user.click(deployButton); + + await screen.findByText(/Error deploying model/i); + + await waitFor(() => { + expect(screen.queryByText('Model deployed successfully.')).not.toBeInTheDocument(); + }); + }); +}); From 8228704dd1865ab0616d288b0e91e34e9865b3b3 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Fri, 19 Sep 2025 19:05:26 +0000 Subject: [PATCH 10/12] add rollback handling and summary display for installation errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce rollback tracking in the installation process to record reverted steps during failures. Update the response structure to include a «rollbacks» field and propagate this data through the installation workflow. Enhance the UI to display rollback summaries using «addInfoToast» when installation errors occur. Modify tests to verify rollback behavior and ensure accurate reporting of reverted steps. Improve error handling and state management by adding a reference to track the last rollback summary and avoid duplicate toasts. --- .../__tests__/model-register.test.tsx | 7 +++++- .../components/model-register.tsx | 20 +++++++++++++--- .../install-dashboard-assistant.test.ts | 23 ++++++++++++++++++- .../trigger-ai-assistant-installer.ts | 13 +++++++++-- ...stall-dashboard-assistant-response.test.ts | 13 +++++++++++ .../install-dashboard-assistant-response.ts | 8 ++++++- .../entities/installation-progress-manager.ts | 11 +++++++++ .../domain/types/installation-result.ts | 1 + .../hooks/use-assistant-installation.ts | 3 ++- .../infrastructure/installation-manager.ts | 1 + 10 files changed, 91 insertions(+), 9 deletions(-) diff --git a/public/dashboard-assistant/components/__tests__/model-register.test.tsx b/public/dashboard-assistant/components/__tests__/model-register.test.tsx index 22172da3..0a14a0ba 100644 --- a/public/dashboard-assistant/components/__tests__/model-register.test.tsx +++ b/public/dashboard-assistant/components/__tests__/model-register.test.tsx @@ -70,7 +70,7 @@ jest.mock('../../modules/installation-manager/hooks/use-assistant-installation', reset: jest.fn(), isLoading: false, error, - result: { success: false }, + result: { success: false, rollbacks: ['Create Agent', 'Create Connector'] }, modelData: undefined, progress, isSuccess: false, @@ -97,5 +97,10 @@ describe('ModelRegister', () => { await waitFor(() => { expect(screen.queryByText('Model deployed successfully.')).not.toBeInTheDocument(); }); + + expect(await screen.findByText(/Rollback summary/i)).toBeInTheDocument(); + expect( + await screen.findByText(/Reverted steps: Create Agent, Create Connector/i) + ).toBeInTheDocument(); }); }); diff --git a/public/dashboard-assistant/components/model-register.tsx b/public/dashboard-assistant/components/model-register.tsx index e1464786..b4300927 100644 --- a/public/dashboard-assistant/components/model-register.tsx +++ b/public/dashboard-assistant/components/model-register.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useState, useCallback, useEffect } from 'react'; +import React, { useState, useCallback, useEffect, useRef } from 'react'; import { EuiButton, EuiButtonEmpty, @@ -50,7 +50,7 @@ const ModelRegisterComponent = ({ onDeployed, }: ModelRegisterProps) => { const [isDeployed, setIsDeployed] = useState(false); - const { addSuccessToast, addErrorToast } = useToast(); + const { addSuccessToast, addErrorToast, addInfoToast } = useToast(); const { install: startInstallationProcess, setModel, @@ -58,7 +58,9 @@ const ModelRegisterComponent = ({ error: installationError, progress: installationProgress, isSuccess: isInstallationSuccessful, + result: installationResult, } = useAssistantInstallation(); + const lastRollbackKeyRef = useRef(null); useEffect(() => { if (installationError) { @@ -67,7 +69,19 @@ const ModelRegisterComponent = ({ `${installationError}. Rolling back current installation. Please, verify data provided and try again.` ); } - }, [addErrorToast, addSuccessToast, installationError]); + }, [addErrorToast, installationError]); + + useEffect(() => { + if (installationError && installationResult?.rollbacks?.length) { + const rollbackSummary = installationResult.rollbacks.join(', '); + if (lastRollbackKeyRef.current !== rollbackSummary) { + addInfoToast('Rollback summary', `Reverted steps: ${rollbackSummary}`); + lastRollbackKeyRef.current = rollbackSummary; + } + } else if (!installationError) { + lastRollbackKeyRef.current = null; + } + }, [addInfoToast, installationError, installationResult]); useEffect(() => { if (isInstallationSuccessful) { diff --git a/public/dashboard-assistant/modules/installation-manager/application/use-cases/install-dashboard-assistant.test.ts b/public/dashboard-assistant/modules/installation-manager/application/use-cases/install-dashboard-assistant.test.ts index abf83cbb..b0812062 100644 --- a/public/dashboard-assistant/modules/installation-manager/application/use-cases/install-dashboard-assistant.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/application/use-cases/install-dashboard-assistant.test.ts @@ -8,7 +8,11 @@ import { triggerAIAssistantInstaller } from './trigger-ai-assistant-installer'; describe('installDashboardAssistantUseCase', () => { it('returns success with data when orchestrator resolves', async () => { const data = { agentId: 'agent-1', modelId: 'model-1' }; - const execute = jest.fn().mockResolvedValue({ data }); + const execute = jest.fn().mockResolvedValue({ + success: true, + message: 'Dashboard assistant installed successfully', + data, + }); const orchestrator = { execute } as any; const useCase = triggerAIAssistantInstaller(orchestrator); @@ -23,6 +27,23 @@ describe('installDashboardAssistantUseCase', () => { expect(res.data).toEqual(data); }); + it('returns failure payload with rollbacks when orchestrator reports failure', async () => { + const execute = jest.fn().mockResolvedValue({ + success: false, + message: 'Installation failed during step "Create Model"', + data: {}, + rollbacks: [{ step: 'Create Connector' }, { step: 'Persist ML Commons settings' }], + }); + const orchestrator = { execute } as any; + + const useCase = triggerAIAssistantInstaller(orchestrator); + const res = await useCase({} as any); + + expect(res.success).toBe(false); + expect(res.message).toContain('Create Model'); + expect(res.rollbacks).toEqual(['Create Connector', 'Persist ML Commons settings']); + }); + it('returns failure with error message when orchestrator throws Error', async () => { const execute = jest.fn().mockRejectedValue(new Error('boom')); const orchestrator = { execute } as any; diff --git a/public/dashboard-assistant/modules/installation-manager/application/use-cases/trigger-ai-assistant-installer.ts b/public/dashboard-assistant/modules/installation-manager/application/use-cases/trigger-ai-assistant-installer.ts index bcdc0e9d..a8a64049 100644 --- a/public/dashboard-assistant/modules/installation-manager/application/use-cases/trigger-ai-assistant-installer.ts +++ b/public/dashboard-assistant/modules/installation-manager/application/use-cases/trigger-ai-assistant-installer.ts @@ -17,10 +17,19 @@ export const triggerAIAssistantInstaller = ( try { const result = await installationOrchestrator.execute(request); + if (result.success) { + return { + success: true, + message: result.message, + data: result.data, + }; + } + return { - success: true, - message: 'Dashboard assistant installed successfully', + success: false, + message: result.message, data: result.data, + rollbacks: result.rollbacks?.map((rollback) => rollback.step), }; } catch (error) { return { diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts index c0b643c9..743ebab8 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts @@ -16,6 +16,7 @@ describe('InstallDashboardAssistantResponse', () => { expect(res.progress).toBe(progress); expect(res.data?.agentId).toBe('agent-123'); expect(res.error).toBeUndefined(); + expect(res.rollbacks).toBeUndefined(); }); it('failure() should create a failed response with error and progress', () => { @@ -28,6 +29,18 @@ describe('InstallDashboardAssistantResponse', () => { expect(res.progress).toBe(progress); expect(res.error).toBe(error); expect(res.data?.agentId).toBeUndefined(); + expect(res.rollbacks).toBeUndefined(); + }); + + it('failure() should accept rollbacks to describe reverted steps', () => { + const progress = new InstallationProgress(); + const res = InstallDashboardAssistantResponse.failure( + 'Failed with rollbacks', + progress, + ['Create Connector', 'Persist ML Commons settings'] + ); + + expect(res.rollbacks).toEqual(['Create Connector', 'Persist ML Commons settings']); }); it('inProgress() should create an in-progress response with progress', () => { diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.ts index d22d7168..fb49559f 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.ts @@ -14,6 +14,7 @@ export interface IInstallDashboardAssistantResponse { modelId?: string; agentId?: string; }; + rollbacks?: string[]; } export class InstallDashboardAssistantResponse implements IInstallDashboardAssistantResponse { @@ -27,6 +28,7 @@ export class InstallDashboardAssistantResponse implements IInstallDashboardAssis modelId?: string; }; public error?: string; + public rollbacks?: string[]; private constructor(params: { success: boolean; @@ -34,6 +36,7 @@ export class InstallDashboardAssistantResponse implements IInstallDashboardAssis progress: InstallationProgress; agentId?: string; error?: string; + rollbacks?: string[]; }) { this.success = params.success; this.message = params.message; @@ -42,6 +45,7 @@ export class InstallDashboardAssistantResponse implements IInstallDashboardAssis agentId: params.agentId, }; this.error = params.error; + this.rollbacks = params.rollbacks; } public static success( @@ -58,13 +62,15 @@ export class InstallDashboardAssistantResponse implements IInstallDashboardAssis public static failure( error: string, - progress: InstallationProgress + progress: InstallationProgress, + rollbacks?: string[] ): InstallDashboardAssistantResponse { return new InstallDashboardAssistantResponse({ success: false, message: error, progress, error, + rollbacks, }); } diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts index f860d56c..b11a11cd 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.ts @@ -14,12 +14,17 @@ interface RollbackError { message: string; } +interface RollbackAction { + step: string; +} + export class InstallationProgressManager { private readonly progress: InstallationProgress; // Prevent concurrent executions private inProgress = false; private completedSteps: InstallationAIAssistantStep[] = []; private rollbackErrors: RollbackError[] = []; + private rollbacks: RollbackAction[] = []; constructor( steps: InstallationAIAssistantStep[], @@ -63,6 +68,7 @@ export class InstallationProgressManager { this.inProgress = true; this.progress.startStep(i); this.rollbackErrors = []; + this.rollbacks = []; try { await executor(); this.completedSteps.push(step); @@ -81,6 +87,10 @@ export class InstallationProgressManager { return this.rollbackErrors.length > 0 ? [...this.rollbackErrors] : undefined; } + public getRollbacks(): RollbackAction[] | undefined { + return this.rollbacks.length > 0 ? [...this.rollbacks] : undefined; + } + private succeedStep(stepIndex: number, step: InstallationAIAssistantStep): void { this.progress.completeStep(stepIndex, StepResultState.SUCCESS, step.getSuccessMessage()); } @@ -119,6 +129,7 @@ export class InstallationProgressManager { ): Promise { try { await step.rollback(request, context, failure); + this.rollbacks.push({ step: step.getName() }); } catch (error) { const normalizedError = error instanceof Error ? error : new Error(String(error)); this.rollbackErrors.push({ step: step.getName(), message: normalizedError.message }); diff --git a/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts b/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts index 2be24c48..058538fa 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/types/installation-result.ts @@ -18,6 +18,7 @@ export interface InstallationResult { [key: string]: any; }; errors?: InstallationError[]; + rollbacks?: Array<{ step: string }>; rollbackErrors?: Array<{ step: string; message: string; diff --git a/public/dashboard-assistant/modules/installation-manager/hooks/use-assistant-installation.ts b/public/dashboard-assistant/modules/installation-manager/hooks/use-assistant-installation.ts index c8637854..41adf854 100644 --- a/public/dashboard-assistant/modules/installation-manager/hooks/use-assistant-installation.ts +++ b/public/dashboard-assistant/modules/installation-manager/hooks/use-assistant-installation.ts @@ -63,7 +63,8 @@ export function useAssistantInstallation() { return InstallDashboardAssistantResponse.failure( response.message ?? 'Installation failed', - lastProgress + lastProgress, + response.rollbacks ); } catch (err) { const errorMessage = err instanceof Error ? err.message : 'Unknown error occurred'; diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts index e08aab27..1269d78d 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/installation-manager.ts @@ -64,6 +64,7 @@ export class InstallationManager implements IInstallationManager { message: step.message || 'Unknown error', details: step.error, })), + rollbacks: progressManager.getRollbacks(), rollbackErrors: progressManager.getRollbackErrors(), }; } From 70e7ed1e83ec10b9b219ae90871bdc5cc1bea049 Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Fri, 19 Sep 2025 19:45:01 +0000 Subject: [PATCH 11/12] refactor test imports and rollback methods for consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Standardize imports across test files by replacing inline React imports with named imports. Simplify code structure for readability by reformatting multiline arrays and conditionals. Remove the «override» keyword from rollback methods in multiple classes to align with TypeScript conventions and ensure compatibility. Improve readability in «derivePossibleCauses» by reformatting conditional checks for network issues. These changes enhance code consistency, maintainability, and readability. --- .../components/__tests__/model-register.test.tsx | 14 +++++--------- .../install-dashboard-assistant-response.test.ts | 9 ++++----- .../entities/installation-progress-manager.test.ts | 4 +--- .../infrastructure/steps/create-agent-step.ts | 2 +- .../infrastructure/steps/create-connector-step.ts | 2 +- .../infrastructure/steps/create-model-step.ts | 2 +- .../infrastructure/steps/register-agent-step.ts | 2 +- .../steps/test-model-connection-step.ts | 9 ++++++--- .../steps/update-ml-commons-settings-step.ts | 2 +- 9 files changed, 21 insertions(+), 25 deletions(-) diff --git a/public/dashboard-assistant/components/__tests__/model-register.test.tsx b/public/dashboard-assistant/components/__tests__/model-register.test.tsx index 0a14a0ba..1b242445 100644 --- a/public/dashboard-assistant/components/__tests__/model-register.test.tsx +++ b/public/dashboard-assistant/components/__tests__/model-register.test.tsx @@ -3,15 +3,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React from 'react'; +import React, { useState, useCallback, useEffect } from 'react'; import userEvent from '@testing-library/user-event'; import { render, screen, waitFor } from '../../../../test/test_utils'; +import { ExecutionState } from '../../modules/installation-manager/domain'; jest.mock('../model-form', () => { - const React = require('react'); - const MockModelForm = ({ onChange, onValidationChange }: any) => { - React.useEffect(() => { + useEffect(() => { onChange?.({ modelProvider: 'OpenAI', model: 'gpt-4o', @@ -31,9 +30,6 @@ jest.mock('../model-form', () => { }); jest.mock('../../modules/installation-manager/hooks/use-assistant-installation', () => { - const React = require('react'); - const { ExecutionState } = require('../../modules/installation-manager/domain'); - const failedStep = { stepName: 'Create Model', state: ExecutionState.FAILED, @@ -58,9 +54,9 @@ jest.mock('../../modules/installation-manager/hooks/use-assistant-installation', return { __esModule: true, useAssistantInstallation: () => { - const [error, setError] = React.useState(undefined); + const [error, setError] = useState(undefined); - const install = React.useCallback(async () => { + const install = useCallback(async () => { setError('Steps: "Create Model" has failed'); }, []); diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts index 743ebab8..d7e8b38b 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/install-dashboard-assistant-response.test.ts @@ -34,11 +34,10 @@ describe('InstallDashboardAssistantResponse', () => { it('failure() should accept rollbacks to describe reverted steps', () => { const progress = new InstallationProgress(); - const res = InstallDashboardAssistantResponse.failure( - 'Failed with rollbacks', - progress, - ['Create Connector', 'Persist ML Commons settings'] - ); + const res = InstallDashboardAssistantResponse.failure('Failed with rollbacks', progress, [ + 'Create Connector', + 'Persist ML Commons settings', + ]); expect(res.rollbacks).toEqual(['Create Connector', 'Persist ML Commons settings']); }); diff --git a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts index 1fbfe541..c99d1073 100644 --- a/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts +++ b/public/dashboard-assistant/modules/installation-manager/domain/entities/installation-progress-manager.test.ts @@ -125,9 +125,7 @@ describe('InstallationProgressManager', () => { ).rejects.toThrow('boom'); expect(rollbackOrder).toEqual(['S3', 'S2', 'S1']); - expect(mgr.getRollbackErrors()).toEqual([ - { step: 'S2', message: 'S2 rollback failed' }, - ]); + expect(mgr.getRollbackErrors()).toEqual([{ step: 'S2', message: 'S2 rollback failed' }]); }); it('prevents concurrent step execution', async () => { diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts index 714742dc..4f60711f 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-agent-step.ts @@ -95,7 +95,7 @@ export class CreateAgentStep extends InstallationAIAssistantStep { return 'Failed to create agent. Please check the configuration and try again.'; } - public override async rollback( + public async rollback( _request: InstallAIDashboardAssistantDto, context: InstallationContext, _error: Error diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts index e9432060..9b3910e0 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-connector-step.ts @@ -72,7 +72,7 @@ export class CreateConnectorStep extends InstallationAIAssistantStep { return 'Failed to create connector. Please check the configuration and try again.'; } - public override async rollback( + public async rollback( _request: InstallAIDashboardAssistantDto, context: InstallationContext, _error: Error diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts index 24f165b8..b5dcf96d 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/create-model-step.ts @@ -63,7 +63,7 @@ export class CreateModelStep extends InstallationAIAssistantStep { return 'Failed to create model. Please check the configuration and try again.'; } - public override async rollback( + public async rollback( _request: InstallAIDashboardAssistantDto, context: InstallationContext, _error: Error diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts index 84824075..6eaf4c79 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/register-agent-step.ts @@ -48,7 +48,7 @@ export class RegisterAgentStep extends InstallationAIAssistantStep { return 'Failed to register agent. Please check the configuration and try again.'; } - public override async rollback( + public async rollback( _request: InstallAIDashboardAssistantDto, _context: InstallationContext, _error: Error diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts index 88e6bca2..728ab902 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/test-model-connection-step.ts @@ -48,7 +48,8 @@ const extractStatusCode = (error: unknown): number | undefined => { const derivePossibleCauses = (error: unknown, modelId?: string): string[] => { const causes = new Set(); const status = extractStatusCode(error); - const message = error instanceof Error ? error.message.toLowerCase() : String(error).toLowerCase(); + const message = + error instanceof Error ? error.message.toLowerCase() : String(error).toLowerCase(); if (status === 401 || status === 403) { causes.add('Verify the API key; it may be incorrect or lack permissions.'); @@ -64,7 +65,9 @@ const derivePossibleCauses = (error: unknown, modelId?: string): string[] => { const indicatesNetworkIssue = !status && - (error instanceof TypeError || message.includes('failed to fetch') || message.includes('network')); + (error instanceof TypeError || + message.includes('failed to fetch') || + message.includes('network')); if (indicatesNetworkIssue) { causes.add('Check the API URL for typos and ensure the endpoint is reachable.'); @@ -116,7 +119,7 @@ export class TestModelConnectionStep extends InstallationAIAssistantStep { return 'Failed to test model connection. Please check the model configuration and try again.'; } - public override async rollback( + public async rollback( _request: InstallAIDashboardAssistantDto, _context: InstallationContext, _error: Error diff --git a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts index 3989dd96..d22d04dc 100644 --- a/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts +++ b/public/dashboard-assistant/modules/installation-manager/infrastructure/steps/update-ml-commons-settings-step.ts @@ -59,7 +59,7 @@ export class UpdateMlCommonsSettingsStep extends InstallationAIAssistantStep { return 'Failed to update ML Commons settings. Please check the configuration and try again.'; } - public override async rollback( + public async rollback( _request: InstallAIDashboardAssistantDto, _context: InstallationContext, _error: Error From 1018ab2d59e5135207e9989e66bd96de7135f5de Mon Sep 17 00:00:00 2001 From: Guido Modarelli Date: Fri, 19 Sep 2025 23:26:10 +0000 Subject: [PATCH 12/12] =?UTF-8?q?replace=20named=20imports=20with=20explic?= =?UTF-8?q?it=20=C2=ABReact=C2=BB=20references=20in=20mocks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update mocked components and hooks to use explicit «React» references instead of named imports. This change ensures compatibility with Jest's module mocking and avoids potential conflicts with React's internal APIs. Additionally, replace JSX syntax with «React.createElement» in the mocked «MockModelForm» for consistency and clarity in the mock's implementation. --- .../__tests__/model-register.test.tsx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/public/dashboard-assistant/components/__tests__/model-register.test.tsx b/public/dashboard-assistant/components/__tests__/model-register.test.tsx index 1b242445..3abe8bf0 100644 --- a/public/dashboard-assistant/components/__tests__/model-register.test.tsx +++ b/public/dashboard-assistant/components/__tests__/model-register.test.tsx @@ -3,14 +3,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useState, useCallback, useEffect } from 'react'; +import React from 'react'; import userEvent from '@testing-library/user-event'; import { render, screen, waitFor } from '../../../../test/test_utils'; -import { ExecutionState } from '../../modules/installation-manager/domain'; jest.mock('../model-form', () => { + const _React = jest.requireActual('react'); + const MockModelForm = ({ onChange, onValidationChange }: any) => { - useEffect(() => { + _React.useEffect(() => { onChange?.({ modelProvider: 'OpenAI', model: 'gpt-4o', @@ -20,7 +21,7 @@ jest.mock('../model-form', () => { onValidationChange?.(true); }, [onChange, onValidationChange]); - return
; + return _React.createElement('div', { 'data-testid': 'mock-model-form' }); }; return { @@ -30,6 +31,11 @@ jest.mock('../model-form', () => { }); jest.mock('../../modules/installation-manager/hooks/use-assistant-installation', () => { + const React = jest.requireActual('react'); + const { ExecutionState } = jest.requireActual< + typeof import('../../modules/installation-manager/domain') + >('../../modules/installation-manager/domain'); + const failedStep = { stepName: 'Create Model', state: ExecutionState.FAILED, @@ -54,9 +60,9 @@ jest.mock('../../modules/installation-manager/hooks/use-assistant-installation', return { __esModule: true, useAssistantInstallation: () => { - const [error, setError] = useState(undefined); + const [error, setError] = React.useState(undefined); - const install = useCallback(async () => { + const install = React.useCallback(async () => { setError('Steps: "Create Model" has failed'); }, []);