fix: avoid duplicate text when a page is edited externally in sync mode

This commit is contained in:
Zef Hemel
2026-08-09 16:07:14 +02:00
parent 754e8525b9
commit e34d6edb3c
7 changed files with 328 additions and 47 deletions
+5 -27
View File
@@ -403,43 +403,21 @@ export class Client {
this.space = space;
let lastSaveTimestamp: number | undefined;
const updateLastSaveTimestamp = () => {
lastSaveTimestamp = Date.now();
};
this.eventHook.addLocalListener(
"editor:pageSaving",
updateLastSaveTimestamp,
);
this.eventHook.addLocalListener(
"editor:documentSaving",
updateLastSaveTimestamp,
);
this.eventHook.addLocalListener(
"file:changed",
(path: string, oldHash: number, newHash: number) => {
(path: string, oldHash: number, _newHash: number, ownWrite: boolean) => {
if (
!this.space.watchInterval ||
this.currentPath() !== path ||
oldHash === undefined
oldHash === undefined ||
// our own save landing, not somebody else's edit
ownWrite
) {
return;
}
if (isMarkdownPath(path)) {
// Live path: precise echo suppression by our own last write's hash;
// content-level no-op inside applyExternalPatches covers the rest
if (newHash === this.contentManager.lastSavedHash) {
return;
}
this.contentManager.reloadPageContent().catch(console.error);
} else if (
!lastSaveTimestamp ||
lastSaveTimestamp < Date.now() - 5000
) {
} else {
// Documents (non-markdown) keep the reload path
this.ui.flashNotification("Document changed elsewhere, reloading");
void this.reloadEditor();
+117
View File
@@ -66,6 +66,7 @@ type ReadPageResult = { text: string; meta: PageMeta };
function makeClientStub(opts: {
initialDoc: string;
readPage: () => Promise<ReadPageResult>;
writePage?: (name: string, text: string) => Promise<PageMeta>;
hasFullIndexCompleted?: () => Promise<boolean>;
getObjectByRef?: () => Promise<PageMeta | undefined>;
}) {
@@ -89,6 +90,7 @@ function makeClientStub(opts: {
},
ui: {
viewState,
flashNotification: () => {},
viewDispatch: (action: {
type: string;
path?: string;
@@ -105,6 +107,8 @@ function makeClientStub(opts: {
},
space: {
readPage: opts.readPage,
writePage:
opts.writePage ?? (async () => pageMeta("2026-01-01T00:00:00.000")),
unwatchFile: () => {},
watchFile: () => {},
},
@@ -404,3 +408,116 @@ describe("ContentManager.reloadPageContent editor:pageReloaded notification", ()
).toBe("journal-page");
});
});
// The base only moves forward when a write's *response* arrives. A read that
// resolves before it sees content newer than the base it gets diffed against,
// and the merge re-inserts the difference into a document that already has it.
describe("ContentManager merge base vs. in-flight writes (regression: text duplicates while typing)", () => {
// Lets the debounced save() timeout fire and promise chains settle.
const flush = () => new Promise((resolve) => setTimeout(resolve, 0));
function typeInto(
client: ReturnType<typeof makeClientStub>,
appended: string,
) {
client.editorView.dispatch({
changes: { from: client.editorView.state.doc.length, insert: appended },
});
client.viewState.unsavedChanges = true;
}
test("a fetch that returns content from a save still in flight does not duplicate it", async () => {
let resolveRead!: (r: ReadPageResult) => void;
let resolveWrite!: (meta: PageMeta) => void;
let readCall = 0;
const client = makeClientStub({
initialDoc: "",
readPage: () => {
readCall++;
if (readCall === 1) {
return Promise.resolve({
text: "one\n",
meta: pageMeta("2026-01-01T00:00:00.000"),
});
}
return new Promise<ReadPageResult>((resolve) => {
resolveRead = resolve;
});
},
writePage: () =>
new Promise<PageMeta>((resolve) => {
resolveWrite = resolve;
}),
});
client.currentPathValue = "index.md";
const cm = new ContentManager(client as unknown as Client);
await cm.loadPage({ path: "index.md" }, false);
typeInto(client, "two");
// A foreign change is reported, so a fetch starts. No write in flight yet.
const reload = cm.reloadPageContent();
// Only now does the autosave fire, and the user keeps typing after it.
void cm.save(true);
await flush();
typeInto(client, "three");
// The fetch returns what that save is putting on disk...
resolveRead({
text: "one\ntwo",
meta: pageMeta("2026-01-01T00:00:05.000"),
});
await flush();
// ...and only then does the write's response carry the base forward.
resolveWrite(pageMeta("2026-01-01T00:00:05.000"));
await reload;
expect(client.editorView.state.sliceDoc()).toBe("one\ntwothree");
});
test("two saves acknowledged out of order leave the base on the newer one", async () => {
// save() does not wait for the previous write, and nothing orders the two
// responses.
const resolvers: ((meta: PageMeta) => void)[] = [];
let diskText = "one\n";
let diskModified = "2026-01-01T00:00:00.000";
const client = makeClientStub({
initialDoc: "",
readPage: async () => ({ text: diskText, meta: pageMeta(diskModified) }),
writePage: () =>
new Promise<PageMeta>((resolve) => {
resolvers.push(resolve);
}),
});
client.currentPathValue = "index.md";
const cm = new ContentManager(client as unknown as Client);
await cm.loadPage({ path: "index.md" }, false);
typeInto(client, "two");
void cm.save(true);
await flush();
typeInto(client, "three");
void cm.save(true);
await flush();
expect(resolvers).toHaveLength(2);
// The later write is acknowledged first; the earlier one lands after.
resolvers[1](pageMeta("2026-01-01T00:00:09.000"));
await flush();
resolvers[0](pageMeta("2026-01-01T00:00:05.000"));
await flush();
// Keeps the document ahead of disk, so the merge below is a real one
// rather than a same-text no-op.
typeInto(client, "four");
// With the base dragged back to the earlier write, this re-inserts "three".
diskText = "one\ntwothree";
diskModified = "2026-01-01T00:00:09.000";
await cm.reloadPageContent();
expect(client.editorView.state.sliceDoc()).toBe("one\ntwothreefour");
});
});
+48 -11
View File
@@ -50,8 +50,8 @@ export class ContentManager {
// fetched content is older than what's already applied (out-of-order
// resolution of overlapping reloads for the same page)
private lastKnownDiskModified?: number;
// lastModified of our own most recent write, for echo suppression
lastSavedHash?: number;
// Resolves once the in-flight write has updated the base above
private pendingWrite?: Promise<void>;
debouncedUpdateEvent = throttle(() => {
this.client.eventHook
.dispatchEvent("editor:updated")
@@ -107,14 +107,28 @@ export class ContentManager {
this.client.currentName(),
);
const text = this.client.editorView.state.sliceDoc(0);
this.client.space
.writePage(this.client.currentName(), text)
const writePromise = this.client.space.writePage(
this.client.currentName(),
text,
);
// Separate from the promise save() returns: waiters need the base
// updated, not the events and meta fetch that follow it.
const baseUpdated = writePromise.then(
(meta) =>
this.adoptOwnWriteAsBase(
text,
parsePageMetaLastModified(meta.lastModified),
),
() => {}, // errors are reported by the .catch() below
);
this.pendingWrite = baseUpdated;
void baseUpdated.then(() => {
if (this.pendingWrite === baseUpdated) {
this.pendingWrite = undefined;
}
});
writePromise
.then(async (meta) => {
this.lastKnownDiskText = text;
this.lastSavedHash = parsePageMetaLastModified(
meta.lastModified,
);
this.lastKnownDiskModified = this.lastSavedHash;
this.client.ui.viewDispatch({ type: "page-saved" });
await this.client.dispatchAppEvent(
"editor:pageSaved",
@@ -161,6 +175,23 @@ export class ContentManager {
});
}
/**
* Records the text we just wrote as the new base, unless a newer state was
* adopted while this write was in flight: dragging the base backwards would
* make the next merge apply that newer change a second time.
*/
private adoptOwnWriteAsBase(text: string, modified: number | undefined) {
if (
modified !== undefined &&
this.lastKnownDiskModified !== undefined &&
modified < this.lastKnownDiskModified
) {
return;
}
this.lastKnownDiskText = text;
this.lastKnownDiskModified = modified;
}
async reloadEditor() {
if (!this.client.systemReady) return;
@@ -351,8 +382,6 @@ export class ContentManager {
}
}
this.lastSavedHash = parsePageMetaLastModified(doc.meta.lastModified);
// This could create an invalid editor state, but that doesn't matter, we'll update it later
this.switchToPageEditor();
@@ -569,6 +598,14 @@ export class ContentManager {
return this.reloadEditor();
}
const doc = await this.client.space.readPage(getNameFromPath(path));
// A write that finished during the read left the base behind what the read
// returned, so merging now would re-insert our own text. After the read,
// not before: it's writes overlapping the *fetch* that strand the base.
// Not covered by ownWrite -- a write overlapping another space operation
// dispatches no event of its own, and surfaces later via a probe instead.
if (this.pendingWrite) {
await this.pendingWrite;
}
// The user may have navigated to a different page while this fetch was
// in flight. Applying now would diff/merge page-A content against
// whatever page is currently open -- bail out rather than corrupt it.
@@ -0,0 +1,84 @@
import { describe, expect, test } from "vitest";
import type { FileMeta } from "@silverbulletmd/silverbullet/type/index";
import { EventedSpacePrimitives } from "./evented_space_primitives.ts";
import type { SpacePrimitives } from "./space_primitives.ts";
import type { EventHook } from "../plugos/hooks/event.ts";
import type { DataStore } from "../data/datastore.ts";
function meta(name: string, lastModified: number): FileMeta {
return {
name,
lastModified,
created: lastModified,
contentType: "text/markdown",
size: 0,
perm: "rw",
};
}
function setup(lastModified = 2000) {
const events: { name: string; args: unknown[] }[] = [];
const wrapped = {
readFile: async (path: string) => ({
data: new Uint8Array(),
meta: meta(path, lastModified),
}),
writeFile: async (path: string) => meta(path, lastModified),
getFileMeta: async (path: string) => meta(path, lastModified),
} as unknown as SpacePrimitives;
const store = new Map<string, unknown>();
const ds = {
get: async (key: string[]) => store.get(key.join("/")),
set: async (key: string[], value: unknown) => {
store.set(key.join("/"), value);
},
} as unknown as DataStore;
const eventHook = {
dispatchEvent: async (name: string, ...args: unknown[]) => {
events.push({ name, args });
return [];
},
} as unknown as EventHook;
const sp = new EventedSpacePrimitives(wrapped, eventHook, ds);
return { sp, events };
}
const fileChanged = (events: { name: string; args: unknown[] }[]) =>
events.filter((e) => e.name === "file:changed");
// file:changed fires from inside writeFile, before it returns, so ownWrite is
// a listener's only way to tell its own write from somebody else's.
describe("EventedSpacePrimitives file:changed ownWrite flag", () => {
test("a write of our own is flagged as such", async () => {
const { sp, events } = setup();
await sp.enable();
await sp.writeFile("index.md", new Uint8Array());
expect(fileChanged(events).map((e) => e.args)).toEqual([
["index.md", undefined, 2000, true],
]);
});
test("a change merely observed by a metadata probe is not", async () => {
const { sp, events } = setup();
await sp.enable();
await sp.getFileMeta("index.md");
expect(fileChanged(events).map((e) => e.args)).toEqual([
["index.md", undefined, 2000, false],
]);
});
test("a change merely observed by a read is not", async () => {
const { sp, events } = setup();
await sp.enable();
await sp.readFile("index.md");
expect(fileChanged(events).map((e) => e.args)).toEqual([
["index.md", undefined, 2000, false],
]);
});
});
+13 -4
View File
@@ -7,7 +7,9 @@ import { sleep } from "@silverbulletmd/silverbullet/lib/async";
/**
* Events exposed:
* - file:changed (string, oldHash, newHash)
* - file:changed (string, oldHash, newHash, ownWrite): dispatched from inside
* writeFile, before it returns, so ownWrite is a listener's only way to tell
* our own write from someone else's.
* - file:deleted (string)
* - file:listed (FileMeta[])
* - file:initial: triggered in case of an initially empty snapshot, after the first set of events has gone out
@@ -199,7 +201,7 @@ export class EventedSpacePrimitives implements SpacePrimitives {
try {
const newMeta = await this.wrapped.writeFile(path, data, meta);
if (this.operationCount === 1) {
await this.triggerEventsAndCache(path, newMeta.lastModified);
await this.triggerEventsAndCache(path, newMeta.lastModified, true);
}
if (path.endsWith(".md")) {
const pageName = path.substring(0, path.length - 3);
@@ -216,14 +218,21 @@ export class EventedSpacePrimitives implements SpacePrimitives {
/**
* @param name
* @param newHash
* @param ownWrite
* @return whether something changed in the snapshot
*/
async triggerEventsAndCache(name: string, newHash: number) {
async triggerEventsAndCache(name: string, newHash: number, ownWrite = false) {
const oldHash = this.spaceSnapshot[name];
// if (oldHash && newHash && oldHash !== newHash) {
if (oldHash !== newHash) {
// Page changed since last cached metadata, trigger event
await this.dispatchEvent("file:changed", name, oldHash, newHash);
await this.dispatchEvent(
"file:changed",
name,
oldHash,
newHash,
ownWrite,
);
}
this.updateInSnapshot(name, newHash);
await this.saveSnapshot();
+3 -5
View File
@@ -3,7 +3,7 @@ An attempt at documenting the changes/new features introduced in each release.
## Edge
Whenever a commit is pushed to the `main` branch, within ~5 minutes, it will be released as a docker image with the `:v2` tag, and a binary in the [edge release](https://github.com/silverbulletmd/silverbullet/releases/tag/edge). If you want to live on the bleeding edge of SilverBullet goodness (or regression) this is where to do it.
* **Live external edits**: changes made to pages by other programs (or users, but don’t rely on this for real-time collaboration) now show up in an open pages almost instantly, instead of waiting for the next sync. The client applies them to the open page as minimal, cursor-preserving edits that land in the undo history, so `Cmd/Ctrl-z` reverts an external edit like any other. The externally inserted text is briefly highlighted with a marker showing where it landed, fading after a few seconds.
* **Live external edits**: changes made to pages by other programs (or users, but don’t rely on this for real-time collaboration) now show up in an open pages almost instantly, instead of waiting for the next sync. The client applies them to the open page as minimal, cursor-preserving edits land in the undo history, so `Cmd/Ctrl-z` reverts an external edit like any other.
* This relies on active file-system, watching which can be configured server-wide with the new `SB_FS_WATCH` environment variable (`auto` (default) / `poll` / `off`): set `poll` when the space lives on a network mount (NFS/SMB) where changes made may not result in FS events.
* **Inline comments** ([[Comment]]): any HTML comment (`<!-- remember this -->`) is now a parsed and rendered as a note.
* [[Space Manager|Multi-space]] mode: the [[Runtime API]] (`runtimeApi`) is now **on by default** for new and existing spaces, instead of off. It only actually runs when the server found a Chrome or Chromium install at startup and only booted upon first use of the API.
@@ -23,8 +23,7 @@ Whenever a commit is pushed to the `main` branch, within ~5 minutes, it will be
## 2.10.0
* [[Space Manager]]: multi-space hosting with multiple accounts is here. A fresh install pointed at an empty folder opens a browser-based first-run **setup wizard** that creates an admin account and your first space, then serves it in place with no restart. One server can host any number of [[Space|spaces]], each bound to a URL prefix or hostname.
* [[Baked Sections]]: bake `${...}` Lua expressions and widgets into
HTML-comment-delimited markdown (`<!--#lua EXPR -->` … `<!--/lua-->`).
* [[Baked Sections]]: bake `${...}` Lua expressions and widgets into HTML-comment-delimited markdown (`<!--#lua EXPR -->` … `<!--/lua-->`).
* Space Lua: **code complete now shows documentation** (where available), all available via [[API/spacelua]] reflection APIs.
* Backend and CLI have been ported to Rust ([see background on this](https://no.silverbullet.plus/tech-stacks)), both should be behavior preserving (that is: you shouldn’t really notice):
* The server backend (previously written in Go) has now been replaced by an adapted version of [SilverBullet+](https://silverbullet.plus/)’s backend written in Rust, more unifying those code bases.
@@ -290,8 +289,7 @@ Whenever a commit is pushed to the `main` branch, within ~5 minutes, it will be
* `item` and `task` now also index (wiki) links and inherited (wiki) links (links appearing in parent nodes), as [requested here](https://community.silverbullet.md/t/coming-from-logseq-outlines-and-linked-mentions/290) under `links` and `ilinks`. Updated the "Linked Tasks" widget now to rely on `ilinks`.
* Rewrote snippet text for links (used in [[Linked Mention|Linked Mentions]]) to be more contextual, now also includes child bullet items, see [community discussion](https://community.silverbullet.md/t/coming-from-logseq-outlines-and-linked-mentions/290).
* For consistency with items, `task` `refs` now point to the item’s position resulting in a slight positional shift, if you have code relying on this, you may have to adjust it.
* Disabled indexing all paragraph text by default, this caused significant indexing overhead. [See discussion](https://community.silverbullet.md/t/who-is-using-paragraph-for-queries/3686).
To re-enable: `config.set("index.paragraph.all", true)`
* Disabled indexing all paragraph text by default, this caused significant indexing overhead. [See discussion](https://community.silverbullet.md/t/who-is-using-paragraph-for-queries/3686). To re-enable: `config.set("index.paragraph.all", true)`
* Better link support in frontmatter (by [Tomasz Gorochowik](https://github.com/silverbulletmd/silverbullet/pull/1711))
* The `page:index` event now also receives a `text` and `meta` attributes.
* [[Transclusions]] improvements:
+58
View File
@@ -8,6 +8,7 @@ import {
redoChord,
spawnServerProcess,
test,
waitForSaveAndReadFromServer,
waitForServer,
} from "./fixtures.ts";
@@ -160,6 +161,63 @@ test("external edit merges with unsaved local typing", async ({
.toContain("LOCAL");
});
test("the client's own save is not treated as an external change", async ({
page,
sbServer,
}) => {
// Not what this test is about: an SSE notification makes the client probe
// file metadata, and a probe overlapping our own write suppresses that
// write's event entirely (operationCount guard in EventedSpacePrimitives),
// making the count below timing-dependent.
await page.route("**/.events", (route) => route.fulfill({ status: 404 }));
await gotoSilverBulletPage(page, sbServer);
const editor = page.locator(".cm-content");
await expect(editor).toContainText("Hello world");
await editor.click();
await page.evaluate(() => {
const client = (globalThis as any).client;
(globalThis as any).__reloads = 0;
(globalThis as any).__ownWriteEvents = 0;
client.eventHook.addLocalListener(
"file:changed",
(name: string, _o: number, _n: number, ownWrite: boolean) => {
if (name === "index.md" && ownWrite) {
(globalThis as any).__ownWriteEvents++;
}
},
);
const cm = client.contentManager;
const original = cm.reloadPageContent.bind(cm);
cm.reloadPageContent = (...args: unknown[]) => {
(globalThis as any).__reloads++;
return original(...args);
};
client.editorView.dispatch({
selection: { anchor: client.editorView.state.doc.length },
});
});
await page.keyboard.type("typed by me");
const saved = await waitForSaveAndReadFromServer(page, sbServer, "index.md");
expect(saved).toBe("Hello world\ntyped by me");
await page.waitForTimeout(1500);
// The save was announced, and the announcement said we caused it...
expect(
await page.evaluate(() => (globalThis as any).__ownWriteEvents),
).toBeGreaterThan(0);
// ...so it never reached the path that fetches the page back and merges it.
expect(await page.evaluate(() => (globalThis as any).__reloads)).toBe(0);
await expect(page.locator(".sb-external-edit")).toHaveCount(0);
expect(
await page.evaluate(() =>
(globalThis as any).client.editorView.state.sliceDoc(),
),
).toBe("Hello world\ntyped by me");
});
test("undo isolates the external edit from the user's own prior typing", async ({
sbPage,
sbServer,