Skip to content

feat(list-conversations): paginate and find conversations by participant - #38

Open
rainsjm wants to merge 4 commits into
mainfrom
feat/paginate-list-conversations
Open

feat(list-conversations): paginate and find conversations by participant#38
rainsjm wants to merge 4 commits into
mainfrom
feat/paginate-list-conversations

Conversation

@rainsjm

@rainsjm rainsjm commented Aug 11, 2026

Copy link
Copy Markdown

The problem

While troubleshooting an automation where I wanted it to DM myself, I realized that it couldn't find that conversation because the tool didn't have access to my full list of DMs. list-conversations returned one unpaginated page — about 20 conversations. Tested against a real workspace with 239 conversations, only 20 were visible. The other 92% could not be reached by any caller.

What this enables

  • Reach every conversation, not the first 20. Standard limit + cursor, matching search-content and get-mentions.
  • Find a conversation by who's in it. Ask for the DM with two specific people and get that one conversation — no scanning, no guessing. An empty userIds finds the conversation with only you.
  • One call, not a paging loop. Participant lookups walk the pages internally, so callers don't page and decide when to stop. That decision is where automations went wrong.

Notes for review

  • Fixes archived handling. includeArchived: false was leaking archived conversations into the default listing, and includeArchived: true was returning every archived conversation twice. Verified against a live workspace: 237 active + 2 archived = 239 combined.
  • A partial result never passes as a complete one. If the cursor stalls or the scan runs long, it throws. Silent truncation is what caused the original bug, so it's treated as a failure rather than a result.
  • Not covered: no workspace on hand exceeds one page, so multi-page cursor advancement is tested against mocks only.
  • Not included: keyword search. This filters by participant, not content — the API has no text search for conversations, and message content is already served by search-content.

Also bumps @doist/comms-sdk 0.9.0 → 1.0.1, which is where the pagination parameters live. Its only breaking change is Node >= 24, already required here.

🤖 Generated with Claude Code

rainsjm and others added 2 commits August 10, 2026 21:48
0.9.0's `getConversations` accepts only `{workspaceId, archived}`, so
`list-conversations` could never page past the server's default window.
1.0.x adds `limit` and the compound `(olderThan, beforeId)` cursor, and
converts `olderThan` to `older_than_ts` internally rather than letting a
Date reach the generic snake-casing.

The only breaking change in 1.0.0 is `node >=24` / `npm >=11`, which this
repo already requires (CI runs 24 and 26).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tool fetched a single unpaginated page, so it could only ever see the
first ~20 conversations in a workspace. Anything further down was
invisible, and the short list was indistinguishable from a complete one —
an automation looking for a specific DM would silently conclude it did not
exist.

Adds the repo's standard `limit` (1..100, default 50) and an opaque
`cursor`, returning `hasMore` alongside them, matching the contract
search-content and get-mentions already use. The server's cursor is the
compound (lastActive, id) of the last row; that is base64-encoded into a
single string so callers never assemble it by hand.

Adds `userIds` + `matchMode` to look a conversation up by who is in it.
`exact` (the default) matches the participant set of you plus userIds —
the same rule the backend dedupes on, so it agrees with what
create-conversation resolves for the same people, and an empty array finds
the conversation containing only you. `includes` returns every
conversation containing those users. Filtered lookups walk the pages
internally, so a caller gets an answer in one call rather than paging and
deciding when to stop.

Three guards keep a partial result from passing as a complete one:

- A full page yielding no unseen rows means the cursor has stopped
  advancing, and throws rather than truncating.
- The scan refuses to run past MAX_SCAN_PAGES.
- Exhaustion is judged by a page adding nothing new, never by a page
  coming back shorter than requested. The server is free to cap a page
  below SCAN_PAGE_SIZE, and reading its cap as the end of the list would
  end every scan at the first page — reintroducing the bug this fixes.
  The cost is one extra request per scan. The caller-driven path does
  still assume `limit` is honoured; that assumption is stated where it is
  made.

A capped `includes` scan resumes from the row it stopped on, not the end
of that row's page, which would skip everything in between.

Also fixes archived handling. `includeArchived: false` sent no `archived`
param at all, which returns the server's unfiltered stream — so archived
conversations leaked into the default listing. `includeArchived: true`
then fetched that same unfiltered stream *plus* an archived-only one and
concatenated them, double-counting every archived row. Now false sends
`archived: false` and true omits it, which also keeps pagination on a
single cursor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@doistbot
doistbot requested a review from henningmu August 11, 2026 08:55

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds pagination (limit/cursor) and participant filtering (userIds/matchMode) to list-conversations, fixes archived-conversation handling, and bumps the Comms SDK — so callers can finally reach every conversation, not just the first 20.

