feat(syncs): Expose listRecords for use in syncs by rbwest · Pull Request #5676 · NangoHQ/nango

rbwest · 2026-03-19T19:41:30Z

listRecords needs to be exposed in sync code to allow for two-layer sync for MobileFirst to reduce sync load

NAN-5073: Expose listRecords for use in syncs

In a sample integrations repo:

Point towards local copy of CLI/runner SDK
Create sync that uses newly exposed nango.listRecords function
Compile and dry run sync

Expose listRecords async iterator in sync runners

This PR introduces a new listRecords async iterator to sync execution paths, enabling scripts to iterate persisted records page-by-page. The change is wired through the runner SDK, CLI sync runner, type definitions, telemetry allowlists, and persist client request handling, with abort checks during iteration and batching.

Supporting updates include a shared fetchRecordsPage helper for getRecordsByIds, optional limit propagation to the persist API, parser/compile validations, mock updates, and expanded unit tests covering abort behavior and multi-page cursor flows.

This summary was automatically generated by @propel-code-bot

linear · 2026-03-19T19:41:34Z

NAN-5073 Expose listRecords for use in syncs

propel-code-bot

Suggested change to correct model-name validation for listRecords argument handling.

Status: Changes Suggested | Risk: Medium

Issues Identified & Suggestions

Fix model-name validation to check first argument for listRecords: packages/cli/lib/services/parser.service.ts

Review Details

📁 9 files reviewed | 💬 1 comments

Instruction Files

└── .claude/
    ├── agents/
    │   └── nango-docs-migrator.md
    └── skills/
        ├── agent-builder-skill/
        │   ├── EXAMPLES.md
        │   └── SKILL.md
        ├── creating-integration-docs/
        │   └── SKILL.md
        └── creating-skills-skill/
            └── SKILL.md

👍 / 👎 individual comments to help improve reviews for you

propel-code-bot · 2026-03-19T19:43:37Z

            'deleteRecordsFromPreviousExecutions',
-            'trackDeletesStart',
+            'getRecordsByIds',
+            'listRecords',


[Logic] The model-name validation below only runs when args.length > 1 and always checks the last argument. For listRecords(model, options?), the model is the first arg, so invalid model names will never be flagged (both when options is omitted and when it’s present).

Fix: handle listRecords explicitly (or map method → model-arg index) so the model name check uses args[0] and runs when args.length >= 1.

Context for Agents

The model-name validation below only runs when `args.length > 1` and always checks the **last** argument. For `listRecords(model, options?)`, the model is the **first** arg, so invalid model names will never be flagged (both when `options` is omitted and when it’s present). Fix: handle `listRecords` explicitly (or map method → model-arg index) so the model name check uses `args[0]` and runs when `args.length >= 1`. File: packages/cli/lib/services/parser.service.ts Line: 95

Changed the call signature to have model be the final argument like all other functions. Will update validation if the team requests it instead. Assuming the model is the final argument does feel a bit hacky 🤷

I must admit it's a weird convention. Also, it's the only required argument out of the 3 which feels weird to have to potentially have to call with (undefined, undefined, 'FooModel'). I'd prefer for it to be the first argument

Per @kaposke, this class is actually deprecated and only used for the zero yaml migration script. I've reverted the call signature back to the orginal (model: string, options?: {...}), added an @deprecated comment to this file, and skipped updating the model arg validation.

…n-syncs

TBonnin · 2026-03-20T15:18:32Z

+    listRecords<T extends object = any>(
+        cursor: string | undefined,
+        limit: number | undefined,
+        model: string


I'd pass an object like the node client equivalent function.
limit should have a default value when not provided. Maybe quite a small number to discourage any heavy processing

I originally had it that way, but there was a check in parser.service.ts that looks for the model arg in the last position per this AI note:

const modelArg = args[args.length - 1];

Want me to revert the signature to:

public async listRecords<T = any>( model: string, options?: { cursor?: string; limit?: number; } )

And update the parser to allow this?

optional cursor/limit before the model looks weird. I think we should modify the parser to allow optional params after the model.

TBonnin · 2026-03-20T15:25:26Z

+            throw res.error;
+        }
+
+        const raw = res.unwrap();


unwrap is not to be used outside of tests.
since you have an isErr() guard you can get the value with res.value.

