From cf861ebf5a61305faede47af5d5a46e090a65e3f Mon Sep 17 00:00:00 2001 From: Himanshu-Sangshetti Date: Mon, 20 Jul 2026 11:55:23 +0530 Subject: [PATCH] fix(integrations): address review feedback on Zapier and n8n Zapier: - Bound the poll budget under Zapier's step timeout and default Wait for Completion off; timeout message notes the add likely still succeeded - Add offline unit tests (mocked z.request) and run them in CI - Expose a Page input on Get Memories - Encode memory_id in the delete path - Update E2E to test the default no-wait path (add then search-retry) n8n: - Clarify the Infer description (controls extraction, not waiting) - Add an App ID field and require at least one entity id before add - Pass itemIndex into pollEvent errors - Unwrap event results so wait-path output matches search and get many --- .github/workflows/zapier-mem0-checks.yml | 10 +- .../n8n-nodes-mem0/nodes/Mem0/Mem0.node.ts | 54 +++++--- integrations/zapier-mem0/README.md | 4 +- .../zapier-mem0/creates/add_memory.js | 20 ++- .../zapier-mem0/creates/delete_memory.js | 4 +- integrations/zapier-mem0/package.json | 3 +- .../zapier-mem0/searches/get_memories.js | 20 ++- integrations/zapier-mem0/test/mem0.test.js | 31 +++-- integrations/zapier-mem0/test/unit.test.js | 124 ++++++++++++++++++ 9 files changed, 231 insertions(+), 39 deletions(-) create mode 100644 integrations/zapier-mem0/test/unit.test.js diff --git a/.github/workflows/zapier-mem0-checks.yml b/.github/workflows/zapier-mem0-checks.yml index 232ba4a9f..b8a6ffc0f 100644 --- a/.github/workflows/zapier-mem0-checks.yml +++ b/.github/workflows/zapier-mem0-checks.yml @@ -3,9 +3,10 @@ name: zapier-mem0 checks # On PRs this is invoked by ci-gate.yml (the single required check); # push-to-main and manual runs remain standalone. # -# CI runs `zapier validate` (offline schema + style checks). The end-to-end -# jest suite is skipped here because it hits the live Mem0 API — it runs -# locally with MEM0_API_KEY set (see the package README). +# CI runs `zapier validate` (offline schema + style checks) plus the offline +# jest unit suite (test/unit.test.js — mocked z.request, no network). The +# end-to-end jest suite is skipped here because it hits the live Mem0 API — it +# runs locally with MEM0_API_KEY set (see the package README). on: workflow_dispatch: push: @@ -38,3 +39,6 @@ jobs: - name: Validate Zapier app definition run: cd integrations/zapier-mem0 && npx zapier-platform-cli@19 validate + + - name: Run offline unit tests + run: cd integrations/zapier-mem0 && pnpm test:unit diff --git a/integrations/n8n-nodes-mem0/nodes/Mem0/Mem0.node.ts b/integrations/n8n-nodes-mem0/nodes/Mem0/Mem0.node.ts index bb61d77c0..d3ff9a0d5 100644 --- a/integrations/n8n-nodes-mem0/nodes/Mem0/Mem0.node.ts +++ b/integrations/n8n-nodes-mem0/nodes/Mem0/Mem0.node.ts @@ -163,24 +163,31 @@ export class Mem0 implements INodeType { default: '', }, { - displayName: 'Run ID', - name: 'run_id', + displayName: 'App ID', + name: 'app_id', type: 'string', default: '', }, - { - displayName: 'Metadata (JSON)', - name: 'metadata', - type: 'json', - default: '', - }, { displayName: 'Infer', name: 'infer', type: 'boolean', default: true, description: - 'Whether to run LLM extraction (async). Turn off to store messages verbatim (synchronous).', + 'Whether to run LLM extraction over the messages. Turn off to store them verbatim. ' + + 'This controls extraction only — use "Wait for Completion" to control whether the node waits.', + }, + { + displayName: 'Metadata (JSON)', + name: 'metadata', + type: 'json', + default: '', + }, + { + displayName: 'Run ID', + name: 'run_id', + type: 'string', + default: '', }, ], }, @@ -315,6 +322,7 @@ export class Mem0 implements INodeType { const userId = this.getNodeParameter('userId', i, '') as string; if (userId) body.user_id = userId; if (addFields.agent_id) body.agent_id = addFields.agent_id; + if (addFields.app_id) body.app_id = addFields.app_id; if (addFields.run_id) body.run_id = addFields.run_id; if (addFields.metadata) { try { @@ -329,14 +337,23 @@ export class Mem0 implements INodeType { } } + // API requires at least one entity id — fail clearly instead of a raw 4xx. + if (!body.user_id && !body.agent_id && !body.run_id && !body.app_id) { + throw new NodeOperationError( + this.getNode(), + 'Add requires at least one of User ID, Agent ID, Run ID, or App ID', + { itemIndex: i }, + ); + } + const addResp = await request('POST', '/v3/memories/add/', body); const waitForCompletion = this.getNodeParameter('waitForCompletion', i, true) as boolean; const addStatus = addResp.status as string | undefined; const isTerminal = addStatus === 'SUCCEEDED' || addStatus === 'FAILED'; - // infer=true returns {event_id, status:PENDING|RUNNING}; poll until terminal. + // Add returns {event_id, status:PENDING|RUNNING}; poll until terminal when asked to wait. if (waitForCompletion && addResp.event_id && !isTerminal) { - responseData = await pollEvent(request, addResp.event_id as string, this); + responseData = await pollEvent(request, addResp.event_id as string, this, i); } else if (addStatus === 'FAILED') { throw new NodeOperationError( this.getNode(), @@ -344,7 +361,7 @@ export class Mem0 implements INodeType { { itemIndex: i }, ); } else { - // Sync path (infer=false) returns {status:SUCCEEDED, results:[...]}; unwrap for consistency. + // If the response is already terminal, unwrap results; otherwise return as-is. responseData = Array.isArray(addResp.results) ? (addResp.results as IDataObject[]) : addResp; @@ -419,21 +436,28 @@ async function pollEvent( request: (m: IHttpRequestMethods, u: string) => Promise, eventId: string, ctx: IExecuteFunctions, -): Promise { + itemIndex: number, +): Promise { for (let attempt = 0; attempt < MAX_POLL_ATTEMPTS; attempt++) { const event = await request('GET', `/v1/event/${eventId}/`); const status = event.status as string; if (status === 'SUCCEEDED') { - return event; + // Match the shape of search/getAll (a clean array); fall back to the envelope. + return Array.isArray(event.results) ? (event.results as IDataObject[]) : event; } if (status === 'FAILED') { const reason = (event.error as string) || (event.message as string) || 'unknown error'; - throw new NodeOperationError(ctx.getNode(), `Mem0 memory event ${eventId} failed: ${reason}`); + throw new NodeOperationError( + ctx.getNode(), + `Mem0 memory event ${eventId} failed: ${reason}`, + { itemIndex }, + ); } await sleep(POLL_INTERVAL_MS); } throw new NodeOperationError( ctx.getNode(), `Timed out waiting for memory event ${eventId} to complete`, + { itemIndex }, ); } diff --git a/integrations/zapier-mem0/README.md b/integrations/zapier-mem0/README.md index 31d0275c4..445603244 100644 --- a/integrations/zapier-mem0/README.md +++ b/integrations/zapier-mem0/README.md @@ -13,7 +13,9 @@ Built with the [Zapier Platform CLI](https://docs.zapier.com/platform/quickstart | Search | **Search Memories** | `POST /v3/memories/search/` | | Search | **Get Memories** | `POST /v3/memories/` | -**Add Memory** runs LLM extraction asynchronously by default; the action polls the event until it completes and returns the resulting memories. Turn off **Wait for Completion** to return immediately, or set **Infer = false** to store verbatim (synchronous). +**Add Memory** runs LLM extraction asynchronously and returns immediately with an event ID by default. Turn on **Wait for Completion** to have the action poll until extraction finishes and return the resulting memories — note that extraction can take longer than Zapier allows a single step to run, and a timeout there does **not** mean the add failed (it typically still completes server-side). Set **Infer = false** to store the message verbatim instead of extracting. + +**Get Memories** returns one page at a time; use the **Page** and **Limit** fields to page through larger result sets. ## Authentication diff --git a/integrations/zapier-mem0/creates/add_memory.js b/integrations/zapier-mem0/creates/add_memory.js index 081713870..6dc8e4f92 100644 --- a/integrations/zapier-mem0/creates/add_memory.js +++ b/integrations/zapier-mem0/creates/add_memory.js @@ -1,7 +1,8 @@ 'use strict'; const POLL_INTERVAL_MS = 1500; -const MAX_POLL_ATTEMPTS = 40; // ~60s +// Bounded so the poll budget stays under Zapier's per-step execution timeout. +const MAX_POLL_ATTEMPTS = 12; // Polls GET /v1/event/{id}/ until the async memory-addition event resolves. const pollEvent = async (z, eventId) => { @@ -18,7 +19,8 @@ const pollEvent = async (z, eventId) => { await new Promise((resolve) => setTimeout(resolve, POLL_INTERVAL_MS)); } throw new z.errors.Error( - `Timed out waiting for memory event ${eventId} to complete`, + `Timed out waiting for memory event ${eventId}. The add was accepted and is ` + + `likely still completing on the server — a timeout here does not mean it failed.`, 'Mem0Timeout', 408, ); @@ -28,7 +30,8 @@ const perform = async (z, bundle) => { // Zapier boolean fields can arrive as the strings 'true'/'false'; coerce // explicitly so "Infer = No" / "Wait = No" are honored. const infer = String(bundle.inputData.infer) !== 'false'; - const wait = String(bundle.inputData.waitForCompletion) !== 'false'; + // Waiting is opt-in (the poll path can exceed Zapier's step timeout). + const wait = String(bundle.inputData.waitForCompletion) === 'true'; const body = { messages: [{ role: bundle.inputData.role || 'user', content: bundle.inputData.content }], @@ -56,7 +59,7 @@ const perform = async (z, bundle) => { const data = response.data; - // infer=true returns {event_id, status:PENDING|RUNNING}; optionally wait. + // Add returns {event_id, status:PENDING|RUNNING}; poll only when opted in. if (wait && data.event_id && data.status !== 'SUCCEEDED' && data.status !== 'FAILED') { return pollEvent(z, data.event_id); } @@ -99,14 +102,17 @@ module.exports = { label: 'Infer', type: 'boolean', default: 'true', - helpText: 'Run LLM extraction (async). Turn off to store verbatim (synchronous).', + helpText: 'Run LLM extraction over the message. Turn off to store it verbatim.', }, { key: 'waitForCompletion', label: 'Wait for Completion', type: 'boolean', - default: 'true', - helpText: 'Poll until extraction finishes and return the resulting memories.', + default: 'false', + helpText: + 'Poll until extraction finishes and return the resulting memories. ' + + 'Leave off (default) to return immediately with an event ID — extraction can take ' + + 'longer than Zapier allows this step to run, and a timeout does not mean the add failed.', }, ], sample: { status: 'SUCCEEDED', event_id: '00000000-0000-0000-0000-000000000000', results: [] }, diff --git a/integrations/zapier-mem0/creates/delete_memory.js b/integrations/zapier-mem0/creates/delete_memory.js index db7e2d3f4..39771bb26 100644 --- a/integrations/zapier-mem0/creates/delete_memory.js +++ b/integrations/zapier-mem0/creates/delete_memory.js @@ -1,9 +1,9 @@ 'use strict'; const perform = async (z, bundle) => { - // Trailing slash required (Django APPEND_SLASH). + // Trailing slash required (Django APPEND_SLASH); id encoded so a stray slash can't mistarget the path. const response = await z.request({ - url: `/v1/memories/${bundle.inputData.memory_id}/`, + url: `/v1/memories/${encodeURIComponent(bundle.inputData.memory_id)}/`, method: 'DELETE', }); return response.data || { message: 'Deleted', memory_id: bundle.inputData.memory_id }; diff --git a/integrations/zapier-mem0/package.json b/integrations/zapier-mem0/package.json index b7c917cae..9a777c7d7 100644 --- a/integrations/zapier-mem0/package.json +++ b/integrations/zapier-mem0/package.json @@ -22,7 +22,8 @@ "license": "MIT", "main": "index.js", "scripts": { - "test": "jest --testTimeout 120000" + "test": "jest --testTimeout 120000", + "test:unit": "jest test/unit.test.js" }, "engines": { "node": ">=18", diff --git a/integrations/zapier-mem0/searches/get_memories.js b/integrations/zapier-mem0/searches/get_memories.js index d3a3f12cb..fdea67679 100644 --- a/integrations/zapier-mem0/searches/get_memories.js +++ b/integrations/zapier-mem0/searches/get_memories.js @@ -7,7 +7,10 @@ const perform = async (z, bundle) => { const response = await z.request({ url: '/v3/memories/', method: 'POST', - params: { page: 1, page_size: Math.max(1, Math.floor(Number(bundle.inputData.limit) || 50)) }, + params: { + page: Math.max(1, Math.floor(Number(bundle.inputData.page) || 1)), + page_size: Math.max(1, Math.floor(Number(bundle.inputData.limit) || 50)), + }, body, }); const data = response.data; @@ -25,7 +28,20 @@ module.exports = { perform, inputFields: [ { key: 'user_id', label: 'User ID', type: 'string', required: true }, - { key: 'limit', label: 'Limit', type: 'integer', default: '50' }, + { + key: 'limit', + label: 'Limit', + type: 'integer', + default: '50', + helpText: 'Max memories per page. Use Page to page through larger result sets.', + }, + { + key: 'page', + label: 'Page', + type: 'integer', + default: '1', + helpText: 'Which page of results to return (1-based).', + }, ], sample: { id: '00000000-0000-0000-0000-000000000000', memory: 'User loves hiking' }, }, diff --git a/integrations/zapier-mem0/test/mem0.test.js b/integrations/zapier-mem0/test/mem0.test.js index 14a0e0115..282ee2fdd 100644 --- a/integrations/zapier-mem0/test/mem0.test.js +++ b/integrations/zapier-mem0/test/mem0.test.js @@ -14,6 +14,18 @@ const authData = { const userId = `zapier-e2e-${Date.now()}`; +// Retry an async op until `done` is satisfied or attempts run out. Extraction is +// async, so the default Add returns before the memory is searchable. +const until = async (fn, done, { attempts = 30, delayMs = 2000 } = {}) => { + let last; + for (let i = 0; i < attempts; i++) { + last = await fn(); + if (done(last)) return last; + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } + return last; +}; + // The E2E suite hits the live Mem0 API, so it only runs when MEM0_API_KEY is // set (locally / with a secret). In CI without a key it is skipped, not failed. const describeE2E = authData.apiKey ? describe : describe.skip; @@ -26,22 +38,25 @@ describeE2E('Mem0 Zapier integration (E2E)', () => { }); it('adds, searches, lists, and deletes a memory', async () => { - // Add (async, wait for completion) + // Add via the default path: returns immediately with an event id. const added = await appTester(App.creates.add_memory.operation.perform, { authData, inputData: { content: 'I love hiking in the Alps and my favorite food is sushi', user_id: userId, - waitForCompletion: true, }, }); - expect(added.status).toBe('SUCCEEDED'); + expect(added.event_id).toBeDefined(); - // Search - const found = await appTester(App.searches.search_memories.operation.perform, { - authData, - inputData: { query: 'outdoor activities', user_id: userId, limit: 5 }, - }); + // Extraction is async; retry search until the memory is indexed. + const found = await until( + () => + appTester(App.searches.search_memories.operation.perform, { + authData, + inputData: { query: 'outdoor activities', user_id: userId, limit: 5 }, + }), + (r) => Array.isArray(r) && r.length > 0, + ); expect(Array.isArray(found)).toBe(true); expect(found.length).toBeGreaterThan(0); diff --git a/integrations/zapier-mem0/test/unit.test.js b/integrations/zapier-mem0/test/unit.test.js new file mode 100644 index 000000000..1b7945f8f --- /dev/null +++ b/integrations/zapier-mem0/test/unit.test.js @@ -0,0 +1,124 @@ +'use strict'; + +/* global describe, it, expect */ + +// Offline unit tests: they mock `z.request`, so they run unconditionally in CI +// (unlike the live E2E suite in mem0.test.js, gated on MEM0_API_KEY). They cover +// what `zapier validate` can't: boolean coercion, URL join, metadata, array shapes. + +const addMemory = require('../creates/add_memory'); +const deleteMemory = require('../creates/delete_memory'); +const searchMemories = require('../searches/search_memories'); +const getMemories = require('../searches/get_memories'); +const { includeApiKey } = require('../middleware'); + +// Minimal `z` stub: hands back queued responses and records every request. +const makeZ = (responses = []) => { + const queue = [...responses]; + const requests = []; + return { + requests, + request: async (opts) => { + requests.push(opts); + const next = queue.shift(); + return next !== undefined ? next : { data: {} }; + }, + errors: { + Error: class Mem0Error extends Error { + constructor(message, name, status) { + super(message); + this.name = name || 'Error'; + this.status = status; + } + }, + }, + }; +}; + +describe('add_memory (offline)', () => { + it('coerces infer="false" to a boolean and does not poll by default', async () => { + const z = makeZ([{ data: { event_id: 'e1', status: 'PENDING' } }]); + const res = await addMemory.operation.perform(z, { + inputData: { content: 'hi', user_id: 'u1', infer: 'false' }, + }); + // waitForCompletion defaults off -> a single request (the add), no poll. + expect(z.requests).toHaveLength(1); + expect(z.requests[0].body.infer).toBe(false); + expect(res.status).toBe('PENDING'); + }); + + it('polls the event only when waitForCompletion="true"', async () => { + const z = makeZ([ + { data: { event_id: 'e1', status: 'PENDING' } }, + { data: { status: 'SUCCEEDED', results: [{ id: 'm1' }] } }, + ]); + const res = await addMemory.operation.perform(z, { + inputData: { content: 'hi', user_id: 'u1', waitForCompletion: 'true' }, + }); + expect(z.requests).toHaveLength(2); + expect(z.requests[1].url).toBe('/v1/event/e1/'); + expect(res.status).toBe('SUCCEEDED'); + }); + + it('throws a clear error on invalid JSON metadata', async () => { + const z = makeZ(); + await expect( + addMemory.operation.perform(z, { + inputData: { content: 'hi', user_id: 'u1', metadata: '{not json' }, + }), + ).rejects.toThrow('Metadata must be valid JSON.'); + }); +}); + +describe('search / get array-shape enforcement (offline)', () => { + it('search unwraps an object {results:[...]} into an array', async () => { + const z = makeZ([{ data: { results: [{ id: 'm1' }] } }]); + const res = await searchMemories.operation.perform(z, { + inputData: { query: 'x', user_id: 'u1' }, + }); + expect(Array.isArray(res)).toBe(true); + expect(res).toHaveLength(1); + }); + + it('get_memories returns [] when the API returns neither array nor results', async () => { + const z = makeZ([{ data: {} }]); + const res = await getMemories.operation.perform(z, { inputData: { user_id: 'u1' } }); + expect(Array.isArray(res)).toBe(true); + expect(res).toHaveLength(0); + }); + + it('get_memories forwards page and page_size as numbers', async () => { + const z = makeZ([{ data: { results: [] } }]); + await getMemories.operation.perform(z, { + inputData: { user_id: 'u1', page: '2', limit: '10' }, + }); + expect(z.requests[0].params).toEqual({ page: 2, page_size: 10 }); + }); +}); + +describe('delete_memory (offline)', () => { + it('encodes the memory id in the URL path', async () => { + const z = makeZ([{ data: {} }]); + await deleteMemory.operation.perform(z, { inputData: { memory_id: 'a/b c' } }); + expect(z.requests[0].url).toBe('/v1/memories/a%2Fb%20c/'); + }); +}); + +describe('includeApiKey middleware (offline)', () => { + it('prepends the base URL and injects the auth header', () => { + const req = includeApiKey( + { url: '/v3/memories/' }, + null, + { authData: { apiKey: 'k', baseUrl: 'https://api.mem0.ai/' } }, + ); + expect(req.url).toBe('https://api.mem0.ai/v3/memories/'); + expect(req.headers.Authorization).toBe('Token k'); + }); + + it('leaves absolute URLs untouched', () => { + const req = includeApiKey({ url: 'https://other.example/x' }, null, { + authData: { apiKey: 'k' }, + }); + expect(req.url).toBe('https://other.example/x'); + }); +});