feat(ai-chat): insertFootnote/insertImage/replaceImage для in-app агента (#410) #418

Merged
vvzvlad merged 2 commits from feat/410-agent-footnote-image into develop 2026-07-07 21:39:01 +03:00
12 changed files with 400 additions and 103 deletions
@@ -249,6 +249,17 @@ roles:
DEDUP. Identical `^[...]` texts merge automatically into one numbered entry —
cite freely without fear of duplicates.
WHICH WRITE PATH PARSES `^[...]`. The `^[...]` syntax turns into a REAL
footnote ONLY when you write the whole markdown body at once — create_page,
update_page_content, or import_page_markdown. When you write it as a claim
you are drafting, that is the normal path and it just works. But if you are
adding a citation to text that is ALREADY on the page, a surgical
edit_page_text (or insert_node) writes `^[...]` as a LITERAL string — it does
NOT parse, and the reader sees the raw `^[...]`. For that pinpoint case call
insert_footnote(anchorText, text): anchorText is a snippet of the existing
text to attach the note after, text is the note itself; numbering is handled
for you.
═══════════════════════════════════════════════
PROSE, NOT NOTES
═══════════════════════════════════════════════
@@ -249,6 +249,17 @@ roles:
DEDUP. Identical `^[...]` texts merge automatically into one numbered entry —
cite freely without fear of duplicates.
WHICH WRITE PATH PARSES `^[...]`. The `^[...]` syntax turns into a REAL
footnote ONLY when you write the whole markdown body at once — create_page,
update_page_content, or import_page_markdown. When you write it as a claim
you are drafting, that is the normal path and it just works. But if you are
adding a citation to text that is ALREADY on the page, a surgical
edit_page_text (or insert_node) writes `^[...]` as a LITERAL string — it does
NOT parse, and the reader sees the raw `^[...]`. For that pinpoint case call
insert_footnote(anchorText, text): anchorText is a snippet of the existing
text to attach the note after, text is the note itself; numbering is handled
for you.
═══════════════════════════════════════════════
PROSE, NOT NOTES
═══════════════════════════════════════════════
@@ -652,6 +652,125 @@ describe('AiChatToolsService #294 changed execute wirings', () => {
});
});
/**
* #410 — the footnote + image tools were promoted from MCP-only into the shared
* registry and are now wired in-app. Assert they are REGISTERED in the in-app
* toolset and forward their args to the client with the correct arg->method
* mapping (the schema fields `imageUrl`/`attachmentId` map onto the client's
* positional `url`/`oldAttachmentId`). A field destructured under the wrong name
* would silently pass `undefined` (execute is `any`-cast, so tsc won't catch it).
*/
describe('AiChatToolsService #410 footnote + image tools', () => {
const calls: Record<string, unknown[][]> = {
insertFootnote: [],
insertImage: [],
replaceImage: [],
};
const fakeClient: Partial<DocmostClientLike> = {
insertFootnote: (...args: unknown[]) => {
calls.insertFootnote.push(args);
return Promise.resolve({ success: true, footnoteId: 'fn1', reused: false });
},
insertImage: (...args: unknown[]) => {
calls.insertImage.push(args);
return Promise.resolve({ success: true, attachmentId: 'att1' });
},
replaceImage: (...args: unknown[]) => {
calls.replaceImage.push(args);
return Promise.resolve({ success: true, replaced: 1 });
},
};
const tokenServiceStub = {
generateAccessToken: jest.fn().mockResolvedValue('access-token'),
generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
};
let service: AiChatToolsService;
beforeEach(() => {
for (const k of Object.keys(calls)) calls[k].length = 0;
jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue(
mockLoaded(function () {
return fakeClient as DocmostClientLike;
} as unknown as loader.DocmostClientCtor),
);
service = new AiChatToolsService(
tokenServiceStub as never,
{} as never,
{} as never,
{} as never,
{} as never,
{
asSink: () => ({ put: jest.fn(), has: jest.fn(), evict: jest.fn() }),
} as never,
);
});
afterEach(() => jest.restoreAllMocks());
const buildTools = () =>
service.forUser(
{ id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never,
'session-1',
'ws-1',
'chat-1',
);
it('registers all three tools in the in-app toolset', async () => {
const tools = await buildTools();
expect(tools.insertFootnote).toBeDefined();
expect(tools.insertImage).toBeDefined();
expect(tools.replaceImage).toBeDefined();
});
it('insertFootnote forwards (pageId, anchorText, text) positionally', async () => {
const tools = await buildTools();
const r = await tools.insertFootnote.execute(
{ pageId: 'p1', anchorText: 'the claim', text: 'See source.' } as never,
{} as never,
);
expect(calls.insertFootnote).toEqual([['p1', 'the claim', 'See source.']]);
expect(r).toMatchObject({ footnoteId: 'fn1' });
});
it('insertImage maps imageUrl->url and packs the option fields', async () => {
const tools = await buildTools();
await tools.insertImage.execute(
{
pageId: 'p1',
imageUrl: 'https://x/img.png',
align: 'center',
alt: 'A',
replaceText: '[img]',
afterText: undefined,
} as never,
{} as never,
);
expect(calls.insertImage).toEqual([
[
'p1',
'https://x/img.png',
{ align: 'center', alt: 'A', replaceText: '[img]', afterText: undefined },
],
]);
});
it('replaceImage maps attachmentId->oldAttachmentId and imageUrl->url', async () => {
const tools = await buildTools();
await tools.replaceImage.execute(
{
pageId: 'p1',
attachmentId: 'att-old',
imageUrl: 'https://x/new.png',
align: 'right',
alt: 'B',
} as never,
{} as never,
);
expect(calls.replaceImage).toEqual([
['p1', 'att-old', 'https://x/new.png', { align: 'right', alt: 'B' }],
]);
});
});
/**
* getCurrentPage selection contract (#388): the tool surfaces the selection that
* was sanitized + nested onto the resolved open-page context (last forUser arg).
@@ -697,6 +697,38 @@ export class AiChatToolsService {
},
),
// Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#410).
// Promoted from MCP-only so the in-app agent can attach a REAL footnote to
// already-written text instead of leaving a literal `^[...]` string.
insertFootnote: sharedTool(
sharedToolSpecs.insertFootnote,
async ({ pageId, anchorText, text }) =>
await client.insertFootnote(pageId, anchorText, text),
),
// Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#410).
// The schema field is `imageUrl`; the client method takes it positionally.
insertImage: sharedTool(
sharedToolSpecs.insertImage,
async ({ pageId, imageUrl, align, alt, replaceText, afterText }) =>
await client.insertImage(pageId, imageUrl, {
align,
alt,
replaceText,
afterText,
}),
),
// Schema + description live in @docmost/mcp's SHARED_TOOL_SPECS (#410).
replaceImage: sharedTool(
sharedToolSpecs.replaceImage,
async ({ pageId, attachmentId, imageUrl, align, alt }) =>
await client.replaceImage(pageId, attachmentId, imageUrl, {
align,
alt,
}),
),
// Schema + description now live in @docmost/mcp's SHARED_TOOL_SPECS (#294).
// The table reference parameter was unified to `table` (was `tableRef`).
tableInsertRow: sharedTool(
@@ -141,6 +141,33 @@ export interface DocmostClientLike {
doc?: unknown,
title?: string,
): Promise<Record<string, unknown>>;
// Attach an author-inline footnote after the first occurrence of anchorText;
// numbering + the footnotes list are derived server-side.
insertFootnote(
pageId: string,
anchorText: string,
text: string,
): Promise<Record<string, unknown>>;
// Download a web image and insert it into the page (append, or replace/after a
// text anchor). `url` is the image http(s) URL.
insertImage(
pageId: string,
url: string,
opts?: {
align?: 'left' | 'center' | 'right';
alt?: string;
replaceText?: string;
afterText?: string;
},
): Promise<Record<string, unknown>>;
// Swap an existing image (by its attachmentId) for a new one fetched from a web
// URL, repointing every reference in the live document.
replaceImage(
pageId: string,
oldAttachmentId: string,
url: string,
opts?: { align?: 'left' | 'center' | 'right'; alt?: string },
): Promise<Record<string, unknown>>;
tableInsertRow(
pageId: string,
tableRef: string,
@@ -27,13 +27,26 @@ import type { DocmostClientLike } from './docmost-client.loader';
*/
describe('tool tier metadata (#332)', () => {
it('core set is the documented 13 + searchInPage (14)', () => {
expect(CORE_TOOL_KEYS).toHaveLength(14);
it('core set is the documented 13 + searchInPage + insertFootnote (15)', () => {
expect(CORE_TOOL_KEYS).toHaveLength(15);
expect(CORE_TOOL_SET.has('searchInPage')).toBe(true); // #330, promoted to core
expect(CORE_TOOL_SET.has('insertFootnote')).toBe(true); // #410, promoted to core
// loadTools is a meta-tool, not a normal core key.
expect(CORE_TOOL_SET.has(LOAD_TOOLS_NAME)).toBe(false);
});
it('#410 image tools are DEFERRED, footnote tool is CORE', () => {
// insert_footnote is core (symmetric with editPageText); the image tools stay
// deferred (rare, fat — loaded on demand). Assert both the spec tier and the
// CORE_TOOL_SET membership so a future tier edit that desyncs them fails here.
expect(SHARED_TOOL_SPECS.insertFootnote.tier).toBe('core');
expect(CORE_TOOL_SET.has('insertFootnote')).toBe(true);
expect(SHARED_TOOL_SPECS.insertImage.tier).toBe('deferred');
expect(CORE_TOOL_SET.has('insertImage')).toBe(false);
expect(SHARED_TOOL_SPECS.replaceImage.tier).toBe('deferred');
expect(CORE_TOOL_SET.has('replaceImage')).toBe(false);
});
it('SHARED_TOOL_SPECS tier agrees with CORE_TOOL_SET for every shared tool', () => {
for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
const isCoreByTier = spec.tier === 'core';
@@ -38,10 +38,13 @@ export interface ToolCatalogEntry {
}
/**
* CORE (always-active) in-app tool keys — 13 frequent/tiny tools. `searchInPage`
* (#330) is added to core on top of the issue's original tier list: it is
* frequent for the editorial roles this feature targets. `loadTools` is active
* too but is not a normal tool key (it is added to activeTools separately).
* CORE (always-active) in-app tool keys — 13 frequent/tiny tools + `searchInPage`
* (#330) + `insertFootnote` (#410). `searchInPage` is core because it is frequent
* for the editorial roles this feature targets; `insertFootnote` is core so the
* footnote tool is NOT hidden while its natural sibling `editPageText` is always
* active (that asymmetry is exactly what pushed the agent to write literal
* `^[...]`). `loadTools` is active too but is not a normal tool key (it is added
* to activeTools separately).
*/
export const CORE_TOOL_KEYS = [
'searchPages',
@@ -60,6 +63,9 @@ export const CORE_TOOL_KEYS = [
// #330 search_in_page — frequent for editorial sweeps; core despite predating
// the issue's tier list.
'searchInPage',
// #410 insert_footnote — core so pinpoint citations to already-written text
// don't degrade into literal `^[...]`; kept symmetric with editPageText.
'insertFootnote',
] as const;
/** O(1) membership test for the core tier. */
+11 -95
View File
@@ -427,43 +427,10 @@ registerShared(SHARED_TOOL_SPECS.deleteNode, async ({ pageId, nodeId }) => {
});
// Tool: insert_image
// MCP-only by design (NOT in the shared registry): the in-app AI-chat agent
// exposes no image tools (insert/replace), so there is no second layer to unify
// — a SHARED_TOOL_SPECS entry's tier/catalogLine are in-app metadata and the
// catalog-partition test forbids a spec without a live in-app tool (#294).
server.registerTool(
"insert_image",
{
description:
"Download an image from a web (http/https) URL and insert it into " +
"a page in one step. By default " +
"appends the image at the end of the page. With replaceText, replaces the " +
"first top-level block whose text contains that string (handy for " +
'swapping a text placeholder like "[image: foo.png]" for the real image). ' +
"With afterText, inserts the image right after the first block containing " +
"that string. Preserves all other block ids.",
inputSchema: {
pageId: z.string().min(1),
imageUrl: z
.string()
.min(1)
.describe("http(s) URL of the image to download and upload"),
align: z.enum(["left", "center", "right"]).optional(),
alt: z.string().optional(),
replaceText: z
.string()
.optional()
.describe(
"Replace the first top-level block whose text contains this string with the image",
),
afterText: z
.string()
.optional()
.describe(
"Insert the image right after the first top-level block whose text contains this string",
),
},
},
// Schema + description now live in the shared registry (#410) so BOTH this MCP
// server and the in-app AI-chat agent expose it. The execute body is unchanged.
registerShared(
SHARED_TOOL_SPECS.insertImage,
async ({ pageId, imageUrl, align, alt, replaceText, afterText }) => {
const result = await docmostClient.insertImage(pageId, imageUrl, {
align,
@@ -476,34 +443,9 @@ server.registerTool(
);
// Tool: replace_image
// MCP-only by design (see insert_image): no in-app equivalent, stays inline.
server.registerTool(
"replace_image",
{
description:
"Replace an existing image on a page with a new image fetched from a web " +
"(http/https) URL: uploads the new file as a NEW " +
"attachment (fresh clean URL that renders and busts browser caches), then " +
"repoints every image node referencing the old attachmentId (recursively, " +
"incl. callouts/tables) via the live document, preserving comments, " +
"alignment and alt. The old attachment is left as an unreferenced orphan " +
"(Docmost has no API to delete a single attachment; it is removed only when " +
"the page/space is deleted). In-place byte overwrite is avoided because some " +
"Docmost versions corrupt the attachment (HTTP 500) on overwrite.",
inputSchema: {
pageId: z.string().min(1),
attachmentId: z
.string()
.min(1)
.describe("attachmentId of the image currently in the page to replace"),
imageUrl: z
.string()
.min(1)
.describe("http(s) URL of the new image to download"),
align: z.enum(["left", "center", "right"]).optional(),
alt: z.string().optional(),
},
},
// Schema + description now live in the shared registry (#410).
registerShared(
SHARED_TOOL_SPECS.replaceImage,
async ({ pageId, attachmentId, imageUrl, align, alt }) => {
const result = await docmostClient.replaceImage(
pageId,
@@ -826,36 +768,10 @@ server.registerTool(
);
// Tool: insert_footnote
// MCP-only by design (see insert_image): the in-app AI-chat agent exposes no
// footnote tool, so there is no second layer to unify — stays inline (#294).
server.registerTool(
"insert_footnote",
{
description:
"Insert an AUTHOR-INLINE footnote: you specify only WHERE (anchorText) " +
"and WHAT (text). The footnote marker is placed right after anchorText in " +
"the body, and the bottom footnotes list + the numbering are derived " +
"deterministically server-side. You do NOT assign a number, and you " +
"never see or edit the footnotes list — so footnotes cannot end up out " +
"of order, orphaned, or as a raw '[^id]' block. If a footnote with the " +
"SAME text already exists, its number is REUSED (one definition, several " +
"references). The write is atomic and won't clobber concurrent edits; if " +
"anchorText is not found, nothing is written and an error is returned.",
inputSchema: {
pageId: z.string().min(1),
anchorText: z
.string()
.min(1)
.describe(
"A snippet of existing body text; the footnote marker is inserted " +
"immediately after its first occurrence (mark-safe).",
),
text: z
.string()
.min(1)
.describe("The footnote content as markdown (becomes the definition)."),
},
},
// Schema + description now live in the shared registry (#410) so the in-app
// AI-chat agent exposes it too. The execute body is unchanged.
registerShared(
SHARED_TOOL_SPECS.insertFootnote,
async ({ pageId, anchorText, text }) => {
const result = await docmostClient.insertFootnote(pageId, anchorText, text);
return jsonContent(result);
+15
View File
@@ -305,6 +305,21 @@ export function applyTextEdits(
continue;
}
// HARD-REFUSE inline footnote tokens (#410). `^[...]` in a `replace` is
// markdown that only becomes a real footnote when a whole markdown body is
// written (create_page / update_page_content / import_page_markdown). Written
// through edit_page_text it stays a LITERAL string in the text — the exact
// failure mode #410 fixes — so refuse it here (defense-in-depth) and point the
// caller at insert_footnote, mirroring the formatting-marker refusal above.
if (/\^\[[\s\S]*?\]/.test(edit.replace)) {
failed.push({
find: edit.find,
reason:
"edit_page_text writes the replacement as LITERAL text, so a `^[...]` footnote token does not parse into a real footnote (it would appear verbatim in the page). To add a footnote to existing text, use insert_footnote (anchorText = where, text = the note).",
});
continue;
}
// Gather every inline block in document order (recurse the whole tree so
// nested containers — callouts, list items, table cells, blockquotes — are
// all covered).
+112
View File
@@ -1003,4 +1003,116 @@ export const SHARED_TOOL_SPECS = {
text: z.string().describe('The new cell text.'),
}),
},
// --- footnote + image write tools (promoted from inline MCP-only, #410) ---
//
// These three were previously registered inline in index.ts as MCP-only,
// because the in-app AI-chat agent had no equivalent. #410 promotes them so the
// in-app agent (esp. the Researcher role) can attach real footnotes/images
// instead of writing literal `^[...]` / placeholder text via editPageText. The
// schema + description are MOVED VERBATIM from the old inline registrations so
// external MCP clients see identical tool names, fields and text.
insertFootnote: {
mcpName: 'insert_footnote',
inAppKey: 'insertFootnote',
description:
'Insert an AUTHOR-INLINE footnote: you specify only WHERE (anchorText) ' +
'and WHAT (text). The footnote marker is placed right after anchorText in ' +
'the body, and the bottom footnotes list + the numbering are derived ' +
'deterministically server-side. You do NOT assign a number, and you ' +
"never see or edit the footnotes list — so footnotes cannot end up out " +
"of order, orphaned, or as a raw '[^id]' block. If a footnote with the " +
'SAME text already exists, its number is REUSED (one definition, several ' +
"references). The write is atomic and won't clobber concurrent edits; if " +
'anchorText is not found, nothing is written and an error is returned.',
// CORE for the in-app agent (#410): keeping it deferred would recreate the
// original asymmetry (footnote tool hidden while editPageText is core), which
// is exactly what makes the agent fall back to literal `^[...]`.
tier: 'core',
catalogLine:
'insertFootnote — attach a numbered footnote right after a snippet of existing body text.',
buildShape: (z) => ({
pageId: z.string().min(1),
anchorText: z
.string()
.min(1)
.describe(
'A snippet of existing body text; the footnote marker is inserted ' +
'immediately after its first occurrence (mark-safe).',
),
text: z
.string()
.min(1)
.describe('The footnote content as markdown (becomes the definition).'),
}),
},
insertImage: {
mcpName: 'insert_image',
inAppKey: 'insertImage',
description:
'Download an image from a web (http/https) URL and insert it into ' +
'a page in one step. By default ' +
'appends the image at the end of the page. With replaceText, replaces the ' +
'first top-level block whose text contains that string (handy for ' +
'swapping a text placeholder like "[image: foo.png]" for the real image). ' +
'With afterText, inserts the image right after the first block containing ' +
'that string. Preserves all other block ids.',
tier: 'deferred',
catalogLine:
'insertImage — download a web image and insert it into a page.',
buildShape: (z) => ({
pageId: z.string().min(1),
imageUrl: z
.string()
.min(1)
.describe('http(s) URL of the image to download and upload'),
align: z.enum(['left', 'center', 'right']).optional(),
alt: z.string().optional(),
replaceText: z
.string()
.optional()
.describe(
'Replace the first top-level block whose text contains this string with the image',
),
afterText: z
.string()
.optional()
.describe(
'Insert the image right after the first top-level block whose text contains this string',
),
}),
},
replaceImage: {
mcpName: 'replace_image',
inAppKey: 'replaceImage',
description:
'Replace an existing image on a page with a new image fetched from a web ' +
'(http/https) URL: uploads the new file as a NEW ' +
'attachment (fresh clean URL that renders and busts browser caches), then ' +
'repoints every image node referencing the old attachmentId (recursively, ' +
'incl. callouts/tables) via the live document, preserving comments, ' +
'alignment and alt. The old attachment is left as an unreferenced orphan ' +
'(Docmost has no API to delete a single attachment; it is removed only when ' +
'the page/space is deleted). In-place byte overwrite is avoided because some ' +
'Docmost versions corrupt the attachment (HTTP 500) on overwrite.',
tier: 'deferred',
catalogLine:
'replaceImage — swap an existing page image for one fetched from a web URL.',
buildShape: (z) => ({
pageId: z.string().min(1),
attachmentId: z
.string()
.min(1)
.describe('attachmentId of the image currently in the page to replace'),
imageUrl: z
.string()
.min(1)
.describe('http(s) URL of the new image to download'),
align: z.enum(['left', 'center', 'right']).optional(),
alt: z.string().optional(),
}),
},
} satisfies Record<string, SharedToolSpec>;
@@ -28,8 +28,9 @@ import { DocmostClient } from "../../build/index.js";
// in the server's DocmostClientLike interface (the in-app per-user tool adapter
// only — it is a SUBSET of the DocmostClient surface — covers only what the in-app adapter
// consumes; the standalone MCP transport (packages/mcp/src/index.ts) calls additional
// client methods (insertImage/replaceImage/deleteComment/updateComment/insertFootnote)
// that this guard does NOT track — the MCP transport's own typecheck covers those). Full type-derivation
// client methods (deleteComment/updateComment) that this guard does NOT track — the
// MCP transport's own typecheck covers those. insertImage/replaceImage/insertFootnote
// were MCP-only but are now in-app-consumed too (#410), so they ARE tracked below. Full type-derivation
// of DocmostClientLike from this class is deferred (see the staged plan in
// docmost-client.loader.ts): the package emits no declarations and the real
// (inferred, concrete) return types conflict with the host's loose
@@ -76,6 +77,10 @@ const HOST_CONTRACT_METHODS = [
"restorePageVersion",
"transformPage",
"stashPage",
// write (image / footnote) — MCP-only until #410 promoted them to in-app tools
"insertImage",
"replaceImage",
"insertFootnote",
// write (comment)
"createComment",
"resolveComment",
@@ -140,6 +140,36 @@ test("typo fix wrapped in markdown still applies (not refused)", () => {
assert.deepEqual(node.marks, [{ type: "bold" }]);
});
// ---------------------------------------------------------------------------
// (iv) #410 footnote token: a `replace` containing `^[...]` is refused into
// failed[] (it would be written as a LITERAL string, never a real footnote).
// Nothing is applied; the reason points at insert_footnote.
// ---------------------------------------------------------------------------
test("replace containing a `^[...]` footnote token is refused, not applied", () => {
const input = doc(paragraph(textNode("The claim stands.")));
const snapshot = JSON.parse(JSON.stringify(input));
const { doc: out, results, failed } = applyTextEdits(input, [
{ find: "The claim stands.", replace: "The claim stands.^[See source, p.42]" },
]);
assert.equal(results.length, 0, "nothing applied");
assert.equal(failed.length, 1, "one refused edit");
assert.equal(failed[0].find, "The claim stands.");
assert.match(failed[0].reason, /insert_footnote/);
// The document is byte-for-byte untouched — no literal `^[` was written.
assert.deepEqual(out, snapshot);
});
test("a plain replace with no footnote token still applies (no false positive)", () => {
const input = doc(paragraph(textNode("a caret ^ and a bracket ] apart")));
const { results, failed } = applyTextEdits(input, [
{ find: "apart", replace: "separate" },
]);
assert.equal(failed.length, 0, "not refused");
assert.equal(results.length, 1, "applied");
});
// ---------------------------------------------------------------------------
// A plain text fix is unaffected by the refuse logic.
// ---------------------------------------------------------------------------