From 18622acf56c7870e2f8d6ee48e8dca970a77cb9b Mon Sep 17 00:00:00 2001 From: Anton Arapov Date: Fri, 29 May 2026 10:17:32 +0200 Subject: [PATCH] fix(entries): correct merged-timeline pagination on includeLinkedPersons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two off-by-one-class bugs in the v1.6.6 fan-out merge path: 1. Under-fetch for page > 1 — only `perPage` candidates were pulled per linked party, so page 2 sliced from too small a pool and could come back short/empty. 2. False end-of-feed on an exactly-full page 1 — nextPage was `merged.length > start + perPage`, which reported undefined when the page filled exactly perPage even though linked parties still had older entries upstream (the per-party Link rel=next was discarded). Fix: fetch min(page × perPage, 100) candidates per party (mergedTimelineCandidatePerParty), and preserve each party's upstream next-page signal when the requested window fits within the per-party fetch cap (mergedTimelineNextPage). fanOutPartyEntries now returns {entries, nextPage} per party so the signal is available. Very deep pagination on a large multi-person org can still be approximate (documented). Default single-GET path unchanged. Cherry-picked from the substantive part of codex PR #62, dropping that PR's v1.6.6→v1.7.0 doc rewrites and the wire-trace-v166→v170 rename — the probe scripts are iteration markers (v163–v167), not release versions, and renaming only v166 would break the sequence and the NOTES §32 references. The release-version bump is a release-cut decision, deferred. 537 tests (+1). Bundle 168.08 KB stdio / 195.95 KB http. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 17 +++++++++ HOWTO.md | 4 +- src/tools/entries.ts | 76 +++++++++++++++++++++++++++++--------- tests/bundle-shape.test.ts | 2 +- tests/entries.test.ts | 31 ++++++++++++++++ 5 files changed, 110 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79dd3f5..9cacc63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,23 @@ versions adhere to [Semantic Versioning](https://semver.org). ### Fixed +- **`list_party_entries` merged-timeline pagination (the + `includeLinkedPersons` path).** Two off-by-one-class bugs in the + fan-out merge: + (1) it under-fetched for `page > 1` — only `perPage` candidates + were pulled per linked party, so a second page sliced from too + small a pool and could come back short or empty; + (2) it falsely reported `nextPage: undefined` on an exactly-full + page 1 even when linked parties still had older entries upstream + (the per-party `Link rel=next` signal was discarded), so callers + stopped paging early. + Now the connector fetches `min(page × perPage, 100)` candidates per + party and preserves each party's upstream next-page signal when the + requested window fits within the per-party fetch cap. Very deep + pagination on a large multi-person org can still be approximate + (documented on the schema). The default (`includeLinkedPersons` + omitted) single-GET path is unchanged. + - **Tool annotations now emit `{readOnlyHint, destructiveHint}` explicitly on every tool — never rely on MCP spec defaults.** Per spec, `destructiveHint` defaults to `true`. The pre-fix diff --git a/HOWTO.md b/HOWTO.md index 1c70d27..8ebf2b3 100644 --- a/HOWTO.md +++ b/HOWTO.md @@ -11,7 +11,7 @@ npm install npm test ``` -536 tests, all mocked — no Capsule API calls happen, no token needed. The suite has three layers: +537 tests, all mocked — no Capsule API calls happen, no token needed. The suite has three layers: - **Per-tool unit tests** (e.g. `tests/parties.test.ts`): import the tool function, mock `undici.fetch`, assert on the URL, method, body, and response handling. Most tests live here. - **MCP-protocol integration tests** (`tests/mcp-integration.test.ts`): drive a real `McpServer` through the wire protocol via the SDK's in-memory transport pair, with `undici.fetch` still mocked. Catches the layer between "tool function works" and "MCP correctly registers and dispatches the tool". Includes the `get_attachment` content-type routing logic (which lives in `server.ts`, not the tool function). @@ -46,7 +46,7 @@ for the contributor-facing summary. npm run build ``` -Produces `dist/index.js` (stdio entry, ~167 KB, with `#!/usr/bin/env node` shebang and the executable bit set) and `dist/http.js` (HTTP entry, ~195 KB, no shebang). Each is fully self-contained — tsup runs as two separate configs so the stdio entry can be invoked directly via npx while the HTTP entry isn't a CLI. tsup target is Node 22 (undici 8 requires Node 22+ for the `webidl.util.markAsUncloneable` runtime API). +Produces `dist/index.js` (stdio entry, ~168 KB, with `#!/usr/bin/env node` shebang and the executable bit set) and `dist/http.js` (HTTP entry, ~196 KB, no shebang). Each is fully self-contained — tsup runs as two separate configs so the stdio entry can be invoked directly via npx while the HTTP entry isn't a CLI. tsup target is Node 22 (undici 8 requires Node 22+ for the `webidl.util.markAsUncloneable` runtime API). `npm run build` also chains `npm run build:icon` (`scripts/build-icon.mjs`), which regenerates `src/icon.ts` from the canonical `assets/icon.svg`. The TypeScript file is committed (so typecheck works without a build step) but is **generated** — edit the SVG, then run the build. A drift-guard test (`tests/icon-source.test.ts`) fails CI if the two ever fall out of sync. diff --git a/src/tools/entries.ts b/src/tools/entries.ts index 81e6dde..9ca5e48 100644 --- a/src/tools/entries.ts +++ b/src/tools/entries.ts @@ -25,11 +25,16 @@ export const listPartyEntriesSchema = z.object({ "Default is `false` — single GET, existing behaviour unchanged. " + "WHY THIS FLAG EXISTS: Capsule's API files each entry against exactly one party row (verified v1.6.6 wire-trace probe 4 — POST /entries rejects multi-party bodies with 422 'entry must be linked to either a party, opportunity or kase'). For an organisation with multiple contacts, captured emails almost always land on a person row, not the org. As a result, `list_party_entries(orgId)` with `includeLinkedPersons: false` will miss recent customer-facing email — even though the org's own `lastContactedAt` is updated by the activity. This flag is the correct call for any 'what's new with $ORG?' question. " + "WHEN `partyId` IS A PERSON: silently no-op — persons have no linked-people relationship in Capsule's data model, so the flag is functionally inert (the connector still issues a cheap `/people` check; the response is empty). " + - "LATENCY: 1 + N round trips for an org with N linked people, concurrency-capped (typical: 2-3 waves for N=10). Use `includeLinkedPersons: false` for fast pre-screen reads where you only need the org-row entries (e.g. invoice/contract notes that are typically filed at the org level). " + - "PAGINATION CAVEAT: `page` and `perPage` apply to the MERGED window. The connector fetches `perPage` entries from each party then slices the caller's window; for deep pagination (`page > 1` with a large multi-person org) the slice may be approximate — the latest entries are correct, but ordering deeper into the feed can lose entries from parties with many records. For LLM-driven 'what's the latest' queries (the typical use), this is invisible.", + "LATENCY: 1 + N round trips for an org with N linked people, concurrency-capped (typical: 2-3 waves for N=10). Linked-person enumeration reads the first 100 linked people; use list_employees for explicit pagination when an organisation has more contacts than that. Use `includeLinkedPersons: false` for fast pre-screen reads where you only need the org-row entries (e.g. invoice/contract notes that are typically filed at the org level). " + + "PAGINATION CAVEAT: `page` and `perPage` apply to the MERGED window. The connector fetches enough entries from each party to cover the requested merged window (up to Capsule's per-party cap of 100) then slices the caller's window; very deep pagination with a large multi-person org can still be approximate. For LLM-driven 'what's the latest' queries (the typical use), this is invisible.", ), }); +interface PartyEntriesPage { + entries: unknown[]; + nextPage: number | undefined; +} + /** * Fetch `entries` arrays for multiple party ids in parallel, * concurrency-capped by `getBatchConcurrency()`. Throws on the first @@ -40,9 +45,9 @@ async function fanOutPartyEntries( partyIds: number[], embed: string | undefined, perPage: number, -): Promise { +): Promise { const concurrency = getBatchConcurrency(); - const results: unknown[][] = new Array(partyIds.length); + const results: PartyEntriesPage[] = new Array(partyIds.length); let cursor = 0; async function worker(): Promise { while (true) { @@ -50,12 +55,15 @@ async function fanOutPartyEntries( cursor += 1; if (i >= partyIds.length) return; const id = partyIds[i]!; - const { data } = await capsuleGet<{ entries: unknown[] }>(`/parties/${id}/entries`, { - embed, - page: 1, - perPage, - }); - results[i] = data.entries; + const { data, nextPage } = await capsuleGet<{ entries: unknown[] }>( + `/parties/${id}/entries`, + { + embed, + page: 1, + perPage, + }, + ); + results[i] = { entries: data.entries, nextPage }; } } const workers: Promise[] = []; @@ -66,6 +74,30 @@ async function fanOutPartyEntries( return results; } +function mergedTimelineCandidatePerParty(page: number, perPage: number): number { + return Math.min(page * perPage, 100); +} + +function mergedTimelineNextPage( + page: number, + perPage: number, + mergedLength: number, + upstreamHasNextPage: boolean, +): number | undefined { + const requestedWindowEnd = page * perPage; + if (mergedLength > requestedWindowEnd) return page + 1; + + // When the requested window fits within the per-party fetch cap, + // an upstream Link rel=next means there are older entries beyond + // our candidate set even if the merged slice length is exactly + // `perPage`. Preserve that signal instead of falsely ending the + // merged feed at page 1. + const coveredRequestedWindow = requestedWindowEnd <= 100; + if (coveredRequestedWindow && upstreamHasNextPage) return page + 1; + + return undefined; +} + export async function listPartyEntries(input: z.infer) { const { partyId, embed, page, perPage, includeLinkedPersons } = input; @@ -81,7 +113,8 @@ export async function listPartyEntries(input: z.infer100 linked persons on a single org see partial - // coverage. Documented in the schema description. + // coverage. The schema description calls this out and points to + // list_employees for explicit linked-person pagination. const { data: peopleData } = await capsuleGet<{ parties?: { id: number }[] }>( `/parties/${partyId}/people`, { page: 1, perPage: 100 }, @@ -99,10 +132,14 @@ export async function listPartyEntries(input: z.infer(); const merged: Array<{ id: number; entryAt?: string }> = []; - for (const arr of perPartyEntries) { - for (const raw of arr) { + for (const { entries } of perPartyPages) { + for (const raw of entries) { const e = raw as { id: number; entryAt?: string }; if (typeof e?.id !== "number") continue; if (seen.has(e.id)) continue; @@ -134,7 +171,12 @@ export async function listPartyEntries(input: z.infer start + perPage ? page + 1 : undefined; + const nextPage = mergedTimelineNextPage( + page, + perPage, + merged.length, + perPartyPages.some((p) => p.nextPage !== undefined), + ); return { entries: slice, ...(nextPage !== undefined ? { nextPage } : {}) }; } diff --git a/tests/bundle-shape.test.ts b/tests/bundle-shape.test.ts index c1d00b0..72eaa1d 100644 --- a/tests/bundle-shape.test.ts +++ b/tests/bundle-shape.test.ts @@ -54,7 +54,7 @@ describe.skipIf(!distExists)("bundle shape (post-build canary)", () => { const httpKb = statSync(HTTP_PATH).size / 1024; // Floor catches "the bundler produced an empty file"; ceiling // catches "we accidentally inlined a giant dependency". The - // current values (~167 / ~195 KB) sit comfortably in the band. + // current values (~168 / ~196 KB) sit comfortably in the band. expect(stdioKb).toBeGreaterThan(MIN_KB); expect(stdioKb).toBeLessThan(MAX_KB); expect(httpKb).toBeGreaterThan(MIN_KB); diff --git a/tests/entries.test.ts b/tests/entries.test.ts index 83f02b0..d5bc626 100644 --- a/tests/entries.test.ts +++ b/tests/entries.test.ts @@ -210,6 +210,37 @@ describe("listPartyEntries", () => { // 5 candidates, slice goes 0..2 → nextPage signals more remain. expect((result as { nextPage?: number }).nextPage).toBe(2); }); + + it("preserves upstream nextPage when the merged page is exactly full", async () => { + // Regression: if one linked person's first page is exactly full + // and Capsule sends Link rel=next, the merged result still has a + // next page even though merged.length === perPage. + mockFetch(200, { parties: [{ id: 8 }] }); + mockFetch(200, { entries: [] }); + mockFetch( + 200, + { + entries: [ + { id: 11, type: "email", entryAt: "2026-05-27T11:00:00Z" }, + { id: 10, type: "email", entryAt: "2026-05-27T10:00:00Z" }, + ], + }, + { + Link: '; rel="next"', + }, + ); + + const { listPartyEntries } = await import("../src/tools/entries.js"); + const result = await listPartyEntries({ + partyId: 7, + page: 1, + perPage: 2, + includeLinkedPersons: true, + }); + + expect(result.entries.map((e: { id: number }) => e.id)).toEqual([11, 10]); + expect((result as { nextPage?: number }).nextPage).toBe(2); + }); }); describe("listOpportunityEntries", () => {