Updated, check now!

TBonnin · 2026-03-20T15:26:55Z

+            };
+            if (cursor !== undefined) {
+                pageOptions.cursor = cursor;
            }


you can do

const pageOptions = { externalIds: Array.from(externalIdMap.keys()), ...(cursor ? { cursor } : {} ) }

Updated, check now!

TBonnin · 2026-03-20T15:27:16Z


            for (const record of records) {
-                const stringId = String(record.id);
+                const stringId = String((record as Record<string, unknown>)['id']);


do you need the casting?

It was because records were getting type T from the cast in fetchRecordsPage instead of carrying the type NangoRecord from the call to the persist client.

I've adjusted the types to be more explicit (from T[] to NangoRecord<T>[] and T = any to T=Record<string, any>) so it's no longer required. Let me know if I'm missing some issue with the new types though.

TBonnin · 2026-03-20T15:28:10Z

+        }
+        if (limit !== undefined) {
+            pageOptions.limit = limit;
+        }


same comment about using the spread operator.

Updated, check now!

TBonnin · 2026-03-20T15:42:41Z

-            'setMetadata',
-            'proxy',
-            'get',
+            'listRecords', // 1. Allow await nango.listRecords() at compile time


I think listRecords needs to be added to disallowedActionCalls. below

Updated, check now!

TBonnin · 2026-03-20T15:47:08Z

+        cursor: string | undefined,
+        limit: number | undefined,
+        model: string
+    ): Promise<{ records: T[]; next_cursor: string | null }>;


slightly unrelated question. Should we deprecate getRecordsByIds and make ids and optional field of listRecords. Since they are basically the same thing under the hood.

Yeah, I think that made sense, but wasn't sure what the process for deprecating looks like. Is there any action we want to take now, like an @deprecated tag on the function?

yes a @deprecated tag to start with. don't worry in this PR you can do it later

…arity

…n-syncs

kaposke · 2026-03-23T16:29:46Z

+        mockPersistClient.getRecords = vi.fn().mockResolvedValueOnce(Ok({ records, nextCursor: null }));
+
+        const nango = new NangoSyncRunner({ ...nangoProps }, { persistClient: mockPersistClient, locks });
+        const result = await nango.listRecords(undefined, undefined, 'SomeModel');


Yeah I'll have to agree with the other comments, this signature doesn't look good

…n-syncs

…tches

TBonnin · 2026-03-25T13:45:46Z

-    ): Promise<{ records: T[]; next_cursor: string | null }>;
+    listRecords<T extends Record<string, any> = Record<string, any>>(
+        model: string,
+        options?: { cursor?: string; }


let's keep the limit options

Hmm... I was going back and forth on this yesterday and removed it after discussing with Marcin and Agustin.

Can you explain how you would expect the limit to apply when returning an iterator? If there is some limit on the number of rows the iterator will yield, then we need to provide a nextCursor for the user to know to pickup the remaining records. If we return nextCursor, the user will have to paginate via cursor to get all records. At that point, what's the point having an iterator vs just paginating like it was originally?

TBonnin · 2026-03-25T13:51:14Z

-            ...(cursor ? { cursor } : {}),
-            ...(limit ? { limit } : {})
-        };
+        let cursor: string | undefined = options?.cursor ?? undefined;


not sure I understand what this line is achieving?

Just storing cursor from the param. ?? undefined was a rogue autocomplete I didn't catch I think. Removed, thanks!

TBonnin · 2026-03-25T13:53:06Z

+
+            for (const record of records) {
+                yield record;
+            }


should the function generate records or page of records? wdyt?
how is listRecords going to be used in the webhook debouncing use case for instance?

I thought returning a single record at a time made it easier to use the iterator. The webhook debouncing use case will want to get all records, de-dupe ids into a map, then fetch the data for every unique id in the map. I think this could work fine returning individual records or pages, but individual records felt cleaner.

TBonnin · 2026-03-25T13:54:14Z

+
+            if (!next_cursor) {
+                break;
+            }


is this break necessary. isn't the while condition be enough to exit the loop?

Yes, sorry. It's a relic from the original implementation of the loop. Changed it from while(true)...break to a do...while and forgot to remove. Fixed now.

…n-syncs

