From 0932e9572e2650165b260fc591d1f02204c17aae Mon Sep 17 00:00:00 2001 From: Stephen Wright Date: Tue, 6 Oct 2026 13:46:39 +0000 Subject: [PATCH] feat(Databricks Node): Guard the Vector Store insert against the wrong index (#39699) --- .../DatabricksVectorStore.test.ts | 34 +++++++-- .../DatabricksVectorStore.ts | 37 ++++++++-- .../VectorStoreDatabricks.node.test.ts | 73 ++++++++++++++++++- .../VectorStoreDatabricks.node.ts | 14 ++-- 4 files changed, 137 insertions(+), 21 deletions(-) diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.test.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.test.ts index cf2bf247779..c62daae56dd 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.test.ts @@ -460,16 +460,38 @@ describe('DatabricksVectorStore', () => { ).rejects.toThrow('Failed primary keys: row-7'); }); - it('rejects the primary key as the content column without a request', async () => { - const store = await createStore(directDescribe, { contentColumn: 'id' }); + it.each([ + ['txet', 'Index cat.sch.idx has no column txet. Select a Content Column from: text, source'], + ['id', 'Column id is the primary key of cat.sch.idx. Select another Content Column'], + [ + 'embedding', + 'Column embedding holds the embedding vector of cat.sch.idx. Select another Content Column', + ], + ])('rejects %s as the content column before embedding anything', async (column, message) => { + const store = await createStore(directDescribe, { contentColumn: column }); await expect(store.addDocuments([{ pageContent: 'hello', metadata: {} }])).rejects.toThrow( - 'primary key', + message, ); - expect(fetchMock).not.toHaveBeenCalledWith( - expect.stringContaining('upsert-data'), - expect.anything(), + expect(embeddings.embedDocuments).not.toHaveBeenCalled(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('rejects an index that declares an empty schema, rather than letting Databricks reject it', async () => { + const emptySchema = { + ...directDescribe, + direct_access_index_spec: { + ...directDescribe.direct_access_index_spec, + schema_json: '{}', + }, + }; + const store = await createStore(emptySchema, { contentColumn: 'text' }); + + await expect(store.addDocuments([{ pageContent: 'hello', metadata: {} }])).rejects.toThrow( + 'Index cat.sch.idx has no column text, and declares no other column to select', ); + expect(embeddings.embedDocuments).not.toHaveBeenCalled(); + expect(fetchMock).not.toHaveBeenCalled(); }); it('rejects a managed Delta Sync index without a request', async () => { diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.ts index 897f9ebc6cb..b303dfbf024 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/DatabricksVectorStore.ts @@ -299,7 +299,7 @@ export class DatabricksVectorStore extends VectorStore { options?: { ids?: string[] }, ): Promise { if (documents.length === 0) return []; - this.assertDirectAccess(); + this.assertWritableContentColumn(); const vectors = await this.embeddings.embedDocuments(documents.map((doc) => doc.pageContent)); return await this.addVectors(vectors, documents, options); } @@ -309,14 +309,8 @@ export class DatabricksVectorStore extends VectorStore { documents: DocumentInterface[], options?: { ids?: string[] }, ): Promise { - const vectorColumn = this.assertDirectAccess(); + const vectorColumn = this.assertWritableContentColumn(); const { primaryKey, name } = this.index; - // The row below would write the document text over its own primary key - if (this.contentColumn === primaryKey) { - throw new UserError( - `Column ${primaryKey} is the primary key of ${name}. Select another content column`, - ); - } const schemaColumns = new Set(this.index.schemaColumns ?? []); const ids = documents.map((doc, i) => options?.ids?.[i] ?? doc.id ?? randomUUID()); @@ -347,6 +341,33 @@ export class DatabricksVectorStore extends VectorStore { return ids; } + /** Both writes below would land on a column the row already fills from somewhere else. */ + private assertWritableContentColumn(): string { + const vectorColumn = this.assertDirectAccess(); + const { primaryKey, name, schemaColumns } = this.index; + if (this.contentColumn === primaryKey) { + throw new UserError( + `Column ${primaryKey} is the primary key of ${name}. Select another Content Column`, + ); + } + if (this.contentColumn === vectorColumn) { + throw new UserError( + `Column ${vectorColumn} holds the embedding vector of ${name}. Select another Content Column`, + ); + } + // An index that declares an empty schema is known, not unknown: letting it + // through trades this message for a raw Databricks schema error. + if (schemaColumns !== undefined && !schemaColumns.includes(this.contentColumn)) { + const selectable = schemaColumns.filter((column) => !this.reserved.has(column)); + throw new UserError( + selectable.length > 0 + ? `Index ${name} has no column ${this.contentColumn}. Select a Content Column from: ${selectable.join(', ')}` + : `Index ${name} has no column ${this.contentColumn}, and declares no other column to select`, + ); + } + return vectorColumn; + } + // Databricks enforces `index_type` here; a Delta Sync index rejects upsert-data even when it holds its own vectors private assertDirectAccess(): string { const { indexType, embedding, name } = this.index; diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.test.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.test.ts index 1dd4e6b867c..c5a22b3ce73 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.test.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.test.ts @@ -1,6 +1,7 @@ /* eslint-disable n8n-nodes-base/node-param-display-name-miscased */ /* eslint-disable n8n-nodes-base/node-param-display-name-miscased-id */ import type { Embeddings } from '@langchain/core/embeddings'; +import type { Tool } from '@langchain/core/tools'; import { proxyFetch } from '@n8n/ai-utilities'; import { DATABRICKS_PARTNER_USER_AGENT } from 'n8n-nodes-base/dist/nodes/Databricks/constants'; import { createMockExecuteFunction } from 'n8n-nodes-base/test/nodes/Helpers'; @@ -327,6 +328,60 @@ describe('VectorStoreDatabricks', () => { }); }); + describe('supplyData in retrieve-as-tool mode', () => { + // A managed-embedding index rejects query_vector, so the agent's query has to reach the + // store as text. The factory picks that path from this node's `searchByText`. + it('searches the agent query as text and never embeds it', async () => { + const store = { similaritySearchWithScore: vi.fn().mockResolvedValue([]) }; + mockedFromExistingIndex.mockResolvedValue(store as unknown as DatabricksVectorStore); + const ctx = setupContext({ + ...baseParams, + mode: 'retrieve-as-tool', + toolDescription: 'Company policies. Use for any policy question.', + topK: 3, + }); + ctx.addInputData = vi.fn().mockReturnValue({ index: 0 }); + ctx.addOutputData = vi.fn(); + + const { response } = await node.supplyData.call(ctx, 0); + await (response as Tool).invoke({ input: 'what is the leave policy' }); + + expect(store.similaritySearchWithScore).toHaveBeenCalledWith( + 'what is the leave policy', + 3, + undefined, + ); + expect(embeddings.embedQuery).not.toHaveBeenCalled(); + }); + + // An agent-driven search must use the node's own configuration. The community package + // drops the mode and the filter on this path, which is the regression this pins. + it('carries the configured search mode, metadata filter and limit into the tool search', async () => { + const store = { similaritySearchWithScore: vi.fn().mockResolvedValue([]) }; + mockedFromExistingIndex.mockResolvedValue(store as unknown as DatabricksVectorStore); + const ctx = setupContext({ + ...baseParams, + mode: 'retrieve-as-tool', + toolDescription: 'Company policies. Use for any policy question.', + topK: 3, + options: { searchMode: 'HYBRID', searchFilterJson: { source: 'hr' } }, + }); + ctx.addInputData = vi.fn().mockReturnValue({ index: 0 }); + ctx.addOutputData = vi.fn(); + + const { response } = await node.supplyData.call(ctx, 0); + await (response as Tool).invoke({ input: 'what is the leave policy' }); + + expect(mockedFromExistingIndex).toHaveBeenCalledWith( + embeddings, + expect.objectContaining({ queryType: 'HYBRID' }), + ); + expect(store.similaritySearchWithScore).toHaveBeenCalledWith('what is the leave policy', 3, { + source: 'hr', + }); + }); + }); + describe('execute in insert mode', () => { it('adds the loaded documents through the store', async () => { const documents = [ @@ -369,7 +424,7 @@ describe('VectorStoreDatabricks', () => { let httpRequestWithAuthentication: ReturnType; let ctx: ILoadOptionsFunctions; - const setupSearchContext = (host: string) => { + const setupSearchContext = (host: string, mode = 'load') => { httpRequestWithAuthentication = vi.fn( async (_type: string, options: { url: string; qs: Record }) => options.url.endsWith('/endpoints') @@ -378,6 +433,7 @@ describe('VectorStoreDatabricks', () => { ); ctx = { getCredentials: vi.fn().mockResolvedValue({ ...mockCredential, host }), + getCurrentNodeParameter: vi.fn((path: string) => (path === 'mode' ? mode : undefined)), getNode: vi.fn().mockReturnValue(nodeDef), helpers: { httpRequestWithAuthentication }, } as unknown as ILoadOptionsFunctions; @@ -401,6 +457,21 @@ describe('VectorStoreDatabricks', () => { } }); + // Only insert writes, so only insert may narrow the list; the read modes query both types. + it.each([ + ['insert', ['cat.sch.zeta']], + ['load', ['cat.sch.alpha', 'cat.sch.beta', 'cat.sch.zeta']], + ['retrieve', ['cat.sch.alpha', 'cat.sch.beta', 'cat.sch.zeta']], + ['retrieve-as-tool', ['cat.sch.alpha', 'cat.sch.beta', 'cat.sch.zeta']], + ])('offers the writable indexes in %s mode', async (mode, expected) => { + setupSearchContext('https://ws.example.com/', mode); + + const result = await methods.listSearch.searchIndexes.call(ctx); + + expect(result.results.map((entry) => entry.value)).toEqual(expected); + expect(ctx.getCurrentNodeParameter).toHaveBeenCalledWith('mode'); + }); + it('does not let the bearer follow a cross-origin redirect', async () => { setupSearchContext('https://ws.example.com'); diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.ts index 53c12f34e6a..290b7416732 100644 --- a/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/vector_store/VectorStoreDatabricks/VectorStoreDatabricks.node.ts @@ -231,6 +231,7 @@ async function searchIndexes( this: ILoadOptionsFunctions, filter?: string, ): Promise { + const insertOnly = this.getCurrentNodeParameter('mode') === 'insert'; const credentials = await this.getCredentials( DATABRICKS_CREDENTIAL_TYPE, ); @@ -252,12 +253,13 @@ async function searchIndexes( { endpoint_name: endpoint.name }, (page: ListPage) => page.vector_indexes, ); - // The type tells the user which indexes accept inserts - return indexes.map((index) => ({ - name: index.name, - value: index.name, - description: `${index.index_type} - ${endpoint.name}`, - })); + return indexes + .filter((index) => !insertOnly || index.index_type === 'DIRECT_ACCESS') + .map((index) => ({ + name: index.name, + value: index.name, + description: `${index.index_type} - ${endpoint.name}`, + })); }), ) )