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 96fb737c9d
commit dd64c2ea05
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 { getCollabToken, performLogin } from "./lib/auth-utils.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 vm from "node:vm";
// 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
// HTTP status as `.status`, so detect an auth failure via either the raw
// 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 isAuthError = axiosStatus === 401 ||
axiosStatus === 403 ||
@@ -687,7 +689,12 @@ export class DocmostClient {
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)`);
}
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.
@@ -709,7 +716,12 @@ export class DocmostClient {
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)`);
}
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
@@ -733,7 +745,13 @@ export class DocmostClient {
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)`);
}
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.
@@ -828,9 +846,11 @@ export class DocmostClient {
*/
async updatePage(pageId, content, title) {
await this.ensureAuthenticated();
if (title) {
await this.client.post("/pages/update", { pageId, title });
}
// Write the BODY first, then the title (#159 split-brain). If the collab
// 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 mutation;
try {
@@ -849,6 +869,10 @@ export class DocmostClient {
}
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 {
success: true,
modified: true,
@@ -968,7 +992,9 @@ export class DocmostClient {
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`");
}
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`");
}
if (node.marks !== undefined) {
@@ -976,7 +1002,9 @@ export class DocmostClient {
throw new Error("invalid ProseMirror document: `marks` must be an array");
}
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`");
}
}
@@ -1035,11 +1063,14 @@ export class DocmostClient {
// the markdown link path (which TipTap sanitizes), raw JSON could otherwise
// inject javascript:/data: link hrefs or media srcs straight into the 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) {
await this.client.post("/pages/update", { pageId, title });
}
const collabToken = await this.getCollabTokenWithReauth();
const mutation = await replacePageContent(pageId, doc, collabToken, this.apiUrl);
return {
success: true,
modified: true,
@@ -1056,9 +1087,7 @@ export class DocmostClient {
async exportPageMarkdown(pageId) {
await this.ensureAuthenticated();
const page = await this.getPageRaw(pageId);
const body = page.content
? convertProseMirrorToMarkdown(page.content)
: "";
const body = page.content ? convertProseMirrorToMarkdown(page.content) : "";
let comments = [];
try {
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
// miss usually means the text differs from what's on the page.
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}`);
}

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

@@ -49,10 +49,7 @@ import {
} from "./lib/json-edit.js";
import { getCollabToken, performLogin } from "./lib/auth-utils.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,
@@ -305,7 +302,9 @@ export class DocmostClient {
// getCollabToken wraps the AxiosError in a plain Error but attaches the
// HTTP status as `.status`, so detect an auth failure via either the raw
// 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 isAuthError =
axiosStatus === 401 ||
@@ -597,11 +596,7 @@ export class DocmostClient {
* sidebar requests and is bounded by that method's 10000-node cap (and skips
* soft-deleted pages server-side).
*/
async listPages(
spaceId?: string,
limit: number = 50,
tree: boolean = false,
) {
async listPages(spaceId?: string, limit: number = 50, tree: boolean = false) {
await this.ensureAuthenticated();
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)`,
);
}
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,
(liveDoc) => {
deleted = false;
const { doc: nd, deleted: del } = deleteTableRow(liveDoc, tableRef, index);
const { doc: nd, deleted: del } = deleteTableRow(
liveDoc,
tableRef,
index,
);
deleted = del;
if (!deleted) return null; // table not found -> skip the write entirely
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)`,
);
}
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)`,
);
}
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, {
headers: {
...form2.getHeaders(),
Authorization:
this.client.defaults.headers.common["Authorization"],
Authorization: this.client.defaults.headers.common["Authorization"],
},
timeout: 60000,
});
@@ -1065,10 +1079,11 @@ export class DocmostClient {
async updatePage(pageId: string, content: string, title?: string) {
await this.ensureAuthenticated();
if (title) {
await this.client.post("/pages/update", { pageId, title });
}
// Write the BODY first, then the title (#159 split-brain). If the collab
// 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 mutation;
try {
@@ -1095,6 +1110,11 @@ export class DocmostClient {
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 {
success: true,
modified: true,
@@ -1169,9 +1189,7 @@ export class DocmostClient {
for (const mark of node.marks) {
if (mark && mark.type === "link" && mark.attrs) {
if (!this.isSafeUrl(mark.attrs.href, "link")) {
throw new Error(
`unsafe link href rejected: "${mark.attrs.href}"`,
);
throw new Error(`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`",
);
}
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`",
);
@@ -1242,7 +1264,11 @@ export class DocmostClient {
);
}
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`",
);
@@ -1317,10 +1343,8 @@ export class DocmostClient {
// inject javascript:/data: link hrefs or media srcs straight into the doc.
this.validateDocUrls(doc);
if (title) {
await this.client.post("/pages/update", { pageId, title });
}
// 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,
@@ -1329,6 +1353,11 @@ export class DocmostClient {
this.apiUrl,
);
// Body persisted successfully — now it is safe to set the title.
if (title) {
await this.client.post("/pages/update", { pageId, title });
}
return {
success: true,
modified: true,
@@ -1346,9 +1375,7 @@ export class DocmostClient {
async exportPageMarkdown(pageId: string): Promise<string> {
await this.ensureAuthenticated();
const page = await this.getPageRaw(pageId);
const body = page.content
? convertProseMirrorToMarkdown(page.content)
: "";
const body = page.content ? convertProseMirrorToMarkdown(page.content) : "";
let comments: any[] = [];
try {
comments = await this.listComments(pageId);
@@ -1562,7 +1589,8 @@ export class DocmostClient {
pageId,
applied: results,
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.`
: "Text edits applied (node ids and formatting preserved).",
verify: mutation.verify,
@@ -1623,7 +1651,11 @@ export class DocmostClient {
this.apiUrl,
(liveDoc) => {
replaced = 0;
const { doc: nd, replaced: r } = replaceNodeById(liveDoc, nodeId, target);
const { doc: nd, replaced: r } = replaceNodeById(
liveDoc,
nodeId,
target,
);
replaced = r;
// 0 matches -> skip the write. >1 matches -> the id is AMBIGUOUS: Docmost
// duplicates block ids on copy/paste (and copyPageContent writes them
@@ -1714,7 +1746,11 @@ export class DocmostClient {
this.apiUrl,
(liveDoc) => {
inserted = false;
const { doc: nd, inserted: ins } = insertNodeRelative(liveDoc, node, opts);
const { doc: nd, inserted: ins } = insertNodeRelative(
liveDoc,
node,
opts,
);
inserted = ins;
if (!inserted) return null; // anchor not found -> skip the write entirely
return nd;
@@ -1729,7 +1765,7 @@ export class DocmostClient {
// 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.
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}`,
@@ -2157,7 +2193,11 @@ export class DocmostClient {
* subtree): pages updated after `since` are scanned and their comments
* filtered by createdAt > since.
*/
async checkNewComments(spaceId: string, since: string, parentPageId?: string) {
async checkNewComments(
spaceId: string,
since: string,
parentPageId?: string,
) {
await this.ensureAuthenticated();
const sinceDate = new Date(since);
@@ -2457,8 +2497,7 @@ export class DocmostClient {
response = await axios.post(uploadUrl, form2, {
headers: {
...form2.getHeaders(),
Authorization:
this.client.defaults.headers.common["Authorization"],
Authorization: this.client.defaults.headers.common["Authorization"],
},
timeout: 60000,
});
@@ -2871,8 +2910,7 @@ export class DocmostClient {
async diffPageVersions(pageId: string, from?: string, to?: string) {
await this.ensureAuthenticated();
const isCurrent = (v?: string) =>
v == null || v === "" || v === "current";
const isCurrent = (v?: string) => v == null || v === "" || v === "current";
const resolveSide = async (
v?: string,
@@ -2993,7 +3031,9 @@ export class DocmostClient {
throw new Error(`transform did not compile: ${e?.message ?? e}`);
}
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(
"f(d, c)",