diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cacc63..573ac19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,10 +62,17 @@ versions adhere to [Semantic Versioning](https://semver.org). 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. + NEXT window still falls strictly within the per-party fetch cap. + The merge has an inherent ceiling — a top-100-per-party merge + reliably orders only the most-recent ~100 entries across the org + + its people — so at exactly that ceiling the feed now ends honestly + (the next-page signal uses a strict `< 100`, not `<= 100`, to avoid + promising a phantom empty page) rather than reporting "approximate." + The schema description states the ceiling explicitly and directs a + specific contact's deeper history to `list_party_entries` on that + person's id (the default single-GET path paginates natively with no + ceiling). The default (`includeLinkedPersons` omitted) path is + unchanged. - **Tool annotations now emit `{readOnlyHint, destructiveHint}` explicitly on every tool — never rely on MCP spec defaults.** Per @@ -81,10 +88,13 @@ versions adhere to [Semantic Versioning](https://semver.org). update_party, every `batch_*`) were getting the same UI weight as `delete_party` in some client paths. - After the fix, every tool emits a full hint pair: + After the fix, every tool emits a full hint pair (counts reflect + the full v1.7.0 catalog, including `delete_tag_definition` added + later in this same release): - reads → `{readOnlyHint: true, destructiveHint: false}` (49) - - destructive (delete_* + remove_track + remove_additional_party) - → `{readOnlyHint: false, destructiveHint: true}` (7) + - destructive (delete_* incl. delete_tag_definition, + remove_track + + remove_additional_party) → `{readOnlyHint: false, + destructiveHint: true}` (8) - all other writes → `{readOnlyHint: false, destructiveHint: false}` (31, was `undefined` before) @@ -784,7 +794,7 @@ pointer. ### Privacy / hygiene - **Tightened `cache.evict` path field.** The evicted key was being - logged in its raw form (e.g. `GET /parties/254022621?embed=tags`), + logged in its raw form (e.g. `GET /parties/123456789?embed=tags`), which leaked record IDs and (in the case of `/parties/search?q=…`) search terms into operator logs. Now redacted to `GET /parties/:id` shape. Same `redactPath` helper used for all diff --git a/HOWTO.md b/HOWTO.md index 8ebf2b3..0af0068 100644 --- a/HOWTO.md +++ b/HOWTO.md @@ -11,7 +11,7 @@ npm install npm test ``` -537 tests, all mocked — no Capsule API calls happen, no token needed. The suite has three layers: +538 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). diff --git a/OPTIMIZATIONS.md b/OPTIMIZATIONS.md index e04bdaa..1b32806 100644 --- a/OPTIMIZATIONS.md +++ b/OPTIMIZATIONS.md @@ -571,7 +571,7 @@ after. - Tool **arguments** are never logged — only the field NAMES that were present (`argFields: ["conditions", "page"]`). Search queries, party IDs, custom-field values, etc. stay out of operator logs. -- Capsule API **paths** are redacted: `/parties/254022621/notes` → +- Capsule API **paths** are redacted: `/parties/123456789/notes` → `/parties/:id/notes`, `/parties/search?q=Acme` → `/parties/search`. Numeric IDs and query strings never appear. The shape stays for analytics ("top endpoints", "p95 latency per endpoint"). diff --git a/glama.json b/glama.json index 3948488..db50cb1 100644 --- a/glama.json +++ b/glama.json @@ -1,6 +1,6 @@ { "$schema": "https://glama.ai/mcp/schemas/server.json", - "description": "MCP server for Capsule CRM. 86 tools across contacts, opportunities, projects, tasks, timeline activity, structured filters with field/operator conditions, saved searches with sort, workflow tracks, file attachments (read + write), audit of deleted records, and batch fetches up to 50 records. Read-only mode supported via CAPSULE_MCP_READONLY=1. Read tools are annotated with readOnlyHint so MCP clients can identify safe calls for auto-approval.", + "description": "MCP server for Capsule CRM. 88 tools across contacts, opportunities, projects, tasks, timeline activity, structured filters with field/operator conditions, saved searches with sort, workflow tracks, file attachments (read + write), audit of deleted records, and batch fetches up to 50 records. Read-only mode supported via CAPSULE_MCP_READONLY=1. Read tools are annotated with readOnlyHint so MCP clients can identify safe calls for auto-approval.", "categories": ["crm", "productivity"], "maintainers": ["arapov"] } diff --git a/src/log.ts b/src/log.ts index eb8199f..16f2049 100644 --- a/src/log.ts +++ b/src/log.ts @@ -120,10 +120,10 @@ export function logEvent( * adjacent metadata) across log aggregators. * * Patterns redacted: - * /parties/254022621 -> /parties/:id + * /parties/123456789 -> /parties/:id * /parties/1,2,3 -> /parties/:id (multi-id GET) - * /parties/254022621/notes -> /parties/:id/notes - * /parties/254022621/notes/456 -> /parties/:id/notes/:id + * /parties/123456789/notes -> /parties/:id/notes + * /parties/123456789/notes/456 -> /parties/:id/notes/:id * /parties/search?q=Acme -> /parties/search (query dropped) * * Tag-list paths (`/parties/tags`, `/opportunities/tags`, diff --git a/src/tools/confirm-flag.ts b/src/tools/confirm-flag.ts index c0f01fd..671d541 100644 --- a/src/tools/confirm-flag.ts +++ b/src/tools/confirm-flag.ts @@ -1,9 +1,10 @@ /** * Shared `confirm: true` literal for destructive write tools. * - * Capsule's connector gates 7 destructive/removal tools behind an explicit + * Capsule's connector gates 8 destructive/removal tools behind an explicit * `confirm: true` flag (delete_party / _opportunity / _project / _task - * / _entry, plus remove_track and remove_additional_party). Zod's + * / _entry / _tag_definition, plus remove_track and + * remove_additional_party). Zod's * default error on a missing or `false` value of a `z.literal(true)` * reads `"Invalid input: expected true"` — technically correct but * unhelpful at a callsite, especially for an LLM caller trying to diff --git a/src/tools/entries.ts b/src/tools/entries.ts index 9ca5e48..c48e04c 100644 --- a/src/tools/entries.ts +++ b/src/tools/entries.ts @@ -26,7 +26,7 @@ export const listPartyEntriesSchema = z.object({ "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). 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.", + "PAGINATION CAVEAT: `page` and `perPage` apply to the MERGED window, and the merge has a hard ceiling — it reliably orders only the most-recent ~100 entries across the org + its people (each party is fetched at Capsule's per-party cap of 100, and a top-100-per-party merge is correct only up to global position 100). Paging past that ceiling (`page × perPage > 100`) returns no further entries and ends the feed; it does NOT continue into older history. To read a specific contact's full timeline beyond the merged ceiling, call `list_party_entries` on that person's id directly (the default single-GET path paginates natively with no ceiling). For the LLM-driven 'what's the latest with $ORG' query this is the typical use of, the first page is exact and the ceiling is never reached.", ), }); @@ -87,13 +87,21 @@ function mergedTimelineNextPage( 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; + // When the NEXT window still falls strictly within the per-party + // fetch cap (100), an upstream Link rel=next means there are older + // entries beyond our candidate set even though the merged slice was + // exactly full — preserve that signal instead of falsely ending the + // feed (the v1.6.6 regression this guards). + // + // Strict `<` (not `<=`): the merge of "top-100 per party" reliably + // orders only the global top ~100 entries. At `requestedWindowEnd + // == 100` we are AT that ceiling — page+1 would need candidates + // beyond 100 that we never fetched, so promising it would yield a + // phantom empty page. End honestly at the ceiling instead; the + // schema description directs deeper per-contact history to + // list_party_entries on the specific person. + const nextWindowWithinCap = requestedWindowEnd < 100; + if (nextWindowWithinCap && upstreamHasNextPage) return page + 1; return undefined; } diff --git a/tests/entries.test.ts b/tests/entries.test.ts index d5bc626..73e092a 100644 --- a/tests/entries.test.ts +++ b/tests/entries.test.ts @@ -241,6 +241,45 @@ describe("listPartyEntries", () => { expect(result.entries.map((e: { id: number }) => e.id)).toEqual([11, 10]); expect((result as { nextPage?: number }).nextPage).toBe(2); }); + + it("does NOT promise a next page at the 100-entry merge ceiling (no phantom page)", async () => { + // The merge reliably orders only the global top ~100 (each party + // capped at 100 candidates). At a window whose end is exactly 100, + // page+1 would need candidates beyond the cap we never fetched — + // so even though a linked person still has an upstream Link + // rel=next, the feed must END here rather than promise a page that + // would come back empty. Guards the `<` (not `<=`) ceiling check. + mockFetch(200, { parties: [{ id: 8 }] }); // /people + mockFetch(200, { entries: [] }); // org (id 7) — empty + // Linked person returns a full cap of 100 entries AND signals more + // upstream. page=4 perPage=25 → requestedWindowEnd === 100. + const hundred = Array.from({ length: 100 }, (_v, i) => ({ + id: 1000 + i, + type: "email", + entryAt: `2026-05-27T${String(23 - Math.floor(i / 5)).padStart(2, "0")}:00:00Z`, + })); + mockFetch( + 200, + { entries: hundred }, + { + Link: '; rel="next"', + }, + ); + + const { listPartyEntries } = await import("../src/tools/entries.js"); + const result = await listPartyEntries({ + partyId: 7, + page: 4, + perPage: 25, + includeLinkedPersons: true, + }); + + // page 4 of 25 = the window ending exactly at the 100 ceiling. + expect(result.entries).toHaveLength(25); + // No phantom page 5: the feed ends honestly at the ceiling even + // though the person had an upstream rel=next. + expect((result as { nextPage?: number }).nextPage).toBeUndefined(); + }); }); describe("listOpportunityEntries", () => { diff --git a/tests/log-events.test.ts b/tests/log-events.test.ts index a51abc6..91d3108 100644 --- a/tests/log-events.test.ts +++ b/tests/log-events.test.ts @@ -25,7 +25,7 @@ vi.mock("undici", () => ({ fetch: vi.fn() })); describe("redactPath", () => { it("redacts single numeric IDs", () => { - expect(redactPath("/parties/254022621")).toBe("/parties/:id"); + expect(redactPath("/parties/123456789")).toBe("/parties/:id"); expect(redactPath("/opportunities/1234")).toBe("/opportunities/:id"); expect(redactPath("/kases/9")).toBe("/kases/:id"); }); @@ -36,7 +36,7 @@ describe("redactPath", () => { }); it("redacts nested numeric IDs", () => { - expect(redactPath("/parties/254022621/notes")).toBe("/parties/:id/notes"); + expect(redactPath("/parties/123456789/notes")).toBe("/parties/:id/notes"); expect(redactPath("/parties/123/notes/456")).toBe("/parties/:id/notes/:id"); }); @@ -212,7 +212,7 @@ describe("end-to-end: tool.call, capsule.request, tool.chain via the MCP wire", ); await log.withRequestContext({ clientId: "cap-client" }, async () => { - await client.callTool({ name: "get_party", arguments: { id: 254022621 } }); + await client.callTool({ name: "get_party", arguments: { id: 123456789 } }); }); const reqs = parseEvents("capsule.request"); @@ -220,9 +220,9 @@ describe("end-to-end: tool.call, capsule.request, tool.chain via the MCP wire", const r = reqs[0]!; // Path is the full URL pathname (Capsule API base is // /api/v2/...). The load-bearing assertion: the raw party id - // 254022621 must NOT appear; `:id` must. + // 123456789 must NOT appear; `:id` must. expect(String(r["path"])).toBe("/api/v2/parties/:id"); - expect(String(r["path"])).not.toContain("254022621"); + expect(String(r["path"])).not.toContain("123456789"); expect(r["method"]).toBe("GET"); expect(r["status"]).toBe(200); expect(typeof r["durationMs"]).toBe("number");