diff --git a/apps/server/src/collaboration/extensions/persistence.disconnect-flush.spec.ts b/apps/server/src/collaboration/extensions/persistence.disconnect-flush.spec.ts new file mode 100644 index 00000000..7b24d82a --- /dev/null +++ b/apps/server/src/collaboration/extensions/persistence.disconnect-flush.spec.ts @@ -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(); + }); +}); diff --git a/apps/server/src/collaboration/extensions/persistence.extension.ts b/apps/server/src/collaboration/extensions/persistence.extension.ts index 4512ec0e..52ecadf0 100644 --- a/apps/server/src/collaboration/extensions/persistence.extension.ts +++ b/apps/server/src/collaboration/extensions/persistence.extension.ts @@ -2,6 +2,7 @@ import { afterUnloadDocumentPayload, Extension, onChangePayload, + onDisconnectPayload, onLoadDocumentPayload, onStoreDocumentPayload, } from '@hocuspocus/server'; @@ -164,6 +165,40 @@ export class PersistenceExtension implements Extension { 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) { const { documentName, document, context } = data; diff --git a/apps/server/src/integrations/git-sync/services/yjs-body-merge.callout.spec.ts b/apps/server/src/integrations/git-sync/services/yjs-body-merge.callout.spec.ts new file mode 100644 index 00000000..16a9ad14 --- /dev/null +++ b/apps/server/src/integrations/git-sync/services/yjs-body-merge.callout.spec.ts @@ -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 { + 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'); + }); +}); diff --git a/packages/git-sync/src/engine/cycle.ts b/packages/git-sync/src/engine/cycle.ts index d666a1fe..985e9645 100644 --- a/packages/git-sync/src/engine/cycle.ts +++ b/packages/git-sync/src/engine/cycle.ts @@ -85,14 +85,30 @@ export async function runCycle(deps: RunCycleDeps): Promise { await vault.assertGitAvailable(); await vault.ensureRepo(); - // 2. Refuse to run on top of an unresolved merge (SPEC §9): a prior - // conflicting pull leaves the vault mid-merge; the next checkout would fail. + // 2. RECOVER from a vault left mid-merge by a PRIOR cycle (SPEC §9 wedge fix). + // 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()) { log( - `vault has an unresolved merge — resolve it (or 'git merge --abort') ` + - `and re-run (SPEC §9); skipping cycle.`, + `vault was left mid-merge by a prior cycle — aborting the stale merge and ` + + `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). diff --git a/packages/git-sync/src/engine/git.ts b/packages/git-sync/src/engine/git.ts index 894e0ee4..f334263d 100644 --- a/packages/git-sync/src/engine/git.ts +++ b/packages/git-sync/src/engine/git.ts @@ -414,6 +414,65 @@ export class VaultGit { 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 { + 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 { + 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 { + 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 { + await this.runRaw(["reset", "--hard", "HEAD"]); + } + /** * List tracked files on the current branch (paths relative to the vault * root, forward-slash separated). An optional glob (a git pathspec) narrows diff --git a/packages/git-sync/src/engine/pull.ts b/packages/git-sync/src/engine/pull.ts index 6e79f552..771735bd 100644 --- a/packages/git-sync/src/engine/pull.ts +++ b/packages/git-sync/src/engine/pull.ts @@ -218,7 +218,15 @@ export function computePullActions(input: PullActionsInput): PullActions { */ export interface ApplyPullActionsDeps { client: Pick; - git: Pick; + git: Pick< + VaultGit, + | "stageAll" + | "commit" + | "checkout" + | "merge" + | "listUnmergedPaths" + | "commitMerge" + >; /** Write a file by ABSOLUTE path (mkdir of the parent is done internally). */ writeFile: (absPath: string, text: string) => Promise; /** Recursive mkdir of an ABSOLUTE directory path. */ @@ -240,6 +248,13 @@ export interface ApplyResult { failed: number; committed: boolean; 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], }); - // Merge docmost -> main. Conflicts are surfaced and left in git (SPEC §9); - // we never push to Docmost. Push to a git remote is deferred (SPEC §7). + // Merge docmost -> main. A CONFLICT must NOT wedge the whole space (the + // 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); const merge = await git.merge(DOCMOST_BRANCH); + let conflictedPaths: string[] = []; 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( - "pull: merge of docmost -> main CONFLICTED. Conflict markers were left " + - "in the vault for manual resolution (SPEC §9). Nothing is pushed to " + - "Docmost (read-only). Resolve locally, then re-run.", + `pull: merge of docmost -> main CONFLICTED on ${conflictedPaths.length} ` + + `page(s): ${conflictedPaths.join(", ")}. Committed the merge WITH ` + + `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) { log(`pull: merge of docmost -> main failed: ${merge.output}`); } 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, + }; } diff --git a/packages/git-sync/src/lib/docmost-schema.ts b/packages/git-sync/src/lib/docmost-schema.ts index 3595866b..304a7c92 100644 --- a/packages/git-sync/src/lib/docmost-schema.ts +++ b/packages/git-sync/src/lib/docmost-schema.ts @@ -49,8 +49,18 @@ function getStyleProperty(element: HTMLElement, propertyName: string): string | 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 => value && CALLOUT_TYPES.includes(value.toLowerCase()) ? value.toLowerCase() diff --git a/packages/git-sync/test/apply-pull-actions.test.ts b/packages/git-sync/test/apply-pull-actions.test.ts index 762046e6..c4d07096 100644 --- a/packages/git-sync/test/apply-pull-actions.test.ts +++ b/packages/git-sync/test/apply-pull-actions.test.ts @@ -66,6 +66,10 @@ function makeGit(merge: { ok: boolean; conflict: boolean; output?: string } = { order.push('merge'); 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 { git, @@ -403,7 +407,11 @@ describe('applyPullActions — commit subject reflects ACTUAL counts', () => { }); 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 g = makeGit({ ok: false, conflict: true, output: 'CONFLICT' }); const fs = makeFs(); @@ -415,6 +423,10 @@ describe('applyPullActions — merge result is surfaced, not swallowed', () => { ); expect(res.merge.conflict).toBe(true); 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 () => { diff --git a/packages/git-sync/test/cycle-roundtrip.test.ts b/packages/git-sync/test/cycle-roundtrip.test.ts index e6533f5a..eb6253b6 100644 --- a/packages/git-sync/test/cycle-roundtrip.test.ts +++ b/packages/git-sync/test/cycle-roundtrip.test.ts @@ -129,7 +129,7 @@ describe("runCycle against a REAL VaultGit (integration)", () => { 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; 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 git.stageAll(); 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(() => {}); + expect(await git.isMergeInProgress()).toBe(true); const client = makeEmptyClientFake(); const res = await runCycle({ @@ -162,8 +163,25 @@ describe("runCycle against a REAL VaultGit (integration)", () => { log: () => undefined, }); - expect(res).toEqual({ ran: false, skipped: "merge-in-progress" }); - expect(client.listSpaceTree).not.toHaveBeenCalled(); - expect(client.createPage).not.toHaveBeenCalled(); + // WEDGE FIX: the cycle does NOT skip forever — it aborts the stale merge and + // RUNS the full pull/push. The space is no longer frozen. + 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); }); }); diff --git a/packages/git-sync/test/cycle.test.ts b/packages/git-sync/test/cycle.test.ts index 848dd2c2..276ed940 100644 --- a/packages/git-sync/test/cycle.test.ts +++ b/packages/git-sync/test/cycle.test.ts @@ -24,6 +24,10 @@ function fakeVault(overrides: Record = {}) { stageAll: rec("stageAll"), commit: rec("commit", { committed: false }), 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), revParse: vi.fn(async () => "0000000000000000000000000000000000000000"), diffNameStatus: vi.fn(async () => [] as any[]), @@ -64,16 +68,47 @@ function baseDeps(vault: any, over: Partial = {}): RunCycleDeps { } describe("runCycle (composition)", () => { - it("short-circuits with skipped:'merge-in-progress' and runs no pull/push", async () => { - const vault = fakeVault({ isMergeInProgress: vi.fn(async () => true) }); + it("RECOVERS from a vault left mid-merge: aborts the stale merge and continues (no wedge)", async () => { + // 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 res = await runCycle(deps); - expect(res).toEqual({ ran: false, skipped: "merge-in-progress" }); - // Never advanced to the pull (listSpaceTree) or push. - expect(deps.client.listSpaceTree).not.toHaveBeenCalled(); - expect(vault.order).not.toContain("checkout:docmost"); + // The stale merge was aborted and the cycle RAN (no permanent wedge). + expect(vault.abortMerge).toHaveBeenCalledTimes(1); + expect(res.ran).toBe(true); + 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 () => { diff --git a/packages/git-sync/test/docmost-schema-attrs.test.ts b/packages/git-sync/test/docmost-schema-attrs.test.ts index 201882e7..b89fe6a4 100644 --- a/packages/git-sync/test/docmost-schema-attrs.test.ts +++ b/packages/git-sync/test/docmost-schema-attrs.test.ts @@ -65,9 +65,21 @@ describe('clampCalloutType', () => { expect(clampCalloutType('success')).toBe('success'); }); - it('falls back to "info" for unknown types', () => { - expect(clampCalloutType('note')).toBe('info'); + it('PRESERVES every editor-canonical type (note/default no longer flattened)', () => { + // 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('banana')).toBe('info'); }); it('falls back to "info" for empty string and null', () => {