Few things worth tightening:

  • Cursor-exhaustion inference can still hide conversations. Both the unfiltered pagination path and the internal participant scan risk marking hasMore false (or returning a partial result) when a server-capped page is simply shorter than the requested size. Since silent truncation is the exact bug this PR set out to fix, the pagination logic should either emit a continuation cursor for non-empty pages unless the API explicitly signals exhaustion, or use a lookahead request to confirm.
  • A stalled cursor is treated as exhaustion instead of throwing. If a subsequent fetch repeats the same capped page, the scan sees no unseen rows but page.length < SCAN_PAGE_SIZE and quietly returns a partial/no-match result — contradicting the stated fail-on-stall guarantee. Detect repeated multi-row pages and throw rather than silently truncating.
  • LLM usage guidelines in src/mcp-server.ts are out of sync. The tool instructions still describe list-conversations as a simple listing tool and don't mention participant filtering (userIds to find a specific DM by who's in it) or pagination via cursor/limit. AGENTS.md requires updating these guidelines when adding tool features, and the LLM relies on them to decide when and how to invoke the tool.

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (3)
  • P3 src/tools/list-conversations.ts:203: In includes mode, an empty userIds array (targetIds.every(...) with nothing to check) is vacuously true and matches every conversation. The parameter description at L24 says "An empty array matches the conversation containing only you", but that contract only holds in exact mode. A caller passing { userIds: [], matchMode: 'includes' } would get every conversation back — surprising and contrary to the documented behavior. Fix: either extend the empty-array special case to includes mode (e.g., early-return the self-conversation matcher when targetIds.length === 0 regardless of mode), or update the description to clarify that the empty-array behaviour is specific to exact mode.
  • P3 src/tools/list-conversations.ts:266: getSessionUser() is called on every participant scan, but buildParticipantMatcher's includes branch never uses sessionUserId — only exact does. For an includes query that finds matches on the first page this doubles the API call count (one getSessionUser + one getConversations). Guard the call with matchMode === 'exact', or fetch the id lazily inside the exact branch of the matcher.
  • P3 src/tools/list-conversations.ts:215: [...expected] creates a new array each time the predicate runs during a scan. Since expected is built once when the matcher is created, hoist the spread to the outer scope: ts const expected = new Set([...targetIds, sessionUserId]) const expectedArr = [...expected] return (conversation) => { const participants = new Set(conversation.userIds) return ( participants.size === expected.size && expectedArr.every((id) => participants.has(id)) ) } This avoids re-allocating the array for every size-matching conversation in a workspace-wide walk.

Share FeedbackReview Logs

Comment thread src/tools/list-conversations.ts Outdated
Comment thread src/tools/list-conversations.ts Outdated
rainsjm and others added 2 commits August 11, 2026 10:10
- Treat any repeated multi-row page as a stalled cursor, not exhaustion.
  The previous rule only threw on a page of SCAN_PAGE_SIZE, so a server
  that caps pages lower could repeat one indefinitely and be read as the
  end of the list — the silent truncation this tool exists to prevent.
  Exhaustion is now only the cursor's own boundary row echoed back alone;
  page size plays no part in the decision.

- An empty `userIds` now means "the conversation containing only me" in
  both match modes. `includes` checks every requested id is present, which
  is vacuously true for an empty list, so it previously returned the whole
  workspace — contradicting the documented contract.

- Only resolve the session user when the query actually needs it. An
  `includes` query never uses it, so it was an extra round trip per scan.

- Hoist the expected-participant array out of the exact matcher instead of
  rebuilding it for every size-matching row.

- Update the list-conversations usage guidelines in mcp-server.ts, per
  AGENTS.md. They still described a plain listing tool, so an LLM had no
  way to know it can now find a conversation by participants — the feature
  is unusable if the guidance never mentions it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The caller-driven path reported hasMore false whenever a page came back
shorter than the requested limit, which assumes the server always honours
`limit`. A page cap that is dynamic, or sized by payload rather than row
count, breaks that assumption without any visible symptom: the caller
stops early believing it has the complete list. That is the exact failure
this tool was changed to eliminate, and the internal scan already refuses
to make the same assumption.

A non-empty page now always carries a continuation cursor, and only an
empty page ends the listing. The cost is one extra request per full
listing. The text output says "More results may be available" rather than
asserting what a single page cannot know.

Earlier reasoning for keeping the inference rested on one probe of a live
workspace returning 237 rows to a single request. That rules out a fixed
row-count cap below 100, but not a dynamic or size-based one, so it was
never enough to justify the carve-out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@scottlovegrove scottlovegrove left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering whether when there's a filter being applied, we should ignore limits/cursors and just fetch all conversations and filter, otherwise you could end up in a situation where you end up with 1 result, but "hasMore: true" and there's another conversation in the second page, which is a very weird situation to be in that you've asked for 20 (for example), it's returned one, and then told you there are more when there may not even be more for that filter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants