diff --git a/apps/web/lib/connector.test.ts b/apps/web/lib/connector.test.ts index e597098e..5b634bd9 100644 --- a/apps/web/lib/connector.test.ts +++ b/apps/web/lib/connector.test.ts @@ -3,7 +3,7 @@ import { createPluckServer } from "@pluck/mcp-server"; import { twoUsers } from "@pluck/db/testing"; import { expect, test } from "vitest"; import { connectorFor } from "./connector"; -import { addTag, newList, pluckAny, readItem, readList, setListVisibility, tagsOf, updateMe } from "./library"; +import { addTag, newList, pluckAny, readItem, readList, setListVisibility, tagIdsFor, tagsOf, updateMe } from "./library"; const FIX = "http://127.0.0.1:4180"; const ALL = ["library:read", "library:write"]; @@ -71,6 +71,18 @@ test("save_item checks the tags and the list first: an unknown one saves nothing expect((await call("list_items", { status: "inbox" })).items).toHaveLength(before); }); +test("save_item reads tags as REST does: ids or names, and an unknown one is refused with the same nearest Tags", async () => { + const { a } = await twoUsers(); + const { call } = await connect(a); + const { tag } = await addTag(a, "Reading"); + const byName = await call("save_item", { text: "Tagged by name", kind: "note", tagIds: ["reading"] }); + expect(byName.saved[0].tags).toEqual(["Reading"]); + const refused = await call("save_item", { text: "Must not land", kind: "note", tagIds: ["Readng"] }); + expect(refused.error).toMatchObject({ code: "unknown_tags", details: { unknown: ["Readng"], nearest: [{ id: tag.id, name: "Reading" }] } }); + await expect(tagIdsFor(a, ["Readng"])).rejects.toMatchObject(refused.error); + expect((await call("list_items", { status: "inbox" })).items).toHaveLength(1); +}); + test("an error that is not one of ours stays on our side: logged, and the AI tool gets a plain internal error", async () => { const { a } = await twoUsers(); const seen: string[] = []; @@ -161,6 +173,18 @@ test("create_list is private with AI notes; update_list changes everything but t expect((await call("list_lists")).lists.map((l: { id: string }) => l.id)).toEqual([list.id]); }); +test("the connector has no way to make a list public, even when the person could", async () => { + const { a } = await twoUsers(); + await updateMe(a, { handle: `ai-${Date.now().toString(36)}` }); + const list = await newList(a, { name: "Mine" }); + await setListVisibility(a, list.id, "public"); + const lib = connectorFor(a); + expect(Object.keys(lib).filter((k) => /visib|public|share/i.test(k))).toEqual([]); + // A caller that ignores the types still cannot pass "public" through. + await (lib.makePrivate as (...args: unknown[]) => Promise)(list.id, "public"); + expect((await readList(a, list.id)).list.visibility).toBe("private"); +}); + test("another person's list is not found, and nothing is changed", async () => { const { a, b } = await twoUsers(); const { call } = await connect(a); diff --git a/apps/web/lib/connector.ts b/apps/web/lib/connector.ts index bb07a5fb..6d9454eb 100644 --- a/apps/web/lib/connector.ts +++ b/apps/web/lib/connector.ts @@ -1,7 +1,7 @@ import type { PluckLibrary } from "@pluck/mcp-server"; import { addItems, addTag, batch, browse, deleteItem, digest, editEntry, editList, highlightsOf, membershipsOf, newList, pluckAny, readItem, readList, - readText, removeItem, reorder, search, setListVisibility, tagsOf, listsOf, + readText, removeItem, reorder, search, setListVisibility, tagIdsFor, tagsOf, listsOf, } from "./library"; type Defer = (task: () => Promise) => void; @@ -19,6 +19,7 @@ export function connectorFor(userId: string, { defer }: { defer?: Defer } = {}): pluck: (body) => pluckAny(userId, body, { defer }), batch: (body) => batch(userId, body, { by }), tags: () => tagsOf(userId), + tagIdsFor: (refs) => tagIdsFor(userId, refs), createTag: (name) => addTag(userId, name, { by }), lists: () => listsOf(userId), list: (id) => readList(userId, id), @@ -28,7 +29,7 @@ export function connectorFor(userId: string, { defer }: { defer?: Defer } = {}): removeFromList: (id, itemId) => removeItem(userId, id, itemId), editEntry: (id, itemId, edit) => editEntry(userId, id, itemId, edit, { by }), reorder: (id, itemIds) => reorder(userId, id, itemIds), - setVisibility: async (id, visibility) => void (await setListVisibility(userId, id, visibility)), + makePrivate: async (id) => void (await setListVisibility(userId, id, "private")), digest: (limit) => digest(userId, { limit }), deleteItem: (id) => deleteItem(userId, id), }; diff --git a/docs/mcp.md b/docs/mcp.md index a45a3576..0036930f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -43,7 +43,7 @@ A tool is only listed when the token carries its scope. | `search_items` | Text search; `query, kinds[], tagIds[], listId, status, since, limit (max 25), cursor` | | `list_items` | Browse by status; `status, kinds[], sort (newest, untouched), limit (max 25), cursor` | | `get_item` | One item with its facts, its kept text (paged at about 20k tokens, follow `textCursor`), note, highlights and lists | -| `save_item` | Pluck a URL, every URL in a block of text, or a note, todo or thing; `url or text, kind, title, note, tagIds[], listId, dueAt`. Saving a link twice brings it back to the Inbox. Unknown `tagIds` or `listId` refuse the call before anything is saved | +| `save_item` | Pluck a URL, every URL in a block of text, or a note, todo or thing; `url or text, kind, title, note, tagIds[], listId, dueAt`. Saving a link twice brings it back to the Inbox. `tagIds` take existing tag ids or names; unknown tags (refused with the nearest existing ones) or an unknown `listId` refuse the call before anything is saved | | `batch_update` | One change over up to 100 items: status, kind, due date, note, add or remove tags. Unknown tags are refused with the nearest existing ones; tags are never created here | | `list_tags` | Tags with counts, and whether an AI made them; paged with `cursor` | | `create_tag` | A new tag, only after the person agreed. Marked as made by AI | diff --git a/packages/mcp/src/library.ts b/packages/mcp/src/library.ts index df9f4f06..3c35023d 100644 --- a/packages/mcp/src/library.ts +++ b/packages/mcp/src/library.ts @@ -16,6 +16,7 @@ export interface PluckLibrary { pluck(body: CreateItemBody): Promise; batch(body: BatchBody): Promise<{ matched: number; updated: number }>; tags(): Promise; + tagIdsFor(refs: string[]): Promise; // ids or names of existing Tags; unknown ones throw unknown_tags with the nearest createTag(name: string): Promise<{ tag: Tag; created: boolean }>; lists(): Promise; list(id: string): Promise<{ list: List; entries: ListEntry[] }>; @@ -25,7 +26,7 @@ export interface PluckLibrary { removeFromList(id: string, itemId: string): Promise; editEntry(id: string, itemId: string, edit: { note?: string | null; ticked?: boolean }): Promise; reorder(id: string, itemIds: string[]): Promise; - setVisibility(id: string, visibility: "private" | "public"): Promise; + makePrivate(id: string): Promise; // there is no way to make a List public here: only the person does that, in Pluck digest(limit: number): Promise; deleteItem(id: string): Promise; } diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 209cd3a4..573d3d3b 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -177,27 +177,23 @@ export function createPluckServer(lib: PluckLibrary, { scopes, version = "1.0.0" kind: Kind.optional(), title: z.string().max(500).optional(), note: z.string().max(10_000).optional(), - tagIds: z.array(ObjectIdString).max(20).optional().describe("Existing tags only."), + tagIds: z.array(z.string().min(1).max(64)).max(20).optional().describe("Existing tag ids or names; unknown ones are refused with the nearest matches."), listId: ObjectIdString.optional(), dueAt: DueAt.optional(), }) .refine((b) => (b.url === undefined) !== (b.text === undefined), { message: "Send url or text." }), annotations: WRITE, // a link twice is one item, but a note twice is two }, - async ({ tagIds, listId, ...body }) => { + async ({ tagIds: tagRefs, listId, ...body }) => { // Check the tags and the list before anything is saved: an error after the pluck would leave the item made, // and an AI tool that retries would make it again. if (listId) await lib.list(listId); - if (tagIds?.length) { - const known = new Set((await lib.tags()).map((t) => t.id)); - const unknown = tagIds.filter((t) => !known.has(t)); - if (unknown.length) throw Object.assign(new Error("Use existing tags only: list_tags has them."), { code: "unknown_tags", details: { unknown } }); - } + const tagIds = tagRefs?.length ? await lib.tagIdsFor(tagRefs) : []; const res = await lib.pluck({ ...body, source: "mcp" }); const ids = res.results.map((r) => r.item.id); - if (ids.length && tagIds?.length) await lib.batch({ itemIds: ids, patch: { addTagIds: tagIds } }); + if (ids.length && tagIds.length) await lib.batch({ itemIds: ids, patch: { addTagIds: tagIds } }); if (ids.length && listId) await lib.addToList(listId, ids); - const items = tagIds?.length ? await Promise.all(ids.map((id) => lib.item(id))) : res.results.map((r) => r.item); + const items = tagIds.length ? await Promise.all(ids.map((id) => lib.item(id))) : res.results.map((r) => r.item); const n = await names(); return { saved: items.map((item, i) => ({ ...card(item, n), created: res.results[i]!.created })), rejected: res.rejected }; }, @@ -277,7 +273,7 @@ export function createPluckServer(lib: PluckLibrary, { scopes, version = "1.0.0" for (const a of add ?? []) await lib.addToList(id, [a.itemId], a.note); for (const e of entries ?? []) await lib.editEntry(id, e.itemId, { note: e.note, ticked: e.ticked }); if (order) await lib.reorder(id, order); - if (visibility) await lib.setVisibility(id, visibility); + if (visibility) await lib.makePrivate(id); const { list } = await lib.list(id); return { list: listRow(list) }; },