Adding an LLM/MCP tool
One tool definition, two consumers. Every tool under packages/trilium-core/src/services/llm/tools/ is declared once via defineTools({...}) and consumed by BOTH the in-app LLM chat AND the external MCP server. The wiring rules below all fall out of that fact — internalize it before touching anything.
Footgun #1 (the big one): execute MUST be synchronous
No async, no await, no returned Promise. This is not a style preference — better-sqlite3 transactions are synchronous, so an async execute lets getSql().transactional() commit before the awaited work runs, silently corrupting entity-change/Becca tracking.
The type system is built to make this a compile error (tool_registry.ts:21,32,40):
type NotAPromise<T> = T & { then?: void }; // line 21
// ...
execute: (args: any) => NotAPromise<object>; // lines 32 (mutating) & 40 (read-only)
A Promise has then: Function, which violates then?: void → typecheck rejects it. It regressed twice anyway (git show 09be2822e0 "fix(llm): some tools were async", a93029f789 "fix(llm): misuse of transactions in tool use due to async") — the NotAPromise guard is the durable fix. Do not weaken it (no as any, no widening the return type). If you need data, fetch it synchronously through Becca / the sync services; the tools deliberately reuse the same logic as ETAPI without HTTP.
Footgun #2: mutates: true is load-bearing wiring, not a label
Both consumers branch on it to wrap the call in a transaction. Forget it on a write tool and execute runs outside a transaction — no error, just broken entity-change tracking.
- LLM chat:
tool_registry.ts:65-66—def.mutates ? (args) => getSql().transactional(() => def.execute(args)) : def.execute - MCP:
mcp_server.ts:29-33(note: it lives inapps/server/src/services/mcp/, not underllm/) — the same branch, inside its owncls.init
Rule: any tool that writes (setContent, save, setAttribute, createNewNote, branch/clone/move, deleteNote, markAsDeleted) gets mutates: true. Read-only tools omit it (or mutates: false).
Footgun #3: a new module is invisible until registered
allToolRegistries (packages/trilium-core/src/services/llm/tools/index.ts:34) is the single wiring point iterated by both mcp_server.ts:51 and base_provider.ts:389 (llm/providers/base_provider.ts, Object.assign(tools, registry.toToolSet())). Adding a tool to an existing module (e.g. another entry in note_tools.ts) needs no wiring. Creating a new module means: export const xTools = defineTools({...}), add the export/import lines in index.ts, and append xTools to the allToolRegistries array. Miss the array and chat + MCP both never see it.
Node-only tools take the other door. A tool that needs something core cannot have in the browser
(the in-app documentation reader, for instance) lives in apps/server/src/services/llm/tools/
(doc_notes.ts, help_tools.ts) and registers at server startup via
registerToolRegistryLoader(async () => (await import("./tools/x.js")).xTools) from core's light
tools/registration.ts (see registerServerLlmExtensions); the first chat turn or MCP request
resolves the loader through resolveToolRegistries() and appends the registry to the same array.
Register the loader, never a static import — the loader form is what keeps the tool stack (zod, the
AI SDK) out of the server's startup path. Standalone simply runs without it. Put a tool there only if it genuinely cannot run under sqlite-wasm —
core is the default home, and anything placed in the server loses the standalone and desktop
consumers.
Footgun #4: return { error: "..." } — never throw
The pipeline keys off the literal error property; a thrown exception escapes the contract. Every guard does return { error: "Note not found" } (note_tools.ts:72,87,110). Service calls that can throw are wrapped in try/catch that converts to { error } (see create_note in note_tools.ts):
try {
const { note } = noteService.createNewNote({ parentNoteId, title, content: htmlContent, type });
return { success: true, noteId: note.noteId, /* ... */ };
} catch (err) {
return { error: err instanceof Error ? err.message : "Failed to create note" };
}
Footgun #5: protected / system-note guards are mandatory and ordered
Skipping these lets the LLM corrupt protected or system notes. Apply in this order (see note_tools.ts):
| Check | Guard | Returns |
|---|---|---|
| Note exists | !becca.getNote(id) |
{ error: "Note not found" } |
| Not protected | !note.isContentAvailable() |
{ error: "Note is protected..." } |
| Right content kind | !note.hasStringContent() |
{ error: "Cannot ... note type: ${note.type}" } |
| Stored content is text | typeof note.getContent() !== "string" |
{ error: "Note has binary content" } |
| Rename/delete | note.isProtected |
{ error: "...cannot be renamed/deleted" } |
| Delete/move/clone a system note | PROTECTED_SYSTEM_NOTES.has(noteId) |
{ error: "Cannot delete system notes" } |
PROTECTED_SYSTEM_NOTES lives in helpers.ts:19 = new Set(["root", "_hidden", "_share", "_lbRoot", "_globalNoteMap"]). For attribute writes, also guard attributeService.isAttributeDangerous(type, name) and (for relations) a missing target note (attribute_tools.ts:75).
Mirror ETAPI's field choices, but never import its mappers. A tool that returns a note should
pick the same fields ETAPI's response does, so the two surfaces describe an entity the same way —
but inline that mapping in the tool. This used to be a discipline; since the tools moved into
packages/trilium-core it is also structural, because ETAPI lives in apps/server/src/etapi/ and
core cannot import from an app. If you find yourself wanting a shared mapper, the type belongs in
@triliumnext/commons, not in a cross-layer import.
The recipe (ordered)
- Pick or create the module (
{note,attribute,attachment,hierarchy,icon,skill}_tools.ts, or a new*_tools.ts). Declare the tool insidedefineTools({...}). - Give it
description(string the LLM reads),inputSchema(z.object({...})with.describe()on each field), andexecute— synchronous (footgun #1). - For writes, add
mutates: true(footgun #2). - Guard inputs in order and
return { error }on every failure branch (footguns #4, #5); wrap throwing service calls in try/catch. - New module only: register it in
allToolRegistries(footgun #3). - Add the client-side friendly name in
apps/client/src/translations/en/translation.jsonunderllm.tools.<tool_name>, imperative tense ("Create note", not "Creating note"). English only — other locales come via Weblate (see CLAUDE.md / translating-locales). - Write the spec with the
getTool()+cls.init()harness — see spec-harness.md.
Decision table — guards & test harness per tool shape
| Tool does… | mutates |
Required guards (in order) | Spec harness |
|---|---|---|---|
| read-only (search/get) | omit | !note → error; isContentAvailable() for content reads |
plain getTool(name).execute(args); mock search.findResultsWithQuery if it searches |
| edit existing note content | true |
!note → not found; !isContentAvailable() → protected; !hasStringContent() → bad type; binary getContent() → binary |
Pattern A: buildNote + stub setContent/saveRevision (no CLS) |
| create / move / clone (service writes) | true |
parent !isContentAvailable(); wrap service call in try/catch → { error } |
Pattern B: cls.init(() => createNewNote(...)) to seed, cls.init(() => getTool(...).execute(...)) to call |
| rename / delete | true |
PROTECTED_SYSTEM_NOTES.has(noteId) first; then !note; then note.isProtected |
Pattern A (mock deleteNote/save) or Pattern B |
The two harness patterns are spelled out fully in the reference — don't re-derive the boilerplate.
Quick verification checklist (before you finish)
-
executeis notasyncand returns no Promise (typecheck rejects it otherwise — runpnpm typecheck). - Every write tool has
mutates: true. - A brand-new module is in
allToolRegistries(index.ts:34). - All failure branches
return { error }; the onlythrows are service calls wrapped in try/catch. - Protected/system-note guards present and ordered (table above).
- Friendly name added under
llm.tools.<name>inen/translation.json, imperative tense. - Spec covers happy path and every guard branch, asserting the literal
{ error: ... }object — and that the success path does NOT leak anerrorproperty (expect(result).not.toHaveProperty("error")).
Reference map
| File | When to open |
|---|---|
| references/spec-harness.md | Writing the *_tools.spec.ts — the getTool() iterator, Pattern A (mock persistence, no CLS), Pattern B (real becca + cls.init), and the error-object assertions. |
Cross-links: writing-unit-tests (general Vitest patterns, the CoreApiTester, becca/froca fixtures, single-file run commands), translating-locales (why en-only and how Weblate picks up the rest), analyzing-coverage (chasing the spec to 100%).