From c9293e316bf74f37d6a442832c68047928093c9f Mon Sep 17 00:00:00 2001 From: agent_coder Date: Thu, 9 Jul 2026 23:45:11 +0300 Subject: [PATCH 1/2] =?UTF-8?q?feat(mcp):=20createComment=20=E2=80=94=20?= =?UTF-8?q?=D0=BF=D0=BE=D0=B4=D1=81=D0=BA=D0=B0=D0=B7=D0=BA=D0=B8=20=D1=81?= =?UTF-8?q?=D0=B0=D0=BC=D0=BE=D0=BA=D0=BE=D1=80=D1=80=D0=B5=D0=BA=D1=86?= =?UTF-8?q?=D0=B8=D0=B8=20=D1=8F=D0=BA=D0=BE=D1=80=D1=8F=20(closest-block,?= =?UTF-8?q?=20markdown-strip,=20multi-block)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createComment — топ-хотспот ошибок агента (промахи по якорю, слепые ретраи). Портированы аффордансы самокоррекции из editPageText: - Closest-block hint: общий хелпер closestBlockHint вынесен в text-normalize.ts (json-edit.ts теперь тоже его зовёт), подключён во все 3 throw-а createComment. - Markdown-strip fallback в comment-anchor.ts согласованно по всем 4 функциям (can/count/apply/get) через единый resolveAnchorSelection: exact-verbatim wins глобально, stripped — только если raw не якорится нигде; soft warning как в editPageText. Инвариант уникальности suggestion (0/1/>=2) сохранён: raw-unique никогда не запускает fallback -> не может стать ambiguous. Хранимый selection остаётся СЫРОЙ подстрокой документа (strip только для поиска). - Multi-block detection: явное сообщение 'selection spans multiple blocks' когда per-block поиск провалился, но выделение есть в объединённом тексте блоков. - tool-spec createComment: копировать selection дословно из getPage/searchInPage. Известное мелкое ограничение (нит внутреннего ревью): детектор multi-block использует raw selection, поэтому markdown-стилизованное выделение через границу блоков получит generic-подсказку вместо spans-multiple-blocks (редко, guidance всё равно корректный). closes #408 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/mcp/src/client.ts | 134 +++++++++++++++--- packages/mcp/src/lib/comment-anchor.ts | 90 ++++++++++-- packages/mcp/src/lib/json-edit.ts | 32 ++--- packages/mcp/src/lib/text-normalize.ts | 34 +++++ packages/mcp/src/tool-specs.ts | 10 +- .../mcp/test/mock/create-comment.test.mjs | 91 ++++++++++++ .../mcp/test/unit/comment-anchor.test.mjs | 68 +++++++++ 7 files changed, 406 insertions(+), 53 deletions(-) diff --git a/packages/mcp/src/client.ts b/packages/mcp/src/client.ts index d370a2b6..b968d753 100644 --- a/packages/mcp/src/client.ts +++ b/packages/mcp/src/client.ts @@ -40,6 +40,7 @@ import { deleteNodeById, assertUnambiguousMatch, insertNodeRelative, + blockPlainText, buildOutline, getNodeByRef, readTable, @@ -59,10 +60,12 @@ import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { diffDocs, summarizeChange } from "./lib/diff.js"; import { applyAnchorInDoc, - canAnchorInDoc, countAnchorMatches, getAnchoredText, + resolveAnchorSelection, + normalizeForMatch, } from "./lib/comment-anchor.js"; +import { closestBlockHint } from "./lib/text-normalize.js"; import { blockText, walk, @@ -2474,6 +2477,64 @@ export class DocmostClient { }; } + /** Plain text of each TOP-LEVEL block of `doc`, for anchor-failure hints. */ + private topLevelBlockTexts(doc: any): string[] { + const content = doc && Array.isArray(doc.content) ? doc.content : []; + return content + .map((b: any) => blockPlainText(b)) + .filter((t: string) => t.length > 0); + } + + /** + * True when per-block anchoring failed but the (normalized) selection DOES + * appear in the blocks' joined plain text — i.e. it straddles a block + * boundary. Blocks are joined with a newline (collapsed to one space by + * normalizeForMatch) so a selection whose parts are separated by a paragraph + * break still matches. Callers only reach here after single-block anchoring + * (incl. the markdown-strip fallback) has already failed. + */ + private selectionSpansMultipleBlocks( + blockTexts: string[], + selection: string, + ): boolean { + const normSel = normalizeForMatch(selection).norm.trim(); + if (normSel.length === 0) return false; + const joined = normalizeForMatch(blockTexts.join("\n")).norm; + return joined.indexOf(normSel) !== -1; + } + + /** + * Build the actionable error for a create_comment anchor MISS, porting + * edit_page_text's self-correction affordances: an explicit "spans multiple + * blocks" message when the selection straddles a block boundary, otherwise a + * "closest block text" hint quoting the block that holds the selection's + * longest token. `live` switches the wording between the pre-check (reading the + * persisted page) and the post-create live-anchor failure (which rolls back). + */ + private anchorNotFoundError( + doc: any, + selection: string, + live: boolean, + ): Error { + const blockTexts = this.topLevelBlockTexts(doc); + const rolled = live ? " The comment was rolled back." : ""; + if (this.selectionSpansMultipleBlocks(blockTexts, selection)) { + return new Error( + "create_comment: the selection spans multiple blocks; anchor on a " + + "contiguous fragment within a SINGLE paragraph/block (<=250 chars)." + + rolled, + ); + } + const where = live ? "in the live document" : "in the page"; + return new Error( + `create_comment: could not find the selection text ${where} to anchor ` + + "the comment. Provide the EXACT contiguous text from a single " + + "paragraph/block (<=250 chars)." + + closestBlockHint(blockTexts, selection) + + rolled, + ); + } + /** * Create an inline comment anchored to its `selection` text, or a reply. * @@ -2535,6 +2596,10 @@ export class DocmostClient { // Captured in the pre-check below (which already reads the page) and used as // payload.selection. Ordinary comments keep sending the raw agent selection. let anchoredSelection: string | null = null; + // Set when the anchor matched only after stripping markdown from the + // selection (the strip fallback); surfaced as a soft warning like + // edit_page_text does, so a stale-markdown selection is flagged. + let anchorNormalized = false; // For a top-level comment, fail BEFORE creating anything when the selection // is not present in the persisted document — this avoids leaving an orphan @@ -2550,10 +2615,7 @@ export class DocmostClient { // rejected BEFORE creating the comment. const matches = countAnchorMatches(page.content, selection); if (matches === 0) { - throw new Error( - "create_comment: could not find the selection text in the page to anchor the comment. " + - "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).", - ); + throw this.anchorNotFoundError(page.content, selection, false); } if (matches >= 2) { throw new Error( @@ -2567,18 +2629,27 @@ export class DocmostClient { // null despite countAnchorMatches===1 (shouldn't happen), fall back to // the raw agent selection below rather than crash. anchoredSelection = getAnchoredText(page.content, selection); - } else if (!canAnchorInDoc(page.content, selection)) { - throw new Error( - "create_comment: could not find the selection text in the page to anchor the comment. " + - "Provide the EXACT contiguous text from a single paragraph/block (<=250 chars).", - ); + anchorNormalized = resolveAnchorSelection( + page.content, + selection, + ).normalized; + } else { + const resolved = resolveAnchorSelection(page.content, selection); + if (!resolved.found) { + throw this.anchorNotFoundError(page.content, selection, false); + } + anchorNormalized = resolved.normalized; } } catch (e) { - // Rethrow our own "not found"/"ambiguous" errors; swallow read/network - // errors so the live anchor step can still try (and enforce) anchoring. + // Rethrow our own "not found"/"ambiguous"/"spans multiple blocks" errors; + // swallow read/network errors so the live anchor step can still try (and + // enforce) anchoring. if ( e instanceof Error && (e.message.startsWith("create_comment: could not find the selection") || + e.message.startsWith( + "create_comment: the selection spans multiple blocks", + ) || e.message.startsWith( "create_comment: the suggestion's selection is ambiguous", )) @@ -2650,6 +2721,10 @@ export class DocmostClient { // Set inside the transform when a suggestion's live anchor is ambiguous // (>=2 occurrences), so the rollback path can surface the right error. let ambiguousInLiveDoc = false; + // Captured inside the transform on a not-found abort, so the rollback path + // can surface the closest-block / spans-multiple-blocks hint built from the + // LIVE document (the pre-check page is not in scope there). + let liveNotFoundError: Error | null = null; try { const collabToken = await this.getCollabTokenWithReauth(); // Open the collab doc by the canonical UUID, never the slugId (#260). The @@ -2677,6 +2752,13 @@ export class DocmostClient { const liveCount = countAnchorMatches(doc, selection as string); if (liveCount !== 1) { ambiguousInLiveDoc = liveCount >= 2; + if (liveCount === 0) { + liveNotFoundError = this.anchorNotFoundError( + doc, + selection as string, + true, + ); + } return null; } } @@ -2686,6 +2768,11 @@ export class DocmostClient { } // Selection text not found in the LIVE document: abort the write. The // rollback + throw below turns this into a hard error. + liveNotFoundError = this.anchorNotFoundError( + doc, + selection as string, + true, + ); return null; }, ); @@ -2702,13 +2789,28 @@ export class DocmostClient { // suggestion, was ambiguous) in the live document. Roll back the comment // and surface a hard error. await this.safeDeleteComment(newCommentId); - throw new Error( - ambiguousInLiveDoc - ? "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique." - : "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back", + if (ambiguousInLiveDoc) { + throw new Error( + "create_comment: the suggestion's selection is ambiguous in the live document (multiple occurrences); the comment was rolled back. Expand the selection with surrounding context so it is unique.", + ); + } + throw ( + liveNotFoundError ?? + new Error( + "create_comment: failed to anchor the comment (selection not found in the live document); the comment was rolled back", + ) ); } + // Soft warning (like edit_page_text): the selection only matched after + // stripping markdown, so the caller likely quoted a styled fragment. + if (anchorNormalized) { + result.warning = + "The selection matched only after stripping markdown syntax; the comment " + + "was anchored on the document's plain text. Copy the selection verbatim " + + "from get_page / search_in_page output to avoid this."; + } + result.anchored = true; return result; } diff --git a/packages/mcp/src/lib/comment-anchor.ts b/packages/mcp/src/lib/comment-anchor.ts index cb494b1b..5c94250b 100644 --- a/packages/mcp/src/lib/comment-anchor.ts +++ b/packages/mcp/src/lib/comment-anchor.ts @@ -17,8 +17,19 @@ * comparing and match across maximal runs of consecutive text nodes within a * single block, while mapping every normalized character back to its raw index * so the mark lands on the exact original characters. + * + * MARKDOWN-STRIP FALLBACK: when the agent copies a selection that still carries + * inline markdown (`**bold**`, `` `code` ``, `[t](u)`), the raw locator will not + * match the document's plain text. Exactly like edit_page_text's json-edit + * fallback, we first try the verbatim selection and, ONLY if it anchors nowhere + * in the whole document, retry with `stripInlineMarkdown` applied. This decision + * is made ONCE per public call in `resolveAnchorSelection`, so the four public + * entry points (can/count/get/apply) all agree on which locator matched — the + * suggestion-uniqueness gate depends on count and can/get never disagreeing. */ +import { stripInlineMarkdown } from "./text-normalize.js"; + /** Typographic double-quote variants mapped to ASCII `"`. */ const DOUBLE_QUOTES = "«»„“”‟〝〞＂"; /** Typographic single-quote/apostrophe variants mapped to ASCII `'`. */ @@ -214,15 +225,17 @@ function reconstructRawText(blockContent: any[], match: AnchorMatch): string { * un-appliable (spurious 409). */ export function getAnchoredText(doc: any, selection: string): string | null { + const { selection: effective, found } = resolveAnchorSelection(doc, selection); + if (!found) return null; const visit = (node: any, depth: number): string | null => { if (depth > MAX_DEPTH || !node || typeof node !== "object") return null; if (!Array.isArray(node.content)) return null; - const match = findAnchorInBlock(node.content, selection); + const match = findAnchorInBlock(node.content, effective); if (match) return reconstructRawText(node.content, match); for (const child of node.content) { if (child && typeof child === "object" && Array.isArray(child.content)) { - const found = visit(child, depth + 1); - if (found !== null) return found; + const foundText = visit(child, depth + 1); + if (foundText !== null) return foundText; } } return null; @@ -231,12 +244,11 @@ export function getAnchoredText(doc: any, selection: string): string | null { } /** - * Depth-first, document-order check for whether `selection` can be anchored - * anywhere in `doc`. At each node with an array `content`, first try to match - * within that node's own content, then recurse into children that themselves - * have a `content` array. + * RAW (no markdown-strip fallback) depth-first check that `selection` anchors + * somewhere in `doc`. This is the primitive `resolveAnchorSelection` builds on; + * public callers should use `canAnchorInDoc`, which adds the strip fallback. */ -export function canAnchorInDoc(doc: any, selection: string): boolean { +function rawCanAnchorInDoc(doc: any, selection: string): boolean { const visit = (node: any, depth: number): boolean => { if (depth > MAX_DEPTH || !node || typeof node !== "object") return false; if (!Array.isArray(node.content)) return false; @@ -251,6 +263,43 @@ export function canAnchorInDoc(doc: any, selection: string): boolean { return visit(doc, 0); } +/** + * Decide the locator that ACTUALLY anchors `selection` in `doc`, applying the + * markdown-strip fallback once (so every public entry point agrees): + * - EXACT WINS: if the verbatim selection anchors anywhere, use it as-is. + * - FALLBACK: only if the verbatim selection anchors nowhere, and the + * markdown-stripped form differs and DOES anchor, use the stripped form and + * flag `normalized` so callers can surface a soft warning. + * - otherwise `found` is false and `selection` is returned unchanged. + * + * The stripped form is used ONLY to LOCATE the anchor; getAnchoredText still + * reconstructs and stores the RAW document substring, so the strip never leaks + * into what gets persisted. + */ +export function resolveAnchorSelection( + doc: any, + selection: string, +): { selection: string; found: boolean; normalized: boolean } { + if (rawCanAnchorInDoc(doc, selection)) { + return { selection, found: true, normalized: false }; + } + const stripped = stripInlineMarkdown(selection); + if (stripped !== selection && rawCanAnchorInDoc(doc, stripped)) { + return { selection: stripped, found: true, normalized: true }; + } + return { selection, found: false, normalized: false }; +} + +/** + * Depth-first, document-order check for whether `selection` can be anchored + * anywhere in `doc` (with the markdown-strip fallback). At each node with an + * array `content`, first try to match within that node's own content, then + * recurse into children that themselves have a `content` array. + */ +export function canAnchorInDoc(doc: any, selection: string): boolean { + return resolveAnchorSelection(doc, selection).found; +} + /** * Split the matched text nodes and splice the comment mark across the range. * `blockContent` is mutated IN PLACE. `match.startChild..endChild` are all text @@ -315,7 +364,7 @@ function spliceCommentMark( * not use this. (Note: counts OCCURRENCES, not just matching blocks, so two * occurrences inside one block are correctly reported as 2.) */ -export function countAnchorMatches(doc: any, selection: string): number { +function rawCountAnchorMatches(doc: any, selection: string): number { const normSel = normalizeForMatch(selection).norm.trim(); if (normSel.length === 0) return 0; @@ -369,6 +418,25 @@ export function countAnchorMatches(doc: any, selection: string): number { return total; } +/** + * Uniqueness gate for suggestions, with the SAME markdown-strip fallback as the + * other entry points so count never disagrees with can/get/apply. EXACT WINS: if + * the verbatim selection occurs at all, return its raw occurrence count (so a + * selection that is unique raw stays unique — the fallback never runs and cannot + * introduce a spurious second match). Only when the verbatim selection is absent + * do we count occurrences of the markdown-stripped form. + */ +export function countAnchorMatches(doc: any, selection: string): number { + const raw = rawCountAnchorMatches(doc, selection); + if (raw > 0) return raw; + const stripped = stripInlineMarkdown(selection); + if (stripped !== selection) { + const strippedCount = rawCountAnchorMatches(doc, stripped); + if (strippedCount > 0) return strippedCount; + } + return 0; +} + /** * Depth-first (same order as canAnchorInDoc) over `doc`; on the FIRST block * whose content matches `selection`, splice the comment mark across the matched @@ -380,10 +448,12 @@ export function applyAnchorInDoc( selection: string, commentId: string, ): boolean { + const { selection: effective, found } = resolveAnchorSelection(doc, selection); + if (!found) return false; const visit = (node: any, depth: number): boolean => { if (depth > MAX_DEPTH || !node || typeof node !== "object") return false; if (!Array.isArray(node.content)) return false; - const match = findAnchorInBlock(node.content, selection); + const match = findAnchorInBlock(node.content, effective); if (match) { spliceCommentMark(node.content, match, commentId); return true; diff --git a/packages/mcp/src/lib/json-edit.ts b/packages/mcp/src/lib/json-edit.ts index 3f32d5f6..fde9b877 100644 --- a/packages/mcp/src/lib/json-edit.ts +++ b/packages/mcp/src/lib/json-edit.ts @@ -12,7 +12,11 @@ * re-import for small wording fixes. */ -import { stripInlineMarkdown, stripBalancedWrappers } from "./text-normalize.js"; +import { + stripInlineMarkdown, + stripBalancedWrappers, + closestBlockHint, +} from "./text-normalize.js"; export interface TextEdit { find: string; @@ -381,29 +385,9 @@ export function applyTextEdits( } else { // Append a bounded "closest text" hint: find the FIRST block that // contains the longest whitespace-delimited token (>= 3 chars) of the - // (stripped, then raw) locator, and quote that block's plain text. - reason = "text not found in the document."; - const tokenSource = stripped.length > 0 ? stripped : edit.find; - const longestToken = tokenSource - .split(/\s+/) - .filter((t) => t.length >= 3) - .sort((a, b) => b.length - a.length)[0]; - if (longestToken) { - const hitBlock = blockPlain.find((plain) => - plain.includes(longestToken), - ); - if (hitBlock) { - // Truncate by code point (spread iterates by code point) so a - // surrogate pair is never split; append the ellipsis only when the - // text was actually longer than the limit. - const points = [...hitBlock]; - const snippet = - points.length > 120 - ? points.slice(0, 120).join("") + "…" - : hitBlock; - reason += ` Closest block text: "${snippet}".`; - } - } + // (stripped, then raw) locator, and quote that block's plain text. Shared + // with create_comment via closestBlockHint so both give the same hint. + reason = "text not found in the document." + closestBlockHint(blockPlain, edit.find); } failed.push({ find: edit.find, reason }); continue; diff --git a/packages/mcp/src/lib/text-normalize.ts b/packages/mcp/src/lib/text-normalize.ts index 7feeb69f..cd545366 100644 --- a/packages/mcp/src/lib/text-normalize.ts +++ b/packages/mcp/src/lib/text-normalize.ts @@ -92,6 +92,40 @@ export function stripBalancedWrappers(s: string): string { * ORIGINAL string is returned so a locator can never normalize down to "" and * match everything. */ +/** + * Build a bounded "closest text" hint for an anchor/find MISS, shared by + * edit_page_text (json-edit) and create_comment (client) so both surface the + * same self-correction affordance. + * + * Take the longest whitespace-delimited token (>= 3 chars) of the locator + * (markdown-stripped first, so `**bold**` contributes `bold`), find the FIRST + * of `blockTexts` that contains it, and return ` Closest block text: "…".` with + * the block quoted (truncated to 120 code points + ellipsis). Returns "" when + * no token qualifies or no block contains it, so the caller can append it + * unconditionally. + */ +export function closestBlockHint( + blockTexts: string[], + locator: string, +): string { + if (typeof locator !== "string" || locator.length === 0) return ""; + const stripped = stripInlineMarkdown(locator); + const tokenSource = stripped.length > 0 ? stripped : locator; + const longestToken = tokenSource + .split(/\s+/) + .filter((t) => t.length >= 3) + .sort((a, b) => b.length - a.length)[0]; + if (!longestToken) return ""; + const hitBlock = blockTexts.find((plain) => plain.includes(longestToken)); + if (!hitBlock) return ""; + // Truncate by code point (spread iterates by code point) so a surrogate pair + // is never split; append the ellipsis only when the text was actually longer. + const points = [...hitBlock]; + const snippet = + points.length > 120 ? points.slice(0, 120).join("") + "…" : hitBlock; + return ` Closest block text: "${snippet}".`; +} + export function stripInlineMarkdown(s: string): string { if (typeof s !== "string" || s.length === 0) return s; diff --git a/packages/mcp/src/tool-specs.ts b/packages/mcp/src/tool-specs.ts index f9e6813f..72f6c57d 100644 --- a/packages/mcp/src/tool-specs.ts +++ b/packages/mcp/src/tool-specs.ts @@ -771,9 +771,13 @@ export const SHARED_TOOL_SPECS = { 'The comment is anchored inline to the given exact `selection` text ' + '(which gets highlighted); page-level comments are NOT supported. A ' + 'new top-level comment REQUIRES a `selection`. Replies inherit the ' + - "parent's anchor and take no selection. If the call fails with a " + - '"selection not found" error, retry with a corrected EXACT selection ' + - 'copied verbatim from a single paragraph/block. You may also attach a ' + + "parent's anchor and take no selection. Always COPY the `selection` " + + 'VERBATIM from get_page / search_in_page output — do NOT quote it from ' + + 'memory (stale-memory quoting is the top cause of anchor misses). If the ' + + 'call fails with a "selection not found" error, the error quotes the ' + + "closest block text (or says the selection spans multiple blocks); retry " + + "with a corrected EXACT selection copied verbatim from a single " + + 'paragraph/block. You may also attach a ' + '`suggestedText` proposing a replacement for the `selection` (a human ' + 'applies it from the UI); when set, the `selection` must occur exactly ' + 'once in the page. Reversible via the comment UI.', diff --git a/packages/mcp/test/mock/create-comment.test.mjs b/packages/mcp/test/mock/create-comment.test.mjs index bcf70c44..e9d903aa 100644 --- a/packages/mcp/test/mock/create-comment.test.mjs +++ b/packages/mcp/test/mock/create-comment.test.mjs @@ -548,3 +548,94 @@ test("suggestedText: the stored selection is the doc's RAW typographic substring ); assert.equal(createPayload.suggestedText, "goodbye"); }); + +// ----------------------------------------------------------------------------- +// 8) #408: a not-found selection error QUOTES the closest block text so the +// model can self-correct instead of blind-retrying. +// ----------------------------------------------------------------------------- +test("a not-found selection error includes a 'Closest block text' hint", async () => { + let createCalls = 0; + const { baseURL } = await 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/pages/info") { + sendJson(res, 200, { + data: { + id: "page-1", + content: { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "The quick brown fox jumps" }] }, + ], + }, + }, + }); + return; + } + if (req.url === "/api/comments/create") { + createCalls++; + sendJson(res, 200, { data: { id: "should-not-happen" } }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + await assert.rejects( + () => client.createComment("page-1", "body", "inline", "quick brown cat"), + /Closest block text: "The quick brown fox jumps"/, + "a not-found selection must quote the closest block text", + ); + assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss"); +}); + +// ----------------------------------------------------------------------------- +// 9) #408: a selection that straddles two blocks gets the explicit +// "spans multiple blocks" message instead of a bare not-found. +// ----------------------------------------------------------------------------- +test("a selection spanning multiple blocks gets the explicit spans-multiple-blocks message", async () => { + let createCalls = 0; + const { baseURL } = await 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/pages/info") { + sendJson(res, 200, { + data: { + id: "page-1", + content: { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "the quick brown" }] }, + { type: "paragraph", content: [{ type: "text", text: "fox jumps over" }] }, + ], + }, + }, + }); + return; + } + if (req.url === "/api/comments/create") { + createCalls++; + sendJson(res, 200, { data: { id: "should-not-happen" } }); + return; + } + sendJson(res, 404, { message: "not found" }); + }); + + const client = new DocmostClient(baseURL, "user@example.com", "pw"); + await assert.rejects( + () => client.createComment("page-1", "body", "inline", "brown fox"), + /spans multiple blocks/, + "a cross-block selection must report the spans-multiple-blocks hint", + ); + assert.equal(createCalls, 0, "/comments/create must NOT be called on a miss"); +}); diff --git a/packages/mcp/test/unit/comment-anchor.test.mjs b/packages/mcp/test/unit/comment-anchor.test.mjs index 87e61def..70f4a5bb 100644 --- a/packages/mcp/test/unit/comment-anchor.test.mjs +++ b/packages/mcp/test/unit/comment-anchor.test.mjs @@ -8,6 +8,7 @@ import { applyAnchorInDoc, countAnchorMatches, getAnchoredText, + resolveAnchorSelection, } from "../../build/lib/comment-anchor.js"; const COMMENT_ID = "cmt-123"; @@ -308,3 +309,70 @@ test("getAnchoredText returns null when the selection does not anchor", () => { const doc = paragraphDoc([{ type: "text", text: "hello world" }]); assert.equal(getAnchoredText(doc, "not present"), null); }); + +// --------------------------------------------------------------------------- +// #408 MARKDOWN-STRIP FALLBACK. A selection copied with inline markdown still +// carries `**`/`` ` ``/`[t](u)` markers the plain document text lacks. When the +// verbatim selection anchors nowhere, all four entry points retry with the +// markdown stripped — consistently, so the suggestion-uniqueness gate stays +// coherent — while what gets STORED remains the raw document substring. +// --------------------------------------------------------------------------- +test("a markdown-styled selection anchors against plain doc text via the strip fallback", () => { + const doc = paragraphDoc([{ type: "text", text: "a bold word here" }]); + // The agent quoted "**bold** word" from a styled view; the doc is plain text. + const sel = "**bold** word"; + const resolved = resolveAnchorSelection(doc, sel); + assert.equal(resolved.found, true, "strip fallback finds the anchor"); + assert.equal(resolved.normalized, true, "reports the soft-warning flag"); + assert.equal(canAnchorInDoc(doc, sel), true); + assert.equal(countAnchorMatches(doc, sel), 1); + + const ok = applyAnchorInDoc(doc, sel, COMMENT_ID); + assert.equal(ok, true); + const marked = doc.content[0].content.filter((p) => commentMark(p)); + assert.equal(marked.map((m) => m.text).join(""), "bold word", + "the mark lands on the plain-text span"); +}); + +test("getAnchoredText stores the RAW doc substring even when matched via the strip fallback", () => { + // Doc uses a smart apostrophe; the agent typed ASCII + markdown emphasis. + const doc = paragraphDoc([{ type: "text", text: "it’s bold now" }]); + const stored = getAnchoredText(doc, "it's **bold**"); + assert.equal(stored, "it’s bold", + "stored selection is the raw document text, not the stripped/ASCII locator"); +}); + +test("the strip fallback does not flip a raw-unique selection to ambiguous", () => { + // "config" appears twice, but the raw phrase "config value" appears once. + const doc = { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "the config value here" }] }, + { type: "paragraph", content: [{ type: "text", text: "another config here" }] }, + ], + }; + // Raw phrase is unique -> exactly 1, and no strip happens (nothing to strip). + assert.equal(countAnchorMatches(doc, "config value"), 1); + assert.equal(resolveAnchorSelection(doc, "config value").normalized, false); +}); + +test("EXACT WINS: a raw match short-circuits the strip fallback (count reflects raw)", () => { + // A literal "**" run exists raw once; its stripped form would also appear. + const doc = paragraphDoc([{ type: "text", text: "use **stars** and stars" }]); + // Raw "**stars**" occurs once -> count 1 from the verbatim locator; the + // fallback (which would find two "stars") never runs. + assert.equal(countAnchorMatches(doc, "**stars**"), 1); + assert.equal(resolveAnchorSelection(doc, "**stars**").normalized, false); +}); + +test("a markdown selection whose stripped form is ambiguous is counted as ambiguous", () => { + const doc = { + type: "doc", + content: [ + { type: "paragraph", content: [{ type: "text", text: "first config here" }] }, + { type: "paragraph", content: [{ type: "text", text: "second config here" }] }, + ], + }; + // Verbatim "**config**" matches nothing; stripped "config" matches twice. + assert.equal(countAnchorMatches(doc, "**config**"), 2); +}); From 72c2d1687e1915c0add0faa6f4c4d0bd20be5dd5 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Fri, 10 Jul 2026 00:06:28 +0300 Subject: [PATCH 2/2] =?UTF-8?q?docs(mcp):=20=D0=BF=D0=BE=D1=87=D0=B8=D0=BD?= =?UTF-8?q?=D0=B8=D1=82=D1=8C=20=D0=BE=D1=81=D0=B8=D1=80=D0=BE=D1=82=D0=B5?= =?UTF-8?q?=D0=B2=D1=88=D0=B8=D0=B9=20docstring=20+=20=D1=83=D1=82=D0=BE?= =?UTF-8?q?=D1=87=D0=BD=D0=B8=D1=82=D1=8C=20=D0=BA=D0=BE=D0=BC=D0=BC=D0=B5?= =?UTF-8?q?=D0=BD=D1=82=20=D0=BF=D1=80=D0=BE=20exact-wins=20(=D1=80=D0=B5?= =?UTF-8?q?=D0=B2=D1=8C=D1=8E=20#427)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - text-normalize.ts: closestBlockHint был вставлен между docstring'ом stripInlineMarkdown и его определением -> docstring осиротел. closestBlockHint перенесён ПОСЛЕ stripInlineMarkdown, каждый docstring снова примыкает к своей функции. Поведение не менялось (только порядок объявлений). - comment-anchor.ts: header-коммент завышал маршрутизацию — countAnchorMatches НЕ зовёт resolveAnchorSelection, у него своя параллельная реализация exact-wins. Коммент уточнён: can/get/apply идут через resolveAnchorSelection, count держит свой счётчик-примитив, синхронный с ним; обе реализации exact-wins должны держаться в синхроне при правках. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/mcp/src/lib/comment-anchor.ts | 12 ++++--- packages/mcp/src/lib/text-normalize.ts | 46 +++++++++++++------------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/packages/mcp/src/lib/comment-anchor.ts b/packages/mcp/src/lib/comment-anchor.ts index 5c94250b..4c16456d 100644 --- a/packages/mcp/src/lib/comment-anchor.ts +++ b/packages/mcp/src/lib/comment-anchor.ts @@ -22,10 +22,14 @@ * inline markdown (`**bold**`, `` `code` ``, `[t](u)`), the raw locator will not * match the document's plain text. Exactly like edit_page_text's json-edit * fallback, we first try the verbatim selection and, ONLY if it anchors nowhere - * in the whole document, retry with `stripInlineMarkdown` applied. This decision - * is made ONCE per public call in `resolveAnchorSelection`, so the four public - * entry points (can/count/get/apply) all agree on which locator matched — the - * suggestion-uniqueness gate depends on count and can/get never disagreeing. + * in the whole document, retry with `stripInlineMarkdown` applied. `canAnchorInDoc`, + * `getAnchoredText` and `applyAnchorInDoc` share this decision via + * `resolveAnchorSelection`. `countAnchorMatches` keeps its OWN parallel exact-wins + * implementation (it needs a raw match COUNT, not a single resolved locator), kept + * deliberately in sync with `resolveAnchorSelection`: raw match ⇒ use raw, else fall + * back to the stripped count. All four therefore agree on which locator matched — + * the suggestion-uniqueness gate depends on count and can/get never disagreeing, so + * these two exact-wins implementations MUST stay in sync if either is changed. */ import { stripInlineMarkdown } from "./text-normalize.js"; diff --git a/packages/mcp/src/lib/text-normalize.ts b/packages/mcp/src/lib/text-normalize.ts index cd545366..997cbff0 100644 --- a/packages/mcp/src/lib/text-normalize.ts +++ b/packages/mcp/src/lib/text-normalize.ts @@ -92,6 +92,29 @@ export function stripBalancedWrappers(s: string): string { * ORIGINAL string is returned so a locator can never normalize down to "" and * match everything. */ +export function stripInlineMarkdown(s: string): string { + if (typeof s !== "string" || s.length === 0) return s; + + // 1 + 2. Shared link/image and balanced-wrapper passes. + let out = stripWrappersAndLinks(s); + + // 3. Trim leading/trailing decoration: whitespace, leftover markdown markers, + // and emoji (Extended_Pictographic plus the VS16 / ZWJ joiners, plus the + // regional-indicator range U+1F1E6–U+1F1FF for flag emoji, which are NOT + // Extended_Pictographic). The `u` flag enables the Unicode property escape. + // Anchored runs only — interior text and sentence punctuation are untouched. + const DECORATION = + "[\\s*_~\\x60\\p{Extended_Pictographic}\\u{1F1E6}-\\u{1F1FF}\\u{FE0F}\\u{200D}]+"; + out = out + .replace(new RegExp("^" + DECORATION, "u"), "") + .replace(new RegExp(DECORATION + "$", "u"), ""); + + // 4. Never normalize a locator down to nothing. + if (out.length === 0) return s; + + return out; +} + /** * Build a bounded "closest text" hint for an anchor/find MISS, shared by * edit_page_text (json-edit) and create_comment (client) so both surface the @@ -125,26 +148,3 @@ export function closestBlockHint( points.length > 120 ? points.slice(0, 120).join("") + "…" : hitBlock; return ` Closest block text: "${snippet}".`; } - -export function stripInlineMarkdown(s: string): string { - if (typeof s !== "string" || s.length === 0) return s; - - // 1 + 2. Shared link/image and balanced-wrapper passes. - let out = stripWrappersAndLinks(s); - - // 3. Trim leading/trailing decoration: whitespace, leftover markdown markers, - // and emoji (Extended_Pictographic plus the VS16 / ZWJ joiners, plus the - // regional-indicator range U+1F1E6–U+1F1FF for flag emoji, which are NOT - // Extended_Pictographic). The `u` flag enables the Unicode property escape. - // Anchored runs only — interior text and sentence punctuation are untouched. - const DECORATION = - "[\\s*_~\\x60\\p{Extended_Pictographic}\\u{1F1E6}-\\u{1F1FF}\\u{FE0F}\\u{200D}]+"; - out = out - .replace(new RegExp("^" + DECORATION, "u"), "") - .replace(new RegExp(DECORATION + "$", "u"), ""); - - // 4. Never normalize a locator down to nothing. - if (out.length === 0) return s; - - return out; -}