address cubic comments

packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts

@@ -647,6 +647,42 @@ describe('AgentIntegrationsController integration credentials', () => {
 		);
 	});
 
+	it.each([
+		['unpublished', null, true],
+		['published', 'v1', false],
+	])(
+		'defaults external resource deletion to %s agent behavior',
+		async (_status, activeVersionId, expectedDeleteExternalResource) => {
+			const agentRepository = mock<AgentRepository>();
+			agentRepository.findByIdAndProjectId.mockResolvedValue({
+				id: 'agent-1',
+				projectId: 'project-1',
+				activeVersionId,
+				integrations: [{ type: 'slack', credentialId: 'cred-slack' }],
+			} as never);
+			const slackAppSetupService = mock<SlackAppSetupService>();
+			const { controller } = makeController({ agentRepository, slackAppSetupService });
+
+			await controller.disconnectIntegration(
+				{
+					params: { projectId: 'project-1' },
+					user: { id: 'user-1' },
+				} as never,
+				undefined as never,
+				'agent-1',
+				{ type: 'slack', credentialId: 'cred-slack' },
+			);
+
+			expect(slackAppSetupService.deleteManagedAppForCredential).toHaveBeenCalledWith({
+				projectId: 'project-1',
+				agentId: 'agent-1',
+				credentialId: 'cred-slack',
+				user: { id: 'user-1' },
+				deleteExternalResource: expectedDeleteExternalResource,
+			});
+		},
+	);
+
 	it('returns a warning while still disconnecting when the managed Slack app remains', async () => {
 		const agentRepository = mock<AgentRepository>();
 		const agent = {

packages/cli/src/modules/agents/agent-integration-management.service.ts

@@ -97,13 +97,16 @@ export class AgentIntegrationManagementService {
 			credentialId: options.credentialId,
 		});
 		const integration = persisted ?? (parsed.success ? parsed.data : undefined);
+		// if agent is not published fallback to delete external resource
+		const deleteExternalResource =
+			options.deleteExternalResource ?? options.agent.activeVersionId === null;
 		const warning = persisted
 			? await this.registry.get(options.type)?.onRemove?.({
 					agentId: options.agent.id,
 					projectId: options.agent.projectId,
 					credentialId: options.credentialId,
 					user: options.user,
-					deleteExternalResource: options.deleteExternalResource,
+					deleteExternalResource,
 				})
 			: undefined;
 

packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts

@@ -675,7 +675,7 @@ describe('Slack setup services', () => {
 		});
 	});
 
