diff --git a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts index 9abd1c50..c5a5195a 100644 --- a/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts +++ b/apps/server/src/core/ai-chat/tools/ai-chat-tools.service.ts @@ -649,13 +649,21 @@ export class AiChatToolsService { listComments: tool({ description: - 'List ALL comments on a page in one call, including RESOLVED ' + - 'threads — filter by resolvedAt when you need only open ones. ' + - 'Content is returned as Markdown.', + 'List comments on a page in one call. By DEFAULT only ACTIVE ' + + 'threads are returned; resolved threads (a resolved top-level ' + + 'comment and all its replies) are hidden and their count reported ' + + 'as `resolvedThreadsHidden` so you can re-query with ' + + '`includeResolved: true` to see everything. Returns ' + + '`{ items, resolvedThreadsHidden }`. Content is returned as Markdown.', inputSchema: modelFriendlyInput({ pageId: z.string().describe('The id of the page.'), + includeResolved: z + .boolean() + .optional() + .describe('default only active threads; true — include resolved'), }), - execute: async ({ pageId }) => await client.listComments(pageId), + execute: async ({ pageId, includeResolved }) => + await client.listComments(pageId, includeResolved), }), getComment: tool({ diff --git a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts index 7b5a9a4e..5a316ce5 100644 --- a/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts +++ b/apps/server/src/core/ai-chat/tools/docmost-client.loader.ts @@ -56,7 +56,12 @@ export interface DocmostClientLike { getPageJson(pageId: string): Promise>; getNode(pageId: string, nodeId: string): Promise>; getTable(pageId: string, tableRef: string): Promise>; - listComments(pageId: string): Promise; + // Returns `{ items, resolvedThreadsHidden }`. DEFAULT (includeResolved unset/ + // false) hides resolved threads wholesale; pass true for the full feed. + listComments( + pageId: string, + includeResolved?: boolean, + ): Promise<{ items: unknown[]; resolvedThreadsHidden: number }>; getComment( commentId: string, ): Promise<{ data: Record; success: boolean }>; diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index cd29d494..ab96b458 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -807,8 +807,14 @@ export class DocmostClient { await this.ensureAuthenticated(); const resultData = await this.getPageRaw(pageId); + // Agent read: hide resolved-comment anchors so the agent sees only active + // discussions. Active anchors are kept. (The lossless export_page_markdown + // round-trip deliberately does NOT pass this flag — resolved anchors there + // must be preserved.) let content = resultData.content - ? convertProseMirrorToMarkdown(resultData.content) + ? convertProseMirrorToMarkdown(resultData.content, { + dropResolvedCommentAnchors: true, + }) : ""; // Always fetch subpages to provide context to the agent @@ -1774,7 +1780,10 @@ export class DocmostClient { const body = page.content ? convertProseMirrorToMarkdown(page.content) : ""; let comments: any[] = []; try { - comments = await this.listComments(pageId); + // Lossless export: include RESOLVED threads so the export -> import + // round-trip preserves every comment. This is exactly why the active-only + // filter is an opt-in (default false) on listComments. + comments = (await this.listComments(pageId, true)).items; } catch (e) { // A comments fetch failure must not lose the body; export with [] and let // the caller see the (empty) comments block. Log under DEBUG only. @@ -2343,8 +2352,21 @@ export class DocmostClient { } } - /** List all comments on a page (cursor-paginated), content as markdown. */ - async listComments(pageId: string) { + /** + * List comments on a page (cursor-paginated), content as markdown. + * + * DEFAULT (`includeResolved = false`) hides RESOLVED THREADS WHOLESALE so the + * agent sees only active discussions: a top-level comment with `resolvedAt` + * set AND every reply under it (a reply of a closed thread is part of the + * closed thread) are dropped from `items`. `resolvedThreadsHidden` reports how + * many resolved top-level threads were hidden so the agent can re-query with + * `includeResolved: true` to see everything. Active threads always stay. + * + * Returns `{ items, resolvedThreadsHidden }` (NOT a bare array) — callers that + * need the full feed (lossless export, transformPage, checkNewComments) pass + * `includeResolved: true` and read `.items`. + */ + async listComments(pageId: string, includeResolved = false) { await this.ensureAuthenticated(); let allComments: any[] = []; let cursor: string | null = null; @@ -2360,7 +2382,7 @@ export class DocmostClient { cursor = data.meta?.nextCursor || null; } while (cursor); - return allComments.map((comment: any) => { + const mapped = allComments.map((comment: any) => { const markdown = comment.content ? convertProseMirrorToMarkdown( this.parseCommentContent(comment.content), @@ -2368,6 +2390,31 @@ export class DocmostClient { : ""; return filterComment(comment, markdown); }); + + if (includeResolved) { + return { items: mapped, resolvedThreadsHidden: 0 }; + } + + // Ids of RESOLVED top-level threads (a top-level comment has no + // parentCommentId). A whole thread is hidden when its root is resolved. + const resolvedRootIds = new Set( + mapped + .filter((c) => !c.parentCommentId && c.resolvedAt != null) + .map((c) => c.id), + ); + + const items = mapped.filter((c) => { + // Hide the resolved root itself and every reply anchored to it. A reply's + // own resolvedAt is irrelevant — its membership follows the parent thread. + // ASSUMPTION: Docmost's comment model is FLAT — a reply's parentCommentId + // always points at the thread ROOT (no reply-of-reply nesting), so a single + // level of parent lookup covers a whole thread. If nested replies are ever + // introduced, a deep reply of a resolved thread would need a root-walk here. + if (!c.parentCommentId) return !resolvedRootIds.has(c.id); + return !resolvedRootIds.has(c.parentCommentId); + }); + + return { items, resolvedThreadsHidden: resolvedRootIds.size }; } async getComment(commentId: string) { @@ -2742,7 +2789,9 @@ export class DocmostClient { const results: any[] = []; for (const page of pagesInScope) { try { - const comments = await this.listComments(page.id); + // Full feed (incl. resolved): a "new comments since" scan reports all + // recent activity; the active-only filter is scoped to list_comments. + const comments = (await this.listComments(page.id, true)).items; const newComments = comments.filter( (c: any) => new Date(c.createdAt) > sinceDate, ); @@ -3488,7 +3537,9 @@ export class DocmostClient { const deleteComments = opts.deleteComments ?? false; await this.ensureAuthenticated(); - const comments = await this.listComments(pageId); + // Full feed (incl. resolved): a page transform (e.g. comments -> footnotes) + // must operate on every comment, so it opts into the unfiltered feed. + const comments = (await this.listComments(pageId, true)).items; // ctx handed to the sandbox. consume() records ids; helpers are the pure // transform primitives. log is captured from console.log inside the sandbox. diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 6bbcea7c..51e0f6d9 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -712,15 +712,24 @@ server.registerTool( "list_comments", { description: - "List ALL comments on a page in one call (pagination is handled " + - "internally), including RESOLVED threads — filter by resolvedAt when you " + - "need only open ones. Content is returned as Markdown.", + "List comments on a page in one call (pagination is handled " + + "internally). By DEFAULT only ACTIVE threads are returned; resolved " + + "threads (a resolved top-level comment and all its replies) are hidden " + + "and their count reported as `resolvedThreadsHidden` so you can re-query " + + "with `includeResolved: true` to see everything. Returns " + + "`{ items, resolvedThreadsHidden }`. Content is returned as Markdown.", inputSchema: { pageId: z.string().describe("ID of the page"), + includeResolved: z + .boolean() + .optional() + .describe( + "default only active threads; true — include resolved", + ), }, }, - async ({ pageId }) => { - const comments = await docmostClient.listComments(pageId); + async ({ pageId, includeResolved }) => { + const comments = await docmostClient.listComments(pageId, includeResolved); return jsonContent(comments); }, ); diff --git a/packages/mcp/src/lib/markdown-converter.ts b/packages/mcp/src/lib/markdown-converter.ts index 10a9f775..b39f608b 100644 --- a/packages/mcp/src/lib/markdown-converter.ts +++ b/packages/mcp/src/lib/markdown-converter.ts @@ -9,4 +9,7 @@ * many existing `./markdown-converter.js` importers (client.ts, tests) do not * have to move. */ -export { convertProseMirrorToMarkdown } from "@docmost/prosemirror-markdown"; +export { + convertProseMirrorToMarkdown, + type ConvertProseMirrorToMarkdownOptions, +} from "@docmost/prosemirror-markdown"; diff --git a/packages/mcp/test/mock/list-comments-resolved.test.mjs b/packages/mcp/test/mock/list-comments-resolved.test.mjs new file mode 100644 index 00000000..19188b9e --- /dev/null +++ b/packages/mcp/test/mock/list-comments-resolved.test.mjs @@ -0,0 +1,160 @@ +// gitmost #328 Channel 2: DocmostClient.listComments hides RESOLVED THREADS +// wholesale by default (a resolved top-level comment AND every reply under it), +// returning `{ items, resolvedThreadsHidden }`. `includeResolved: true` returns +// the full feed. These tests stand a local http.createServer in for Docmost and +// mock the /auth/login + /comments (paginated) routes. +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import http from "node:http"; +import { DocmostClient } from "../../build/client.js"; + +function readBody(req) { + return new Promise((resolve) => { + let raw = ""; + req.on("data", (c) => (raw += c)); + req.on("end", () => resolve(raw)); + }); +} + +function startServer(handler) { + return new Promise((resolve) => { + const server = http.createServer(handler); + server.listen(0, "127.0.0.1", () => { + const { port } = server.address(); + resolve({ server, baseURL: `http://127.0.0.1:${port}/api` }); + }); + }); +} + +function closeServer(server) { + return new Promise((resolve) => server.close(resolve)); +} + +function sendJson(res, status, obj, extraHeaders = {}) { + res.writeHead(status, { "Content-Type": "application/json", ...extraHeaders }); + res.end(JSON.stringify(obj)); +} + +const openServers = []; +async function spawn(handler) { + const { server, baseURL } = await startServer(handler); + openServers.push(server); + return { server, baseURL }; +} + +after(async () => { + await Promise.all(openServers.map((s) => closeServer(s))); +}); + +// A minimal ProseMirror comment body (a paragraph of text). +const body = (t) => ({ + type: "doc", + content: [{ type: "paragraph", content: [{ type: "text", text: t }] }], +}); + +// Feed: an ACTIVE thread whose REPLY is resolved (root active, reply resolved — +// the thread must STAY, because a thread is gated only by its ROOT's resolvedAt) +// and a RESOLVED thread (root + reply). +const FEED = [ + { + id: "a", + pageId: "page-1", + parentCommentId: null, + resolvedAt: null, + createdAt: "2026-01-01T00:00:00.000Z", + creatorId: "u1", + content: body("active root"), + }, + { + id: "a1", + pageId: "page-1", + parentCommentId: "a", + // A RESOLVED reply under an ACTIVE root: the thread is NOT hidden (only a + // resolved ROOT hides a thread), so this reply survives the default filter. + resolvedAt: "2026-02-15T00:00:00.000Z", + createdAt: "2026-01-01T01:00:00.000Z", + creatorId: "u1", + content: body("resolved reply of an active thread"), + }, + { + id: "r", + pageId: "page-1", + parentCommentId: null, + resolvedAt: "2026-02-01T00:00:00.000Z", + createdAt: "2026-01-02T00:00:00.000Z", + creatorId: "u1", + content: body("resolved root"), + }, + { + id: "r1", + pageId: "page-1", + parentCommentId: "r", + resolvedAt: null, + createdAt: "2026-01-02T01:00:00.000Z", + creatorId: "u1", + content: body("resolved reply"), + }, +]; + +function commentsServer() { + return spawn(async (req, res) => { + await readBody(req); + if (req.url === "/api/auth/login") { + sendJson(res, 200, { success: true }, { + "Set-Cookie": "authToken=t; Path=/; HttpOnly", + }); + return; + } + if (req.url === "/api/comments") { + // Single page, no cursor. + sendJson(res, 200, { data: { items: FEED, meta: { nextCursor: null } } }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); +} + +test("default hides the resolved thread (root + its reply) and counts it", async () => { + const { baseURL } = await commentsServer(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + const result = await client.listComments("page-1"); + assert.equal(Array.isArray(result.items), true, "returns { items, ... }"); + const ids = result.items.map((c) => c.id).sort(); + assert.deepEqual(ids, ["a", "a1"], "only the active thread remains"); + assert.equal(result.resolvedThreadsHidden, 1, "one resolved thread hidden"); +}); + +test("includeResolved:true returns EVERYTHING with zero hidden", async () => { + const { baseURL } = await commentsServer(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + const result = await client.listComments("page-1", true); + const ids = result.items.map((c) => c.id).sort(); + assert.deepEqual(ids, ["a", "a1", "r", "r1"], "all four comments returned"); + assert.equal(result.resolvedThreadsHidden, 0, "nothing hidden with the flag"); +}); + +test("the reply of a resolved thread is hidden with the thread", async () => { + const { baseURL } = await commentsServer(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + const result = await client.listComments("page-1"); + const ids = result.items.map((c) => c.id); + assert.equal(ids.includes("r1"), false, "the resolved thread's reply is gone"); + assert.equal(ids.includes("r"), false, "the resolved root is gone"); +}); + +test("an ACTIVE thread whose REPLY is resolved is NOT hidden", async () => { + // A thread is gated only by its ROOT's resolvedAt. `a1` is a resolved reply + // under the active root `a`, so both must survive the default filter and the + // thread must not be counted as hidden. + const { baseURL } = await commentsServer(); + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + + const result = await client.listComments("page-1"); + const ids = result.items.map((c) => c.id).sort(); + assert.equal(ids.includes("a"), true, "active root stays"); + assert.equal(ids.includes("a1"), true, "its resolved reply stays with the thread"); + assert.equal(result.resolvedThreadsHidden, 1, "only the resolved-root thread is hidden"); +}); diff --git a/packages/prosemirror-markdown/src/lib/index.ts b/packages/prosemirror-markdown/src/lib/index.ts index 10bb757d..f95e6a6d 100644 --- a/packages/prosemirror-markdown/src/lib/index.ts +++ b/packages/prosemirror-markdown/src/lib/index.ts @@ -16,6 +16,7 @@ export { export type { DocmostMdMeta } from "./markdown-document.js"; export { convertProseMirrorToMarkdown } from "./markdown-converter.js"; +export type { ConvertProseMirrorToMarkdownOptions } from "./markdown-converter.js"; export { markdownToProseMirror } from "./markdown-to-prosemirror.js"; diff --git a/packages/prosemirror-markdown/src/lib/markdown-converter.ts b/packages/prosemirror-markdown/src/lib/markdown-converter.ts index 63c39333..2d173e52 100644 --- a/packages/prosemirror-markdown/src/lib/markdown-converter.ts +++ b/packages/prosemirror-markdown/src/lib/markdown-converter.ts @@ -33,13 +33,36 @@ import { */ const MAX_NODE_DEPTH = 400; +/** + * Options for {@link convertProseMirrorToMarkdown}. + */ +export interface ConvertProseMirrorToMarkdownOptions { + /** + * When true, an inline comment anchor whose Comment mark is `resolved` + * emits its BARE text (no `` wrapper), so an agent + * reading the page never sees resolved-comment anchors. ACTIVE (unresolved) + * anchors still emit their wrapper. Defaults to false — a zero-behavior + * change for every existing caller, including the lossless git-sync export + * path where resolved anchors MUST be preserved for round-tripping. + */ + dropResolvedCommentAnchors?: boolean; +} + /** * Convert ProseMirror/TipTap JSON content to Markdown * Supports all Docmost-specific node types and extensions */ -export function convertProseMirrorToMarkdown(content: any): string { +export function convertProseMirrorToMarkdown( + content: any, + options: ConvertProseMirrorToMarkdownOptions = {}, +): string { if (!content || !content.content) return ""; + // Closure flag read by both `case "comment"` emitters (the top-level marks + // loop and the raw-HTML inlineToHtml path). Off by default; the agent-read + // callers (mcp getPage / in-app AI chat) pass it true. + const dropResolvedCommentAnchors = options.dropResolvedCommentAnchors === true; + // Escape a value interpolated into an HTML double-quoted attribute value // (textAlign, colors, image src, math `text`, all data-* attrs, etc.). In the // ATTRIBUTE context only the quote that delimits the value and the ampersand @@ -508,6 +531,11 @@ export function convertProseMirrorToMarkdown(content: any): string { // commentId/resolved). const cid = mark.attrs?.commentId; if (cid) { + // Hide resolved anchors from agent reads: drop the wrapper and + // keep only the bare text. Active anchors keep their wrapper. + if (mark.attrs?.resolved && dropResolvedCommentAnchors) { + break; + } const resolvedAttr = mark.attrs?.resolved ? ` data-resolved="true"` : ""; @@ -1177,6 +1205,11 @@ export function convertProseMirrorToMarkdown(content: any): string { // Inline comment anchor inside a raw-HTML container (columns / // spanned table cells), so commented text there also round-trips. if (mark.attrs?.commentId) { + // Hide resolved anchors from agent reads: drop the wrapper and + // keep only the bare text. Active anchors keep their wrapper. + if (mark.attrs?.resolved && dropResolvedCommentAnchors) { + break; + } const r = mark.attrs?.resolved ? ` data-resolved="true"` : ""; t = `${t}`; } diff --git a/packages/prosemirror-markdown/test/resolved-comment-anchors.test.ts b/packages/prosemirror-markdown/test/resolved-comment-anchors.test.ts new file mode 100644 index 00000000..2b83f223 --- /dev/null +++ b/packages/prosemirror-markdown/test/resolved-comment-anchors.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from 'vitest'; +// Import the converter DIRECTLY from src (NOT the docmost-client barrel, which +// pulls in collaboration.ts and mutates the global DOM at import time), matching +// the other converter unit tests (see markdown-converter-html-marks.test.ts). +import { convertProseMirrorToMarkdown } from '../src/lib/markdown-converter.js'; + +// gitmost #328 Channel 1: the `dropResolvedCommentAnchors` converter option +// hides RESOLVED comment anchors from agent reads while keeping ACTIVE anchors. +// The option defaults to false (zero behavior change for the lossless git-sync +// export path). Two emitters read it: the top-level marks loop and the raw-HTML +// inlineToHtml path (inside columns / spanned table cells). + +const text = (t: string, marks?: any[]) => + marks ? { type: 'text', text: t, marks } : { type: 'text', text: t }; +const para = (...inline: any[]) => ({ type: 'paragraph', content: inline }); +const doc = (...nodes: any[]) => ({ type: 'doc', content: nodes }); + +const commentMark = (commentId: string, resolved: boolean) => ({ + type: 'comment', + attrs: { commentId, resolved }, +}); + +// A columns node (raw-HTML container) so its children render via the +// blockToHtml -> inlineToHtml path (the SECOND `case "comment"` emitter). +const oneColumn = (...blocks: any[]) => ({ + type: 'columns', + attrs: { layout: 'two' }, + content: [{ type: 'column', content: blocks }], +}); + +describe('#328 Channel 1 — top-level emitter: dropResolvedCommentAnchors', () => { + const resolvedDoc = doc( + para(text('kept '), text('resolved', [commentMark('r1', true)])), + ); + const activeDoc = doc( + para(text('kept '), text('active', [commentMark('a1', false)])), + ); + + it('drops a RESOLVED anchor (bare text) WITH the flag', () => { + const out = convertProseMirrorToMarkdown(resolvedDoc, { + dropResolvedCommentAnchors: true, + }); + expect(out).toBe('kept resolved'); + expect(out).not.toContain('data-comment-id'); + }); + + it('PRESERVES a RESOLVED anchor WITHOUT the flag (default off)', () => { + const out = convertProseMirrorToMarkdown(resolvedDoc); + expect(out).toContain( + 'resolved', + ); + }); + + it('KEEPS an ACTIVE anchor in BOTH cases', () => { + const withFlag = convertProseMirrorToMarkdown(activeDoc, { + dropResolvedCommentAnchors: true, + }); + const withoutFlag = convertProseMirrorToMarkdown(activeDoc); + expect(withFlag).toContain('active'); + expect(withoutFlag).toContain('active'); + }); +}); + +describe('#328 Channel 1 — raw-HTML inlineToHtml emitter (columns)', () => { + const resolvedCol = doc( + oneColumn(para(text('resolved', [commentMark('r1', true)]))), + ); + const activeCol = doc( + oneColumn(para(text('active', [commentMark('a1', false)]))), + ); + + it('drops a RESOLVED anchor (bare text) WITH the flag', () => { + const out = convertProseMirrorToMarkdown(resolvedCol, { + dropResolvedCommentAnchors: true, + }); + expect(out).toContain('

resolved

'); + expect(out).not.toContain('data-comment-id'); + }); + + it('PRESERVES a RESOLVED anchor WITHOUT the flag', () => { + const out = convertProseMirrorToMarkdown(resolvedCol); + expect(out).toContain( + 'resolved', + ); + }); + + it('KEEPS an ACTIVE anchor in BOTH cases', () => { + const withFlag = convertProseMirrorToMarkdown(activeCol, { + dropResolvedCommentAnchors: true, + }); + const withoutFlag = convertProseMirrorToMarkdown(activeCol); + expect(withFlag).toContain('active'); + expect(withoutFlag).toContain('active'); + }); +});