propel-code-bot

No issues found; the changes appear solid and aligned with the intended functionality.

Status: No Issues Found | Risk: Low

Review Details

📁 10 files reviewed | 💬 0 comments

Instruction Files

└── .claude/
    ├── agents/
    │   └── nango-docs-migrator.md
    └── skills

…n-syncs

rbwest added 2 commits March 17, 2026 10:45

Init

87e4f21

Cleanup and comment removal

23197dc

propel-code-bot Bot reviewed Mar 19, 2026

View reviewed changes

rbwest added 2 commits March 19, 2026 16:52

Update function signature to place model last as expected in parser

9ea6232

Update snapshot for test

93d09f5

rbwest changed the title ~~feat(syncs) - Expose listRecords for use in syncs~~ feat(syncs): Expose listRecords for use in syncs Mar 19, 2026

This comment was marked as outdated.

Sign in to view

Merge branch 'master' into ryan/nan-5073-expose-listrecords-for-use-i…

23a30b2

…n-syncs

This comment was marked as outdated.

Sign in to view

marcindobry requested a review from a team March 20, 2026 11:56

rbwest added 2 commits March 20, 2026 08:17

Fix limit check in persist client

e9249a8

Merge branch 'master' into ryan/nan-5073-expose-listrecords-for-use-i…

14a44af

…n-syncs

This comment was marked as outdated.

Sign in to view

TBonnin reviewed Mar 20, 2026

View reviewed changes

rbwest added 5 commits March 20, 2026 13:16

PR comment - remove unnecessary unwrap

3223000

PR comment - use spread

7841101

PR comment - adjust types on sync runner to avoid cast and improve cl…

44c86ca

…arity

PR comment - Remove comment and add listRecords to disallowedActionCalls

7a005aa

Merge branch 'master' into ryan/nan-5073-expose-listrecords-for-use-i…

aa06066

…n-syncs

kaposke reviewed Mar 23, 2026

View reviewed changes

rbwest added 2 commits March 24, 2026 12:04

Re-factor listRecords to return iterator and update call signature

3b020ec

Merge branch 'master' into ryan/nan-5073-expose-listrecords-for-use-i…

87a6285

…n-syncs

This comment was marked as outdated.

Sign in to view

Fix snapshot

b9f74ae

This comment was marked as outdated.

Sign in to view

PR comment - throwIfAbortedOrKilled in loop to prevent unnecessary fe…

1e2fcf1

…tches

TBonnin reviewed Mar 25, 2026

View reviewed changes

PR comments - cleanup

43fe020

TBonnin approved these changes Mar 25, 2026

View reviewed changes

rbwest added 2 commits March 25, 2026 11:02

Merge branch 'master' into ryan/nan-5073-expose-listrecords-for-use-i…

feb5e23

…n-syncs

PR comments - cleanup

977976a

This comment was marked as outdated.

Sign in to view

PR comments - add throw at top of getRecordsByIds in case of empty ids

729e949

This comment was marked as outdated.

Sign in to view

rbwest added 2 commits March 25, 2026 14:03

PR comment - missing import

151a2ad

Merge branch 'master' into ryan/nan-5073-expose-listrecords-for-use-i…

400d1f9

…n-syncs

propel-code-bot Bot reviewed Mar 25, 2026

View reviewed changes

rbwest added 2 commits March 25, 2026 14:21

Update snapshot

8fc7290

Merge branch 'master' into ryan/nan-5073-expose-listrecords-for-use-i…

42b2190

…n-syncs

rbwest added this pull request to the merge queue Mar 25, 2026

Merged via the queue into master with commit cd8294b Mar 25, 2026
25 checks passed

rbwest deleted the ryan/nan-5073-expose-listrecords-for-use-in-syncs branch March 25, 2026 22:17

rbwest self-assigned this May 6, 2026

Conversation

rbwest commented Mar 19, 2026 • edited by propel-code-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear Bot commented Mar 19, 2026

Uh oh!

propel-code-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

propel-code-bot Bot Mar 19, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rbwest Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rbwest Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rbwest commented Mar 19, 2026 •

edited by propel-code-bot Bot

Loading

rbwest Mar 20, 2026 •

edited

Loading

rbwest Mar 20, 2026 •

edited

Loading