mirror of
https://github.com/DayuanJiang/next-ai-draw-io.git
synced 2026-10-12 04:29:51 +08:00
fix(mcp-server): review fixes for the version cards and History
The shell's version cards (shell/node-versions-source.ts): - a version's change and undo target are the state it replaced, the History entry right before its first copy, not the card before it: after a restore those differ, and undo went to the wrong version (and not where restore_version steps_back=1 goes) - a card restores the newest copy of its content, as restore_version does, so page settings the user changed (a "user" copy) are kept - a blank page after a drawing is a clear of the canvas, a version of its own; only the blank page before any drawing is hidden - numbers and changes are keyed by content, not by the first copy's id, so a version keeps them when its first copy drops out of the server's 20-entry buffer; the caches start over for another server state (the process restarted: entry ids name other content) The server's History (src/history.ts): - firstCopyIds compares each entry with the first of every group only: a bare model matches any page name, so "same content" is not transitive, and a card could show one document and restore another - the time and pages fields had no reader; pages parsed every XML once more on every write Reading History (src/http-server.ts, shell/mcp-sync-core.ts): - GET /api/state and a push's answer carry a History key (entry count, newest id, the entry on the canvas); the shell reads History again only when it changes, so a hand edit no longer downloads every entry's XML and thumbnail - a failed History read is told again at the next poll - a History list from a state the poll has not seen yet is dropped
This commit is contained in:
dayuan.jiang
@@ -43,8 +43,6 @@ function entry(
|
||||
svg: "",
|
||||
xml,
|
||||
source: null,
|
||||
time: 1000 + id,
|
||||
pages: 1,
|
||||
firstId: id,
|
||||
...extra,
|
||||
}
|
||||
@@ -123,7 +121,7 @@ describe("buildVersions", () => {
|
||||
})
|
||||
|
||||
it("keeps the numbers once given, and gives new versions the next", () => {
|
||||
const numbers = new Map<number, number>()
|
||||
const numbers = new Map<string, number>()
|
||||
const changes = new Map()
|
||||
buildVersions(list([entry(0, A), entry(1, AB)], 1), numbers, changes)
|
||||
// The oldest entry dropped out of the server's buffer
|
||||
@@ -135,6 +133,138 @@ describe("buildVersions", () => {
|
||||
expect(versions.map((v) => v.number)).toEqual([2, 3])
|
||||
})
|
||||
|
||||
it("counts a write from the state it replaced, not from the card before, after a restore", () => {
|
||||
// History: A, AB, ABC, A again (restored), AD: the AI drew D on A
|
||||
const AD = doc(cell("a", "A") + cell("d", "D"))
|
||||
const { versions } = buildVersions(
|
||||
list(
|
||||
[
|
||||
entry(0, A),
|
||||
entry(1, AB),
|
||||
entry(2, ABC),
|
||||
entry(3, A, { firstId: 0, source: "restore" }),
|
||||
entry(4, AD),
|
||||
],
|
||||
4,
|
||||
),
|
||||
new Map(),
|
||||
new Map(),
|
||||
)
|
||||
expect(versions.map((v) => v.id)).toEqual(["0", "1", "2", "4"])
|
||||
const latest = versions[3]
|
||||
expect(latest.beforeXml).toBe(A)
|
||||
expect(latest.summary.shapesAdded).toBe(1)
|
||||
expect(latest.summary.shapesRemoved).toBe(0)
|
||||
expect(latest.beforeId).toBe("0")
|
||||
// The first drawing replaced nothing a card shows
|
||||
expect(versions[0].beforeId).toBeNull()
|
||||
expect(versions[1].beforeId).toBe("0")
|
||||
})
|
||||
|
||||
it("restores the newest copy of a version: a later copy may carry the user's page settings", () => {
|
||||
// The user set a background on A (kept as a "user" entry, the same
|
||||
// content to the server), then the AI added B
|
||||
const yellow = A.replace(
|
||||
"<mxGraphModel>",
|
||||
'<mxGraphModel background="#ffcc00">',
|
||||
)
|
||||
const { versions } = buildVersions(
|
||||
list(
|
||||
[
|
||||
entry(0, A),
|
||||
entry(1, yellow, { firstId: 0, source: "user" }),
|
||||
entry(2, AB),
|
||||
],
|
||||
2,
|
||||
),
|
||||
new Map(),
|
||||
new Map(),
|
||||
)
|
||||
expect(versions).toHaveLength(2)
|
||||
expect(versions[0].id).toBe("0")
|
||||
expect(versions[0].entryId).toBe(1)
|
||||
expect(versions[0].xml).toBe(yellow)
|
||||
expect(versions[0].source).toBeNull()
|
||||
})
|
||||
|
||||
it("shows a clear of the canvas as a version, hiding only the blank page before any drawing", () => {
|
||||
const blank = `<mxfile><diagram name="Page-1" id="page-1"><mxGraphModel><root><mxCell id="0"/><mxCell id="1" parent="0"/></root></mxGraphModel></diagram></mxfile>`
|
||||
// The server folds the cleared page into the first blank (same
|
||||
// content); then AB is restored
|
||||
const entries = [
|
||||
entry(0, blank),
|
||||
entry(1, A),
|
||||
entry(2, AB),
|
||||
entry(3, blank, { firstId: 0 }),
|
||||
entry(4, AB, { firstId: 2, source: "restore" }),
|
||||
]
|
||||
const cleared = buildVersions(
|
||||
list(entries.slice(0, 4), 3),
|
||||
new Map(),
|
||||
new Map(),
|
||||
)
|
||||
expect(cleared.versions.map((v) => v.id)).toEqual(["1", "2", "3"])
|
||||
expect(cleared.onCanvasId).toBe("3")
|
||||
expect(cleared.versions[2].beforeId).toBe("2")
|
||||
expect(cleared.versions[2].summary.shapesRemoved).toBe(2)
|
||||
const back = buildVersions(list(entries, 4), new Map(), new Map())
|
||||
expect(back.versions.map((v) => v.id)).toEqual(["1", "2", "3"])
|
||||
expect(back.onCanvasId).toBe("2")
|
||||
// A second clear is a version of its own; a restore of a clear is
|
||||
// a copy of the newest one
|
||||
const twice = buildVersions(
|
||||
list(
|
||||
[
|
||||
...entries,
|
||||
entry(5, blank, { firstId: 0 }),
|
||||
entry(6, blank, { firstId: 0, source: "restore" }),
|
||||
],
|
||||
6,
|
||||
),
|
||||
new Map(),
|
||||
new Map(),
|
||||
)
|
||||
expect(twice.versions.map((v) => [v.id, v.number])).toEqual([
|
||||
["1", 1],
|
||||
["2", 2],
|
||||
["3", 3],
|
||||
["5", 4],
|
||||
])
|
||||
expect(twice.onCanvasId).toBe("5")
|
||||
})
|
||||
|
||||
it("keeps a version's number and change when its first copy drops out of the server's buffer", () => {
|
||||
const numbers = new Map<string, number>()
|
||||
const changes = new Map()
|
||||
const docs = Array.from({ length: 20 }, (_, i) =>
|
||||
doc(
|
||||
Array.from({ length: i + 1 }, (_, j) =>
|
||||
cell(`c${j}`, `C${j}`),
|
||||
).join(""),
|
||||
),
|
||||
)
|
||||
const full = docs.map((xml, i) => entry(i, xml))
|
||||
buildVersions(list(full, 19), numbers, changes)
|
||||
// v1 restored: its copy is added and the original, the oldest
|
||||
// entry, drops out; the copy is now the first with that content
|
||||
const after = [
|
||||
...full.slice(1),
|
||||
entry(20, docs[0], { firstId: 20, source: "restore" }),
|
||||
]
|
||||
const { versions, onCanvasId } = buildVersions(
|
||||
list(after, 20),
|
||||
numbers,
|
||||
changes,
|
||||
)
|
||||
const v1 = versions.find((v) => v.id === "20")
|
||||
expect(v1?.number).toBe(1)
|
||||
expect(v1?.fromScratch).toBe(true)
|
||||
expect(v1?.summary.shapesAdded).toBe(1)
|
||||
expect(v1?.beforeXml).toBe("")
|
||||
expect(versions.map((v) => v.number)).not.toContain(21)
|
||||
expect(onCanvasId).toBe("20")
|
||||
})
|
||||
|
||||
it("lets the newest version on the canvas wait for its picture, older ones show none", () => {
|
||||
const { versions } = buildVersions(
|
||||
list([entry(0, A), entry(1, AB)], 1),
|
||||
@@ -229,6 +359,76 @@ describe("useNodeVersions", () => {
|
||||
expect(result.current.canRedo).toBe(false)
|
||||
})
|
||||
|
||||
it("numbers the History of another server state anew", async () => {
|
||||
// The process restarted: entry ids start over and name other content
|
||||
const server = fakeSync(list([entry(0, A), entry(1, AB)], 1))
|
||||
const { result } = renderHook(() => useNodeVersions(server.sync), {
|
||||
wrapper,
|
||||
})
|
||||
await flush()
|
||||
expect(result.current.versions.map((v) => v.number)).toEqual([1, 2])
|
||||
const AB2 = doc(cell("a", "A") + cell("b", "B"), "p2")
|
||||
const ABC2 = doc(cell("a", "A") + cell("b", "B") + cell("c", "C"), "p2")
|
||||
await act(async () =>
|
||||
server.serve({
|
||||
entries: [entry(0, AB2), entry(1, ABC2)],
|
||||
stateId: "S2",
|
||||
currentId: 1,
|
||||
}),
|
||||
)
|
||||
expect(result.current.versions.map((v) => v.number)).toEqual([1, 2])
|
||||
const [first, second] = result.current.versions
|
||||
expect(first.fromScratch).toBe(true)
|
||||
expect(first.summary.shapesAdded).toBe(2)
|
||||
expect(second.pageId).toBe("p2")
|
||||
})
|
||||
|
||||
it("undoes to the version the newest one replaced, after a restore in between", async () => {
|
||||
// History: A, AB, ABC, A again (restored), AD; undo from AD goes to
|
||||
// A (its newest copy), as restore_version does, not to ABC
|
||||
const AD = doc(cell("a", "A") + cell("d", "D"))
|
||||
const server = fakeSync(
|
||||
list(
|
||||
[
|
||||
entry(0, A),
|
||||
entry(1, AB),
|
||||
entry(2, ABC),
|
||||
entry(3, A, { firstId: 0, source: "restore" }),
|
||||
entry(4, AD),
|
||||
],
|
||||
4,
|
||||
),
|
||||
)
|
||||
const { result } = renderHook(() => useNodeVersions(server.sync), {
|
||||
wrapper,
|
||||
})
|
||||
await flush()
|
||||
expect(result.current.canUndo).toBe(true)
|
||||
await act(async () => {
|
||||
result.current.undo()
|
||||
})
|
||||
expect(server.restores).toEqual([{ id: 3, stateId: "S1" }])
|
||||
// Back at A: AD is the undone one
|
||||
await act(async () =>
|
||||
server.serve(
|
||||
list(
|
||||
[
|
||||
entry(0, A),
|
||||
entry(1, AB),
|
||||
entry(2, ABC),
|
||||
entry(3, A, { firstId: 0, source: "restore" }),
|
||||
entry(4, AD),
|
||||
entry(5, A, { firstId: 0, source: "restore" }),
|
||||
],
|
||||
5,
|
||||
),
|
||||
),
|
||||
)
|
||||
expect(result.current.onCanvasId).toBe("0")
|
||||
expect(result.current.undoneId).toBe("4")
|
||||
expect(result.current.canRedo).toBe(true)
|
||||
})
|
||||
|
||||
it("has no versions without a sync", () => {
|
||||
const { result } = renderHook(() => useNodeVersions(null), { wrapper })
|
||||
expect(result.current.versions).toEqual([])
|
||||
|
||||
@@ -1079,6 +1079,18 @@ describe("MCP sync History", () => {
|
||||
expect(t.notices).toEqual(["restoreFailed"])
|
||||
})
|
||||
|
||||
it("drops a History list of a state the poll has not seen yet", async () => {
|
||||
// Another process answers on the port; its History has other ids.
|
||||
// Its state is seen at the next poll, which tells the cards again
|
||||
const t = await inStep()
|
||||
const history = t.sync.fetchHistory()
|
||||
t.next("GET", "/api/history").answer({
|
||||
...historyList,
|
||||
body: { ...historyList.body, stateId: "S2" },
|
||||
})
|
||||
expect(await history).toBeNull()
|
||||
})
|
||||
|
||||
it("passes on the entry the server says is on the canvas", async () => {
|
||||
const t = await inStep()
|
||||
const history = t.sync.fetchHistory()
|
||||
@@ -1092,18 +1104,24 @@ describe("MCP sync History", () => {
|
||||
|
||||
// The version cards read History again when the server changed
|
||||
describe("MCP sync server change listener", () => {
|
||||
it("tells once per server version: the first poll, a write, the tab's own edit", async () => {
|
||||
it("tells once per History key: the first poll, a write, the tab's first edit, not its next", async () => {
|
||||
// The server's key names History (count, newest id) and the entry
|
||||
// on the canvas; the version cards read History only when it changes
|
||||
const t = open()
|
||||
let changes = 0
|
||||
const stop = t.sync.onServerChange(() => changes++)
|
||||
t.sync.setReady(true)
|
||||
let poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 2, "<mxfile>A</mxfile>"))
|
||||
t.next("GET").answer(
|
||||
state("S1", 2, "<mxfile>A</mxfile>", { historyKey: "1:0:0" }),
|
||||
)
|
||||
await poll
|
||||
expect(changes).toBe(1)
|
||||
// The same version again: nothing new
|
||||
// The same key again: nothing new
|
||||
poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 2, "<mxfile>A</mxfile>"))
|
||||
t.next("GET").answer(
|
||||
state("S1", 2, "<mxfile>A</mxfile>", { historyKey: "1:0:0" }),
|
||||
)
|
||||
await poll
|
||||
expect(changes).toBe(1)
|
||||
// The thumbnail of the write is stored: its entry has a picture now
|
||||
@@ -1115,29 +1133,82 @@ describe("MCP sync server change listener", () => {
|
||||
expect(changes).toBe(2)
|
||||
// An AI write
|
||||
poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 3, "<mxfile>B</mxfile>"))
|
||||
t.next("GET").answer(
|
||||
state("S1", 3, "<mxfile>B</mxfile>", { historyKey: "2:1:1" }),
|
||||
)
|
||||
await poll
|
||||
expect(changes).toBe(3)
|
||||
// The user's edit is saved: the canvas is at no version now
|
||||
await edit(t, "<mxfile>C</mxfile>")
|
||||
t.next("POST", "/api/state").answer({
|
||||
status: 200,
|
||||
body: { success: true, version: 4 },
|
||||
body: { success: true, version: 4, historyKey: "2:1:null" },
|
||||
})
|
||||
await t.settle()
|
||||
expect(changes).toBe(4)
|
||||
// The next poll sees the version the push already told about
|
||||
// The next poll sees the key the push already told about
|
||||
poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 4, "<mxfile>C</mxfile>"))
|
||||
t.next("GET").answer(
|
||||
state("S1", 4, "<mxfile>C</mxfile>", { historyKey: "2:1:null" }),
|
||||
)
|
||||
await poll
|
||||
expect(changes).toBe(4)
|
||||
// A further edit changes History not at all: no download of it
|
||||
await edit(t, "<mxfile>C2</mxfile>")
|
||||
t.next("POST", "/api/state").answer({
|
||||
status: 200,
|
||||
body: { success: true, version: 5, historyKey: "2:1:null" },
|
||||
})
|
||||
await t.settle()
|
||||
expect(changes).toBe(4)
|
||||
stop()
|
||||
poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 5, "<mxfile>D</mxfile>"))
|
||||
t.next("GET").answer(
|
||||
state("S1", 6, "<mxfile>D</mxfile>", { historyKey: "3:2:2" }),
|
||||
)
|
||||
await poll
|
||||
expect(changes).toBe(4)
|
||||
})
|
||||
|
||||
it("tells once per server version when the server sends no key", async () => {
|
||||
const t = open()
|
||||
let changes = 0
|
||||
t.sync.onServerChange(() => changes++)
|
||||
let poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 2, "<mxfile>A</mxfile>"))
|
||||
await poll
|
||||
poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 2, "<mxfile>A</mxfile>"))
|
||||
await poll
|
||||
expect(changes).toBe(1)
|
||||
poll = t.sync.poll()
|
||||
t.next("GET").answer(state("S1", 3, "<mxfile>B</mxfile>"))
|
||||
await poll
|
||||
expect(changes).toBe(2)
|
||||
})
|
||||
|
||||
it("tells again at the next poll when a History read failed", async () => {
|
||||
// The cards would otherwise keep the old list until the next write
|
||||
const t = await inStep()
|
||||
let changes = 0
|
||||
t.sync.onServerChange(() => changes++)
|
||||
const history = t.sync.fetchHistory()
|
||||
t.next("GET", "/api/history").answer({ status: 500, body: {} })
|
||||
expect(await history).toBeNull()
|
||||
const poll = t.sync.poll()
|
||||
t.next("GET", "/api/state").answer(state("S1", 2, "<mxfile>A</mxfile>"))
|
||||
await poll
|
||||
expect(changes).toBe(1)
|
||||
// Also when the request itself fails
|
||||
const again = t.sync.fetchHistory()
|
||||
t.next("GET", "/api/history").fail()
|
||||
expect(await again).toBeNull()
|
||||
const next = t.sync.poll()
|
||||
t.next("GET", "/api/state").answer(state("S1", 2, "<mxfile>A</mxfile>"))
|
||||
await next
|
||||
expect(changes).toBe(2)
|
||||
})
|
||||
|
||||
it("tells when a rejected edit was kept in History, and when the state was recreated", async () => {
|
||||
const t = await inStep()
|
||||
let changes = 0
|
||||
|
||||
Reference in New Issue
Block a user