fix(git-sync): don't clobber pages with a live editing session; crash-safe body write
Review finding #5: the git -> page body write (writeBody) did a full-body replace
(delete-all + re-insert) on the shared Yjs doc. Applied while a human is editing
the page, it discarded their in-flight changes; and TiptapTransformer.toYdoc ran
AFTER the fragment was cleared, so a conversion failure could leave the page with
an empty body.
Fixes:
- Active-session guard: CollaborationGateway.getActiveEditorCount(documentName)
reports live human (websocket) editor sessions for a doc, excluding server-side
direct connections. writeBody now throws ActiveEditSessionError when an editor
is connected. The engine's push loop already isolates each importPageMarkdown in
try/catch and does not advance the loop-guard on failure, so the write is simply
retried on the next poll once the editor disconnects — never a clobber.
- Crash-safe conversion: build the replacement Yjs update BEFORE opening the
connection / clearing the fragment, so a transform failure can never leave the
body empty.
Also updates the server-side converter gate spec to the corrected round-trip
shape: the block-image hoist no longer leaves a leading empty paragraph (the
git-sync converter fix in 7d39c16b, now reaching the built package).
A true merge of git content into a live Yjs session is out of scope (it needs a
real 3-way text merge with no shared update lineage); deferring the write while a
page is being edited is the safe, owner-approved minimum.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -42,6 +42,23 @@ const GIT_SYNC_PROVENANCE: AuthProvenanceData = {
|
||||
aiChatId: null,
|
||||
};
|
||||
|
||||
/**
|
||||
* Thrown when a git -> page body write is skipped because a human is editing the
|
||||
* page RIGHT NOW (a live collab session). The engine's push loop catches this
|
||||
* per page, records it as a (non-fatal) failure, and does NOT advance the
|
||||
* loop-guard for that page — so the write is retried on the next poll once the
|
||||
* editor disconnects, instead of clobbering their in-flight edits with a
|
||||
* full-body replace (plan §15.6 / review #5).
|
||||
*/
|
||||
export class ActiveEditSessionError extends Error {
|
||||
constructor(pageId: string) {
|
||||
super(
|
||||
`git-sync: page ${pageId} has an active edit session; deferring body write`,
|
||||
);
|
||||
this.name = 'ActiveEditSessionError';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Native, in-process implementation of the engine's `GitSyncClient` seam
|
||||
* (plan §3). Reads go through repositories (PageRepo/SpaceRepo); body writes go
|
||||
@@ -379,7 +396,32 @@ export class GitmostDataSourceService {
|
||||
prosemirrorJson: unknown,
|
||||
userId: string,
|
||||
): Promise<void> {
|
||||
const conn = await this.collabGateway.openDirectConnection(`page.${pageId}`, {
|
||||
const documentName = `page.${pageId}`;
|
||||
|
||||
// Do NOT clobber a page someone is editing right now. The write below is a
|
||||
// full-body replace (delete-all + re-insert); applied over a live editing
|
||||
// session it would discard the user's in-flight changes. If a human editor
|
||||
// is connected, defer: throw so the engine retries on the next poll once
|
||||
// they disconnect (review #5 — "не писать в страницу с активной сессией").
|
||||
if (this.collabGateway.getActiveEditorCount(documentName) > 0) {
|
||||
this.logger.debug(
|
||||
`Skipping git-sync body write for ${documentName}: active edit session`,
|
||||
);
|
||||
throw new ActiveEditSessionError(pageId);
|
||||
}
|
||||
|
||||
// Build the replacement Yjs state BEFORE touching the live doc. If the
|
||||
// transform throws (a malformed/unsupported doc), we must NOT have already
|
||||
// cleared the fragment — otherwise a conversion failure would leave the page
|
||||
// with an empty body (review #5 — crash-safe conversion).
|
||||
const next = TiptapTransformer.toYdoc(
|
||||
prosemirrorJson,
|
||||
'default',
|
||||
tiptapExtensions,
|
||||
);
|
||||
const update = Y.encodeStateAsUpdate(next);
|
||||
|
||||
const conn = await this.collabGateway.openDirectConnection(documentName, {
|
||||
actor: 'git-sync',
|
||||
// PersistenceExtension reads `context.user.id` for lastUpdatedById, so the
|
||||
// service user is required on the context (unlike the bare `{ actor }`
|
||||
@@ -390,12 +432,7 @@ export class GitmostDataSourceService {
|
||||
await conn.transact((doc) => {
|
||||
const fragment = doc.getXmlFragment('default');
|
||||
if (fragment.length > 0) fragment.delete(0, fragment.length);
|
||||
const next = TiptapTransformer.toYdoc(
|
||||
prosemirrorJson,
|
||||
'default',
|
||||
tiptapExtensions,
|
||||
);
|
||||
Y.applyUpdate(doc, Y.encodeStateAsUpdate(next));
|
||||
Y.applyUpdate(doc, update);
|
||||
});
|
||||
} finally {
|
||||
await conn.disconnect();
|
||||
|
||||
Reference in New Issue
Block a user