feat(ai-chat): give the in-app agent insertFootnote/insertImage/replaceImage (#410)

The Researcher role wrote 40 literal `^[...]` and zero real footnotes: its incremental write path (insertNode/editPageText) doesn't parse markdown, and the footnote-capable tool was MCP-only. Promote three tools from inline MCP-only to the shared registry so the in-app agent gets them too. - tool-specs.ts: insertFootnote/insertImage/replaceImage added to SHARED_TOOL_SPECS (mcpName/schema/description moved VERBATIM from the inline registrations — MCP names + behaviour unchanged for external clients). - index.ts: the 3 inline registerTool calls become registerShared; drop the "MCP-only by design" comments. - ai-chat-tools.service.ts: register the 3 in-app via sharedTool -> client.insertFootnote/insertImage/replaceImage (imageUrl->url, attachmentId->oldAttachmentId mapping). - tool-tiers.ts: insertFootnote -> core (else the original asymmetry recurs — footnote tool hidden while editPageText is core); images -> deferred. - research/{en,ru}.yaml FOOTNOTES: `^[...]` parses ONLY on a whole-markdown write (create/update/import); for a pinpoint citation to existing text use insertFootnote; via editPageText/insertNode it stays literal. - json-edit.ts guardrail: an edit_page_text `replace` containing a `^[...]` token is refused into failed[] with an insert_footnote hint, mirroring the existing formatting-marker refusal. (Slightly broader net than that mirror — a literal `^[a-z]` regex class in a replace is also refused; accepted defense-in-depth, has a no-false-positive test.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-07 21:18:01 +03:00
parent abd61041fe
commit 327737b701
11 changed files with 393 additions and 101 deletions
@@ -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. */