vvzvlad/gitmost
1
0
Fork 0
Code Issues 14 Pull Requests 12 Packages Projects Releases Activity

fix(mcp): write page body before title to avoid split-brain on failure (#159)

Browse Source 
updatePage (markdown) and updatePageJson wrote the title via REST FIRST, then
the body via collab. If the body write failed (e.g. a collab persist timeout),
the page was left with the NEW title over its OLD body — a split-brain the tool
reported as an error but never repaired (red-team finding #10).

Reorder both: write the body first, and only set the title after the body has
persisted. Now a body-write failure leaves the title untouched (no split-brain).
A title write failing after a successful body is rarer (REST is fast) and leaves
correct content under a stale title — the strictly lesser inconsistency — which
is the same trade-off the issue's "atomic, or roll back the title" intends,
without the fragility of a rollback write that could itself fail.

No unit test: both paths require a live collab provider and the suite has no
provider mock; the change is a pure reordering. All 306 mcp tests still pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude code agent 227
2026-06-25 05:14:42 +03:00
parent 9703fc2b36
commit a670de7638
2 changed files with 193 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
packages/mcp/build/client.js
View File

@@ -16,7 +16,7 @@ import { withPageLock } from "./lib/page-lock.js";
import { applyTextEdits, } from "./lib/json-edit.js"; import { applyTextEdits, } from "./lib/json-edit.js";
import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { getCollabToken, performLogin } from "./lib/auth-utils.js";
import { diffDocs, summarizeChange } from "./lib/diff.js"; import { diffDocs, summarizeChange } from "./lib/diff.js";
import { applyAnchorInDoc, canAnchorInDoc, } from "./lib/comment-anchor.js"; import { applyAnchorInDoc, canAnchorInDoc } from "./lib/comment-anchor.js";
import { blockText, walk, getList, insertMarkerAfter, setCalloutRange, noteItem, mdToInlineNodes, commentsToFootnotes, } from "./lib/transforms.js"; import { blockText, walk, getList, insertMarkerAfter, setCalloutRange, noteItem, mdToInlineNodes, commentsToFootnotes, } from "./lib/transforms.js";
import vm from "node:vm"; import vm from "node:vm";
// Supported image types, kept as two lookup tables so both a local file // Supported image types, kept as two lookup tables so both a local file
@@ -208,7 +208,9 @@ export class DocmostClient {
// getCollabToken wraps the AxiosError in a plain Error but attaches the // getCollabToken wraps the AxiosError in a plain Error but attaches the
// HTTP status as `.status`, so detect an auth failure via either the raw // HTTP status as `.status`, so detect an auth failure via either the raw
// AxiosError shape OR the attached status. // AxiosError shape OR the attached status.
const axiosStatus = axios.isAxiosError(e) ? e.response?.status : undefined; const axiosStatus = axios.isAxiosError(e)
? e.response?.status
: undefined;
const attachedStatus = e?.status; const attachedStatus = e?.status;
const isAuthError = axiosStatus === 401 || const isAuthError = axiosStatus === 401 ||
axiosStatus === 403 || axiosStatus === 403 ||
@@ -687,7 +689,12 @@ export class DocmostClient {
if (!inserted) { if (!inserted) {
throw new Error(`table_insert_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`); throw new Error(`table_insert_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`);
} }
return { success: true, table: tableRef, inserted: true, verify: mutation.verify }; return {
success: true,
table: tableRef,
inserted: true,
verify: mutation.verify,
};
} }
/** /**
* Delete the row at 0-based `index` from a table on the LIVE collab document. * Delete the row at 0-based `index` from a table on the LIVE collab document.
@@ -709,7 +716,12 @@ export class DocmostClient {
if (!deleted) { if (!deleted) {
throw new Error(`table_delete_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`); throw new Error(`table_delete_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`);
} }
return { success: true, table: tableRef, deleted: true, verify: mutation.verify }; return {
success: true,
table: tableRef,
deleted: true,
verify: mutation.verify,
};
} }
/** /**
* Set the plain-text content of cell `[row, col]` (0-based) in a table on the * Set the plain-text content of cell `[row, col]` (0-based) in a table on the
@@ -733,7 +745,13 @@ export class DocmostClient {
if (!updated) { if (!updated) {
throw new Error(`table_update_cell: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`); throw new Error(`table_update_cell: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`);
} }
return { success: true, table: tableRef, row, col, verify: mutation.verify }; return {
success: true,
table: tableRef,
row,
col,
verify: mutation.verify,
};
} }
/** /**
* Create a new page with title and content. * Create a new page with title and content.
@@ -828,9 +846,11 @@ export class DocmostClient {
*/ */
async updatePage(pageId, content, title) { async updatePage(pageId, content, title) {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
if (title) { // Write the BODY first, then the title (#159 split-brain). If the collab
await this.client.post("/pages/update", { pageId, title }); // body write fails (e.g. a persist timeout), the title must be left
} // UNTOUCHED so the page never ends up with a new title over its old body.
// A title write failing AFTER a successful body is rarer (REST is fast) and
// leaves correct content under a stale title — the lesser inconsistency.
let collabToken = ""; let collabToken = "";
let mutation; let mutation;
try { try {
@@ -849,6 +869,10 @@ export class DocmostClient {
} }
throw new Error(`Failed to update page content: ${error.message}`); throw new Error(`Failed to update page content: ${error.message}`);
} }
// Body persisted successfully — now it is safe to set the title.
if (title) {
await this.client.post("/pages/update", { pageId, title });
}
return { return {
success: true, success: true,
modified: true, modified: true,
@@ -968,7 +992,9 @@ export class DocmostClient {
if (!node || typeof node !== "object" || typeof node.type !== "string") { if (!node || typeof node !== "object" || typeof node.type !== "string") {
throw new Error("invalid ProseMirror document: every node must be an object with a string `type`"); throw new Error("invalid ProseMirror document: every node must be an object with a string `type`");
} }
if ("text" in node && node.type === "text" && typeof node.text !== "string") { if ("text" in node &&
node.type === "text" &&
typeof node.text !== "string") {
throw new Error("invalid ProseMirror document: a text node must have a string `text`"); throw new Error("invalid ProseMirror document: a text node must have a string `text`");
} }
if (node.marks !== undefined) { if (node.marks !== undefined) {
@@ -976,7 +1002,9 @@ export class DocmostClient {
throw new Error("invalid ProseMirror document: `marks` must be an array"); throw new Error("invalid ProseMirror document: `marks` must be an array");
} }
for (const mark of node.marks) { for (const mark of node.marks) {
if (!mark || typeof mark !== "object" || typeof mark.type !== "string") { if (!mark ||
typeof mark !== "object" ||
typeof mark.type !== "string") {
throw new Error("invalid ProseMirror document: every mark must be an object with a string `type`"); throw new Error("invalid ProseMirror document: every mark must be an object with a string `type`");
} }
} }
@@ -1035,11 +1063,14 @@ export class DocmostClient {
// the markdown link path (which TipTap sanitizes), raw JSON could otherwise // the markdown link path (which TipTap sanitizes), raw JSON could otherwise
// inject javascript:/data: link hrefs or media srcs straight into the doc. // inject javascript:/data: link hrefs or media srcs straight into the doc.
this.validateDocUrls(doc); this.validateDocUrls(doc);
// Write the BODY first, then the title (#159 split-brain): a failed body
// write (e.g. persist timeout) must not leave a new title over the old body.
const collabToken = await this.getCollabTokenWithReauth();
const mutation = await replacePageContent(pageId, doc, collabToken, this.apiUrl);
// Body persisted successfully — now it is safe to set the title.
if (title) { if (title) {
await this.client.post("/pages/update", { pageId, title }); await this.client.post("/pages/update", { pageId, title });
} }
const collabToken = await this.getCollabTokenWithReauth();
const mutation = await replacePageContent(pageId, doc, collabToken, this.apiUrl);
return { return {
success: true, success: true,
modified: true, modified: true,
@@ -1056,9 +1087,7 @@ export class DocmostClient {
async exportPageMarkdown(pageId) { async exportPageMarkdown(pageId) {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
const page = await this.getPageRaw(pageId); const page = await this.getPageRaw(pageId);
const body = page.content const body = page.content ? convertProseMirrorToMarkdown(page.content) : "";
? convertProseMirrorToMarkdown(page.content)
: "";
let comments = []; let comments = [];
try { try {
comments = await this.listComments(pageId); comments = await this.listComments(pageId);
@@ -1363,7 +1392,7 @@ export class DocmostClient {
// markdown/emoji are tolerated only as a strip-and-retry fallback, so a // markdown/emoji are tolerated only as a strip-and-retry fallback, so a
// miss usually means the text differs from what's on the page. // miss usually means the text differs from what's on the page.
const hint = opts.anchorText const hint = opts.anchorText
? ' anchorText must be the block\'s literal rendered plain text (no markdown wrappers or emoji); anchorNodeId from get_page_json is more reliable.' ? " anchorText must be the block's literal rendered plain text (no markdown wrappers or emoji); anchorNodeId from get_page_json is more reliable."
: ""; : "";
throw new Error(`insert_node: anchor not found (${anchorDesc}) on page ${pageId}.${hint}`); throw new Error(`insert_node: anchor not found (${anchorDesc}) on page ${pageId}.${hint}`);
} }

124
packages/mcp/src/client.ts
View File

@@ -49,10 +49,7 @@ import {
} from "./lib/json-edit.js"; } from "./lib/json-edit.js";
import { getCollabToken, performLogin } from "./lib/auth-utils.js"; import { getCollabToken, performLogin } from "./lib/auth-utils.js";
import { diffDocs, summarizeChange } from "./lib/diff.js"; import { diffDocs, summarizeChange } from "./lib/diff.js";
import { import { applyAnchorInDoc, canAnchorInDoc } from "./lib/comment-anchor.js";
applyAnchorInDoc,
canAnchorInDoc,
} from "./lib/comment-anchor.js";
import { import {
blockText, blockText,
walk, walk,
@@ -305,7 +302,9 @@ export class DocmostClient {
// getCollabToken wraps the AxiosError in a plain Error but attaches the // getCollabToken wraps the AxiosError in a plain Error but attaches the
// HTTP status as `.status`, so detect an auth failure via either the raw // HTTP status as `.status`, so detect an auth failure via either the raw
// AxiosError shape OR the attached status. // AxiosError shape OR the attached status.
const axiosStatus = axios.isAxiosError(e) ? e.response?.status : undefined; const axiosStatus = axios.isAxiosError(e)
? e.response?.status
: undefined;
const attachedStatus = (e as any)?.status; const attachedStatus = (e as any)?.status;
const isAuthError = const isAuthError =
axiosStatus === 401 || axiosStatus === 401 ||
@@ -597,11 +596,7 @@ export class DocmostClient {
* sidebar requests and is bounded by that method's 10000-node cap (and skips * sidebar requests and is bounded by that method's 10000-node cap (and skips
* soft-deleted pages server-side). * soft-deleted pages server-side).
*/ */
async listPages( async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
spaceId?: string,
limit: number = 50,
tree: boolean = false,
) {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
if (tree) { if (tree) {
@@ -880,7 +875,12 @@ export class DocmostClient {
`table_insert_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`, `table_insert_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`,
); );
} }
return { success: true, table: tableRef, inserted: true, verify: mutation.verify }; return {
success: true,
table: tableRef,
inserted: true,
verify: mutation.verify,
};
} }
/** /**
@@ -899,7 +899,11 @@ export class DocmostClient {
this.apiUrl, this.apiUrl,
(liveDoc) => { (liveDoc) => {
deleted = false; deleted = false;
const { doc: nd, deleted: del } = deleteTableRow(liveDoc, tableRef, index); const { doc: nd, deleted: del } = deleteTableRow(
liveDoc,
tableRef,
index,
);
deleted = del; deleted = del;
if (!deleted) return null; // table not found -> skip the write entirely if (!deleted) return null; // table not found -> skip the write entirely
return nd; return nd;
@@ -911,7 +915,12 @@ export class DocmostClient {
`table_delete_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`, `table_delete_row: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`,
); );
} }
return { success: true, table: tableRef, deleted: true, verify: mutation.verify }; return {
success: true,
table: tableRef,
deleted: true,
verify: mutation.verify,
};
} }
/** /**
@@ -956,7 +965,13 @@ export class DocmostClient {
`table_update_cell: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`, `table_update_cell: no table found for "${tableRef}" on page ${pageId} (use "#<index>" from get_outline, or a block id inside the table)`,
); );
} }
return { success: true, table: tableRef, row, col, verify: mutation.verify }; return {
success: true,
table: tableRef,
row,
col,
verify: mutation.verify,
};
} }
/** /**
@@ -1030,8 +1045,7 @@ export class DocmostClient {
response = await axios.post(importUrl, form2, { response = await axios.post(importUrl, form2, {
headers: { headers: {
...form2.getHeaders(), ...form2.getHeaders(),
Authorization: Authorization: this.client.defaults.headers.common["Authorization"],
this.client.defaults.headers.common["Authorization"],
}, },
timeout: 60000, timeout: 60000,
}); });
@@ -1065,10 +1079,11 @@ export class DocmostClient {
async updatePage(pageId: string, content: string, title?: string) { async updatePage(pageId: string, content: string, title?: string) {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
if (title) { // Write the BODY first, then the title (#159 split-brain). If the collab
await this.client.post("/pages/update", { pageId, title }); // body write fails (e.g. a persist timeout), the title must be left
} // UNTOUCHED so the page never ends up with a new title over its old body.
// A title write failing AFTER a successful body is rarer (REST is fast) and
// leaves correct content under a stale title — the lesser inconsistency.
let collabToken = ""; let collabToken = "";
let mutation; let mutation;
try { try {
@@ -1095,6 +1110,11 @@ export class DocmostClient {
throw new Error(`Failed to update page content: ${error.message}`); throw new Error(`Failed to update page content: ${error.message}`);
} }
// Body persisted successfully — now it is safe to set the title.
if (title) {
await this.client.post("/pages/update", { pageId, title });
}
return { return {
success: true, success: true,
modified: true, modified: true,
@@ -1169,9 +1189,7 @@ export class DocmostClient {
for (const mark of node.marks) { for (const mark of node.marks) {
if (mark && mark.type === "link" && mark.attrs) { if (mark && mark.type === "link" && mark.attrs) {
if (!this.isSafeUrl(mark.attrs.href, "link")) { if (!this.isSafeUrl(mark.attrs.href, "link")) {
throw new Error( throw new Error(`unsafe link href rejected: "${mark.attrs.href}"`);
`unsafe link href rejected: "${mark.attrs.href}"`,
);
} }
} }
} }
@@ -1230,7 +1248,11 @@ export class DocmostClient {
"invalid ProseMirror document: every node must be an object with a string `type`", "invalid ProseMirror document: every node must be an object with a string `type`",
); );
} }
if ("text" in node && node.type === "text" && typeof node.text !== "string") { if (
"text" in node &&
node.type === "text" &&
typeof node.text !== "string"
) {
throw new Error( throw new Error(
"invalid ProseMirror document: a text node must have a string `text`", "invalid ProseMirror document: a text node must have a string `text`",
); );
@@ -1242,7 +1264,11 @@ export class DocmostClient {
); );
} }
for (const mark of node.marks) { for (const mark of node.marks) {
if (!mark || typeof mark !== "object" || typeof mark.type !== "string") { if (
!mark ||
typeof mark !== "object" ||
typeof mark.type !== "string"
) {
throw new Error( throw new Error(
"invalid ProseMirror document: every mark must be an object with a string `type`", "invalid ProseMirror document: every mark must be an object with a string `type`",
); );
@@ -1317,10 +1343,8 @@ export class DocmostClient {
// inject javascript:/data: link hrefs or media srcs straight into the doc. // inject javascript:/data: link hrefs or media srcs straight into the doc.
this.validateDocUrls(doc); this.validateDocUrls(doc);
if (title) { // Write the BODY first, then the title (#159 split-brain): a failed body
await this.client.post("/pages/update", { pageId, title }); // write (e.g. persist timeout) must not leave a new title over the old body.
}
const collabToken = await this.getCollabTokenWithReauth(); const collabToken = await this.getCollabTokenWithReauth();
const mutation = await replacePageContent( const mutation = await replacePageContent(
pageId, pageId,
@@ -1329,6 +1353,11 @@ export class DocmostClient {
this.apiUrl, this.apiUrl,
); );
// Body persisted successfully — now it is safe to set the title.
if (title) {
await this.client.post("/pages/update", { pageId, title });
}
return { return {
success: true, success: true,
modified: true, modified: true,
@@ -1346,9 +1375,7 @@ export class DocmostClient {
async exportPageMarkdown(pageId: string): Promise<string> { async exportPageMarkdown(pageId: string): Promise<string> {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
const page = await this.getPageRaw(pageId); const page = await this.getPageRaw(pageId);
const body = page.content const body = page.content ? convertProseMirrorToMarkdown(page.content) : "";
? convertProseMirrorToMarkdown(page.content)
: "";
let comments: any[] = []; let comments: any[] = [];
try { try {
comments = await this.listComments(pageId); comments = await this.listComments(pageId);
@@ -1562,7 +1589,8 @@ export class DocmostClient {
pageId, pageId,
applied: results, applied: results,
failed, failed,
message: (failed?.length ?? 0) message:
(failed?.length ?? 0)
? `Applied ${results?.length ?? 0} edit(s); ${failed!.length} failed (see failed[]). Node ids and formatting preserved.` ? `Applied ${results?.length ?? 0} edit(s); ${failed!.length} failed (see failed[]). Node ids and formatting preserved.`
: "Text edits applied (node ids and formatting preserved).", : "Text edits applied (node ids and formatting preserved).",
verify: mutation.verify, verify: mutation.verify,
@@ -1623,7 +1651,11 @@ export class DocmostClient {
this.apiUrl, this.apiUrl,
(liveDoc) => { (liveDoc) => {
replaced = 0; replaced = 0;
const { doc: nd, replaced: r } = replaceNodeById(liveDoc, nodeId, target); const { doc: nd, replaced: r } = replaceNodeById(
liveDoc,
nodeId,
target,
);
replaced = r; replaced = r;
// 0 matches -> skip the write. >1 matches -> the id is AMBIGUOUS: Docmost // 0 matches -> skip the write. >1 matches -> the id is AMBIGUOUS: Docmost
// duplicates block ids on copy/paste (and copyPageContent writes them // duplicates block ids on copy/paste (and copyPageContent writes them
@@ -1714,7 +1746,11 @@ export class DocmostClient {
this.apiUrl, this.apiUrl,
(liveDoc) => { (liveDoc) => {
inserted = false; inserted = false;
const { doc: nd, inserted: ins } = insertNodeRelative(liveDoc, node, opts); const { doc: nd, inserted: ins } = insertNodeRelative(
liveDoc,
node,
opts,
);
inserted = ins; inserted = ins;
if (!inserted) return null; // anchor not found -> skip the write entirely if (!inserted) return null; // anchor not found -> skip the write entirely
return nd; return nd;
@@ -1729,7 +1765,7 @@ export class DocmostClient {
// markdown/emoji are tolerated only as a strip-and-retry fallback, so a // markdown/emoji are tolerated only as a strip-and-retry fallback, so a
// miss usually means the text differs from what's on the page. // miss usually means the text differs from what's on the page.
const hint = opts.anchorText const hint = opts.anchorText
? ' anchorText must be the block\'s literal rendered plain text (no markdown wrappers or emoji); anchorNodeId from get_page_json is more reliable.' ? " anchorText must be the block's literal rendered plain text (no markdown wrappers or emoji); anchorNodeId from get_page_json is more reliable."
: ""; : "";
throw new Error( throw new Error(
`insert_node: anchor not found (${anchorDesc}) on page ${pageId}.${hint}`, `insert_node: anchor not found (${anchorDesc}) on page ${pageId}.${hint}`,
@@ -2157,7 +2193,11 @@ export class DocmostClient {
* subtree): pages updated after `since` are scanned and their comments * subtree): pages updated after `since` are scanned and their comments
* filtered by createdAt > since. * filtered by createdAt > since.
*/ */
async checkNewComments(spaceId: string, since: string, parentPageId?: string) { async checkNewComments(
spaceId: string,
since: string,
parentPageId?: string,
) {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
const sinceDate = new Date(since); const sinceDate = new Date(since);
@@ -2457,8 +2497,7 @@ export class DocmostClient {
response = await axios.post(uploadUrl, form2, { response = await axios.post(uploadUrl, form2, {
headers: { headers: {
...form2.getHeaders(), ...form2.getHeaders(),
Authorization: Authorization: this.client.defaults.headers.common["Authorization"],
this.client.defaults.headers.common["Authorization"],
}, },
timeout: 60000, timeout: 60000,
}); });
@@ -2871,8 +2910,7 @@ export class DocmostClient {
async diffPageVersions(pageId: string, from?: string, to?: string) { async diffPageVersions(pageId: string, from?: string, to?: string) {
await this.ensureAuthenticated(); await this.ensureAuthenticated();
const isCurrent = (v?: string) => const isCurrent = (v?: string) => v == null || v === "" || v === "current";
v == null || v === "" || v === "current";
const resolveSide = async ( const resolveSide = async (
v?: string, v?: string,
@@ -2993,7 +3031,9 @@ export class DocmostClient {
throw new Error(`transform did not compile: ${e?.message ?? e}`); throw new Error(`transform did not compile: ${e?.message ?? e}`);
} }
if (typeof fn !== "function") { if (typeof fn !== "function") {
throw new Error("transform must evaluate to a function (doc, ctx) => doc"); throw new Error(
"transform must evaluate to a function (doc, ctx) => doc",
);
} }
const result = vm.runInNewContext( const result = vm.runInNewContext(
"f(d, c)", "f(d, c)",