diff --git a/apps/server/src/collaboration/merge/yjs-body-merge.ts b/apps/server/src/collaboration/merge/yjs-body-merge.ts index 1ec64ced..6d42dcbc 100644 --- a/apps/server/src/collaboration/merge/yjs-body-merge.ts +++ b/apps/server/src/collaboration/merge/yjs-body-merge.ts @@ -20,11 +20,13 @@ import { buildLcsTable } from './lcs'; * no-op (zero Yjs operations). Yjs then CRDT-merges the minimal ops with any * concurrent edits. * - * Limitation (honest): this is a 2-way merge (live vs incoming). For a block that - * BOTH sides changed since the last sync it cannot tell which is newer without a - * common ancestor, so the incoming (git) version wins for that one block. A full - * 3-way merge would need the last-synced base plumbed from the engine; the common - * cases — unchanged resync, and edits to DIFFERENT blocks — are handled losslessly. + * Merge mode: a THREE-WAY merge (live vs incoming vs base) runs whenever the + * engine plumbs the last-synced base (`baseMarkdown` from refs/docmost/last-pushed) + * — which it now does end-to-end — so a block both sides changed is a genuine + * conflict resolved deterministically (git wins that block; the prior state is + * preserved in page history). Only when NO base is available (a brand-new file) + * does it fall back to a 2-way merge (live vs incoming). Common cases — unchanged + * resync and edits to DIFFERENT blocks — are lossless in both modes. */ type XmlNode = Y.XmlElement | Y.XmlText | Y.XmlHook; diff --git a/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.ts b/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.ts index 55157302..d4568af8 100644 --- a/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.ts +++ b/apps/server/src/integrations/git-sync/services/git-sync.orchestrator.ts @@ -140,10 +140,9 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy { // --- one sync cycle for a space ------------------------------- /** - * Build the engine `Settings` for a space. The engine's REST-era fields - * (docmostApiUrl/email/password) are unused on the native path — the - * datasource writes in-process — so they are placeholders; only `vaultPath` - * and the tunables are load-bearing today. + * Build the engine `Settings` for a space. The datasource writes in-process, + * so only `vaultPath`, `docmostSpaceId` and the tunables are load-bearing; the + * dead REST-era fields (docmostApiUrl/email/password) were removed (review). * * `gitRemote` is NOT yet consumed: the vendored engine has no remote-push path * (see engine/git.ts, engine/pull.ts, SPEC §7 — remote push is deferred), so @@ -174,9 +173,6 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy { .where('id', '=', spaceId) .executeTakeFirst(); return { - docmostApiUrl: 'http://native.local', - docmostEmail: 'native@local', - docmostPassword: 'native', docmostSpaceId: spaceId, vaultPath: this.vaultRegistry.vaultPath(spaceId), gitRemote, @@ -493,6 +489,17 @@ export class GitSyncOrchestrator implements OnModuleInit, OnModuleDestroy { onModuleInit(): void { if (!this.environmentService.isGitSyncEnabled()) return; + // GIT_SYNC_REMOTE_TEMPLATE is inert scaffolding: the vendored engine has no + // remote-push path yet (SPEC §7), so setting it does nothing today. Warn once + // at startup so an operator who configured it isn't left with a silent no-op + // (review). Remove this warning when the engine grows a remote-push path. + if (this.environmentService.getGitSyncRemoteTemplate()) { + this.logger.warn( + 'git-sync: GIT_SYNC_REMOTE_TEMPLATE is set but NOT yet consumed — ' + + 'remote push is deferred (SPEC §7); this value currently has no effect.', + ); + } + const ms = this.environmentService.getGitSyncPollIntervalMs(); const handle = setInterval(() => { void this.pollTick(); diff --git a/packages/git-sync/src/engine/client.types.ts b/packages/git-sync/src/engine/client.types.ts index 871e4273..94340543 100644 --- a/packages/git-sync/src/engine/client.types.ts +++ b/packages/git-sync/src/engine/client.types.ts @@ -70,14 +70,16 @@ export interface GitSyncClient { // --- writes (push) -------------------------------------------------------- /** - * Merge a page's body from a self-contained markdown file (meta + body). The - * collab/Yjs write path (SPEC §2/§15.6) — never a raw jsonb overwrite. - * `applyPushActions` reads only an optional `updatedAt` off the result - * (via `extractUpdatedAt`, tolerant of extra fields). + * Merge a page's markdown BODY into the live page. `applyPushActions` passes + * the file's body with the frontmatter AND any git conflict markers already + * stripped — NOT the raw self-contained file — so `fullMarkdown` here is clean + * body text (the datasource re-parses defensively). The collab/Yjs write path + * (SPEC §2/§15.6) — never a raw jsonb overwrite. `applyPushActions` reads only + * an optional `updatedAt` off the result (via `extractUpdatedAt`). * - * `baseMarkdown` is the last-synced version of the file (`refs/docmost/ - * last-pushed`), the common ancestor for a THREE-WAY merge against the live - * doc so concurrent human edits survive (review #5). Optional/null -> 2-way. + * `baseMarkdown` is the last-synced body (from `refs/docmost/last-pushed`, + * likewise stripped), the common ancestor for a THREE-WAY merge against the + * live doc so concurrent human edits survive (review #5). Optional/null -> 2-way. */ importPageMarkdown( pageId: string, diff --git a/packages/git-sync/src/engine/push.ts b/packages/git-sync/src/engine/push.ts index f5f22247..7a1db5bd 100644 --- a/packages/git-sync/src/engine/push.ts +++ b/packages/git-sync/src/engine/push.ts @@ -486,9 +486,10 @@ export const LAST_PUSHED_REF = "refs/docmost/last-pushed"; export const DOCMOST_BRANCH = "docmost"; /** - * Injectable IO for `applyPushActions`. The real `main` (NEXT increment) wires - * these to the live client, `node:fs/promises`, and the vault git wrapper; this - * increment drives them only through FAKES in tests (no live destructive run). + * Injectable IO for `applyPushActions`. In production these are wired to the live + * client (the native GitmostDataSource), `node:fs/promises`, and the vault git + * wrapper — `applyPushActions` runs LIVE via runPush -> runCycle -> the + * orchestrator's driveCycle. Tests substitute FAKES through the same seam. * - `client`: the create/update/delete/move/rename subset of `GitSyncClient`. * - `readFile`/`writeFile`: read a changed file's body / write a file back * (by vault-relative path; the applier does not resolve absolute paths so @@ -645,13 +646,13 @@ export interface ApplyPushResult { } /** - * THIN IO applier for the COMMON push cases (create/update/delete). Exercised - * via FAKES only in this increment — there is no live wiring. + * THIN IO applier for the COMMON push cases (create/update/delete). Runs LIVE in + * production (via runPush -> runCycle -> orchestrator); tests drive it with FAKES. * * - UPDATE: read the file body, then `client.importPageMarkdown(pageId, body)`. * This is the collab/Yjs write path (SPEC §2/§15.6) — NEVER a raw jsonb - * overwrite. The full self-contained markdown (meta + body) is sent as-is; - * `importPageMarkdown` parses the meta/body itself. + * overwrite. The file's markdown is sent as-is; `importPageMarkdown` parses + * the meta/body itself. * - CREATE: derive title/spaceId/parentPageId from the file's current meta, * `client.createPage(...)`, take the assigned pageId from the result, and * write it BACK as the file's `gitmost_id` frontmatter (re-serialized via @@ -1379,8 +1380,8 @@ function extractUpdatedAt(result: unknown): { updatedAt?: string } { // Docmost writes, NO ref advance); an explicit `--apply` is the ONLY path that // builds a client and mutates Docmost. // -// Every external effect is injected (`PushDeps`) so the whole orchestration is -// driven by FAKES in tests — no live Docmost, git, fs, or network. +// Every external effect is injected (`PushDeps`): production wires the live +// Docmost client, git, and fs; tests substitute FAKES through the same seam. /** * The human ("local") git identity used for engine-made commits on `main` in the diff --git a/packages/git-sync/src/engine/settings.ts b/packages/git-sync/src/engine/settings.ts index 4efc9834..4f4eda38 100644 --- a/packages/git-sync/src/engine/settings.ts +++ b/packages/git-sync/src/engine/settings.ts @@ -8,9 +8,6 @@ */ export type Settings = { - docmostApiUrl: string; - docmostEmail: string; - docmostPassword: string; docmostSpaceId: string; vaultPath: string; gitRemote?: string; diff --git a/packages/git-sync/test/cycle-roundtrip.test.ts b/packages/git-sync/test/cycle-roundtrip.test.ts index a4c54807..b1d7ca40 100644 --- a/packages/git-sync/test/cycle-roundtrip.test.ts +++ b/packages/git-sync/test/cycle-roundtrip.test.ts @@ -31,9 +31,6 @@ async function gitAvailable(): Promise { function makeSettings(vaultPath: string): Settings { return { - docmostApiUrl: "https://docmost.example.com", - docmostEmail: "you@example.com", - docmostPassword: "secret", docmostSpaceId: "space-1", vaultPath, pollIntervalMs: 15000, diff --git a/packages/git-sync/test/redteam-push-cycle.test.ts b/packages/git-sync/test/redteam-push-cycle.test.ts index dc70e2d4..7edb4227 100644 --- a/packages/git-sync/test/redteam-push-cycle.test.ts +++ b/packages/git-sync/test/redteam-push-cycle.test.ts @@ -15,9 +15,6 @@ import { serializePageFile } from '../src/lib/page-file'; function makeSettings(): Settings { return { - docmostApiUrl: 'https://docmost.example.com', - docmostEmail: 'you@example.com', - docmostPassword: 'secret', docmostSpaceId: 'space-1', vaultPath: '/vault', pollIntervalMs: 15000, diff --git a/packages/git-sync/test/run-push-realgit.test.ts b/packages/git-sync/test/run-push-realgit.test.ts index a550c9b2..0dc36aed 100644 --- a/packages/git-sync/test/run-push-realgit.test.ts +++ b/packages/git-sync/test/run-push-realgit.test.ts @@ -33,9 +33,6 @@ async function gitAvailable(): Promise { /** A minimal valid Settings fixture (only fields runPush reads matter). */ function makeSettings(vaultPath: string): Settings { return { - docmostApiUrl: 'https://docmost.example.com', - docmostEmail: 'you@example.com', - docmostPassword: 'secret', docmostSpaceId: 'space-1', vaultPath, pollIntervalMs: 15000, diff --git a/packages/git-sync/test/run-push.test.ts b/packages/git-sync/test/run-push.test.ts index 43cfb622..c5c4f060 100644 --- a/packages/git-sync/test/run-push.test.ts +++ b/packages/git-sync/test/run-push.test.ts @@ -18,9 +18,6 @@ function fileFor(pageId: string, body = 'body'): string { /** A minimal valid Settings fixture (only fields runPush reads matter). */ function makeSettings(): Settings { return { - docmostApiUrl: 'https://docmost.example.com', - docmostEmail: 'you@example.com', - docmostPassword: 'secret', docmostSpaceId: 'space-1', vaultPath: '/vault', pollIntervalMs: 15000,