diff --git a/prototypes/zotero-roam-mcp/mocks/roam.ts b/prototypes/zotero-roam-mcp/mocks/roam.ts index e3fb31f..2723770 100644 --- a/prototypes/zotero-roam-mcp/mocks/roam.ts +++ b/prototypes/zotero-roam-mcp/mocks/roam.ts @@ -33,9 +33,7 @@ function getAllPages(){ return []; } -function getCitekeyPages() { - return new Map([]); -} +const getCitekeyPages = fn((): Map => new Map()); function getCitekeyPagesWithEditTime(){ return new Map([]); @@ -63,18 +61,16 @@ function hasBlockChildren(uid) { return [uid_with_existing_block_with_children, existing_page_with_content_uid].includes(uid); } -const importItemMetadata = fn(({ item }, uid) => { - const pageUID = uid || existing_page_uid; - return Promise.resolve({ - args: { blocks: [], uid: pageUID }, - error: null, - page: { new: !uid, title: "@" + item.key, uid: pageUID }, - raw: {}, - success: true - }); -}); +/** The shape both import functions resolve with. Widened past the happy path, so that tests can stub failed and uncertain outcomes. */ +type MockImportOutcome = { + args: { blocks: unknown[], uid: string }, + error: unknown, + page: { new: boolean, title: string, uid: string }, + raw?: Record, + success: boolean | null +}; -const importItemNotes = fn(({ item }, uid) => { +const mockImportOutcome = ({ item }, uid): Promise => { const pageUID = uid || existing_page_uid; return Promise.resolve({ args: { blocks: [], uid: pageUID }, @@ -83,7 +79,11 @@ const importItemNotes = fn(({ item }, uid) => { raw: {}, success: true }); -}); +}; + +const importItemMetadata = fn(mockImportOutcome); + +const importItemNotes = fn(mockImportOutcome); function makeDNP(date: Date | any, { brackets = true }: { brackets?: boolean } = {}) { const thisdate = date.constructor === Date ? date : new Date(date); diff --git a/prototypes/zotero-roam-mcp/src/components/RoamCitekeysContext/index.tsx b/prototypes/zotero-roam-mcp/src/components/RoamCitekeysContext/index.tsx index 484eb5e..f9fffee 100644 --- a/prototypes/zotero-roam-mcp/src/components/RoamCitekeysContext/index.tsx +++ b/prototypes/zotero-roam-mcp/src/components/RoamCitekeysContext/index.tsx @@ -1,4 +1,4 @@ -import { FC, createContext, useCallback, useContext, useMemo, useState } from "react"; +import { FC, createContext, useCallback, useContext, useEffect, useMemo, useState } from "react"; import { getCitekeyPages } from "@services/roam"; @@ -16,6 +16,22 @@ const RoamCitekeysProvider: FC = ({ children }) => { setRoamCitekeys(() => getCitekeyPages()); }, []); + // Imports that don't go through the UI (AI tools, or any other consumer of the import functions) can create citekey pages. + // Without this, the map would stay stale until the next mount, and the UI would treat those pages as missing. + useEffect(() => { + const refreshIfPageCreated = (event: CustomEvent<{ page?: { new?: boolean } }>) => { + if (event.detail?.page?.new) { update(); } + }; + + document.addEventListener("zotero-roam:metadata-added", refreshIfPageCreated); + document.addEventListener("zotero-roam:notes-added", refreshIfPageCreated); + + return () => { + document.removeEventListener("zotero-roam:metadata-added", refreshIfPageCreated); + document.removeEventListener("zotero-roam:notes-added", refreshIfPageCreated); + }; + }, [update]); + const contextValue = useMemo(() => [roamCitekeys, update] as const, [roamCitekeys, update]); return ( diff --git a/prototypes/zotero-roam-mcp/src/loader.tsx b/prototypes/zotero-roam-mcp/src/loader.tsx index 5cc47fd..59bd394 100644 --- a/prototypes/zotero-roam-mcp/src/loader.tsx +++ b/prototypes/zotero-roam-mcp/src/loader.tsx @@ -7,7 +7,7 @@ import ClearCacheButton from "Components/ClearCacheButton"; import { UserSettingsProvider } from "Components/UserSettings"; import ZoteroRoam from "./api"; -import { registerAiTools } from "@services/ai-tools"; +import { registerAiTools, unregisterAiTools } from "@services/ai-tools"; import { clearDefaultHooks } from "@services/events"; import IDBDatabase from "@services/idb"; import { unregisterSmartblockCommands } from "@services/smartblocks"; @@ -59,8 +59,12 @@ function onload({ extensionAPI }){ setup({ settings }); // Expose key functionality to agents connected through Roam's MCP server. - // Tools are tied to the extension: Roam removes them automatically on unload. - registerAiTools({ extensionAPI }); + // The API is experimental, so a failure here must not take down the extension. + try { + registerAiTools({ extensionAPI }); + } catch(e) { + console.error("zoteroRoam: failed to register AI tools", e); + } render( @@ -81,6 +85,11 @@ function onload({ extensionAPI }){ function offload(){ clearDefaultHooks(); + try { + unregisterAiTools(); + } catch(e) { + console.error("zoteroRoam: failed to unregister AI tools", e); + } unregisterSmartblockCommands(); unmountExtensionIfExists(); window.zoteroRoam.deleteDatabase(); diff --git a/prototypes/zotero-roam-mcp/src/services/ai-tools/helpers.ts b/prototypes/zotero-roam-mcp/src/services/ai-tools/helpers.ts index 17134ab..a933ba7 100644 --- a/prototypes/zotero-roam-mcp/src/services/ai-tools/helpers.ts +++ b/prototypes/zotero-roam-mcp/src/services/ai-tools/helpers.ts @@ -1,3 +1,4 @@ +import { getItemYear } from "../../api/helpers"; import { parseDOI } from "../../utils"; import { ZItemTop } from "Types/transforms"; @@ -22,18 +23,11 @@ type SimplifiedItem = { year: string }; -/** Removes the `@` prefix from a citekey, if present */ -function normalizeCitekey(citekey: string) { - return citekey.trim().replace(/^@/, ""); -} - -/** Extracts an item's year of publication, if available */ -function extractYear(item: ZItemTop) { - return !item.meta.parsedDate - ? "" - : isNaN(Number(new Date(item.meta.parsedDate))) - ? "" - : (new Date(item.meta.parsedDate)).getUTCFullYear().toString(); +/** Normalizes an agent-provided citekey: coerces to string, trims, and removes the `@` prefix. + * Handler arguments are only schema-validated when the call comes through Roam's MCP server, so they are coerced here rather than assumed. + */ +function normalizeCitekey(citekey: unknown) { + return String(citekey ?? "").trim().replace(/^@/, ""); } /** Formats a Zotero item into a compact summary for agents */ @@ -48,15 +42,15 @@ function simplifyItemForAgent(item: ZItemTop, { inGraph }: { inGraph: string | f key: item.data.key, library: item.library.type + "s/" + item.library.id, title: item.data.title || "", - year: extractYear(item) + year: getItemYear(item) }; } /** Matches Zotero items against a search string - by citekey, Zotero key, DOI, or title substring. * @returns The matching items, with exact citekey/key/DOI matches sorted before title matches */ -function matchItems(items: ZItemTop[], query: string): ZItemTop[] { - const trimmed = query.trim(); +function matchItems(items: ZItemTop[], query: unknown): ZItemTop[] { + const trimmed = String(query ?? "").trim(); const lowercased = trimmed.toLowerCase(); const citekey = normalizeCitekey(trimmed).toLowerCase(); const doi = parseDOI(trimmed); @@ -65,9 +59,11 @@ function matchItems(items: ZItemTop[], query: string): ZItemTop[] { const partial: ZItemTop[] = []; items.forEach(item => { - if (item.key.toLowerCase() == citekey || item.data.key.toLowerCase() == lowercased || (doi && parseDOI(item.data.DOI) == doi)) { + const itemKey = item.key.toLowerCase(); + + if (itemKey == citekey || item.data.key.toLowerCase() == lowercased || (doi && parseDOI(item.data.DOI) == doi)) { exact.push(item); - } else if (item.key.toLowerCase().includes(citekey) || (item.data.title || "").toLowerCase().includes(lowercased)) { + } else if (itemKey.includes(citekey) || (item.data.title || "").toLowerCase().includes(lowercased)) { partial.push(item); } }); diff --git a/prototypes/zotero-roam-mcp/src/services/ai-tools/index.test.ts b/prototypes/zotero-roam-mcp/src/services/ai-tools/index.test.ts index a1209a4..9502bba 100644 --- a/prototypes/zotero-roam-mcp/src/services/ai-tools/index.test.ts +++ b/prototypes/zotero-roam-mcp/src/services/ai-tools/index.test.ts @@ -6,10 +6,10 @@ import { Roam } from "@services/roam"; import { analyzeUserRequests, setupInitialSettings } from "../../setup"; import ZoteroRoam from "../../api"; -import { aiTools, registerAiTools } from "."; +import { aiTools, registerAiTools, unregisterAiTools } from "."; import { apiKeys, items, libraries, sampleAnnot, sampleNote, samplePDF } from "Mocks"; -import { existing_page_uid, existing_page_with_content_uid, findRoamPage, importItemMetadata, importItemNotes } from "Mocks/roam"; +import { existing_page_uid, existing_page_with_content_uid, findRoamPage, getCitekeyPages, importItemMetadata, importItemNotes } from "Mocks/roam"; const { userLibrary, groupLibrary } = libraries; @@ -25,27 +25,12 @@ const initSettings = setupInitialSettings({}); const blochItem = items.find(it => it.key == "blochImplementingSocialInterventions2021")!; const pintoItem = items.find(it => it.key == "pintoExploringDifferentMethods2021")!; -/** Creates a fake `extensionAPI`, with an in-memory settings store and spies for the AI tools API */ -const makeExtensionAPI = (settings: Record = {}, { withAI = true } = {}): Roam.ExtensionAPI => ({ - settings: { - get: (key: string) => settings[key] as T | undefined, - getAll: () => settings, - set: (key, value) => { - settings[key] = value; - }, - panel: { - create: () => {} - } - }, - ui: { - commandPalette: { - addCommand: async () => {}, - removeCommand: async () => {} - } - }, - ...(withAI - ? { ai: { addTool: vi.fn(() => null), removeTool: vi.fn(() => null) } } - : {}) +/** Creates a fake `extensionAPI` exposing what the AI tools use: the settings store, and the AI tools API */ +const makeExtensionAPI = ({ settings = {}, withAI = true }: { settings?: Record, withAI?: boolean } = {}) => mock({ + settings: { getAll: () => settings }, + ai: withAI + ? { addTool: vi.fn(() => null), removeTool: vi.fn(() => null) } + : undefined }); const getTool = (extensionAPI: Roam.ExtensionAPI, name: string) => { @@ -81,12 +66,56 @@ describe("registerAiTools", () => { }); it("no-ops on Roam builds without extension AI tools", () => { - const extensionAPI = makeExtensionAPI({}, { withAI: false }); + const extensionAPI = makeExtensionAPI({ withAI: false }); expect(registerAiTools({ extensionAPI })).toBe(false); }); }); +describe("unregisterAiTools", () => { + it("removes every registered tool", () => { + const extensionAPI = makeExtensionAPI(); + registerAiTools({ extensionAPI }); + + unregisterAiTools(); + + expect(vi.mocked(extensionAPI.ai!.removeTool).mock.calls.map(([{ name }]) => name)).toEqual([ + "zotero-search-items", + "zotero-import-metadata", + "zotero-import-notes" + ]); + }); + + it("removes each tool only once, however often it is called", () => { + const extensionAPI = makeExtensionAPI(); + registerAiTools({ extensionAPI }); + + unregisterAiTools(); + unregisterAiTools(); + + expect(extensionAPI.ai!.removeTool).toHaveBeenCalledTimes(3); + }); + + it("does not reach a stale API after a failed registration", () => { + const registered = makeExtensionAPI(); + registerAiTools({ extensionAPI: registered }); + registerAiTools({ extensionAPI: makeExtensionAPI({ withAI: false }) }); + + unregisterAiTools(); + + // The failed registration cleared the stored handle, so the earlier API is not touched + expect(registered.ai!.removeTool).not.toHaveBeenCalled(); + }); + + it("replaces the tools of a previous registration", () => { + const first = makeExtensionAPI(); + registerAiTools({ extensionAPI: first }); + registerAiTools({ extensionAPI: makeExtensionAPI() }); + + expect(first.ai!.removeTool).toHaveBeenCalledTimes(3); + }); +}); + describe("AI tool handlers", () => { let client: QueryClient; let extensionAPI: Roam.ExtensionAPI; @@ -94,6 +123,7 @@ describe("AI tool handlers", () => { beforeEach(() => { // The Mocks/roam spies come from @storybook/test's `fn`, which vitest's `clearMocks` doesn't cover findRoamPage.mockClear(); + getCitekeyPages.mockClear(); importItemMetadata.mockClear(); importItemNotes.mockClear(); @@ -134,6 +164,17 @@ describe("AI tool handlers", () => { }); }); + it("reports the uid of an item's Roam page, with a single lookup for all matches", () => { + getCitekeyPages.mockReturnValueOnce(new Map([["@blochImplementingSocialInterventions2021", existing_page_uid]])); + const tool = getTool(extensionAPI, "zotero-search-items"); + + const output = tool.handler({ query: "" }, toolContext()) as { items: { citekey: string, inGraph: string | false }[] }; + + expect(getCitekeyPages).toHaveBeenCalledTimes(1); + expect(output.items.find(it => it.citekey == "@blochImplementingSocialInterventions2021")?.inGraph).toBe(existing_page_uid); + expect(output.items.find(it => it.citekey == "@pintoExploringDifferentMethods2021")?.inGraph).toBe(false); + }); + it("caps the number of returned items to the limit", () => { const tool = getTool(extensionAPI, "zotero-search-items"); @@ -142,6 +183,42 @@ describe("AI tool handlers", () => { expect(output.total).toBe(items.length); expect(output.items.length).toBe(1); }); + + it("pages through matches with the offset", () => { + const tool = getTool(extensionAPI, "zotero-search-items"); + + const firstPage = tool.handler({ query: "", limit: 1 }, toolContext()) as { items: { key: string }[] }; + const secondPage = tool.handler({ query: "", limit: 1, offset: 1 }, toolContext()) as { total: number, items: { key: string }[] }; + + expect(secondPage.total).toBe(items.length); + expect(secondPage.items.length).toBe(1); + expect(secondPage.items[0].key).not.toBe(firstPage.items[0].key); + }); + + // Handlers are callable directly from JS, where Roam's schema validation doesn't apply + it.each([ + ["no query", {}], + ["a null query", { query: null }], + ["a non-string query", { query: 2021 }], + ["a null limit", { query: "", limit: null }], + ["a negative limit", { query: "", limit: -1 }] + ])("survives %s", (_label, args) => { + const tool = getTool(extensionAPI, "zotero-search-items"); + + const output = tool.handler(args, toolContext()) as { total: number, items: unknown[] }; + + expect(output.total).toBeGreaterThanOrEqual(0); + expect(Array.isArray(output.items)).toBe(true); + }); + + it("lists every loaded item for an empty query", () => { + const tool = getTool(extensionAPI, "zotero-search-items"); + + const output = tool.handler({ query: "" }, toolContext()) as { total: number, items: unknown[] }; + + expect(output.total).toBe(items.length); + expect(output.items.length).toBe(items.length); + }); }); describe("zotero-import-metadata", () => { @@ -199,6 +276,58 @@ describe("AI tool handlers", () => { .rejects.toThrow(/No Zotero item found for citekey "@noSuchCitekey2099"/); expect(importItemMetadata).not.toHaveBeenCalled(); }); + + it("resolves the citekey regardless of casing", async () => { + const tool = getTool(extensionAPI, "zotero-import-metadata"); + + await tool.handler({ citekey: "@BLOCHImplementingSocialInterventions2021" }, toolContext()); + + expect(importItemMetadata).toHaveBeenCalledWith( + expect.objectContaining({ item: blochItem }), + expect.anything(), expect.anything(), expect.anything(), expect.anything(), expect.anything() + ); + }); + + it("surfaces the error when the import fails", async () => { + importItemMetadata.mockResolvedValueOnce({ + args: { blocks: [], uid: existing_page_uid }, + error: new Error("Roam rejected the write"), + page: { new: true, title: "@blochImplementingSocialInterventions2021", uid: existing_page_uid }, + success: false + }); + const tool = getTool(extensionAPI, "zotero-import-metadata"); + + await expect(tool.handler({ citekey: "@blochImplementingSocialInterventions2021" }, toolContext())) + .rejects.toThrow("Roam rejected the write"); + }); + + it("surfaces a non-Error failure reason", async () => { + importItemMetadata.mockResolvedValueOnce({ + args: { blocks: [], uid: existing_page_uid }, + error: "a custom function threw a string", + page: { new: true, title: "@blochImplementingSocialInterventions2021", uid: existing_page_uid }, + success: false + }); + const tool = getTool(extensionAPI, "zotero-import-metadata"); + + await expect(tool.handler({ citekey: "@blochImplementingSocialInterventions2021" }, toolContext())) + .rejects.toThrow(/Metadata import failed: a custom function threw a string/); + }); + + // `addBlocksArray` resolves with `success: null` when the formatted output is empty, + // which leaves an empty page behind - an agent can't see that, so it must be an error + it("fails when the import wrote nothing", async () => { + importItemMetadata.mockResolvedValueOnce({ + args: { blocks: [], uid: existing_page_uid }, + error: null, + page: { new: true, title: "@blochImplementingSocialInterventions2021", uid: existing_page_uid }, + success: null + }); + const tool = getTool(extensionAPI, "zotero-import-metadata"); + + await expect(tool.handler({ citekey: "@blochImplementingSocialInterventions2021" }, toolContext())) + .rejects.toThrow(/nothing was written/); + }); }); describe("zotero-import-notes", () => { diff --git a/prototypes/zotero-roam-mcp/src/services/ai-tools/index.ts b/prototypes/zotero-roam-mcp/src/services/ai-tools/index.ts index b370138..bd247dd 100644 --- a/prototypes/zotero-roam-mcp/src/services/ai-tools/index.ts +++ b/prototypes/zotero-roam-mcp/src/services/ai-tools/index.ts @@ -1,54 +1,63 @@ -import { Roam, findRoamPage, hasBlockChildren, importItemMetadata, importItemNotes } from "@services/roam"; +import { Roam, findRoamPage, getCitekeyPages, hasBlockChildren, importItemMetadata, importItemNotes } from "@services/roam"; import { matchItems, normalizeCitekey, simplifyItemForAgent } from "./helpers"; -import { setupInitialSettings } from "../../setup"; +import { getCurrentSettings } from "../../setup"; import { categorizeLibraryItems, identifyChildren } from "../../utils"; -import { UserSettings } from "Types/extension"; -import { ZItemAnnotation, ZItemAttachment, ZItemNote, ZItemTop, ZLibraryContents } from "Types/transforms"; +import { OutcomeMetadataStatus, OutcomePage } from "Types/extension"; +import { ZItemAnnotation, ZItemAttachment, ZItemNote, ZItemTop, isZItemTop } from "Types/transforms"; -/** Retrieves the extension's current settings, merged with defaults. - * Settings are read at call time (rather than captured at registration), so that changes made through the settings dialog are picked up without a reload. - */ -function getCurrentSettings(extensionAPI: Roam.ExtensionAPI): UserSettings { - return setupInitialSettings((extensionAPI.settings.getAll() || {}) as Partial); -} - -/** Retrieves the categorized contents of the user's Zotero libraries, as currently loaded by the extension */ -function getLibraryContents(): ZLibraryContents { - return categorizeLibraryItems(window.zoteroRoam.getItems("all")); -} +/** The maximum number of items `zotero-search-items` will return in one call */ +const SEARCH_LIMIT_MAX = 50; +const SEARCH_LIMIT_DEFAULT = 20; -/** Finds a loaded Zotero item from its citekey. - * @returns The item and the categorized library contents it was found in +/** Finds a loaded Zotero item from its citekey, with its children (PDFs, notes/annotations). + * The citekey is matched case-insensitively, so that an agent transcribing a citekey from prose or a page title still resolves the item. + * @returns The item and its children, as `importItemMetadata` and `importItemNotes` expect them */ -function findItemByCitekey(citekey: string): { item: ZItemTop, libraryContents: ZLibraryContents } { +function findItemWithChildren(citekey: unknown): { item: ZItemTop, pdfs: ZItemAttachment[], notes: (ZItemNote | ZItemAnnotation)[] } { const key = normalizeCitekey(citekey); - const libraryContents = getLibraryContents(); - const item = libraryContents.items.find(it => it.key == key); + const libraryContents = categorizeLibraryItems(window.zoteroRoam.getItems("all")); + const item = libraryContents.items.find(it => it.key.toLowerCase() == key.toLowerCase()); if (!item) { throw new Error(`No Zotero item found for citekey "@${key}". The item may not be loaded yet, or may not have a pinned citation key - use the zotero-search-items tool to find items by title or DOI.`); } - return { item, libraryContents }; + const location = item.library.type + "s/" + item.library.id; + const { pdfs, notes } = identifyChildren(item.data.key, location, { pdfs: libraryContents.pdfs, notes: libraryContents.notes }); + + return { item, pdfs, notes }; } -/** Identifies an item's children (PDFs, notes/annotations) among the library contents */ -function findItemChildren(item: ZItemTop, libraryContents: ZLibraryContents): { pdfs: ZItemAttachment[], notes: (ZItemNote | ZItemAnnotation)[] } { - const location = item.library.type + "s/" + item.library.id; - return identifyChildren(item.data.key, location, { pdfs: libraryContents.pdfs, notes: libraryContents.notes }); +/** Shapes an import's outcome into an agent-facing result. + * @throws If the import failed, or completed with an uncertain outcome (which leaves an empty page behind) - an agent can't see the page, so anything short of a confirmed import is reported as an error. + */ +function reportImportOutcome(item: ZItemTop, outcome: { page: OutcomePage } & OutcomeMetadataStatus, fallbackMessage: string) { + if (outcome.success !== true) { + if (outcome.error) { + throw (outcome.error instanceof Error ? outcome.error : new Error(`${fallbackMessage}: ${String(outcome.error)}`)); + } + throw new Error(`${fallbackMessage}: nothing was written to [[@${item.key}]]. The item's formatted output was empty - check the extension's metadata settings (a custom function or SmartBlock may have returned nothing).`); + } + + return { + citekey: "@" + item.key, + page: outcome.page, + success: outcome.success + }; } /** Generates the list of AI tools to register. * Tool handlers read from `window.zoteroRoam` and from the extension's settings at call time, so they must only be invoked once the extension is fully loaded. + * Handler arguments are validated by Roam against each tool's `inputSchema` before the handler runs, but only for calls made through an agent - handlers are also callable directly from JS, so arguments are coerced rather than trusted. */ const aiTools = ({ extensionAPI }: { extensionAPI: Roam.ExtensionAPI }): Roam.AITool[] => [ { name: "zotero-search-items", - description: "Searches the Zotero items currently loaded by the zoteroRoam extension, by citekey, DOI, Zotero item key, or title substring. Read-only. Returns compact summaries, including each item's citekey and inGraph (the UID of its Roam page if one exists, otherwise false). Use this to find an item's exact citekey before calling zotero-import-metadata or zotero-import-notes. An empty query lists loaded items, up to the limit. Items are matched against the extension's local data, which syncs periodically from the Zotero API - an item added to Zotero moments ago may not be loaded yet.", + description: "Searches the Zotero items currently loaded by the zoteroRoam extension, by citekey, DOI, Zotero item key, or title substring. Read-only. Returns compact summaries, including each item's citekey and inGraph (the UID of its Roam page if one exists, otherwise false). Use this to find an item's exact citekey before calling zotero-import-metadata or zotero-import-notes. An empty query lists loaded items. `total` is the number of matches; when it exceeds the items returned, either narrow the query or page through the rest with `offset`. Items are matched against the extension's local data, which syncs periodically from the Zotero API - an item added to Zotero moments ago may not be loaded yet.", scope: "read", inputSchema: { type: "object", @@ -60,23 +69,33 @@ const aiTools = ({ extensionAPI }: { extensionAPI: Roam.ExtensionAPI }): Roam.AI limit: { type: "integer", minimum: 1, - maximum: 50, - default: 20, + maximum: SEARCH_LIMIT_MAX, + default: SEARCH_LIMIT_DEFAULT, description: "Maximum number of matches to return" + }, + offset: { + type: "integer", + minimum: 0, + default: 0, + description: "Number of matches to skip, for paging through a result set larger than the limit" } }, required: ["query"], additionalProperties: false }, - handler: ({ query, limit = 20 }) => { - const { items } = getLibraryContents(); - const matches = matchItems(items, String(query ?? "")); + handler: ({ query, limit, offset }) => { + const size = Math.min(Number(limit) || SEARCH_LIMIT_DEFAULT, SEARCH_LIMIT_MAX); + const start = Math.max(Number(offset) || 0, 0); + + // `getItems("items")` already excludes children, but is typed as the full union - narrow it back + const matches = matchItems(window.zoteroRoam.getItems("items").filter(isZItemTop), query); + const citekeyPages = getCitekeyPages(); return { total: matches.length, items: matches - .slice(0, Number(limit) || 20) - .map(item => simplifyItemForAgent(item, { inGraph: findRoamPage("@" + item.key) })) + .slice(start, start + size) + .map(item => simplifyItemForAgent(item, { inGraph: citekeyPages.get("@" + item.key) || false })) }; } }, @@ -100,33 +119,26 @@ const aiTools = ({ extensionAPI }: { extensionAPI: Roam.ExtensionAPI }): Roam.AI required: ["citekey"], additionalProperties: false }, - handler: async ({ citekey, allowDuplicate = false }) => { - const { item, libraryContents } = findItemByCitekey(citekey); - const { pdfs, notes } = findItemChildren(item, libraryContents); + handler: async ({ citekey, allowDuplicate }) => { + const { item, pdfs, notes } = findItemWithChildren(citekey); const uid = findRoamPage("@" + item.key); - if (uid && !allowDuplicate && hasBlockChildren(uid)) { + if (uid && allowDuplicate !== true && hasBlockChildren(uid)) { throw new Error(`The page [[@${item.key}]] already has content - its metadata may already have been imported. Pass allowDuplicate: true to import anyway (this will add another copy of the metadata at the top of the page).`); } const settings = getCurrentSettings(extensionAPI); const outcome = await importItemMetadata({ item, pdfs, notes }, uid, settings.metadata, settings.typemap, settings.notes, settings.annotations); - if (outcome.success === false) { - throw (outcome.error instanceof Error ? outcome.error : new Error("Metadata import failed")); - } - return { - citekey: "@" + item.key, - page: outcome.page, - success: outcome.success, + ...reportImportOutcome(item, outcome, "Metadata import failed"), blocksAdded: outcome.args && "blocks" in outcome.args ? outcome.args.blocks.length : null }; } }, { name: "zotero-import-notes", - description: "Imports a Zotero item's notes and PDF annotations into Roam, as blocks on its citekey page ([[@citekey]]) - creating that page if needed. This is the headless equivalent of zoteroRoam's 'Import notes' button: notes and annotations are formatted with the user's notes/annotations settings. Fails if the item has no notes or annotations, or if the citekey doesn't match a loaded item - use zotero-search-items first when unsure.", + description: "Imports a Zotero item's notes and PDF annotations into Roam, as blocks on its citekey page ([[@citekey]]) - creating that page if needed. This is the headless equivalent of zoteroRoam's 'Import notes' button: notes and annotations are formatted with the user's notes/annotations settings. Fails if the item has no notes or annotations, or if the citekey doesn't match a loaded item - use zotero-search-items first when unsure. Unlike zotero-import-metadata this call has no duplicate guard, because the citekey page normally already holds the item's metadata: calling it twice imports the notes twice, so do not retry it blindly (a call that exceeds the handler deadline may still have written its blocks).", scope: "edit", inputSchema: { type: "object", @@ -140,8 +152,7 @@ const aiTools = ({ extensionAPI }: { extensionAPI: Roam.ExtensionAPI }): Roam.AI additionalProperties: false }, handler: async ({ citekey }) => { - const { item, libraryContents } = findItemByCitekey(citekey); - const { notes } = findItemChildren(item, libraryContents); + const { item, notes } = findItemWithChildren(citekey); if (notes.length == 0) { throw new Error(`The item "@${item.key}" has no notes or annotations in Zotero - there is nothing to import.`); @@ -151,33 +162,50 @@ const aiTools = ({ extensionAPI }: { extensionAPI: Roam.ExtensionAPI }): Roam.AI const settings = getCurrentSettings(extensionAPI); const outcome = await importItemNotes({ item, notes }, uid, settings.notes, settings.annotations); - if (outcome.success === false) { - throw (outcome.error instanceof Error ? outcome.error : new Error("Notes import failed")); - } - return { - citekey: "@" + item.key, - page: outcome.page, - success: outcome.success, + ...reportImportOutcome(item, outcome, "Notes import failed"), notesImported: notes.length }; } } ]; +// Stored at registration so that teardown can reach the API: Roam's `onunload` receives no arguments +let registeredWith: Roam.ExtensionAPI | null = null; + /** Registers the extension's AI tools, so that agents connected through Roam's MCP server can invoke them (via `call_extension_tool`). - * No-ops on Roam builds that predate extension AI tools. Safe to call again (re-adding a name updates the tool in place); registered tools are removed automatically when the extension is unloaded. + * No-ops on Roam builds that predate extension AI tools. Safe to call again (re-adding a name updates the tool in place). * @returns Whether the tools were registered */ function registerAiTools({ extensionAPI }: { extensionAPI: Roam.ExtensionAPI }): boolean { const ai = extensionAPI.ai; if (!ai || typeof ai.addTool !== "function") { + registeredWith = null; return false; } + unregisterAiTools(); + aiTools({ extensionAPI }).forEach(tool => ai.addTool(tool)); + registeredWith = extensionAPI; return true; } +/** Unregisters the extension's AI tools. + * Roam removes an extension's tools automatically on unload; this makes the teardown explicit, as for the extension's other registrations, so that a stale tool can never outlive the data it reads. + */ +function unregisterAiTools(): void { + const extensionAPI = registeredWith; + const ai = extensionAPI?.ai; + + registeredWith = null; + + if (!ai || typeof ai.removeTool !== "function") { + return; + } + + aiTools({ extensionAPI: extensionAPI! }).forEach(({ name }) => ai.removeTool({ name })); +} + -export { aiTools, registerAiTools }; +export { aiTools, registerAiTools, unregisterAiTools }; diff --git a/prototypes/zotero-roam-mcp/src/setup.ts b/prototypes/zotero-roam-mcp/src/setup.ts index fe3ba9e..eaeca79 100644 --- a/prototypes/zotero-roam-mcp/src/setup.ts +++ b/prototypes/zotero-roam-mcp/src/setup.ts @@ -184,6 +184,13 @@ export function shouldQueryBePersisted(query: Query){ return defaultShouldDehydrateQuery(query); } +/** Retrieves the extension's current settings from the extensionAPI store, merged with defaults. + * This is the single source of truth for "the extension's settings right now": the settings dialog writes through to the store on every change, so reading at call time always reflects the latest values. + */ +export function getCurrentSettings(extensionAPI: Roam.ExtensionAPI): UserSettings { + return setupInitialSettings((extensionAPI.settings.getAll() || {}) as Partial); +} + /** Generates a merged settings object, combining user settings and defaults. */ export function setupInitialSettings(settingsObject: Partial): UserSettings{ const { @@ -330,7 +337,7 @@ function configRoamDepot({ extensionAPI }: { extensionAPI: Roam.ExtensionAPI }){ // Subsequent loads: merge defaults in-memory only, don't write back // This preserves user settings even if Roam returns incomplete data - const settings = setupInitialSettings(current || {}); + const settings = getCurrentSettings(extensionAPI); const requests = extensionAPI.settings.get("requests") || { dataRequests: [], apiKeys: [],