mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
fix(ai-builder): Make browser credential-setup evals grade the right thing (no-changelog) (#40858)
This commit is contained in:
parent
94bdf85a8a
commit
ccf42e3213
|
|
@ -502,6 +502,7 @@ export {
|
|||
InstanceAiEvalExecutionRequest,
|
||||
InstanceAiEvalAgentExecutionRequest,
|
||||
InstanceAiEvalCredentialAllowlistRequest,
|
||||
InstanceAiEvalCredentialTestTargetRequest,
|
||||
INSTANCE_AI_MEMORY_TASK_WAIT_TIMEOUT_MS,
|
||||
AI_GATEWAY_MANAGED_TAG,
|
||||
InstanceAiEvalRestoreThreadRequest,
|
||||
|
|
|
|||
|
|
@ -3042,6 +3042,18 @@ export class InstanceAiEvalCredentialAllowlistRequest extends Z.class({
|
|||
bypassCredentialTest: z.array(z.string().min(1)).max(50).optional(),
|
||||
}) {}
|
||||
|
||||
/**
|
||||
* Aims the thread's connection tests for one credential type at a provider
|
||||
* stand-in, so the eval checks the captured secret without the real provider.
|
||||
*/
|
||||
export class InstanceAiEvalCredentialTestTargetRequest extends Z.class({
|
||||
threadId: z.string().uuid(),
|
||||
credentialType: z.string().min(1),
|
||||
baseUrl: z.string().url(),
|
||||
/** Credential field the stand-in URL is written into. */
|
||||
urlField: z.string().min(1),
|
||||
}) {}
|
||||
|
||||
/** The id an authored seed gives a data table, agent or folder. ≥8 chars: the
|
||||
* restore remaps ids by whole-document string replace, and a short id would risk
|
||||
* corrupting unrelated substrings — so the restore path refuses shorter ids.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,125 @@
|
|||
import { vi } from 'vitest';
|
||||
|
||||
import { N8nApiError, type N8nClient } from '../clients/n8n-client';
|
||||
import { buildWorkflow } from '../harness/build-workflow';
|
||||
import {
|
||||
resolveCredentialSetupFixture,
|
||||
startCredentialSetupLane,
|
||||
type CredentialSetupLane,
|
||||
} from '../harness/credential-setup-lane';
|
||||
import type { EvalLogger } from '../harness/logger';
|
||||
|
||||
vi.mock('../harness/chat-loop', async (importActual) => {
|
||||
const actual = await importActual<typeof import('../harness/chat-loop')>();
|
||||
return {
|
||||
SSE_SETTLE_DELAY_MS: 0,
|
||||
startSseConnection: vi.fn().mockResolvedValue(undefined),
|
||||
waitForAllActivity: vi.fn().mockResolvedValue(undefined),
|
||||
runMultiTurnConversation: vi.fn().mockResolvedValue(undefined),
|
||||
recordUserTurn: vi.fn(),
|
||||
buildAutoApprovePayload: actual.buildAutoApprovePayload,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../harness/credential-setup-lane', async (importActual) => ({
|
||||
...(await importActual<typeof import('../harness/credential-setup-lane')>()),
|
||||
startCredentialSetupLane: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../outcome/workflow-discovery', () => ({
|
||||
buildAgentOutcome: vi.fn().mockResolvedValue({
|
||||
workflowsCreated: [],
|
||||
executionsRun: [],
|
||||
dataTablesCreated: [],
|
||||
finalText: 'done',
|
||||
workflowJsons: [],
|
||||
}),
|
||||
extractWorkflowIdsFromMessages: vi.fn().mockReturnValue([]),
|
||||
}));
|
||||
|
||||
const silentLogger: EvalLogger = {
|
||||
info: () => {},
|
||||
verbose: () => {},
|
||||
success: () => {},
|
||||
warn: () => {},
|
||||
error: () => {},
|
||||
isVerbose: false,
|
||||
};
|
||||
|
||||
function makeClient(): N8nClient {
|
||||
return {
|
||||
getPersonalProjectId: vi.fn().mockResolvedValue('project-1'),
|
||||
ensureThread: vi.fn().mockResolvedValue(undefined),
|
||||
setThreadCredentialAllowlist: vi.fn().mockResolvedValue(undefined),
|
||||
setThreadCredentialTestTarget: vi.fn().mockResolvedValue(undefined),
|
||||
listCredentialIds: vi.fn().mockResolvedValue([]),
|
||||
listCredentials: vi.fn().mockResolvedValue([]),
|
||||
sendMessage: vi.fn().mockResolvedValue({ runId: 'run-1' }),
|
||||
getThreadMessages: vi.fn().mockResolvedValue({ messages: [] }),
|
||||
listWorkflows: vi.fn().mockResolvedValue([]),
|
||||
} as unknown as N8nClient;
|
||||
}
|
||||
|
||||
async function buildWithLane(
|
||||
lane: Partial<CredentialSetupLane>,
|
||||
client = makeClient(),
|
||||
): Promise<N8nClient> {
|
||||
vi.mocked(startCredentialSetupLane).mockResolvedValue({
|
||||
local: false,
|
||||
close: vi.fn().mockResolvedValue(undefined),
|
||||
...lane,
|
||||
} as CredentialSetupLane);
|
||||
await buildWorkflow({
|
||||
client,
|
||||
skipWorkflowChecks: true,
|
||||
preRunWorkflowIds: new Set<string>(),
|
||||
claimedWorkflowIds: new Set<string>(),
|
||||
logger: silentLogger,
|
||||
conversation: [{ role: 'user', text: 'create a Slack credential' }],
|
||||
credentialSetupSelection: await resolveCredentialSetupFixture({ credentialFixture: 'slack' }),
|
||||
});
|
||||
return client;
|
||||
}
|
||||
|
||||
describe('buildWorkflow credential test target', () => {
|
||||
it('aims the thread at the fixture stand-in before the first message', async () => {
|
||||
const client = await buildWithLane({
|
||||
credentialType: 'slackApi',
|
||||
verifyBaseUrl: 'http://stand-in.test:4100',
|
||||
credentialUrlField: 'url',
|
||||
});
|
||||
|
||||
expect(client.setThreadCredentialTestTarget).toHaveBeenCalledWith(expect.any(String), {
|
||||
credentialType: 'slackApi',
|
||||
baseUrl: 'http://stand-in.test:4100',
|
||||
urlField: 'url',
|
||||
});
|
||||
expect(
|
||||
vi.mocked(client.setThreadCredentialTestTarget).mock.invocationCallOrder[0],
|
||||
).toBeLessThan(vi.mocked(client.sendMessage).mock.invocationCallOrder[0]);
|
||||
});
|
||||
|
||||
it('still builds against a backend without the endpoint', async () => {
|
||||
const client = makeClient();
|
||||
vi.mocked(client.setThreadCredentialTestTarget).mockRejectedValue(
|
||||
new N8nApiError('Not found', 404),
|
||||
);
|
||||
|
||||
await buildWithLane(
|
||||
{
|
||||
credentialType: 'slackApi',
|
||||
verifyBaseUrl: 'http://stand-in.test:4100',
|
||||
credentialUrlField: 'url',
|
||||
},
|
||||
client,
|
||||
);
|
||||
|
||||
expect(client.sendMessage).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('sets no target when the fixture ships no stand-in', async () => {
|
||||
const client = await buildWithLane({ credentialType: 'slackApi', credentialUrlField: 'url' });
|
||||
|
||||
expect(client.setThreadCredentialTestTarget).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,106 @@
|
|||
import { vi } from 'vitest';
|
||||
|
||||
import type { N8nClient } from '../clients/n8n-client';
|
||||
import { buildWorkflow } from '../harness/build-workflow';
|
||||
import { waitForAllActivity, type ConfirmationStrategy } from '../harness/chat-loop';
|
||||
import type { EvalLogger } from '../harness/logger';
|
||||
import type { CapturedEvent } from '../types';
|
||||
|
||||
const proxyAnswer = {
|
||||
kind: 'questions',
|
||||
answers: [{ questionId: 'key_choice', selectedOptions: ['Create a new API key'] }],
|
||||
};
|
||||
|
||||
vi.mock('../harness/chat-loop', async (importActual) => {
|
||||
const actual = await importActual<typeof import('../harness/chat-loop')>();
|
||||
return {
|
||||
SSE_SETTLE_DELAY_MS: 0,
|
||||
startSseConnection: vi.fn().mockResolvedValue(undefined),
|
||||
waitForAllActivity: vi.fn().mockResolvedValue(undefined),
|
||||
runMultiTurnConversation: vi.fn().mockResolvedValue(undefined),
|
||||
recordUserTurn: vi.fn(),
|
||||
buildAutoApprovePayload: actual.buildAutoApprovePayload,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../utils/user-proxy', () => ({
|
||||
UserProxyLlm: class {
|
||||
respondToConfirmation = vi.fn().mockResolvedValue(proxyAnswer);
|
||||
ingestEvents = vi.fn();
|
||||
decideFollowUp = vi.fn().mockResolvedValue({ kind: 'done' });
|
||||
getDecisionStats = vi.fn().mockReturnValue({});
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('../outcome/workflow-discovery', () => ({
|
||||
buildAgentOutcome: vi.fn().mockResolvedValue({
|
||||
workflowsCreated: [],
|
||||
executionsRun: [],
|
||||
dataTablesCreated: [],
|
||||
finalText: 'done',
|
||||
workflowJsons: [],
|
||||
}),
|
||||
extractWorkflowIdsFromMessages: vi.fn().mockReturnValue([]),
|
||||
}));
|
||||
|
||||
const silentLogger: EvalLogger = {
|
||||
info: () => {},
|
||||
verbose: () => {},
|
||||
success: () => {},
|
||||
warn: () => {},
|
||||
error: () => {},
|
||||
isVerbose: false,
|
||||
};
|
||||
|
||||
function makeClient(): N8nClient {
|
||||
return {
|
||||
getPersonalProjectId: vi.fn().mockResolvedValue('project-1'),
|
||||
ensureThread: vi.fn().mockResolvedValue(undefined),
|
||||
setThreadCredentialAllowlist: vi.fn().mockResolvedValue(undefined),
|
||||
sendMessage: vi.fn().mockResolvedValue({ runId: 'run-1' }),
|
||||
getThreadMessages: vi.fn().mockResolvedValue({ messages: [] }),
|
||||
listWorkflows: vi.fn().mockResolvedValue([]),
|
||||
} as unknown as N8nClient;
|
||||
}
|
||||
|
||||
function confirmationEvent(payload: Record<string, unknown>): CapturedEvent {
|
||||
return {
|
||||
type: 'confirmation-request',
|
||||
data: { type: 'confirmation-request', payload: { requestId: 'req-1', ...payload } },
|
||||
} as unknown as CapturedEvent;
|
||||
}
|
||||
|
||||
async function singleTurnStrategy(): Promise<ConfirmationStrategy> {
|
||||
await buildWorkflow({
|
||||
client: makeClient(),
|
||||
skipWorkflowChecks: true,
|
||||
preRunWorkflowIds: new Set<string>(),
|
||||
claimedWorkflowIds: new Set<string>(),
|
||||
logger: silentLogger,
|
||||
conversation: [{ role: 'user', text: 'create a Gemini credential' }],
|
||||
});
|
||||
const strategy = vi.mocked(waitForAllActivity).mock.calls[0]?.[0].confirmationStrategy;
|
||||
if (!strategy) throw new Error('single-turn build passed no confirmation strategy');
|
||||
return strategy;
|
||||
}
|
||||
|
||||
describe('buildWorkflow single-turn confirmations', () => {
|
||||
beforeEach(() => vi.mocked(waitForAllActivity).mockClear());
|
||||
|
||||
it('answers ask-user questions through the user proxy instead of leaving them blank', async () => {
|
||||
const strategy = await singleTurnStrategy();
|
||||
const response = await strategy(
|
||||
confirmationEvent({
|
||||
inputType: 'questions',
|
||||
questions: [{ id: 'key_choice', question: 'Which key?', type: 'single' }],
|
||||
}),
|
||||
);
|
||||
expect(response).toEqual(proxyAnswer);
|
||||
});
|
||||
|
||||
it('keeps auto-approving every other confirmation', async () => {
|
||||
const strategy = await singleTurnStrategy();
|
||||
const response = await strategy(confirmationEvent({ message: 'Open aistudio.google.com' }));
|
||||
expect(response).toEqual({ kind: 'approval', approved: true });
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,46 @@
|
|||
import { vi } from 'vitest';
|
||||
|
||||
import type { N8nClient } from '../clients/n8n-client';
|
||||
import { startBrowserRuntime } from '../harness/browser-runtime';
|
||||
import {
|
||||
resolveCredentialSetupFixture,
|
||||
startCredentialSetupLane,
|
||||
} from '../harness/credential-setup-lane';
|
||||
import type { EvalLogger } from '../harness/logger';
|
||||
|
||||
vi.mock('../harness/browser-runtime', () => ({
|
||||
startBrowserRuntime: vi.fn(),
|
||||
attachToRunningBrowser: vi.fn(),
|
||||
}));
|
||||
|
||||
const silentLogger: EvalLogger = {
|
||||
info: () => {},
|
||||
verbose: () => {},
|
||||
success: () => {},
|
||||
warn: () => {},
|
||||
error: () => {},
|
||||
isVerbose: false,
|
||||
};
|
||||
|
||||
describe('startCredentialSetupLane stand-in', () => {
|
||||
it('exposes the shipped fixture stand-in and its credential field', async () => {
|
||||
vi.mocked(startBrowserRuntime).mockResolvedValue({
|
||||
close: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as Awaited<ReturnType<typeof startBrowserRuntime>>);
|
||||
const selection = await resolveCredentialSetupFixture({ credentialFixture: 'slack' });
|
||||
|
||||
const lane = await startCredentialSetupLane({
|
||||
client: {} as N8nClient,
|
||||
selection,
|
||||
logger: silentLogger,
|
||||
});
|
||||
|
||||
try {
|
||||
expect(lane?.credentialType).toBe('slackApi');
|
||||
expect(lane?.credentialUrlField).toBe('url');
|
||||
expect(lane?.verifyBaseUrl).toMatch(/^http:\/\/.+:\d+$/);
|
||||
} finally {
|
||||
await lane?.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -91,9 +91,13 @@ describe('provider fixtures', () => {
|
|||
urlField: 'host',
|
||||
verify: { path: '/v1beta/models', query: 'key' },
|
||||
},
|
||||
// No `verify`: slackApi's test hardcodes its baseURL, so the stand-in can
|
||||
// never be aimed at it. The value check reports itself unverifiable.
|
||||
{ type: 'slackApi', id: 'slack', host: 'api.slack.com', prefix: 'xoxb-' },
|
||||
{
|
||||
type: 'slackApi',
|
||||
id: 'slack',
|
||||
host: 'api.slack.com',
|
||||
prefix: 'xoxb-',
|
||||
verify: { path: '/api/users.profile.get', header: 'Authorization', scheme: 'Bearer' },
|
||||
},
|
||||
];
|
||||
|
||||
it.each(SHIPPED)('ships the $id fixture keyed on $type', async (row) => {
|
||||
|
|
|
|||
|
|
@ -1000,6 +1000,17 @@ export class N8nClient {
|
|||
});
|
||||
}
|
||||
|
||||
/** Send the thread's connection tests for one credential type to a provider stand-in. */
|
||||
async setThreadCredentialTestTarget(
|
||||
threadId: string,
|
||||
target: { credentialType: string; baseUrl: string; urlField: string },
|
||||
): Promise<void> {
|
||||
await this.fetch('/rest/instance-ai/eval/thread-credential-test-target', {
|
||||
method: 'POST',
|
||||
body: { threadId, ...target },
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Seed an existing thread with a previously exported conversation: the
|
||||
* referenced workflows are recreated (node credentials stripped server-side),
|
||||
|
|
|
|||
|
|
@ -8,5 +8,10 @@
|
|||
"/apps/:appId/install-on-team": "install.html",
|
||||
"/apps/:appId/oauth": "install.html"
|
||||
},
|
||||
"defaultRoute": "/apps"
|
||||
"defaultRoute": "/apps",
|
||||
"verify": {
|
||||
"path": "/api/users.profile.get",
|
||||
"header": "Authorization",
|
||||
"scheme": "Bearer"
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import {
|
|||
waitForAllActivity,
|
||||
runMultiTurnConversation,
|
||||
recordUserTurn,
|
||||
buildAutoApprovePayload,
|
||||
type ConfirmationStrategy,
|
||||
} from './chat-loop';
|
||||
import { runWorkflowChecks, summarizeMissingWorkflowError } from './cleanup';
|
||||
|
|
@ -95,6 +96,7 @@ import {
|
|||
lastAgentText,
|
||||
userTurnsAsText,
|
||||
} from '../utils/conversation-text';
|
||||
import { getEventPayload } from '../utils/confirmation-payload';
|
||||
import { UserProxyLlm, type ProxyDecisionStats } from '../utils/user-proxy';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
|
@ -871,12 +873,31 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise<BuildR
|
|||
'Add a fixture for it (evaluations/fixtures/providers/) before running this case locally.',
|
||||
);
|
||||
}
|
||||
credentialSetupLane = await startCredentialSetupLane({
|
||||
const lane = await startCredentialSetupLane({
|
||||
client,
|
||||
selection: config.credentialSetupSelection,
|
||||
logger,
|
||||
localCredentialType: config.credentialSetupType,
|
||||
});
|
||||
credentialSetupLane = lane;
|
||||
// The agent tests the credential it saves. Without this, that test reaches
|
||||
// the real provider and rejects every key the stand-in issued.
|
||||
if (lane?.credentialType && lane.verifyBaseUrl && lane.credentialUrlField) {
|
||||
try {
|
||||
await client.setThreadCredentialTestTarget(threadId, {
|
||||
credentialType: lane.credentialType,
|
||||
baseUrl: lane.verifyBaseUrl,
|
||||
urlField: lane.credentialUrlField,
|
||||
});
|
||||
} catch (error: unknown) {
|
||||
// An older backend has no endpoint: the agent's own test then reaches the
|
||||
// real provider, as before. Any other failure is a lane boot failure.
|
||||
if (!(error instanceof N8nApiError && error.status === 404)) throw error;
|
||||
logger.info(
|
||||
` Credential-test target endpoint unavailable, agent tests reach the real provider${config.laneTag ?? ''}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
laneBootFailed = false;
|
||||
}
|
||||
|
||||
|
|
@ -1243,6 +1264,12 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise<BuildR
|
|||
openingHandoffContext,
|
||||
observerThresholdTokens,
|
||||
);
|
||||
// An empty answer reads to the agent as "no preference", which steers it
|
||||
// off the path a real user would pick, so questions still get the proxy.
|
||||
const questionProxy = new UserProxyLlm({
|
||||
conversation: [{ role: 'user', text: recordedOpeningMessage }],
|
||||
logger,
|
||||
});
|
||||
try {
|
||||
await waitForAllActivity({
|
||||
client,
|
||||
|
|
@ -1256,6 +1283,13 @@ export async function buildWorkflow(config: BuildWorkflowConfig): Promise<BuildR
|
|||
inactivityTimeoutMs: INACTIVITY_TIMEOUT_MS,
|
||||
logger,
|
||||
proxyResponses,
|
||||
confirmationStrategy: async (event) => {
|
||||
if (getEventPayload(event).inputType !== 'questions') {
|
||||
return buildAutoApprovePayload(event);
|
||||
}
|
||||
questionProxy.ingestEvents(events);
|
||||
return await questionProxy.respondToConfirmation(event);
|
||||
},
|
||||
});
|
||||
} catch (error: unknown) {
|
||||
if (!(error instanceof RunTimeoutError)) throw error;
|
||||
|
|
|
|||
|
|
@ -8261,6 +8261,41 @@ describe('createCredentialAdapter', () => {
|
|||
await expect(credentialService.test('cred-1')).rejects.toBe(refusal);
|
||||
expect(test).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('writes the stand-in URL into the credential field an eval target names', async () => {
|
||||
const decryptForUse = vi.fn().mockResolvedValue({ apiKey: 'secret' });
|
||||
const test = vi.fn().mockResolvedValue({ status: 'OK', message: 'ok' });
|
||||
const { service, mockUser } = adapterWith(decryptForUse, test);
|
||||
const { credentialService } = service.createContext(mockUser, {
|
||||
getCredentialTestTarget: () => ({
|
||||
credentialType: 'slackApi',
|
||||
baseUrl: 'http://stand-in.test:4100',
|
||||
urlField: 'url',
|
||||
}),
|
||||
});
|
||||
|
||||
await credentialService.test('cred-1');
|
||||
|
||||
expect(test).toHaveBeenCalledWith(
|
||||
mockUser.id,
|
||||
expect.objectContaining({
|
||||
data: { apiKey: 'secret', url: 'http://stand-in.test:4100' },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('tests the stored data unchanged when the thread has no target', async () => {
|
||||
const decryptForUse = vi.fn().mockResolvedValue({ apiKey: 'secret' });
|
||||
const test = vi.fn().mockResolvedValue({ status: 'OK', message: 'ok' });
|
||||
const { credentialService, mockUser } = adapterWith(decryptForUse, test);
|
||||
|
||||
await credentialService.test('cred-1');
|
||||
|
||||
expect(test).toHaveBeenCalledWith(
|
||||
mockUser.id,
|
||||
expect.objectContaining({ data: { apiKey: 'secret' } }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getCredentialFillState', () => {
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ import type {
|
|||
InstanceAiAdminSettingsUpdateRequest,
|
||||
InstanceAiEvalAgentExecutionResult,
|
||||
InstanceAiEvalCredentialAllowlistRequest,
|
||||
InstanceAiEvalCredentialTestTargetRequest,
|
||||
InstanceAiEvalExecutionResult,
|
||||
InstanceAiEvalLlmUsage,
|
||||
InstanceAiEvalRestoreThreadRequest,
|
||||
|
|
@ -1106,6 +1107,70 @@ describe('InstanceAiController', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('setThreadCredentialTestTarget', () => {
|
||||
const target = {
|
||||
credentialType: 'slackApi',
|
||||
baseUrl: 'http://stand-in.test:4100',
|
||||
urlField: 'url',
|
||||
};
|
||||
const payload = { threadId: THREAD_ID, ...target };
|
||||
|
||||
it('should require instanceAi:eval scope', () => {
|
||||
expect(scopeOf('setThreadCredentialTestTarget')).toEqual({
|
||||
scope: 'instanceAi:eval',
|
||||
globalOnly: true,
|
||||
});
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
globalConfig.instanceAi.evalInstance = true;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalConfig.instanceAi.evalInstance = false;
|
||||
});
|
||||
|
||||
it('should set the test target for an owned thread', async () => {
|
||||
memoryService.checkThreadOwnership.mockResolvedValue('owned');
|
||||
|
||||
const result = await controller.setThreadCredentialTestTarget(
|
||||
req,
|
||||
res,
|
||||
payload as InstanceAiEvalCredentialTestTargetRequest,
|
||||
);
|
||||
|
||||
expect(result).toEqual({ ok: true });
|
||||
expect(evalCredentialAllowlists.getTestTarget(THREAD_ID, 'slackApi')).toEqual(target);
|
||||
});
|
||||
|
||||
it('should reject a thread that does not exist', async () => {
|
||||
memoryService.checkThreadOwnership.mockResolvedValue('not_found');
|
||||
|
||||
await expect(
|
||||
controller.setThreadCredentialTestTarget(
|
||||
req,
|
||||
res,
|
||||
payload as InstanceAiEvalCredentialTestTargetRequest,
|
||||
),
|
||||
).rejects.toThrow(NotFoundError);
|
||||
});
|
||||
|
||||
it('should refuse the target on an instance that is not an eval instance', async () => {
|
||||
globalConfig.instanceAi.evalInstance = false;
|
||||
evalCredentialAllowlists.clearThread(THREAD_ID);
|
||||
memoryService.checkThreadOwnership.mockResolvedValue('owned');
|
||||
|
||||
await expect(
|
||||
controller.setThreadCredentialTestTarget(
|
||||
req,
|
||||
res,
|
||||
payload as InstanceAiEvalCredentialTestTargetRequest,
|
||||
),
|
||||
).rejects.toThrow(NotFoundError);
|
||||
expect(evalCredentialAllowlists.getTestTarget(THREAD_ID, 'slackApi')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('getEvalThreadMemory', () => {
|
||||
const memory: InstanceAiEvalThreadMemoryResponse = {
|
||||
observations: [{ marker: 'critical', text: 'Posting via HTTP Request', tokenCount: 7 }],
|
||||
|
|
|
|||
|
|
@ -55,3 +55,44 @@ describe('EvalThreadCredentialAllowlistService', () => {
|
|||
expect(service.get('thread-1')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('EvalThreadCredentialAllowlistService test targets', () => {
|
||||
let service: EvalThreadCredentialAllowlistService;
|
||||
const target = {
|
||||
credentialType: 'slackApi',
|
||||
baseUrl: 'http://stand-in.test:4100',
|
||||
urlField: 'url',
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
service = new EvalThreadCredentialAllowlistService();
|
||||
});
|
||||
|
||||
it('returns the target for its credential type only', () => {
|
||||
service.setTestTarget('thread-1', target);
|
||||
|
||||
expect(service.getTestTarget('thread-1', 'slackApi')).toEqual(target);
|
||||
expect(service.getTestTarget('thread-1', 'openAiApi')).toBeUndefined();
|
||||
expect(service.getTestTarget('thread-2', 'slackApi')).toBeUndefined();
|
||||
});
|
||||
|
||||
it('keeps a target for each credential type', () => {
|
||||
const openAi = {
|
||||
credentialType: 'openAiApi',
|
||||
baseUrl: 'http://stand-in.test:4200',
|
||||
urlField: 'url',
|
||||
};
|
||||
service.setTestTarget('thread-1', target);
|
||||
service.setTestTarget('thread-1', openAi);
|
||||
|
||||
expect(service.getTestTarget('thread-1', 'slackApi')).toEqual(target);
|
||||
expect(service.getTestTarget('thread-1', 'openAiApi')).toEqual(openAi);
|
||||
});
|
||||
|
||||
it('forgets the target when the thread is cleared', () => {
|
||||
service.setTestTarget('thread-1', target);
|
||||
service.clearThread('thread-1');
|
||||
|
||||
expect(service.getTestTarget('thread-1', 'slackApi')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,10 +1,19 @@
|
|||
import { Service } from '@n8n/di';
|
||||
|
||||
/** Where the thread's connection tests for one credential type go instead of
|
||||
* the provider. See `InstanceAiEvalCredentialTestTargetRequest`. */
|
||||
export interface EvalCredentialTestTarget {
|
||||
credentialType: string;
|
||||
baseUrl: string;
|
||||
urlField: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-thread credential visibility for evaluation runs. The eval harness
|
||||
* Per-thread credential settings for evaluation runs. The eval harness
|
||||
* declares which credentials a build thread may see; the builder context's
|
||||
* credential `list()` is filtered to that set. Threads without an entry see
|
||||
* the unfiltered instance listing.
|
||||
* the unfiltered instance listing. The harness can also bypass a credential's
|
||||
* connection test, or aim it at a provider stand-in.
|
||||
*/
|
||||
@Service()
|
||||
export class EvalThreadCredentialAllowlistService {
|
||||
|
|
@ -13,6 +22,8 @@ export class EvalThreadCredentialAllowlistService {
|
|||
* without contacting the provider — see `bypassCredentialTest` on
|
||||
* `InstanceAiEvalCredentialAllowlistRequest`. */
|
||||
private readonly testBypassByThread = new Map<string, string[]>();
|
||||
/** Thread id → credential type → target. */
|
||||
private readonly testTargetsByThread = new Map<string, Map<string, EvalCredentialTestTarget>>();
|
||||
|
||||
set(threadId: string, credentialIds: string[], bypassCredentialTest?: string[]): void {
|
||||
this.byThread.set(threadId, [...credentialIds]);
|
||||
|
|
@ -41,8 +52,20 @@ export class EvalThreadCredentialAllowlistService {
|
|||
return this.testBypassByThread.get(threadId)?.includes(credentialId) ?? false;
|
||||
}
|
||||
|
||||
/** Replaces an earlier target for the same credential type only. */
|
||||
setTestTarget(threadId: string, target: EvalCredentialTestTarget): void {
|
||||
const targets = this.testTargetsByThread.get(threadId) ?? new Map();
|
||||
targets.set(target.credentialType, { ...target });
|
||||
this.testTargetsByThread.set(threadId, targets);
|
||||
}
|
||||
|
||||
getTestTarget(threadId: string, credentialType: string): EvalCredentialTestTarget | undefined {
|
||||
return this.testTargetsByThread.get(threadId)?.get(credentialType);
|
||||
}
|
||||
|
||||
clearThread(threadId: string): void {
|
||||
this.byThread.delete(threadId);
|
||||
this.testBypassByThread.delete(threadId);
|
||||
this.testTargetsByThread.delete(threadId);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -171,6 +171,7 @@ import {
|
|||
scopeCredentialProvider,
|
||||
type AgentCredentialProvider,
|
||||
} from './eval/scoped-credential-provider';
|
||||
import type { EvalCredentialTestTarget } from './eval/thread-credential-allowlist.service';
|
||||
import { InstanceAiBuilderDelegateAdapterService } from '@/modules/agents/instance-ai-builder-delegate.adapter';
|
||||
import { DataTableRepository } from '@/modules/data-table/data-table.repository';
|
||||
import { DataTableService } from '@/modules/data-table/data-table.service';
|
||||
|
|
@ -490,6 +491,9 @@ export class InstanceAiAdapterService {
|
|||
* contacting the provider. A predicate rather than a list because the
|
||||
* harness registers bypasses mid-run, after this context is built. */
|
||||
shouldBypassCredentialTest?: (credentialId: string) => boolean;
|
||||
/** Eval-only: the provider stand-in for a credential type's connection test.
|
||||
* A getter for the same reason as the bypass predicate. */
|
||||
getCredentialTestTarget?: (credentialType: string) => EvalCredentialTestTarget | undefined;
|
||||
/** Pre-bound agent for the build-existing-agent flow. When omitted, the
|
||||
* assistant can create one via the build-agent tool. */
|
||||
agentId?: string;
|
||||
|
|
@ -532,6 +536,7 @@ export class InstanceAiAdapterService {
|
|||
getCredentialIdAllowlist,
|
||||
resumeAgentBuild = false,
|
||||
shouldBypassCredentialTest,
|
||||
getCredentialTestTarget,
|
||||
agentId,
|
||||
configEvalsEnabled,
|
||||
setupPanelVariant,
|
||||
|
|
@ -557,6 +562,7 @@ export class InstanceAiAdapterService {
|
|||
projectId,
|
||||
getCredentialIdAllowlist,
|
||||
shouldBypassCredentialTest,
|
||||
getCredentialTestTarget,
|
||||
);
|
||||
return {
|
||||
userId: user.id,
|
||||
|
|
@ -2758,6 +2764,7 @@ export class InstanceAiAdapterService {
|
|||
boundProjectId?: string,
|
||||
getCredentialIdAllowlist?: () => string[] | undefined,
|
||||
shouldBypassCredentialTest?: (credentialId: string) => boolean,
|
||||
getCredentialTestTarget?: (credentialType: string) => EvalCredentialTestTarget | undefined,
|
||||
): InstanceAiCredentialService {
|
||||
const {
|
||||
credentialsService,
|
||||
|
|
@ -2867,15 +2874,18 @@ export class InstanceAiAdapterService {
|
|||
return { success: true, message: 'Connection tested successfully' };
|
||||
}
|
||||
|
||||
const data = await credentialsService.decryptForUse(
|
||||
credential,
|
||||
{ kind: 'user', user },
|
||||
boundProjectId,
|
||||
);
|
||||
// Eval-only: aim the test at the thread's stand-in.
|
||||
const target = getCredentialTestTarget?.(credential.type);
|
||||
const credentialsToTest: ICredentialsDecrypted = {
|
||||
id: credential.id,
|
||||
name: credential.name,
|
||||
type: credential.type,
|
||||
data: await credentialsService.decryptForUse(
|
||||
credential,
|
||||
{ kind: 'user', user },
|
||||
boundProjectId,
|
||||
),
|
||||
data: target ? { ...data, [target.urlField]: target.baseUrl } : data,
|
||||
};
|
||||
|
||||
const result = await credentialsService.test(user.id, credentialsToTest);
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import {
|
|||
InstanceAiEvalExecutionRequest,
|
||||
InstanceAiEvalAgentExecutionRequest,
|
||||
InstanceAiEvalCredentialAllowlistRequest,
|
||||
InstanceAiEvalCredentialTestTargetRequest,
|
||||
InstanceAiEvalRestoreThreadRequest,
|
||||
InstanceAiEvalSeedDataTableRowsRequest,
|
||||
findSeedFolderIssues,
|
||||
|
|
@ -104,6 +105,8 @@ const KEEP_ALIVE_INTERVAL_MS = 15_000;
|
|||
export class InstanceAiController {
|
||||
private readonly gatewayApiKey: string;
|
||||
|
||||
private readonly instanceAiConfig: GlobalConfig['instanceAi'];
|
||||
|
||||
constructor(
|
||||
private readonly instanceAiService: InstanceAiService,
|
||||
private readonly gatewayService: InstanceAiGatewayService,
|
||||
|
|
@ -133,6 +136,7 @@ export class InstanceAiController {
|
|||
private readonly threadTabsService: InstanceAiThreadTabsService,
|
||||
) {
|
||||
this.gatewayApiKey = globalConfig.instanceAi.gatewayApiKey;
|
||||
this.instanceAiConfig = globalConfig.instanceAi;
|
||||
}
|
||||
|
||||
private requireInstanceAiEnabled(): void {
|
||||
|
|
@ -1208,6 +1212,29 @@ export class InstanceAiController {
|
|||
return { ok: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Send the thread's connection tests for one credential type to a provider
|
||||
* stand-in, so the agent's own test checks the captured secret.
|
||||
*/
|
||||
@Post('/eval/thread-credential-test-target')
|
||||
@GlobalScope('instanceAi:eval')
|
||||
async setThreadCredentialTestTarget(
|
||||
req: AuthenticatedRequest,
|
||||
_res: Response,
|
||||
@Body payload: InstanceAiEvalCredentialTestTargetRequest,
|
||||
) {
|
||||
this.requireInstanceAiEnabled();
|
||||
// Redirecting a credential test sends its secret elsewhere, so only a
|
||||
// throwaway eval instance offers it.
|
||||
if (!this.instanceAiConfig.evalInstance) {
|
||||
throw new NotFoundError('Credential test targets are only available on an eval instance');
|
||||
}
|
||||
await this.assertThreadAccess(req.user.id, payload.threadId);
|
||||
const { threadId, ...target } = payload;
|
||||
this.evalCredentialAllowlists.setTestTarget(threadId, target);
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Observational memory for a thread, for a context eval to assert on.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -2692,6 +2692,8 @@ export class InstanceAiService {
|
|||
getCredentialIdAllowlist: () => this.evalCredentialAllowlists.get(threadId),
|
||||
shouldBypassCredentialTest: (credentialId: string) =>
|
||||
this.evalCredentialAllowlists.shouldBypassTest(threadId, credentialId),
|
||||
getCredentialTestTarget: (credentialType: string) =>
|
||||
this.evalCredentialAllowlists.getTestTarget(threadId, credentialType),
|
||||
configEvalsEnabled,
|
||||
setupPanelVariant,
|
||||
mcpConnectionsAvailable,
|
||||
|
|
|
|||
|
|
@ -57,6 +57,13 @@ export class SlackApi implements ICredentialType {
|
|||
type: 'hidden',
|
||||
default: '',
|
||||
},
|
||||
{
|
||||
// Base URL of the connection test only, so an eval can aim the test at a stand-in. Node requests ignore it
|
||||
displayName: 'Base URL',
|
||||
name: 'url',
|
||||
type: 'hidden',
|
||||
default: 'https://slack.com',
|
||||
},
|
||||
{
|
||||
displayName:
|
||||
'We strongly recommend setting up a <a href="https://docs.n8n.io/integrations/builtin/trigger-nodes/n8n-nodes-base.slacktrigger/#verify-the-webhook" target="_blank">signing secret</a> to ensure the authenticity of requests.',
|
||||
|
|
@ -82,7 +89,7 @@ export class SlackApi implements ICredentialType {
|
|||
|
||||
test: ICredentialTestRequest = {
|
||||
request: {
|
||||
baseURL: 'https://slack.com',
|
||||
baseURL: '={{$credentials.url}}',
|
||||
url: '/api/users.profile.get',
|
||||
},
|
||||
rules: [
|
||||
|
|
|
|||
|
|
@ -0,0 +1,27 @@
|
|||
import { NodeHelpers } from 'n8n-workflow';
|
||||
|
||||
import { SlackApi } from '../SlackApi.credentials';
|
||||
|
||||
describe('SlackApi Credential', () => {
|
||||
const credential = new SlackApi();
|
||||
|
||||
it('should read the connection test base URL from the credential', () => {
|
||||
expect(credential.test.request.baseURL).toBe('={{$credentials.url}}');
|
||||
expect(credential.test.request.url).toBe('/api/users.profile.get');
|
||||
});
|
||||
|
||||
// Credentials saved before the `url` field existed store no value for it.
|
||||
// The tester fills defaults in with getNodeParameters before it resolves the request.
|
||||
it('should test a stored credential without a url against slack.com', () => {
|
||||
const data = NodeHelpers.getNodeParameters(
|
||||
credential.properties,
|
||||
{ accessToken: 'xoxb-stored' },
|
||||
true,
|
||||
false,
|
||||
null,
|
||||
null,
|
||||
);
|
||||
|
||||
expect(data).toMatchObject({ accessToken: 'xoxb-stored', url: 'https://slack.com' });
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user