fix(git-sync): unwedge per-page conflicts, preserve callout types, flush collab on disconnect
Addresses QA findings on PR #119 (issues #235/#236). SYNC-WEDGE (HIGH): one same-line conflict on one page froze sync for the WHOLE space in both directions forever. The pull's docmost->main merge left the vault mid-merge, so every later cycle's isMergeInProgress() check returned skipped:"merge-in-progress" and skipped the entire space with no recovery. - pull.ts now COMMITS a conflicting merge with markers in place (commitMerge): cleanly-merged pages land, the conflicted page carries its markers on main and is isolated by the existing push-side conflict-marker skip (markers never reach Docmost), and the next cycle is no longer wedged. conflictedPaths is surfaced. - cycle.ts now RECOVERS a vault left mid-merge by a prior/pre-fix cycle: it aborts the stale merge (merge --abort, hard-reset fallback) and continues, instead of skipping the space forever. - git.ts: listUnmergedPaths / commitMerge / abortMerge / resetHardToHead. CALLOUT TYPE FIDELITY: git-sync's CALLOUT_TYPES was missing "note" and "default" (editor-canonical types), so [!note]/[!default] callouts flattened to [!info] on every round-trip. Aligned the list with @docmost/editor-ext getValidCalloutType. LOSS-ON-FAST-CLOSE: editing a page then closing the tab inside the collab debounce window (~3-18s) lost the edit, because with unloadImmediately:false Hocuspocus does not flush the debounced onStoreDocument on the last-client disconnect. PersistenceExtension.onDisconnect now flushes the pending store (debouncer.executeNow) on the last disconnect only, with no redundant write. DUPLICATION re-verify (#1): the schema-default merge-key normalization is intact; faithful toYdoc-based reproduction shows callout + rich content resync with 0 ops and no growth/strip across cycles -> the re-report was leftover vault data, not a live regression. Locked with a callout regression spec. Tests: git-sync 688 pass (incl. real-VaultGit wedge-recovery integration); server git-sync+collaboration 285 pass; new callout merge/fidelity + onDisconnect-flush specs. tsc --noEmit clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,89 @@
|
|||||||
|
import { PersistenceExtension } from './persistence.extension';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Regression for the QA #119 "loss-on-fast-close" data loss: editing a page then
|
||||||
|
* closing the tab within the collab debounce window (~3-18s) lost the edit
|
||||||
|
* because, with `unloadImmediately: false`, Hocuspocus does NOT flush the
|
||||||
|
* debounced onStoreDocument on a last-client disconnect. PersistenceExtension
|
||||||
|
* now flushes the pending store on the LAST disconnect (and only then).
|
||||||
|
*/
|
||||||
|
describe('PersistenceExtension.onDisconnect flush (loss-on-fast-close)', () => {
|
||||||
|
function makeExt(): PersistenceExtension {
|
||||||
|
// onDisconnect touches none of the injected deps; pass casts.
|
||||||
|
return new PersistenceExtension(
|
||||||
|
null as any,
|
||||||
|
null as any,
|
||||||
|
null as any,
|
||||||
|
null as any,
|
||||||
|
null as any,
|
||||||
|
null as any,
|
||||||
|
null as any,
|
||||||
|
null as any,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeData(opts: {
|
||||||
|
clientsCount: number;
|
||||||
|
isDebounced: boolean;
|
||||||
|
isLoading?: boolean;
|
||||||
|
}) {
|
||||||
|
const executeNow = jest.fn(async () => undefined);
|
||||||
|
const isDebounced = jest.fn(() => opts.isDebounced);
|
||||||
|
return {
|
||||||
|
executeNow,
|
||||||
|
isDebounced,
|
||||||
|
payload: {
|
||||||
|
clientsCount: opts.clientsCount,
|
||||||
|
context: {},
|
||||||
|
document: { isLoading: opts.isLoading ?? false } as any,
|
||||||
|
documentName: 'page.abc',
|
||||||
|
instance: { debouncer: { isDebounced, executeNow } } as any,
|
||||||
|
requestHeaders: {},
|
||||||
|
requestParameters: new URLSearchParams(),
|
||||||
|
socketId: 's',
|
||||||
|
} as any,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
it('flushes the pending store when the LAST client disconnects', async () => {
|
||||||
|
const ext = makeExt();
|
||||||
|
const { executeNow, payload } = makeData({
|
||||||
|
clientsCount: 0,
|
||||||
|
isDebounced: true,
|
||||||
|
});
|
||||||
|
await ext.onDisconnect(payload);
|
||||||
|
expect(executeNow).toHaveBeenCalledTimes(1);
|
||||||
|
expect(executeNow).toHaveBeenCalledWith('onStoreDocument-page.abc');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT flush while other editors remain connected', async () => {
|
||||||
|
const ext = makeExt();
|
||||||
|
const { executeNow, payload } = makeData({
|
||||||
|
clientsCount: 2,
|
||||||
|
isDebounced: true,
|
||||||
|
});
|
||||||
|
await ext.onDisconnect(payload);
|
||||||
|
expect(executeNow).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT write when nothing is pending (already persisted)', async () => {
|
||||||
|
const ext = makeExt();
|
||||||
|
const { executeNow, payload } = makeData({
|
||||||
|
clientsCount: 0,
|
||||||
|
isDebounced: false,
|
||||||
|
});
|
||||||
|
await ext.onDisconnect(payload);
|
||||||
|
expect(executeNow).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT flush a doc that is still loading (load error guard)', async () => {
|
||||||
|
const ext = makeExt();
|
||||||
|
const { executeNow, payload } = makeData({
|
||||||
|
clientsCount: 0,
|
||||||
|
isDebounced: true,
|
||||||
|
isLoading: true,
|
||||||
|
});
|
||||||
|
await ext.onDisconnect(payload);
|
||||||
|
expect(executeNow).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -2,6 +2,7 @@ import {
|
|||||||
afterUnloadDocumentPayload,
|
afterUnloadDocumentPayload,
|
||||||
Extension,
|
Extension,
|
||||||
onChangePayload,
|
onChangePayload,
|
||||||
|
onDisconnectPayload,
|
||||||
onLoadDocumentPayload,
|
onLoadDocumentPayload,
|
||||||
onStoreDocumentPayload,
|
onStoreDocumentPayload,
|
||||||
} from '@hocuspocus/server';
|
} from '@hocuspocus/server';
|
||||||
@@ -164,6 +165,40 @@ export class PersistenceExtension implements Extension {
|
|||||||
return new Y.Doc();
|
return new Y.Doc();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* LOSS-ON-FAST-CLOSE FIX (QA #119). When the LAST editor disconnects, FLUSH any
|
||||||
|
* pending (debounced) store to the DB IMMEDIATELY instead of waiting out the
|
||||||
|
* up-to-10s `debounce` window.
|
||||||
|
*
|
||||||
|
* The collab server runs with `unloadImmediately: false` (collaboration.gateway),
|
||||||
|
* so on a last-client disconnect Hocuspocus does NOT flush the debounced
|
||||||
|
* onStoreDocument — it relies on the timer firing later. A quick edit-then-close
|
||||||
|
* (closing the tab within the debounce window, ~3-18s) therefore left the edit
|
||||||
|
* only in the soon-to-be-unloaded in-memory Y.Doc; meanwhile git-sync mirrored
|
||||||
|
* the STALE/empty DB body to the vault (the reported "59-byte frontmatter-only"
|
||||||
|
* data loss). Running the already-scheduled store now closes that window.
|
||||||
|
*
|
||||||
|
* Gated tightly so it never adds a redundant write: only on the LAST disconnect
|
||||||
|
* (`clientsCount === 0`), only for a fully-loaded doc, and only when a store is
|
||||||
|
* actually pending (`isDebounced`). `executeNow` runs the SAME payload Hocuspocus
|
||||||
|
* scheduled (preserving the edit's context/actor) and clears the timer.
|
||||||
|
*/
|
||||||
|
async onDisconnect(data: onDisconnectPayload) {
|
||||||
|
const { instance, document, documentName, clientsCount } = data;
|
||||||
|
if (clientsCount > 0) return;
|
||||||
|
if (!document || document.isLoading) return;
|
||||||
|
const debounceId = `onStoreDocument-${documentName}`;
|
||||||
|
if (!instance?.debouncer?.isDebounced(debounceId)) return;
|
||||||
|
try {
|
||||||
|
await instance.debouncer.executeNow(debounceId);
|
||||||
|
} catch (err) {
|
||||||
|
this.logger.error(
|
||||||
|
`onDisconnect flush failed for ${documentName}: ` +
|
||||||
|
(err instanceof Error ? err.message : String(err)),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async onStoreDocument(data: onStoreDocumentPayload) {
|
async onStoreDocument(data: onStoreDocumentPayload) {
|
||||||
const { documentName, document, context } = data;
|
const { documentName, document, context } = data;
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,161 @@
|
|||||||
|
import { TiptapTransformer } from '@hocuspocus/transformer';
|
||||||
|
import * as Y from 'yjs';
|
||||||
|
import {
|
||||||
|
markdownToProseMirror,
|
||||||
|
convertProseMirrorToMarkdown,
|
||||||
|
} from '@docmost/git-sync';
|
||||||
|
|
||||||
|
import { tiptapExtensions } from '../../../collaboration/collaboration.util';
|
||||||
|
import { mergeXmlFragments, mergeXmlFragments3Way } from './yjs-body-merge';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Regression for the QA #119 callout findings (body-duplication re-verify +
|
||||||
|
* "callout strips the whole body"). These reproduce the ACTUAL live merge path:
|
||||||
|
*
|
||||||
|
* live = TiptapTransformer.toYdoc(editor JSON, tiptapExtensions) (the
|
||||||
|
* collaboration server's materialization — schema defaults stamped)
|
||||||
|
* git = toYdoc(markdownToProseMirror(convertProseMirrorToMarkdown(editor)))
|
||||||
|
* (the engine round-trip the push side feeds into writePageBody)
|
||||||
|
*
|
||||||
|
* A page containing a callout (with a neighbouring heading + paragraphs) must:
|
||||||
|
* - merge with ZERO ops on an unchanged resync (no duplication — bug #1), and
|
||||||
|
* - NEVER lose blocks / collapse to empty (no strip — bug #2),
|
||||||
|
* across repeated cycles, for every editor-canonical callout type.
|
||||||
|
*/
|
||||||
|
|
||||||
|
const toYdoc = (content: unknown[]) =>
|
||||||
|
TiptapTransformer.toYdoc(
|
||||||
|
{ type: 'doc', content },
|
||||||
|
'default',
|
||||||
|
tiptapExtensions as any,
|
||||||
|
);
|
||||||
|
|
||||||
|
const blockTypes = (f: Y.XmlFragment) =>
|
||||||
|
f.toArray().map((n: any) => n.nodeName);
|
||||||
|
|
||||||
|
function editorPage(calloutType: string) {
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
type: 'heading',
|
||||||
|
attrs: { id: 'h1', level: 1 },
|
||||||
|
content: [{ type: 'text', text: 'Title here' }],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: 'paragraph',
|
||||||
|
attrs: { id: 'p1' },
|
||||||
|
content: [{ type: 'text', text: 'Para before callout' }],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: 'callout',
|
||||||
|
attrs: { type: calloutType },
|
||||||
|
content: [
|
||||||
|
{
|
||||||
|
type: 'paragraph',
|
||||||
|
attrs: { id: 'pc' },
|
||||||
|
content: [{ type: 'text', text: 'Inside the callout' }],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
type: 'paragraph',
|
||||||
|
attrs: { id: 'p2' },
|
||||||
|
content: [{ type: 'text', text: 'Para after callout' }],
|
||||||
|
},
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
async function gitRoundTrip(content: unknown[]): Promise<any[]> {
|
||||||
|
const md = await convertProseMirrorToMarkdown({ type: 'doc', content });
|
||||||
|
const json = await markdownToProseMirror(md);
|
||||||
|
return json.content;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('git-sync callout merge is idempotent + non-destructive (QA #119)', () => {
|
||||||
|
for (const type of ['info', 'note', 'warning', 'danger', 'success', 'default']) {
|
||||||
|
it(`callout(${type}) resyncs with 0 ops and never strips the body`, async () => {
|
||||||
|
const editor = editorPage(type);
|
||||||
|
const gitContent = await gitRoundTrip(editor);
|
||||||
|
|
||||||
|
const liveDoc = toYdoc(editor);
|
||||||
|
const live = liveDoc.getXmlFragment('default');
|
||||||
|
const before = live.toArray().length;
|
||||||
|
expect(before).toBe(4);
|
||||||
|
|
||||||
|
// 2-way: live vs the git round-trip -> no-op (no dup, no strip).
|
||||||
|
let applied = -1;
|
||||||
|
liveDoc.transact(() => {
|
||||||
|
applied = mergeXmlFragments(live, toYdoc(gitContent).getXmlFragment('default'));
|
||||||
|
});
|
||||||
|
expect(applied).toBe(0);
|
||||||
|
expect(live.toArray().length).toBe(before);
|
||||||
|
|
||||||
|
// 3-way across 4 cycles with base == git (the steady-state) -> stable.
|
||||||
|
for (let cycle = 0; cycle < 4; cycle++) {
|
||||||
|
let a = -1;
|
||||||
|
liveDoc.transact(() => {
|
||||||
|
a = mergeXmlFragments3Way(
|
||||||
|
live,
|
||||||
|
toYdoc(gitContent).getXmlFragment('default'),
|
||||||
|
toYdoc(gitContent).getXmlFragment('default'),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
expect(a).toBe(0);
|
||||||
|
expect(live.toArray().length).toBe(before);
|
||||||
|
expect(blockTypes(live)).toEqual([
|
||||||
|
'heading',
|
||||||
|
'paragraph',
|
||||||
|
'callout',
|
||||||
|
'paragraph',
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it('3-way with a stale base (callout JUST added) keeps the callout + neighbours', async () => {
|
||||||
|
// base = the previously-synced version WITHOUT the callout (git round-trip);
|
||||||
|
// the human just inserted the callout -> the merge must KEEP everything.
|
||||||
|
const prev = [
|
||||||
|
{ type: 'heading', attrs: { id: 'h1', level: 1 }, content: [{ type: 'text', text: 'Title here' }] },
|
||||||
|
{ type: 'paragraph', attrs: { id: 'p1' }, content: [{ type: 'text', text: 'Para before callout' }] },
|
||||||
|
{ type: 'paragraph', attrs: { id: 'p2' }, content: [{ type: 'text', text: 'Para after callout' }] },
|
||||||
|
];
|
||||||
|
const editor = editorPage('info');
|
||||||
|
const baseContent = await gitRoundTrip(prev);
|
||||||
|
const gitContent = await gitRoundTrip(editor);
|
||||||
|
|
||||||
|
const liveDoc = toYdoc(editor);
|
||||||
|
const live = liveDoc.getXmlFragment('default');
|
||||||
|
liveDoc.transact(() => {
|
||||||
|
mergeXmlFragments3Way(
|
||||||
|
live,
|
||||||
|
toYdoc(gitContent).getXmlFragment('default'),
|
||||||
|
toYdoc(baseContent).getXmlFragment('default'),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
// Body survives in full — NOT stripped to empty / a lone paragraph.
|
||||||
|
expect(blockTypes(live)).toEqual([
|
||||||
|
'heading',
|
||||||
|
'paragraph',
|
||||||
|
'callout',
|
||||||
|
'paragraph',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('git-sync callout type fidelity (QA "callout type -> [!info]")', () => {
|
||||||
|
for (const type of ['info', 'note', 'warning', 'danger', 'success', 'default']) {
|
||||||
|
it(`preserves callout type "${type}" across the engine round-trip`, async () => {
|
||||||
|
const content = editorPage(type);
|
||||||
|
const gitContent = await gitRoundTrip(content);
|
||||||
|
const co = gitContent.find((b: any) => b.type === 'callout');
|
||||||
|
expect(co?.attrs?.type).toBe(type);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it('flattens a genuinely unknown callout type to info', async () => {
|
||||||
|
const content = editorPage('tip'); // not an editor-canonical type
|
||||||
|
const gitContent = await gitRoundTrip(content);
|
||||||
|
const co = gitContent.find((b: any) => b.type === 'callout');
|
||||||
|
expect(co?.attrs?.type).toBe('info');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -85,14 +85,30 @@ export async function runCycle(deps: RunCycleDeps): Promise<RunCycleResult> {
|
|||||||
await vault.assertGitAvailable();
|
await vault.assertGitAvailable();
|
||||||
await vault.ensureRepo();
|
await vault.ensureRepo();
|
||||||
|
|
||||||
// 2. Refuse to run on top of an unresolved merge (SPEC §9): a prior
|
// 2. RECOVER from a vault left mid-merge by a PRIOR cycle (SPEC §9 wedge fix).
|
||||||
// conflicting pull leaves the vault mid-merge; the next checkout would fail.
|
// A leftover merge used to WEDGE THE WHOLE SPACE: this check returned
|
||||||
|
// `skipped: "merge-in-progress"` so EVERY later cycle skipped the entire
|
||||||
|
// space (all pages, both directions) forever, with no recovery. The pull
|
||||||
|
// phase below no longer leaves the vault mid-merge (it commits a conflicting
|
||||||
|
// merge with markers and isolates the one bad page), but a vault wedged by a
|
||||||
|
// PRE-FIX build (or a manual/interrupted git op) must still self-heal.
|
||||||
|
// So instead of skipping, ABORT the stale half-merge and continue — the
|
||||||
|
// fresh pull re-runs and, on a real conflict, commits-with-markers rather
|
||||||
|
// than re-wedging. A stray unmerged index that `merge --abort` can't clear
|
||||||
|
// (no MERGE_HEAD) is force-cleared with a hard reset to HEAD.
|
||||||
if (await vault.isMergeInProgress()) {
|
if (await vault.isMergeInProgress()) {
|
||||||
log(
|
log(
|
||||||
`vault has an unresolved merge — resolve it (or 'git merge --abort') ` +
|
`vault was left mid-merge by a prior cycle — aborting the stale merge and ` +
|
||||||
`and re-run (SPEC §9); skipping cycle.`,
|
`continuing so the space is not wedged (SPEC §9 recovery).`,
|
||||||
);
|
);
|
||||||
return { ran: false, skipped: "merge-in-progress" };
|
await vault.abortMerge();
|
||||||
|
if (await vault.isMergeInProgress()) {
|
||||||
|
log(
|
||||||
|
`vault still mid-merge after 'merge --abort' — hard-resetting to HEAD ` +
|
||||||
|
`to recover (SPEC §9).`,
|
||||||
|
);
|
||||||
|
await vault.resetHardToHead();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 3. Pull writes happen on `docmost`; be on it BEFORE applying (see docstring).
|
// 3. Pull writes happen on `docmost`; be on it BEFORE applying (see docstring).
|
||||||
|
|||||||
@@ -414,6 +414,65 @@ export class VaultGit {
|
|||||||
return r.code === 0 && r.stdout.trim().length > 0;
|
return r.code === 0 && r.stdout.trim().length > 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The vault-relative (forward-slash) paths with UNMERGED (conflicted) index
|
||||||
|
* entries after a conflicting merge. NUL-delimited + `core.quotepath=false`
|
||||||
|
* (the `runRaw` baseline) so Cyrillic/space paths come back verbatim. Used by
|
||||||
|
* the pull cycle to LOG and ISOLATE the conflicted page(s) when it commits a
|
||||||
|
* conflicted merge instead of leaving the whole vault wedged (SPEC §9 wedge
|
||||||
|
* fix). Returns `[]` on any error (best-effort diagnostics).
|
||||||
|
*/
|
||||||
|
async listUnmergedPaths(): Promise<string[]> {
|
||||||
|
const r = await this.runRaw([
|
||||||
|
"diff",
|
||||||
|
"--name-only",
|
||||||
|
"--diff-filter=U",
|
||||||
|
"-z",
|
||||||
|
]);
|
||||||
|
if (r.code !== 0) return [];
|
||||||
|
return r.stdout.split("\0").filter((p) => p.length > 0);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Commit an IN-PROGRESS (conflicted) merge AS-IS so the vault is NOT left
|
||||||
|
* wedged mid-merge (SPEC §9 wedge fix). A `git merge` that conflicts leaves
|
||||||
|
* `MERGE_HEAD` + unmerged index entries; the next cycle's `isMergeInProgress`
|
||||||
|
* check would then skip the ENTIRE space forever (the reported wedge). Instead
|
||||||
|
* we stage everything — including the conflicted file(s), whose conflict
|
||||||
|
* markers are PRESERVED in the committed tree — and record the two-parent merge
|
||||||
|
* commit. The cleanly-merged pages land normally; the conflicted page carries
|
||||||
|
* its markers on `main`, where the push side isolates it (a per-page push
|
||||||
|
* failure when `autoMergeConflicts` is off; the markers never reach Docmost)
|
||||||
|
* while every other page keeps syncing. Recovery: resolve the markers in git
|
||||||
|
* and the next push sends the clean body.
|
||||||
|
*
|
||||||
|
* `--allow-empty` guards the degenerate case where the staged conflict
|
||||||
|
* resolution nets to no tree change; while `MERGE_HEAD` exists `git commit`
|
||||||
|
* still records the merge commit so the half-merge is cleared.
|
||||||
|
*/
|
||||||
|
async commitMerge(message: string, opts: CommitOptions): Promise<void> {
|
||||||
|
await this.run(["add", "-A"]);
|
||||||
|
await this.commitRaw(message, { ...opts, allowEmpty: true });
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Abort an in-progress merge (`git merge --abort`), restoring the pre-merge
|
||||||
|
* working tree + index. Best-effort: a non-zero exit (e.g. no MERGE_HEAD) is
|
||||||
|
* swallowed. Used by the cycle's RECOVERY path to unwedge a vault that a
|
||||||
|
* PRIOR (pre-fix) cycle left mid-merge, so the fresh pull can re-run instead of
|
||||||
|
* skipping the space forever (SPEC §9 wedge recovery).
|
||||||
|
*/
|
||||||
|
async abortMerge(): Promise<void> {
|
||||||
|
await this.runRaw(["merge", "--abort"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Hard-reset the working tree + index to HEAD (drops a stray half-merge that
|
||||||
|
* `merge --abort` could not clear — no MERGE_HEAD but lingering unmerged
|
||||||
|
* entries). Best-effort recovery primitive (SPEC §9). */
|
||||||
|
async resetHardToHead(): Promise<void> {
|
||||||
|
await this.runRaw(["reset", "--hard", "HEAD"]);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* List tracked files on the current branch (paths relative to the vault
|
* List tracked files on the current branch (paths relative to the vault
|
||||||
* root, forward-slash separated). An optional glob (a git pathspec) narrows
|
* root, forward-slash separated). An optional glob (a git pathspec) narrows
|
||||||
|
|||||||
@@ -218,7 +218,15 @@ export function computePullActions(input: PullActionsInput): PullActions {
|
|||||||
*/
|
*/
|
||||||
export interface ApplyPullActionsDeps {
|
export interface ApplyPullActionsDeps {
|
||||||
client: Pick<GitSyncClient, "getPageJson">;
|
client: Pick<GitSyncClient, "getPageJson">;
|
||||||
git: Pick<VaultGit, "stageAll" | "commit" | "checkout" | "merge">;
|
git: Pick<
|
||||||
|
VaultGit,
|
||||||
|
| "stageAll"
|
||||||
|
| "commit"
|
||||||
|
| "checkout"
|
||||||
|
| "merge"
|
||||||
|
| "listUnmergedPaths"
|
||||||
|
| "commitMerge"
|
||||||
|
>;
|
||||||
/** Write a file by ABSOLUTE path (mkdir of the parent is done internally). */
|
/** Write a file by ABSOLUTE path (mkdir of the parent is done internally). */
|
||||||
writeFile: (absPath: string, text: string) => Promise<void>;
|
writeFile: (absPath: string, text: string) => Promise<void>;
|
||||||
/** Recursive mkdir of an ABSOLUTE directory path. */
|
/** Recursive mkdir of an ABSOLUTE directory path. */
|
||||||
@@ -240,6 +248,13 @@ export interface ApplyResult {
|
|||||||
failed: number;
|
failed: number;
|
||||||
committed: boolean;
|
committed: boolean;
|
||||||
merge: { ok: boolean; conflict: boolean; output: string };
|
merge: { ok: boolean; conflict: boolean; output: string };
|
||||||
|
/**
|
||||||
|
* Vault-relative paths of the page(s) that CONFLICTED in the docmost -> main
|
||||||
|
* merge and were committed WITH conflict markers (so the rest of the space
|
||||||
|
* keeps syncing — SPEC §9 wedge fix). Empty on a clean merge. The push side
|
||||||
|
* isolates these (per-page failure when `autoMergeConflicts` is off).
|
||||||
|
*/
|
||||||
|
conflictedPaths: string[];
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -404,20 +419,47 @@ export async function applyPullActions(
|
|||||||
trailers: [SOURCE_TRAILER],
|
trailers: [SOURCE_TRAILER],
|
||||||
});
|
});
|
||||||
|
|
||||||
// Merge docmost -> main. Conflicts are surfaced and left in git (SPEC §9);
|
// Merge docmost -> main. A CONFLICT must NOT wedge the whole space (the
|
||||||
// we never push to Docmost. Push to a git remote is deferred (SPEC §7).
|
// reported bug: ONE same-line conflict on ONE page froze sync for EVERY page
|
||||||
|
// in both directions because the next cycle's `isMergeInProgress` check kept
|
||||||
|
// skipping the entire space). So instead of leaving the vault mid-merge, we
|
||||||
|
// COMMIT the conflicted merge with markers in place (SPEC §9 wedge fix): the
|
||||||
|
// cleanly-merged pages land, the conflicted page carries its markers on `main`
|
||||||
|
// and is isolated by the push side (a per-page failure when `autoMergeConflicts`
|
||||||
|
// is off — the markers never reach Docmost), and the NEXT cycle is NOT wedged.
|
||||||
|
// Recovery: resolve the markers in git; the next push then sends the clean body.
|
||||||
await git.checkout(DEFAULT_BRANCH);
|
await git.checkout(DEFAULT_BRANCH);
|
||||||
const merge = await git.merge(DOCMOST_BRANCH);
|
const merge = await git.merge(DOCMOST_BRANCH);
|
||||||
|
let conflictedPaths: string[] = [];
|
||||||
if (merge.conflict) {
|
if (merge.conflict) {
|
||||||
|
conflictedPaths = await git.listUnmergedPaths();
|
||||||
|
await git.commitMerge(
|
||||||
|
`docmost: sync with unresolved conflict in ${conflictedPaths.length} page(s)`,
|
||||||
|
{
|
||||||
|
authorName: BOT_AUTHOR_NAME,
|
||||||
|
authorEmail: BOT_AUTHOR_EMAIL,
|
||||||
|
trailers: [SOURCE_TRAILER],
|
||||||
|
},
|
||||||
|
);
|
||||||
log(
|
log(
|
||||||
"pull: merge of docmost -> main CONFLICTED. Conflict markers were left " +
|
`pull: merge of docmost -> main CONFLICTED on ${conflictedPaths.length} ` +
|
||||||
"in the vault for manual resolution (SPEC §9). Nothing is pushed to " +
|
`page(s): ${conflictedPaths.join(", ")}. Committed the merge WITH ` +
|
||||||
"Docmost (read-only). Resolve locally, then re-run.",
|
`conflict markers so the rest of the space keeps syncing (SPEC §9). The ` +
|
||||||
|
`conflicted page(s) are isolated on push (markers never reach Docmost); ` +
|
||||||
|
`resolve the markers in git to recover.`,
|
||||||
);
|
);
|
||||||
} else if (!merge.ok) {
|
} else if (!merge.ok) {
|
||||||
log(`pull: merge of docmost -> main failed: ${merge.output}`);
|
log(`pull: merge of docmost -> main failed: ${merge.output}`);
|
||||||
}
|
}
|
||||||
log("pull: git push to remote is DEFERRED in this increment (SPEC §7).");
|
log("pull: git push to remote is DEFERRED in this increment (SPEC §7).");
|
||||||
|
|
||||||
return { written, movedApplied, deleted, failed, committed, merge };
|
return {
|
||||||
|
written,
|
||||||
|
movedApplied,
|
||||||
|
deleted,
|
||||||
|
failed,
|
||||||
|
committed,
|
||||||
|
merge,
|
||||||
|
conflictedPaths,
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -49,8 +49,18 @@ function getStyleProperty(element: HTMLElement, propertyName: string): string |
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Allowed Docmost callout types; anything else falls back to "info". */
|
/**
|
||||||
const CALLOUT_TYPES = ["info", "warning", "danger", "success"];
|
* Allowed Docmost callout types; anything else falls back to "info".
|
||||||
|
*
|
||||||
|
* This MUST stay in lockstep with the editor's canonical set
|
||||||
|
* (`getValidCalloutType` in `@docmost/editor-ext` callout/utils.ts:
|
||||||
|
* default | info | note | success | warning | danger). A type missing here is
|
||||||
|
* silently flattened to "info" on the markdown -> ProseMirror round-trip, so a
|
||||||
|
* `[!note]` / `[!default]` callout authored in the editor would come back as
|
||||||
|
* `[!info]` after a git sync (the QA "callout type -> [!info]" fidelity loss).
|
||||||
|
* `note` and `default` were previously absent and so were being flattened.
|
||||||
|
*/
|
||||||
|
const CALLOUT_TYPES = ["default", "info", "note", "success", "warning", "danger"];
|
||||||
export const clampCalloutType = (value: string | null | undefined): string =>
|
export const clampCalloutType = (value: string | null | undefined): string =>
|
||||||
value && CALLOUT_TYPES.includes(value.toLowerCase())
|
value && CALLOUT_TYPES.includes(value.toLowerCase())
|
||||||
? value.toLowerCase()
|
? value.toLowerCase()
|
||||||
|
|||||||
@@ -66,6 +66,10 @@ function makeGit(merge: { ok: boolean; conflict: boolean; output?: string } = {
|
|||||||
order.push('merge');
|
order.push('merge');
|
||||||
return { ok: merge.ok, conflict: merge.conflict, output: merge.output ?? '' };
|
return { ok: merge.ok, conflict: merge.conflict, output: merge.output ?? '' };
|
||||||
}),
|
}),
|
||||||
|
listUnmergedPaths: vi.fn(async () => ['Conflicted.md']),
|
||||||
|
commitMerge: vi.fn(async (subject: string) => {
|
||||||
|
order.push(`commitMerge:${subject}`);
|
||||||
|
}),
|
||||||
};
|
};
|
||||||
return {
|
return {
|
||||||
git,
|
git,
|
||||||
@@ -403,7 +407,11 @@ describe('applyPullActions — commit subject reflects ACTUAL counts', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe('applyPullActions — merge result is surfaced, not swallowed', () => {
|
describe('applyPullActions — merge result is surfaced, not swallowed', () => {
|
||||||
it('returns conflict:true on a conflicting merge (no auto-resolve)', async () => {
|
it('COMMITS a conflicting merge with markers (no wedge) and surfaces conflictedPaths', async () => {
|
||||||
|
// Regression for the WEDGE bug (QA #119): a conflicting docmost -> main merge
|
||||||
|
// must NOT be left mid-merge (which wedged the whole space). It is committed
|
||||||
|
// WITH markers so the rest of the space keeps syncing; the conflicted page is
|
||||||
|
// surfaced in `conflictedPaths` and isolated by the push side.
|
||||||
const { client } = makeClient();
|
const { client } = makeClient();
|
||||||
const g = makeGit({ ok: false, conflict: true, output: 'CONFLICT' });
|
const g = makeGit({ ok: false, conflict: true, output: 'CONFLICT' });
|
||||||
const fs = makeFs();
|
const fs = makeFs();
|
||||||
@@ -415,6 +423,10 @@ describe('applyPullActions — merge result is surfaced, not swallowed', () => {
|
|||||||
);
|
);
|
||||||
expect(res.merge.conflict).toBe(true);
|
expect(res.merge.conflict).toBe(true);
|
||||||
expect(res.merge.ok).toBe(false);
|
expect(res.merge.ok).toBe(false);
|
||||||
|
// The merge was COMMITTED (vault no longer mid-merge) and the bad page named.
|
||||||
|
expect(g.git.commitMerge).toHaveBeenCalledTimes(1);
|
||||||
|
expect(res.conflictedPaths).toEqual(['Conflicted.md']);
|
||||||
|
expect(g.order.some((o) => o.startsWith('commitMerge:'))).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('returns ok:false conflict:false on a non-conflict merge failure', async () => {
|
it('returns ok:false conflict:false on a non-conflict merge failure', async () => {
|
||||||
|
|||||||
@@ -129,7 +129,7 @@ describe("runCycle against a REAL VaultGit (integration)", () => {
|
|||||||
expect(onDisk).toContain("new-id");
|
expect(onDisk).toContain("new-id");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("an unresolved merge short-circuits before any client call", async () => {
|
it("RECOVERS a vault left mid-merge instead of wedging the whole space", async () => {
|
||||||
if (!available) return;
|
if (!available) return;
|
||||||
|
|
||||||
dir = await mkdtemp(join(tmpdir(), "docmost-cycle-merge-"));
|
dir = await mkdtemp(join(tmpdir(), "docmost-cycle-merge-"));
|
||||||
@@ -149,8 +149,9 @@ describe("runCycle against a REAL VaultGit (integration)", () => {
|
|||||||
await writeFile(join(dir, "C.md"), "main-side\n", "utf8");
|
await writeFile(join(dir, "C.md"), "main-side\n", "utf8");
|
||||||
await git.stageAll();
|
await git.stageAll();
|
||||||
await git.commit("main edit", { authorName: "h", authorEmail: "h@l" });
|
await git.commit("main edit", { authorName: "h", authorEmail: "h@l" });
|
||||||
// Start a conflicting merge and leave it unresolved.
|
// Start a conflicting merge and leave it unresolved (the wedged state).
|
||||||
await execFileAsync("git", ["-C", dir, "merge", "docmost"]).catch(() => {});
|
await execFileAsync("git", ["-C", dir, "merge", "docmost"]).catch(() => {});
|
||||||
|
expect(await git.isMergeInProgress()).toBe(true);
|
||||||
|
|
||||||
const client = makeEmptyClientFake();
|
const client = makeEmptyClientFake();
|
||||||
const res = await runCycle({
|
const res = await runCycle({
|
||||||
@@ -162,8 +163,25 @@ describe("runCycle against a REAL VaultGit (integration)", () => {
|
|||||||
log: () => undefined,
|
log: () => undefined,
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(res).toEqual({ ran: false, skipped: "merge-in-progress" });
|
// WEDGE FIX: the cycle does NOT skip forever — it aborts the stale merge and
|
||||||
expect(client.listSpaceTree).not.toHaveBeenCalled();
|
// RUNS the full pull/push. The space is no longer frozen.
|
||||||
expect(client.createPage).not.toHaveBeenCalled();
|
expect(res.ran).toBe(true);
|
||||||
|
expect(client.listSpaceTree).toHaveBeenCalled();
|
||||||
|
// And crucially, the vault is NOT left mid-merge afterward (the re-merge of a
|
||||||
|
// genuinely conflicting page is committed-with-markers, not wedged), so the
|
||||||
|
// next cycle can run too.
|
||||||
|
expect(await git.isMergeInProgress()).toBe(false);
|
||||||
|
|
||||||
|
// A SECOND cycle also runs cleanly (proves the wedge is gone for good).
|
||||||
|
const res2 = await runCycle({
|
||||||
|
spaceId: "space-1",
|
||||||
|
client: client as any,
|
||||||
|
vault: git,
|
||||||
|
settings: makeSettings(dir),
|
||||||
|
fs: nodeFs,
|
||||||
|
log: () => undefined,
|
||||||
|
});
|
||||||
|
expect(res2.ran).toBe(true);
|
||||||
|
expect(await git.isMergeInProgress()).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -24,6 +24,10 @@ function fakeVault(overrides: Record<string, any> = {}) {
|
|||||||
stageAll: rec("stageAll"),
|
stageAll: rec("stageAll"),
|
||||||
commit: rec("commit", { committed: false }),
|
commit: rec("commit", { committed: false }),
|
||||||
merge: rec("merge", { ok: true, conflict: false, output: "" }),
|
merge: rec("merge", { ok: true, conflict: false, output: "" }),
|
||||||
|
listUnmergedPaths: vi.fn(async () => [] as string[]),
|
||||||
|
commitMerge: rec("commitMerge"),
|
||||||
|
abortMerge: rec("abortMerge"),
|
||||||
|
resetHardToHead: rec("resetHardToHead"),
|
||||||
readRef: vi.fn(async () => null),
|
readRef: vi.fn(async () => null),
|
||||||
revParse: vi.fn(async () => "0000000000000000000000000000000000000000"),
|
revParse: vi.fn(async () => "0000000000000000000000000000000000000000"),
|
||||||
diffNameStatus: vi.fn(async () => [] as any[]),
|
diffNameStatus: vi.fn(async () => [] as any[]),
|
||||||
@@ -64,16 +68,47 @@ function baseDeps(vault: any, over: Partial<RunCycleDeps> = {}): RunCycleDeps {
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe("runCycle (composition)", () => {
|
describe("runCycle (composition)", () => {
|
||||||
it("short-circuits with skipped:'merge-in-progress' and runs no pull/push", async () => {
|
it("RECOVERS from a vault left mid-merge: aborts the stale merge and continues (no wedge)", async () => {
|
||||||
const vault = fakeVault({ isMergeInProgress: vi.fn(async () => true) });
|
// Regression for the WEDGE bug (QA #119): a vault left mid-merge by a prior
|
||||||
|
// cycle used to skip the WHOLE space forever. Now the cycle aborts the stale
|
||||||
|
// merge and proceeds so the space self-heals.
|
||||||
|
let midMerge = true;
|
||||||
|
const vault = fakeVault({
|
||||||
|
// mid-merge until `abortMerge` clears it (then the cycle continues).
|
||||||
|
isMergeInProgress: vi.fn(async () => midMerge),
|
||||||
|
abortMerge: vi.fn(async () => {
|
||||||
|
midMerge = false;
|
||||||
|
}),
|
||||||
|
});
|
||||||
const deps = baseDeps(vault);
|
const deps = baseDeps(vault);
|
||||||
|
|
||||||
const res = await runCycle(deps);
|
const res = await runCycle(deps);
|
||||||
|
|
||||||
expect(res).toEqual({ ran: false, skipped: "merge-in-progress" });
|
// The stale merge was aborted and the cycle RAN (no permanent wedge).
|
||||||
// Never advanced to the pull (listSpaceTree) or push.
|
expect(vault.abortMerge).toHaveBeenCalledTimes(1);
|
||||||
expect(deps.client.listSpaceTree).not.toHaveBeenCalled();
|
expect(res.ran).toBe(true);
|
||||||
expect(vault.order).not.toContain("checkout:docmost");
|
expect(deps.client.listSpaceTree).toHaveBeenCalledTimes(1);
|
||||||
|
expect(vault.order).toContain("checkout:docmost");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("hard-resets when 'merge --abort' cannot clear a stray unmerged index", async () => {
|
||||||
|
// abortMerge does NOT clear it (no MERGE_HEAD but stray unmerged entries);
|
||||||
|
// the cycle falls back to a hard reset, then proceeds.
|
||||||
|
let midMerge = true;
|
||||||
|
const vault = fakeVault({
|
||||||
|
isMergeInProgress: vi.fn(async () => midMerge),
|
||||||
|
abortMerge: vi.fn(async () => undefined), // leaves it mid-merge
|
||||||
|
resetHardToHead: vi.fn(async () => {
|
||||||
|
midMerge = false;
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
const deps = baseDeps(vault);
|
||||||
|
|
||||||
|
const res = await runCycle(deps);
|
||||||
|
|
||||||
|
expect(vault.abortMerge).toHaveBeenCalledTimes(1);
|
||||||
|
expect(vault.resetHardToHead).toHaveBeenCalledTimes(1);
|
||||||
|
expect(res.ran).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("stages ensureRepo -> ensureBranch(docmost,main) -> checkout(docmost) BEFORE pulling", async () => {
|
it("stages ensureRepo -> ensureBranch(docmost,main) -> checkout(docmost) BEFORE pulling", async () => {
|
||||||
|
|||||||
@@ -65,9 +65,21 @@ describe('clampCalloutType', () => {
|
|||||||
expect(clampCalloutType('success')).toBe('success');
|
expect(clampCalloutType('success')).toBe('success');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('falls back to "info" for unknown types', () => {
|
it('PRESERVES every editor-canonical type (note/default no longer flattened)', () => {
|
||||||
expect(clampCalloutType('note')).toBe('info');
|
// Regression for the QA "callout type -> [!info]" fidelity loss: `note` and
|
||||||
|
// `default` are valid editor callout types and must survive the git
|
||||||
|
// round-trip, not collapse to `info`.
|
||||||
|
expect(clampCalloutType('note')).toBe('note');
|
||||||
|
expect(clampCalloutType('default')).toBe('default');
|
||||||
|
expect(clampCalloutType('info')).toBe('info');
|
||||||
|
expect(clampCalloutType('warning')).toBe('warning');
|
||||||
|
expect(clampCalloutType('danger')).toBe('danger');
|
||||||
|
expect(clampCalloutType('success')).toBe('success');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('falls back to "info" for genuinely unknown types', () => {
|
||||||
expect(clampCalloutType('tip')).toBe('info');
|
expect(clampCalloutType('tip')).toBe('info');
|
||||||
|
expect(clampCalloutType('banana')).toBe('info');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('falls back to "info" for empty string and null', () => {
|
it('falls back to "info" for empty string and null', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user