fix(mcp): drawioCreate — успех с warning на вложенной вставке, а не throw (#494)
Коммит 3. При вставке диаграммы во вложенный контейнер (анкор внутри callout/ячейки таблицы) узел УЖЕ записан и закоммичен мутацией, но тул кидал ошибку «no addressable #<index> handle». Retry-склонный агент воспринимал это как провал записи и повторял drawioCreate → ДУБЛИКАТ диаграммы (тот же класс double-apply, что в инциденте #435). Правка: ветка insertedIndex<0 возвращает success:true с nodeId:null и warning'ом «written NESTED, saved — do NOT re-create; re-read via getOutline/getPageJson (attachmentId …)» вместо throw. Запись подтверждается, агент знает, что хендла нет и как перечитать — и не ретраит приземлившуюся запись. nodeId стал string|null в drawioCreate и наследующих drawioFromGraph/ drawioFromMermaid (там тот же путь) + в интерфейсе IDrawioMixin. Мок-тест: вложенная вставка не кидает, отдаёт success/nodeId:null/warning и пишет диаграмму РОВНО один раз вложенной (краснеет, если вернуть throw). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -57,11 +57,11 @@ import {
|
||||
// Derived from the class below; `implements IDrawioMixin` fails to compile on drift.
|
||||
export interface IDrawioMixin {
|
||||
drawioGet(pageId: string, node: string, format?: "xml" | "svg"): Promise<{ pageId: string; nodeId: string; format: "xml" | "svg"; content: string; meta: { attachmentId: string | null; title: string | null; width: number | null; height: number | null; cellCount: number; hash: string; }; }>;
|
||||
drawioCreate(pageId: string, where: { position: "before" | "after" | "append"; anchorNodeId?: string; anchorText?: string; }, xml: string, title?: string, layout?: "elk"): Promise<{ success: boolean; nodeId: string; attachmentId: string; warnings: string[]; verify?: any; }>;
|
||||
drawioCreate(pageId: string, where: { position: "before" | "after" | "append"; anchorNodeId?: string; anchorText?: string; }, xml: string, title?: string, layout?: "elk"): Promise<{ success: boolean; nodeId: string | null; attachmentId: string; warnings: string[]; verify?: any; }>;
|
||||
drawioUpdate(pageId: string, node: string, xml: string, baseHash: string, layout?: "elk"): Promise<{ success: boolean; nodeId: string; attachmentId: string; warnings: string[]; verify?: any; }>;
|
||||
drawioEditCells(pageId: string, node: string, operations: CellOp[], baseHash: string): Promise<{ success: boolean; nodeId: string; attachmentId: string; warnings: string[]; verify?: any; }>;
|
||||
drawioFromGraph(pageId: string, where: { position: "before" | "after" | "append"; anchorNodeId?: string; anchorText?: string; }, graph: Graph, direction?: "LR" | "RL" | "TB" | "BT", preset?: string, layout?: GraphLayoutMode, node?: string): Promise<{ success: boolean; nodeId: string; attachmentId: string; warnings: string[]; iconsResolved: number; iconsMissing: string[]; verify?: any; }>;
|
||||
drawioFromMermaid(pageId: string, where: { position: "before" | "after" | "append"; anchorNodeId?: string; anchorText?: string; }, mermaid: string, preset?: string): Promise<{ success: boolean; nodeId: string; attachmentId: string; warnings: string[]; iconsResolved: number; iconsMissing: string[]; verify?: any; }>;
|
||||
drawioFromGraph(pageId: string, where: { position: "before" | "after" | "append"; anchorNodeId?: string; anchorText?: string; }, graph: Graph, direction?: "LR" | "RL" | "TB" | "BT", preset?: string, layout?: GraphLayoutMode, node?: string): Promise<{ success: boolean; nodeId: string | null; attachmentId: string; warnings: string[]; iconsResolved: number; iconsMissing: string[]; verify?: any; }>;
|
||||
drawioFromMermaid(pageId: string, where: { position: "before" | "after" | "append"; anchorNodeId?: string; anchorText?: string; }, mermaid: string, preset?: string): Promise<{ success: boolean; nodeId: string | null; attachmentId: string; warnings: string[]; iconsResolved: number; iconsMissing: string[]; verify?: any; }>;
|
||||
}
|
||||
|
||||
export function DrawioMixin<TBase extends GConstructor<DocmostClientContext>>(Base: TBase): GConstructor<DocmostClientContext & IDrawioMixin> & TBase {
|
||||
@@ -164,7 +164,9 @@ export function DrawioMixin<TBase extends GConstructor<DocmostClientContext>>(Ba
|
||||
layout?: "elk",
|
||||
): Promise<{
|
||||
success: boolean;
|
||||
nodeId: string;
|
||||
// `null` when the diagram was written but nested (no addressable "#<index>"
|
||||
// handle) — see the nested-insert branch below (#494).
|
||||
nodeId: string | null;
|
||||
attachmentId: string;
|
||||
warnings: string[];
|
||||
verify?: any;
|
||||
@@ -276,11 +278,28 @@ export function DrawioMixin<TBase extends GConstructor<DocmostClientContext>>(Ba
|
||||
// anchor), where "#<index>" — which addresses only top-level blocks —
|
||||
// cannot reference it. drawio nodes carry no persisted id, so there is no
|
||||
// stable handle for a nested diagram.
|
||||
throw new Error(
|
||||
`drawioCreate: the diagram was inserted on page ${pageId} but not as a ` +
|
||||
`top-level block, so it has no addressable "#<index>" handle. Anchor ` +
|
||||
`on a top-level block (or append) so the diagram can be re-read.`,
|
||||
);
|
||||
//
|
||||
// CRITICAL (#494): the diagram is ALREADY WRITTEN and committed at this
|
||||
// point (the mutation above succeeded). Throwing here reported a FAILURE for
|
||||
// a write that in fact landed, so a retry-prone agent re-ran drawioCreate
|
||||
// and inserted a DUPLICATE diagram (the #435 double-apply class). Return
|
||||
// SUCCESS with nodeId:null and a warning instead: the write is
|
||||
// acknowledged, and the agent is told there is no addressable handle and how
|
||||
// to re-read the diagram — so it never blind-retries a landed write.
|
||||
return {
|
||||
success: true,
|
||||
nodeId: null,
|
||||
attachmentId: att.id,
|
||||
warnings: [
|
||||
...prepared.warnings,
|
||||
`The diagram was written on page ${pageId} but as a NESTED block (not ` +
|
||||
`top-level), so it has no addressable "#<index>" handle. It is saved ` +
|
||||
`— do NOT re-create it. To read or edit it, locate it via getOutline ` +
|
||||
`/ getPageJson (attachmentId ${att.id}). To get a stable "#<index>" ` +
|
||||
`handle, anchor on a top-level block (or append).`,
|
||||
],
|
||||
verify: mutation.verify,
|
||||
};
|
||||
}
|
||||
|
||||
// The returned handle is POSITIONAL ("#<index>"): valid for the immediate
|
||||
@@ -597,7 +616,9 @@ export function DrawioMixin<TBase extends GConstructor<DocmostClientContext>>(Ba
|
||||
node?: string,
|
||||
): Promise<{
|
||||
success: boolean;
|
||||
nodeId: string;
|
||||
// `null` when written nested (no addressable handle) — inherited from
|
||||
// drawioCreate (#494).
|
||||
nodeId: string | null;
|
||||
attachmentId: string;
|
||||
warnings: string[];
|
||||
iconsResolved: number;
|
||||
@@ -682,7 +703,9 @@ export function DrawioMixin<TBase extends GConstructor<DocmostClientContext>>(Ba
|
||||
preset?: string,
|
||||
): Promise<{
|
||||
success: boolean;
|
||||
nodeId: string;
|
||||
// `null` when written nested (no addressable handle) — inherited from
|
||||
// drawioFromGraph/drawioCreate (#494).
|
||||
nodeId: string | null;
|
||||
attachmentId: string;
|
||||
warnings: string[];
|
||||
iconsResolved: number;
|
||||
|
||||
@@ -170,6 +170,65 @@ test("drawioCreate: before/after requires exactly one anchor", async () => {
|
||||
);
|
||||
});
|
||||
|
||||
// #494 — a NESTED insert (anchored inside a callout/table cell) lands a write
|
||||
// that "#<index>" cannot address. The tool used to THROW here even though the
|
||||
// diagram was already committed, so a retry-prone agent re-created a DUPLICATE.
|
||||
// It must now report SUCCESS (nodeId:null + a warning) so the agent never
|
||||
// blind-retries a landed write. This REDDENS if the throw is restored (the
|
||||
// assert.doesNotReject + success asserts would fail).
|
||||
test("drawioCreate: a NESTED insert succeeds with nodeId:null + a warning (no throw, no duplicate)", async () => {
|
||||
const pageDoc = {
|
||||
type: "doc",
|
||||
content: [
|
||||
{
|
||||
type: "callout",
|
||||
attrs: { id: "co1" },
|
||||
content: [
|
||||
{
|
||||
type: "paragraph",
|
||||
attrs: { id: "inner" },
|
||||
content: [{ type: "text", text: "hello inner" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
const { client, calls } = makeClient({ pageDoc });
|
||||
|
||||
let res;
|
||||
await assert.doesNotReject(async () => {
|
||||
res = await client.drawioCreate(
|
||||
"page1",
|
||||
{ position: "after", anchorNodeId: "inner" },
|
||||
MODEL,
|
||||
);
|
||||
});
|
||||
|
||||
// The write is acknowledged as a SUCCESS...
|
||||
assert.equal(res.success, true);
|
||||
// ...but with NO addressable "#<index>" handle (it is nested).
|
||||
assert.equal(res.nodeId, null);
|
||||
assert.equal(res.attachmentId, "att-1");
|
||||
// A warning tells the agent it is saved (do not re-create) and how to re-read.
|
||||
assert.ok(
|
||||
res.warnings.some((w) => /NESTED|do NOT re-create/i.test(w)),
|
||||
"missing the nested-write warning",
|
||||
);
|
||||
|
||||
// The diagram was written EXACTLY ONCE, nested inside the callout (not a
|
||||
// top-level block) — proving it really landed (so a retry would duplicate).
|
||||
assert.equal(calls.uploads.length, 1);
|
||||
assert.equal(calls.mutations.length, 1);
|
||||
const topLevel = calls.mutations[0].doc.content;
|
||||
assert.ok(
|
||||
!topLevel.some((b) => b && b.type === "drawio"),
|
||||
"the diagram must be nested, not a top-level block",
|
||||
);
|
||||
const nested = findDrawio(calls.mutations[0].doc);
|
||||
assert.equal(nested.length, 1, "exactly one diagram written");
|
||||
assert.equal(nested[0].attrs.attachmentId, "att-1");
|
||||
});
|
||||
|
||||
// --- drawioGet ------------------------------------------------------------
|
||||
|
||||
test("drawioGet: decodes the model and returns meta with a hash", async () => {
|
||||
|
||||
Reference in New Issue
Block a user