diff --git a/.github/workflows/test-evals-instance-ai.yml b/.github/workflows/test-evals-instance-ai.yml index ff2af9eaefe..458a672b969 100644 --- a/.github/workflows/test-evals-instance-ai.yml +++ b/.github/workflows/test-evals-instance-ai.yml @@ -164,6 +164,7 @@ jobs: - name: Start n8n containers env: EVALS_ANTHROPIC_KEY: ${{ secrets.EVALS_ANTHROPIC_KEY }} + INSTANCE_AI_BRAVE_SEARCH_API_KEY: ${{ secrets.INSTANCE_AI_BRAVE_SEARCH_API_KEY }} N8N_LICENSE_ACTIVATION_KEY: ${{ secrets.N8N_LICENSE_ACTIVATION_KEY }} N8N_LICENSE_CERT: ${{ secrets.N8N_LICENSE_CERT }} N8N_ENCRYPTION_KEY: ${{ secrets.N8N_ENCRYPTION_KEY }} @@ -224,6 +225,7 @@ jobs: -e N8N_ENABLED_MODULES=instance-ai \ -e N8N_AI_ENABLED=true \ -e N8N_INSTANCE_AI_MODEL_API_KEY="$EVALS_ANTHROPIC_KEY" \ + -e INSTANCE_AI_BRAVE_SEARCH_API_KEY="$INSTANCE_AI_BRAVE_SEARCH_API_KEY" \ -e N8N_AI_ASSISTANT_BASE_URL="" \ -e N8N_INSTANCE_AI_SANDBOX_ENABLED=true \ "${SANDBOX_ARGS[@]}" \ diff --git a/packages/@n8n/ai-utilities/src/web-search/__tests__/brave-search.test.ts b/packages/@n8n/ai-utilities/src/web-search/__tests__/brave-search.test.ts index a0f2218a231..aee7b25de2a 100644 --- a/packages/@n8n/ai-utilities/src/web-search/__tests__/brave-search.test.ts +++ b/packages/@n8n/ai-utilities/src/web-search/__tests__/brave-search.test.ts @@ -113,16 +113,61 @@ describe('braveSearch', () => { expect(q).toBe('webhooks -site:reddit.com'); }); - it('throws on non-OK response', async () => { + it('throws on non-OK response without retrying', async () => { mockFetch.mockResolvedValue({ ok: false, - status: 429, - statusText: 'Too Many Requests', + status: 401, + statusText: 'Unauthorized', }); await expect(braveSearch('BSA-key', 'test', {})).rejects.toThrow( - 'Brave search failed: 429 Too Many Requests', + 'Brave search failed: 401 Unauthorized', ); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('retries a rate-limited request and returns the eventual success', async () => { + mockFetch + .mockResolvedValueOnce({ ok: false, status: 429, statusText: 'Too Many Requests' }) + .mockResolvedValueOnce({ ok: true, json: async () => MOCK_BRAVE_RESPONSE }); + + const result = await braveSearch('BSA-key', 'test', {}); + + expect(mockFetch).toHaveBeenCalledTimes(2); + expect(result.results).toHaveLength(2); + }); + + it('stops waiting out the retry backoff when the search is aborted', async () => { + vi.useFakeTimers(); + try { + const controller = new AbortController(); + mockFetch.mockImplementation(async (_url: string, init: RequestInit) => { + if (init.signal?.aborted) throw new Error('aborted'); + controller.abort(); + return { ok: false, status: 429, statusText: 'Too Many Requests' }; + }); + + const search = braveSearch('BSA-key', 'test', { abortSignal: controller.signal }); + + // No timer is advanced — only an abort-aware backoff lets this settle. + await expect(search).rejects.toThrow('aborted'); + expect(mockFetch).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + + it('gives up after the retry budget and throws the last status', async () => { + mockFetch.mockResolvedValue({ + ok: false, + status: 503, + statusText: 'Service Unavailable', + }); + + await expect(braveSearch('BSA-key', 'test', {})).rejects.toThrow( + 'Brave search failed: 503 Service Unavailable', + ); + expect(mockFetch).toHaveBeenCalledTimes(3); }); it('handles empty results gracefully', async () => { diff --git a/packages/@n8n/ai-utilities/src/web-search/brave-search.ts b/packages/@n8n/ai-utilities/src/web-search/brave-search.ts index 6a93a276064..c7e4008a5f0 100644 --- a/packages/@n8n/ai-utilities/src/web-search/brave-search.ts +++ b/packages/@n8n/ai-utilities/src/web-search/brave-search.ts @@ -3,6 +3,10 @@ import type { WebSearchOptions, WebSearchResponse } from './types'; const BRAVE_SEARCH_PATH = '/res/v1/web/search'; const BRAVE_SEARCH_URL = `https://api.search.brave.com${BRAVE_SEARCH_PATH}`; +/** Brave rate-limits per second — retry so a burst of searches doesn't fail the caller. */ +const MAX_ATTEMPTS = 3; +const RETRY_BASE_MS = 250; + interface BraveWebResult { title: string; url: string; @@ -60,10 +64,41 @@ export async function braveSearch( ...(proxyHeaders ?? { 'X-Subscription-Token': apiKey }), }; - const response = await fetch(`${baseUrl}?${params}`, { - headers, - ...(options.abortSignal ? { signal: options.abortSignal } : {}), - }); + const runSearch = async () => + await fetch(`${baseUrl}?${params}`, { + headers, + ...(options.abortSignal ? { signal: options.abortSignal } : {}), + }); + const isRetryable = (status: number) => status === 429 || status >= 500; + /** Abort-aware so a cancelled run doesn't sit out the delay; the retried fetch + * then rejects on the aborted signal. */ + const backoff = async (ms: number) => + await new Promise((resolve) => { + const signal = options.abortSignal; + if (signal?.aborted) { + resolve(); + return; + } + const timer = setTimeout(() => { + signal?.removeEventListener('abort', onAbort); + resolve(); + }, ms); + function onAbort() { + clearTimeout(timer); + resolve(); + } + signal?.addEventListener('abort', onAbort, { once: true }); + }); + + let response = await runSearch(); + for ( + let attempt = 1; + attempt < MAX_ATTEMPTS && !response.ok && isRetryable(response.status); + attempt++ + ) { + await backoff(RETRY_BASE_MS * 2 ** (attempt - 1)); + response = await runSearch(); + } if (!response.ok) { throw new Error(`Brave search failed: ${response.status} ${response.statusText}`); diff --git a/packages/@n8n/instance-ai/evaluations/README.md b/packages/@n8n/instance-ai/evaluations/README.md index 60b18c8a887..966b5ba12d4 100644 --- a/packages/@n8n/instance-ai/evaluations/README.md +++ b/packages/@n8n/instance-ai/evaluations/README.md @@ -306,6 +306,7 @@ Not yet covered: an automatic "unexpected artifact" fail (a build producing an a | `LANGSMITH_BRANCH` | No | Branch name to tag the experiment with (auto-set in CI) | | `CONTEXT7_API_KEY` | No | Context7 key for API-doc lookups. Improves mock realism for less-common services; the LLM falls back to training data when unset | | `N8N_AI_ASSISTANT_BASE_URL` | No | Set to `""` to bypass the hosted AI proxy and hit Anthropic directly — useful to avoid per-tenant quota during large batch runs | +| `INSTANCE_AI_BRAVE_SEARCH_API_KEY` | No | Set on the **target n8n instance** (note: no `N8N_` prefix) to enable the builder's `web-search` action. Unset = the action returns zero results, which reads to the agent as "nothing found". A licensed instance with `N8N_AI_ASSISTANT_BASE_URL` set routes search through the AI proxy instead and ignores this key | | `N8N_INSTANCE_AI_RUN_DEBUG_ENABLED` | No | Set to `true` on the target n8n instance to capture orchestrator LLM steps and workflow code for the eval LLM debug report (`workflow-eval-llm-debug.html`). Off by default. | **LangSmith caveat:** if `LANGSMITH_API_KEY` is set in `.env.local`, local runs also land in the shared `instance-ai-workflow-evals` dataset. Unset it (or run without `dotenvx`) to keep exploratory runs out of team results. @@ -714,8 +715,9 @@ Write the turns as a screenplay of what the user wants, keeping concrete values | Withhold a value until asked | `[Don't bring up the channel unless the agent asks where to post; then say 'Slack #growth.']` | | Refuse and hold firm on re-ask | `[The user has no channel and won't provide one. If asked — question or setup card, even repeatedly — skip it; never invent one.]` | | Keep the conversation going | `[After each change lands, send the next one from the list, one at a time, until done.]` | +| Refuse network access | `[Deny the web-search request — the user doesn't want it searching the web.]` | -A direction governs only what it covers; otherwise the proxy answers every question (inventing plausible placeholders) and never sets credentials. Setup cards (the "configure your workflow" card) are filled via the wizard — or dismissed when a direction withholds the value — not answered as questions. +A direction governs only what it covers; otherwise the proxy answers every question (inventing plausible placeholders) and never sets credentials. Network-access prompts (`web-search`, `fetch-url`) are the one gate that's granted **without** consulting the proxy LLM, so they cost nothing by default — but while any stage direction is still pending the decision goes to the LLM, which is what makes the refusal above reachable. Setup cards (the "configure your workflow" card) are filled via the wizard — or dismissed when a direction withholds the value — not answered as questions. **Prompt / conversation tips** diff --git a/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts b/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts index 038b08503f7..9ea93164bef 100644 --- a/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts +++ b/packages/@n8n/instance-ai/evaluations/__tests__/user-proxy.test.ts @@ -160,6 +160,25 @@ function domainAccessEvent(requestId: string): CapturedEvent { }; } +function webSearchEvent(requestId: string): CapturedEvent { + return { + timestamp: 100, + type: 'confirmation-request', + data: { + type: 'confirmation-request', + payload: { + requestId, + toolCallId: 'tc-y', + toolName: 'research', + args: {}, + severity: 'info', + message: 'n8n AI wants to search the web for: stripe webhook signing', + webSearch: { query: 'stripe webhook signing' }, + }, + }, + }; +} + function resourceDecisionEvent(requestId: string, options: string[]): CapturedEvent { return { timestamp: 100, @@ -526,6 +545,57 @@ describe('UserProxyLlm.respondToConfirmation', () => { expect(agent.callCount).toBe(0); }); + it('handles web-search events deterministically with allow_all', async () => { + const agent = new FakeAgent(); + const proxy = new UserProxyLlm({ + conversation: [{ role: 'user', text: 'go' }], + agent, + }); + + const response = await proxy.respondToConfirmation(webSearchEvent('req-search')); + expect(response.kind).toBe('domainAccessApprove'); + if (response.kind === 'domainAccessApprove') { + expect(response.domainAccessAction).toBe('allow_all'); + } + expect(agent.callCount).toBe(0); + }); + + it('defers an access gate to the LLM while a stage direction is pending, so a case can deny', async () => { + const agent = new FakeAgent(); + agent.enqueue({ action: 'respond_to_domain_access', response: 'deny' }); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'go' }, + { + role: 'user', + text: '[Refuse the web-search request — the user does not want it searching.]', + }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation(webSearchEvent('req-deny')); + + expect(agent.callCount).toBe(1); + expect(response.kind).toBe('domainAccessDeny'); + }); + + it('still grants an access gate deterministically when the pending script has no stage direction', async () => { + const agent = new FakeAgent(); + const proxy = new UserProxyLlm({ + conversation: [ + { role: 'user', text: 'go' }, + { role: 'user', text: 'also add a retry' }, + ], + agent, + }); + + const response = await proxy.respondToConfirmation(webSearchEvent('req-allow')); + + expect(response.kind).toBe('domainAccessApprove'); + expect(agent.callCount).toBe(0); + }); + it('handles resource-decision events deterministically with first allow option', async () => { const agent = new FakeAgent(); const proxy = new UserProxyLlm({ diff --git a/packages/@n8n/instance-ai/evaluations/harness/schema.ts b/packages/@n8n/instance-ai/evaluations/harness/schema.ts index 410be70c2a4..409dd061802 100644 --- a/packages/@n8n/instance-ai/evaluations/harness/schema.ts +++ b/packages/@n8n/instance-ai/evaluations/harness/schema.ts @@ -15,7 +15,15 @@ export const DEFAULT_DATASETS = ['full']; * (e.g. the mcp-manifest builder) normalize identically. */ export const conversationTurnTextSchema = z .union([z.string(), z.array(z.string())]) - .transform((t) => (Array.isArray(t) ? t.join('\n') : t)); + .transform((t) => (Array.isArray(t) ? t.join('\n') : t)) + // An unclosed `[` fails silently and expensively: the proxy stops seeing a + // stage direction, so it sends the text as dialogue and the case grades a + // conversation it was never meant to have. Easy to do in the array form, + // where the closing bracket lands on a different line from the opening one. + .refine((t) => !t.includes('[') || t.includes(']'), { + message: + 'unbalanced stage direction — text opens `[` but never closes it, so the proxy would send it as dialogue instead of treating it as a direction', + }); export const ConversationTurnSchema = z.object({ role: z.enum(['user', 'assistant']), diff --git a/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts b/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts index d959219d9c0..28ee9242542 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/confirmation-payload.ts @@ -10,9 +10,9 @@ import type { CapturedEvent } from '../types'; /** * Handle confirmation events that carry no user-intent signal — domain access, - * resource decisions, standalone credential requests. The eval grants all - * access, has no credentials, and picks the most-permissive option for - * resource gates. Returns `undefined` for events that need caller-specific + * web search, resource decisions, standalone credential requests. The eval + * grants all access, has no credentials, and picks the most-permissive option + * for resource gates. Returns `undefined` for events that need caller-specific * handling: setup wizards, ask-user questions, plan reviews. */ export function tryInfrastructureResponse( @@ -20,7 +20,8 @@ export function tryInfrastructureResponse( ): InstanceAiConfirmRequest | undefined { const payload = getNestedRecord(event.data, 'payload') ?? {}; - if (getNestedRecord(payload, 'domainAccess')) { + // Web search reuses domain access's approval shape. + if (getNestedRecord(payload, 'domainAccess') || getNestedRecord(payload, 'webSearch')) { return { kind: 'domainAccessApprove', domainAccessAction: 'allow_all' }; } diff --git a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts index 83d8c12d8a2..9f2ffc237c1 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/index.ts @@ -131,7 +131,7 @@ export class UserProxyLlm { } const det = tryDeterministicConfirmationResponse(event); - if (det) { + if (det && !this.deferAccessGateToScript(event)) { this.bumpStat('deterministic'); return this.rememberResponse(requestId, det); } @@ -261,6 +261,15 @@ export class UserProxyLlm { return this.tryScriptedConfirmationResponse(event) ?? buildAutoApprovePayload(event); } + /** Network-access gates (fetch-url domain, web search) are granted deterministically so the + * common case spends no LLM call. A pending stage direction may instruct a refusal though, + * so hand those to the LLM the way plan review already is. */ + private deferAccessGateToScript(event: CapturedEvent): boolean { + const payload = getEventPayload(event); + if (!payload.domainAccess && !payload.webSearch) return false; + return this.remainingUserScriptTurns().some((turn) => hasStageDirection(turn.text)); + } + private tryScriptedConfirmationResponse( event: CapturedEvent, ): InstanceAiConfirmRequest | undefined { diff --git a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts index f4f9478cfa9..ab1e689a256 100644 --- a/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts +++ b/packages/@n8n/instance-ai/evaluations/utils/user-proxy/tools.ts @@ -111,7 +111,7 @@ export const CONFIRMATION_TOOL_DESCRIPTIONS = `Available actions — confirmatio - approve_or_reject(approved, userInput?): A plan-review or free-text confirmation widget is on screen (the event's inputType is plan-review or text). Approve if the plan matches user intent; reject with reason if it diverges. This action only exists as a response to such a widget. -- respond_to_domain_access(response): The agent is asking for domain access permissions. Pick allow_once, allow_all, or deny. Default to allow_all unless the user would deny. +- respond_to_domain_access(response): The agent is asking permission to reach the network — either a specific domain (fetch-url) or a web search. Pick allow_once, allow_all, or deny. Default to allow_all; pick deny ONLY when a [stage direction] tells the user to refuse this kind of access. - pick_resource_decision(decision): The agent is asking the user to pick a gateway resource access option. Pick the option the user would choose.`; diff --git a/packages/@n8n/instance-ai/src/tools/__tests__/research.tool.test.ts b/packages/@n8n/instance-ai/src/tools/__tests__/research.tool.test.ts index e36f64ccac9..eda8e9c3b19 100644 --- a/packages/@n8n/instance-ai/src/tools/__tests__/research.tool.test.ts +++ b/packages/@n8n/instance-ai/src/tools/__tests__/research.tool.test.ts @@ -350,6 +350,39 @@ describe('research tool', () => { expect(tracker.approveWebSearchOnce).not.toHaveBeenCalled(); }); + // The payload both the UI's approval widget and the eval user-proxy send. + it('should grant persistent approval and run the search on resume with allow_all', async () => { + const tracker = { + isHostAllowed: vi.fn(), + approveDomain: vi.fn(), + approveAllDomains: vi.fn(), + approveOnce: vi.fn(), + isWebSearchAllowed: vi.fn().mockReturnValue(false), + approveWebSearch: vi.fn(), + approveWebSearchOnce: vi.fn(), + }; + const context = createMockContext({ domainAccessTracker: tracker as never }); + context.webResearchService!.search = vi.fn().mockResolvedValue({ + query: 'q', + results: [{ title: 'T', url: 'https://example.com/a', snippet: 'S' }], + }); + const suspend = vi.fn(); + + const tool = createResearchTool(context); + const result = (await tool.handler!( + { action: 'web-search' as const, query: 'q' }, + createAgentCtx({ + resumeData: { approved: true, domainAccessAction: 'allow_all' }, + suspend, + }) as never, + )) as { results: unknown[] }; + + expect(tracker.approveWebSearch).toHaveBeenCalled(); + expect(suspend).not.toHaveBeenCalled(); + expect(context.webResearchService!.search).toHaveBeenCalledWith('q', expect.anything()); + expect(result.results).toHaveLength(1); + }); + it('should return empty results when resumed with denial', async () => { const tracker = { isHostAllowed: vi.fn(), diff --git a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts index 48089f2d196..2591eadee84 100644 --- a/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts +++ b/packages/cli/src/modules/instance-ai/__tests__/instance-ai.adapter.service.test.ts @@ -1332,6 +1332,89 @@ function createNodeAdapterForTests( return createNodeAdapterServiceForTests(nodes, { nodeCatalogService }).nodeService; } +// --------------------------------------------------------------------------- +// Web-search provider selection +// --------------------------------------------------------------------------- + +import { braveSearch, searxngSearch } from '@n8n/ai-utilities'; + +describe('web-search provider selection', () => { + type SearchFn = (query: string, options?: Record) => Promise; + type ProxyConfig = { apiUrl: string; getAuthHeaders: () => Promise> }; + + /** `buildSearchMethod` is private, but the precedence it encodes is exactly what + * the `INSTANCE_AI_BRAVE_SEARCH_API_KEY` wiring relies on — assert it directly. */ + function buildSearch(args: { + apiKey?: string; + searxngUrl?: string; + proxyConfig?: ProxyConfig; + }): SearchFn | undefined { + const { service } = createNodeAdapterServiceForTests([]); + const cache = { get: vi.fn().mockReturnValue(undefined), set: vi.fn() }; + const withPrivate = service as unknown as { + buildSearchMethod: ( + apiKey: string, + searxngUrl: string, + cache: unknown, + proxyConfig?: ProxyConfig, + userId?: string, + ) => SearchFn | undefined; + }; + return withPrivate.buildSearchMethod.call( + service, + args.apiKey ?? '', + args.searxngUrl ?? '', + cache, + args.proxyConfig, + 'user-1', + ); + } + + beforeEach(() => { + vi.mocked(braveSearch).mockReset().mockResolvedValue({ query: 'q', results: [] }); + vi.mocked(searxngSearch).mockReset().mockResolvedValue({ query: 'q', results: [] }); + }); + + it('has no search method when neither a Brave key nor a SearXNG URL is set', () => { + // The adapter then serves `{ query, results: [] }`, which the agent cannot + // distinguish from "nothing found" — hence the key in the eval lanes. + expect(buildSearch({})).toBeUndefined(); + }); + + it('searches Brave with the configured key', async () => { + await buildSearch({ apiKey: 'BSA-key' })!('quakes', { maxResults: 3 }); + + expect(braveSearch).toHaveBeenCalledWith( + 'BSA-key', + 'quakes', + expect.objectContaining({ maxResults: 3 }), + ); + expect(searxngSearch).not.toHaveBeenCalled(); + }); + + it('routes through the AI-service proxy in preference to a configured key', async () => { + const proxyConfig: ProxyConfig = { + apiUrl: 'https://proxy.example.com/brave-search', + getAuthHeaders: async () => ({}), + }; + + await buildSearch({ apiKey: 'BSA-key', proxyConfig })!('quakes'); + + expect(braveSearch).toHaveBeenCalledWith( + '', + 'quakes', + expect.objectContaining({ proxyConfig }), + ); + }); + + it('falls back to SearXNG when only a URL is set', async () => { + await buildSearch({ searxngUrl: 'http://searxng:8080' })!('quakes'); + + expect(searxngSearch).toHaveBeenCalledWith('http://searxng:8080', 'quakes', expect.anything()); + expect(braveSearch).not.toHaveBeenCalled(); + }); +}); + describe('createNodeAdapter', () => { it('preserves credential displayOptions in getDescription()', async () => { const adapter = createNodeAdapterForTests([