-	it('refreshes the manager token and auto-installs with a project-owned bot credential', async () => {
+	it('refreshes the manager token during icon setup and auto-installs the bot credential', async () => {
 		credentialsOverwrites.getOverwrites.mockReturnValue({
 			clientId: 'client',
 			clientSecret: 'secret',
@@ -703,7 +703,6 @@ describe('Slack setup services', () => {
 		});
 		requestMock
 			.mockResolvedValueOnce(slackAppCreatedResponse())
-			.mockResolvedValueOnce(slackResponse({ ok: true }))
 			.mockResolvedValueOnce(slackResponse({ ok: false, error: 'token_expired' }))
 			.mockResolvedValueOnce(
 				slackResponse({
@@ -714,6 +713,7 @@ describe('Slack setup services', () => {
 					},
 				}),
 			)
+			.mockResolvedValueOnce(slackResponse({ ok: true }))
 			.mockResolvedValueOnce(
 				slackResponse({
 					ok: true,
@@ -759,8 +759,14 @@ describe('Slack setup services', () => {
 		expect(iconRequest.body.get('app_id')).toBe('A123');
 		expect(iconRequest.body.get('token')).toBe('xoxp-manager');
 		expect(iconRequest.body.get('file')).toBeInstanceOf(Blob);
-		expect(fetchParams(requestMock, 2).get('bot_scopes')).toContain('chat:write');
-		expect(fetchParams(requestMock, 3).get('grant_type')).toBe('refresh_token');
+		expect(fetchParams(requestMock, 2).get('grant_type')).toBe('refresh_token');
+		const retriedIconRequest = requestMock.mock.calls[3]?.[0] as {
+			url: string;
+			body: FormData;
+		};
+		expect(retriedIconRequest.url).toBe('https://slack.com/api/apps.icon.set');
+		expect(retriedIconRequest.body.get('token')).toBe('xoxp-refreshed');
+		expect(fetchParams(requestMock, 4).get('bot_scopes')).toContain('chat:write');
 		expect(fetchParams(requestMock, 4).get('token')).toBe('xoxp-refreshed');
 		expect(credentialsService.update).toHaveBeenCalledWith(
 			'manager',
@@ -788,6 +794,55 @@ describe('Slack setup services', () => {
 		);
 	});
 
+	it('deletes a newly created managed app when icon setup fails', async () => {
+		credentialsOverwrites.getOverwrites.mockReturnValue({
+			clientId: 'client',
+			clientSecret: 'secret',
+			userScope: 'app_configurations:read app_configurations:write managed_apps:install',
+		});
+		credentialsOverwrites.usesManagedAuth.mockReturnValue(true);
+		credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([
+			{ id: 'manager', type: 'slackManagerOAuth2Api' },
+		] as never);
+		credentialsFinderService.findCredentialForUser.mockResolvedValue({
+			id: 'manager',
+			name: 'Slack manager',
+			type: 'slackManagerOAuth2Api',
+		} as CredentialsEntity);
+		credentialsService.decrypt.mockResolvedValue({
+			oauthTokenData: {
+				authed_user: {
+					access_token: 'xoxp-manager',
+					scope: 'app_configurations:read app_configurations:write managed_apps:install',
+				},
+				team: { id: 'T123', name: 'Example workspace' },
+			},
+		});
+		requestMock
+			.mockResolvedValueOnce(slackAppCreatedResponse())
+			.mockResolvedValueOnce(slackResponse({ ok: false, error: 'slack_request_failed' }))
+			.mockResolvedValueOnce(slackResponse({ ok: true }));
+
+		await expect(
+			service.installManagedApp({
+				projectId: 'project-1',
+				agentId: 'agent-1',
+				managerCredentialId: 'manager',
+				workspaceId: 'T123',
+				user,
+			}),
+		).rejects.toThrow('Slack could not set the Slack app icon: slack_request_failed');
+
+		expect(requestMock.mock.calls[2]?.[0]).toEqual(
+			expect.objectContaining({ url: 'https://slack.com/api/apps.manifest.delete' }),
+		);
+		expect(fetchParams(requestMock, 2).get('app_id')).toBe('A123');
+		expect(fetchParams(requestMock, 2).get('token')).toBe('xoxp-manager');
+		expect(cacheService.delete).toHaveBeenCalledWith(
+			'agents:slack-managed-app:project-1:agent-1:manager:T123',
+		);
+	});
+
 	it('returns the child OAuth URL when managed installation needs approval', async () => {
 		credentialsOverwrites.getOverwrites.mockReturnValue({
 			clientId: 'client',
@@ -846,6 +901,77 @@ describe('Slack setup services', () => {
 		expect(credentialsService.createManagedCredential).not.toHaveBeenCalled();
 	});
 
+	it('preserves managed app provenance after fallback OAuth installation', async () => {
+		credentialsOverwrites.getOverwrites.mockReturnValue({
+			clientId: 'client',
+			clientSecret: 'secret',
+			userScope: 'app_configurations:read app_configurations:write managed_apps:install',
+		});
+		credentialsOverwrites.usesManagedAuth.mockReturnValue(true);
+		credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([
+			{ id: 'manager', type: 'slackManagerOAuth2Api' },
+		] as never);
+		credentialsFinderService.findCredentialForUser.mockResolvedValue({
+			id: 'manager',
+			name: 'Slack manager',
+			type: 'slackManagerOAuth2Api',
+		} as CredentialsEntity);
+		credentialsService.decrypt.mockResolvedValue({
+			oauthTokenData: {
+				authed_user: {
+					access_token: 'xoxp-manager',
+					scope: 'app_configurations:read app_configurations:write managed_apps:install',
+				},
+				team: { id: 'T123', name: 'Example workspace' },
+			},
+		});
+		requestMock
+			.mockResolvedValueOnce(slackAppCreatedResponse())
+			.mockResolvedValueOnce(slackResponse({ ok: true }))
+			.mockResolvedValueOnce(slackResponse({ ok: false, error: 'app_approval_request_eligible' }))
+			.mockResolvedValueOnce(slackOAuthResponse());
+		userRepository.findOne.mockResolvedValue(user);
+		credentialsService.createManagedCredential.mockResolvedValue({
+			id: 'bot-credential',
+		} as never);
+		agentIntegrationPersistenceService.saveCredentialIntegration.mockResolvedValue(agent as never);
+
+		const result = await service.installManagedApp({
+			projectId: 'project-1',
+			agentId: 'agent-1',
+			managerCredentialId: 'manager',
+			workspaceId: 'T123',
+			user,
+		});
+		expect(result.status).toBe('manual_install_required');
+		if (result.status !== 'manual_install_required') return;
+
+		const state = new URL(result.installUrl).searchParams.get('state') ?? '';
+		await service.completeInstall({
+			projectId: 'project-1',
+			agentId: 'agent-1',
+			code: 'slack-code',
+			state,
+		});
+
+		expect(credentialsService.createManagedCredential).toHaveBeenCalledWith(
+			{
+				name: 'Example workspace - Support Agent',
+				type: 'slackApi',
+				data: {
+					accessToken: 'xoxb-installed-token',
+					signatureSecret: 'signing-secret',
+					managedAppId: 'A123',
+					teamId: 'T123',
+					managerCredentialId: 'manager',
+				},
+				projectId: 'project-1',
+			},
+			user,
+		);
+		expect(credentialsService.createUnmanagedCredential).not.toHaveBeenCalled();
+	});
+
 	it('exports settings for a managed Slack bot credential', async () => {
 		credentialsOverwrites.getOverwrites.mockReturnValue({
 			clientId: 'client',

packages/cli/src/modules/agents/integrations/platforms/slack/slack-managed-setup.service.ts

@@ -25,14 +25,10 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { CacheService } from '@/services/cache/cache.service';
 
 import { SlackMethodsService } from './slack-methods.service';
-import {
-	childRecord,
-	SLACK_BOT_SCOPES,
-	type SlackAppSetupSession,
-	stringProperty,
-} from './slack-setup.types';
+import { childRecord, SLACK_BOT_SCOPES, type SlackAppSetupSession } from './slack-setup.types';
 import type { Agent } from '../../../entities/agent.entity';
 import { AgentRepository } from '../../../repositories/agent.repository';
+import { stringProperty } from '../../integration-helpers';
 
 const SLACK_MANAGED_APP_CACHE_PREFIX = 'agents:slack-managed-app:';
 const SLACK_APP_SETUP_TTL_MS = 60 * 60 * 1000;
@@ -111,6 +107,9 @@ interface ManagedBotCredentialContext {
 	teamId: string;
 }
 
+type SlackApiParams = Record<string, string> | FormData;
+type SlackApiParamsFactory = (accessToken: string) => SlackApiParams;
+
 function stringsFromScope(value: unknown): Set<string> {
 	if (typeof value !== 'string') return new Set();
 	return new Set(value.split(/[\s,]+/).filter(Boolean));
@@ -715,31 +714,41 @@ export class SlackManagedSetupService {
 		const clientSecret = stringProperty(credentials, 'client_secret');
 		const signingSecret = stringProperty(credentials, 'signing_secret');
 		const oauthAuthorizeUrl = stringProperty(response, 'oauth_authorize_url');
-		if (!appId || !clientId || !clientSecret || !signingSecret || !oauthAuthorizeUrl) {
-			throw new BadRequestError('Slack returned an incomplete app setup response');
-		}
-		await this.setManagedAppIcon(manager, appId);
+		try {
+			if (!appId || !clientId || !clientSecret || !signingSecret || !oauthAuthorizeUrl) {
+				throw new BadRequestError('Slack returned an incomplete app setup response');
+			}
+			await this.setManagedAppIcon(manager, appId);
 
-		const session = {
-			projectId: options.projectId,
-			agentId: options.agentId,
-			userId: options.user.id,
-			appId,
-			clientId,
-			clientSecret,
-			signingSecret,
-			redirectUrl,
-			managerCredentialId: options.managerCredentialId,
-			teamId: options.workspaceId,
-			teamName: workspaceName,
-			oauthAuthorizeUrl,
-		} satisfies ManagedSlackAppSession;
-		await this.cacheService.set(
-			key,
-			await this.cipher.encryptV2(JSON.stringify(session)),
-			SLACK_APP_SETUP_TTL_MS,
-		);
-		return { session, created: true };
+			const session = {
+				projectId: options.projectId,
+				agentId: options.agentId,
+				userId: options.user.id,
+				appId,
+				clientId,
+				clientSecret,
+				signingSecret,
+				redirectUrl,
+				managerCredentialId: options.managerCredentialId,
+				teamId: options.workspaceId,
+				teamName: workspaceName,
+				oauthAuthorizeUrl,
+			} satisfies ManagedSlackAppSession;
+			await this.cacheService.set(
+				key,
+				await this.cipher.encryptV2(JSON.stringify(session)),
+				SLACK_APP_SETUP_TTL_MS,
+			);
+			return { session, created: true };
+		} catch (error) {
+			if (appId) {
+				await Promise.allSettled([
+					this.callManagerSlackApi(manager, 'apps.manifest.delete', { app_id: appId }),
+					this.cacheService.delete(key),
+				]);
+			}
+			throw error;
+		}
 	}
 
 	private async decryptManagedAppSession(
@@ -758,12 +767,11 @@ export class SlackManagedSetupService {
 	private async callManagerSlackApi(
 		manager: ManagerCredentialContext,
 		method: string,
-		params: Record<string, string>,
+		params: Record<string, string> | SlackApiParamsFactory,
 	): Promise<Record<string, unknown>> {
-		let response = await this.methods.callSlackApi(method, {
-			...params,
-			token: manager.accessToken,
-		});
+		const paramsForToken = (accessToken: string): SlackApiParams =>
+			typeof params === 'function' ? params(accessToken) : { ...params, token: accessToken };
+		let response = await this.methods.callSlackApi(method, paramsForToken(manager.accessToken));
 		const error = stringProperty(response, 'error');
 		if (!['invalid_auth', 'token_expired'].includes(error ?? '')) return response;
 
@@ -805,7 +813,7 @@ export class SlackManagedSetupService {
 		await this.credentialsService.update(manager.credential.id, encrypted, updatedData);
 		manager.oauthTokenData = oauthTokenData;
 		manager.accessToken = refreshedAccessToken;
-		response = await this.methods.callSlackApi(method, { ...params, token: refreshedAccessToken });
+		response = await this.methods.callSlackApi(method, paramsForToken(refreshedAccessToken));
 		return response;
 	}
 
@@ -823,15 +831,17 @@ export class SlackManagedSetupService {
 		managedAppId: string,
 	): Promise<void> {
 		const image = await readFile(join(__dirname, 'assets', 'n8n-bot-icon.png'));
-		const formData = new FormData();
-		formData.set('token', manager.accessToken);
-		formData.set('app_id', managedAppId);
-		formData.set(
-			'file',
-			new Blob([new Uint8Array(image)], { type: 'image/png' }),
-			'n8n-bot-icon.png',
-		);
-		const response = await this.methods.callSlackApi('apps.icon.set', formData);
+		const response = await this.callManagerSlackApi(manager, 'apps.icon.set', (accessToken) => {
+			const formData = new FormData();
+			formData.set('token', accessToken);
+			formData.set('app_id', managedAppId);
+			formData.set(
+				'file',
+				new Blob([new Uint8Array(image)], { type: 'image/png' }),
+				'n8n-bot-icon.png',
+			);
+			return formData;
+		});
 		if (response.ok !== true) {
 			throw this.methods.slackError('set the Slack app icon', response);
 		}

packages/cli/src/modules/agents/integrations/platforms/slack/slack-manual-setup.service.ts

@@ -12,12 +12,8 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
 import { CacheService } from '@/services/cache/cache.service';
 
 import { SlackMethodsService } from './slack-methods.service';
-import {
-	childRecord,
-	type SlackAppSetupSession,
-	slackSetupCacheKey,
-	stringProperty,
-} from './slack-setup.types';
+import { childRecord, type SlackAppSetupSession, slackSetupCacheKey } from './slack-setup.types';
+import { stringProperty } from '../../integration-helpers';
 
 export interface CreateSlackAppOptions {
 	projectId: string;

packages/cli/src/modules/agents/integrations/platforms/slack/slack-methods.service.ts

@@ -19,12 +19,12 @@ import {
 	SLACK_BOT_SCOPES,
 	type SlackAppSetupSession,
 	slackSetupCacheKey,
-	stringProperty,
 } from './slack-setup.types';
 import { AgentIntegrationPersistenceService } from '../../../agent-integration-persistence.service';
 import type { Agent } from '../../../entities/agent.entity';
 import { AgentRepository } from '../../../repositories/agent.repository';
 import { ChatIntegrationService } from '../../chat-integration.service';
+import { stringProperty } from '../../integration-helpers';
 
 const SLACK_MANAGED_APP_CACHE_PREFIX = 'agents:slack-managed-app:';
 const SLACK_APP_SETUP_TTL_MS = 60 * 60 * 1000;

packages/cli/src/modules/agents/integrations/platforms/slack/slack-setup.types.ts

@@ -49,11 +49,3 @@ export function childRecord(
 	const child = record[key];
 	return isRecord(child) ? child : undefined;
 }
-
-export function stringProperty(
-	record: Record<string, unknown> | undefined,
-	key: string,
-): string | undefined {
-	const value = record?.[key];
-	return typeof value === 'string' ? value : undefined;
-}

packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelSlackManagedSettings.test.ts

@@ -34,18 +34,18 @@ function mountForm(
 					template:
 						'<input :data-testid="$attrs[\'data-testid\']" :data-label="label" :data-info="infoText" :value="modelValue" @input="$emit(\'update:modelValue\', $event.target.value)" />',
 				},
-				Switch2: {
+				Switch: {
 					props: ['modelValue'],
 					emits: ['update:modelValue'],
 					template:
 						'<button data-testid="slack-managed-app-always-online" @click="$emit(\'update:modelValue\', !modelValue)" />',
 				},
-				Link: {
+				N8nLink: {
 					props: ['href', 'bold'],
 					template: '<a :href="href" :data-bold="bold"><slot /></a>',
 				},
 				N8nIcon: { template: '<i data-testid="external-link-icon" />' },
-				Text: { template: '<span><slot /></span>' },
+				N8nText: { template: '<span><slot /></span>' },
 			},
 		},
 	});

packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelSlackManagedSetup.test.ts

@@ -127,7 +127,7 @@ describe('AgentChannelSlackManagedSetup', () => {
 	it('shows the Slack quick-connect button when no manager credential exists', async () => {
 		const { wrapper, connectManager } = mountSetup(0, 0);
 
-		expect(wrapper.find('[data-testid="slack-manager-credential-select"]').exists()).toBe(false);
+		expect(wrapper.find('[data-test-id="slack-manager-credential-select"]').exists()).toBe(false);
 		const connectButton = wrapper.get('[data-testid="slack-manager-connect"]');
 		expect(connectButton.attributes('icon')).toBe('slack');
 		expect(connectButton.text()).toBe('agents.channels.slack.managed.connect.button');
@@ -148,6 +148,7 @@ describe('AgentChannelSlackManagedSetup', () => {
 		const { wrapper, editManager } = mountSetup(1);
 
 		expect(wrapper.find('[data-testid="slack-manager-connect"]').exists()).toBe(false);
+		expect(wrapper.find('[data-test-id="slack-manager-credential-select"]').exists()).toBe(true);
 		expect(wrapper.find('[data-testid="select-manager"]').exists()).toBe(true);
 		await wrapper.get('[data-testid="slack-manager-edit"]').trigger('click');
 

packages/frontend/editor-ui/src/features/agents/channels/slack/api.test.ts

@@ -0,0 +1,41 @@
+import { ResponseError } from '@n8n/rest-api-client';
+import { describe, expect, it } from 'vitest';
+
+import { getSlackApiErrorCode } from './api';
+
+describe('getSlackApiErrorCode', () => {
+	it('returns the code from a Slack API response error', () => {
+		const error = new ResponseError('Slack request failed', {
+			httpStatusCode: 400,
+			meta: {
+				integrationType: 'slack',
+				code: 'service_limits_exceeded',
+			},
+		});
+
+		expect(getSlackApiErrorCode(error)).toBe('service_limits_exceeded');
+	});
+
+	it('rejects metadata from another integration', () => {
+		const error = new ResponseError('Integration request failed', {
+			httpStatusCode: 400,
+			meta: {
+				integrationType: 'linear',
+				code: 'service_limits_exceeded',
+			},
+		});
+
+		expect(getSlackApiErrorCode(error)).toBeUndefined();
+	});
+
+	it('rejects Slack-shaped metadata outside a response error', () => {
+		const error = {
+			meta: {
+				integrationType: 'slack',
+				code: 'service_limits_exceeded',
+			},
+		};
+
+		expect(getSlackApiErrorCode(error)).toBeUndefined();
+	});
+});

packages/frontend/editor-ui/src/features/agents/channels/slack/useSlackChannelRuntime.test.ts

@@ -36,19 +36,14 @@ vi.mock('@/features/credentials/composables/useCredentialOAuth', () => ({
 	}),
 }));
 
-vi.mock('./api', () => ({
+vi.mock('./api', async (importOriginal) => ({
+	...(await importOriginal<typeof import('./api')>()),
 	createSlackAgentApp: vi.fn(),
 	createSlackManagerCredential: vi.fn(),
 	getSlackManagedSetup: mocks.getSetup,
 	getSlackManagedAppSettings: mocks.getSettings,
 	updateSlackManagedAppSettings: mocks.updateSettings,
 	installSlackManagedApp: mocks.install,
-	getSlackApiErrorCode: (error: unknown) => {
-		if (typeof error !== 'object' || error === null || !('meta' in error)) return undefined;
-		const meta = error.meta;
-		if (typeof meta !== 'object' || meta === null || !('code' in meta)) return undefined;
-		return typeof meta.code === 'string' ? meta.code : undefined;
-	},
 }));
 
 function createRuntime(

packages/frontend/editor-ui/src/features/credentials/composables/useCredentialOAuth.ts

@@ -305,7 +305,7 @@ export function useCredentialOAuth() {
 		} finally {
 			oauthAbortController.value = null;
 			if (!success) {
-				await credentialsStore.deleteCredential({ id: credential.id });
+				await credentialsStore.deleteCredential({ id: credential.id }).catch(() => {});
 			}
 		}
 	}

packages/workflow/test/credential-domain-restrictions.test.ts

@@ -632,7 +632,10 @@ describe('injectDomainRestrictionFields', () => {
 
 		it('does not inject fields when hideDomainRestrictionFields is true', () => {
 			const result = injectDomainRestrictionFields(
-				baseCredential({ hideDomainRestrictionFields: true }),
+				baseCredential({
+					extends: ['oAuth2Api'],
+					hideDomainRestrictionFields: true,
+				}),
 			);
 			expect(result).toHaveLength(1);
 			expect(result[0]).toBe(dummyField);