After merging develop, git-sync's markdown converter still predated two
editor features and silently dropped them on every sync:
- Spoiler mark (#259): had no schema entry, so `<span data-spoiler>` from
the canonical lossless form re-parsed as plain text and the mark was
lost. Register a Spoiler mark mirroring @docmost/editor-ext + the MCP
schema, and emit `<span data-spoiler="true">…</span>` on PM->MD.
- Image caption (#221): the converter assumed no caption attribute existed
(the branch was dead). The image schema now carries a plain-text caption
(data-caption); register it and route a captioned image through the raw
<img> form (same lossless convention as the other Docmost image attrs).
Caption-less images keep the lighter `` form.
Both survive PM->MD->PM unchanged; raw `<span data-spoiler>` / `<img
data-caption>` in incoming markdown parse back. New round-trip tests in
markdown-roundtrip-spoiler-caption.test.ts; schema-surface snapshot updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F8: extract emitMoveWithBody helper (renamesMoves + body update with
basePath=oldPath) and call it at all three rename emission sites (ghost-move
A, ghost-move M, R/C) — byte-identical behavior, single F4 rationale. Helper
placed above computePushActions so the planner JSDoc stays attached.
F7: add an M-side ghost-move test (D+M same pageId) asserting the move and the
body update carry basePath=oldPath — the previously-untested branch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F4: a rename/move + body edit in one diff used to lose the edit (renamed pages
went only into renamesMoves, never updates). Now computePushActions also
emits an updates entry for renames, AND threads the OLD path via a new
UpdateAction.basePath so applyPushActions resolves the 3-way merge base from
the pre-rename file. Without it the base lookup at the new path returns null
and degrades to a 2-way merge that rolls back concurrent Docmost edits; with
it the edited block wins while a concurrent edit to another block survives.
A plain (status M) update carries no basePath and is byte-identical to before.
F5: test the CREATE path stripping conflict markers (autoMergeConflicts on).
F6: .env.example documents GIT_SYNC_REMOTE_TEMPLATE as deferred/inert scaffolding.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
F2: the real-git modify/delete null-edge test docstring overclaimed it
caught loss of the `?? theirs` fallback end-to-end. git itself leaves
theirs in the working tree (stage 3) so commitMerge's `git add -A` would
stage it even with the bug — the assertions pass on broken logic. Reword
to state it verifies the clean-merge happy path; the real F1 regression
guard lives in the fake-fs apply-pull-actions.test.ts.
F3: fill the `round-?` placeholder with `round-2` in both new blocks to
match the file convention (header: 'QA #119 round-2').
Comment-only; no production or test-logic changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The genuine-conflict branch in applyPullActions resolves to `ours ?? theirs`,
but the two stages where a side is ABSENT had NO test — the existing conflict
tests only fed stages where both ours and theirs are non-null. This is the
data-preservation core on the published `main`: a regression (dropping the
`?? theirs`, or wrongly writing on both-null) would silently lose a surviving
Docmost edit or resurrect a both-deleted page.
Adds four tests:
- apply-pull-actions.test.ts (fake-git, controlled stages): modify/delete
(ours=null, theirs!=null -> keep THEIRS) and delete/delete (both null ->
write nothing, deletion staged by commitMerge's `git add -A`).
- pull-conflict-normalize.test.ts (real-git 3-way): modify/delete built by
deleting on main + modifying on docmost (stage 2 absent -> theirs kept,
committed clean, no markers); delete/delete built via a rename/rename(1to2)
on the shared base file, which records the original path as both-deleted
(stages 2 AND 3 absent -> nothing written, deletion committed off main).
Production logic at pull.ts:487-497 held — pure test-coverage fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three more git-sync QA defects from the 2nd live pass on PR #119, plus a
callout-fidelity nit:
1. SPURIOUS conflict leaked raw markers into canonical main (root cause). On an
ordinary round-trip the only difference between the docmost mirror (normalize-
on-write) and a user's raw push is trailing/empty-line normalization, which made
git's line-based docmost->main merge CONFLICT, and the wedge fix then committed
the file WITH literal <<<<<<< / ======= / >>>>>>> markers onto main (git and the
DB silently diverged for cycles). Fix: on a conflict, normalize trailing/empty
lines on BOTH sides (showStage :2:/:3:) before comparing — a trailing-only diff
is recognized as spurious and resolved to the clean normalized form. A GENUINE
same-block conflict is auto-resolved to OURS (git wins, mirroring the live-doc
3-way rule); the docmost side stays on the `docmost` branch + page history. Raw
markers NEVER reach main again.
2. Concurrent UI<->git edit silently lost the UI side. The git->Docmost 3-way merge
ran against a live Y.Doc that hadn't yet received the user's debounced in-flight
edit, so git clean-applied (no conflict detected) and the edit vanished even on a
different block. Fix: flush the pending debounced store before the merge so the
in-flight edit is drained into the live doc first — a different-block edit is
merged, a same-block one is detected and pinned to history (recoverable).
3. Smart-HTTP HEAD flapped to the read-only `docmost` mirror (~1/4 of clones). The
engine transiently checks out `docmost` mid-pull and the host advertises whatever
HEAD resolves to. Fix: VaultGit.pinHeadToMain(); the cycle restores HEAD->main in
a finally; and the upload-pack ref advertisement is served HEAD-pinned under the
per-space lock so it can never observe a mid-cycle HEAD.
4. (callout) clampCalloutType now mirrors the editor's GITHUB_ALERT_TYPE_MAP for
non-schema aliases (tip->success, caution->danger, important->info) instead of
flatly collapsing to info. The editor schema genuinely supports only the six
banner types, so unknown types still fall back to info (by design).
Tests: deterministic real-git trailing-blank round-trip (no conflict, no markers,
in sync over 2 cycles) + genuine-conflict no-marker-leak; HEAD advertisement
stability; pre/post-flush concurrent-edit survival; serveReadAdvertisement lock
pin; widened callout-alias coverage. Engine vitest + server tsc + collaboration /
git-http / orchestrator specs all green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bug #1 (push 503 starvation): an external receive-pack that briefly overlapped
a poll cycle immediately 503'd because the per-space single-writer lock was
held. Add a BOUNDED retry-acquire on the PUSH path only (SpaceLockService
.withSpaceLock acquireRetry: capped exponential backoff up to ~5s); a transient
overlap now waits and succeeds, a genuinely stuck cycle still 503s after the
bound. The poll cycle passes no retry (immediate skip). Push result stays
deterministic: the receive-pack only runs once the lock is held, so a 503 never
leaves a half-applied ref.
Bug #2 (concurrent-edit marker leak + silent same-block loss):
- Marker leak (a): the push UPDATE path stripped markers for the body sent to
Docmost but left raw <<<<<<</>>>>>>> committed on the published `main` vault
forever (autoMergeConflicts ON). Now the cleaned body is written back to the
vault file + recorded in writtenBack so runPush commits it on `main` and the
vault converges to clean bytes.
- Marker leak (b): pin merge.conflictStyle=merge in ensureRepo and teach
stripConflictMarkers/hasConflictMarkers about the diff3 `|||||||` base section
(drop the marker AND the stale base region) so diff3/zdiff3 conflicts can
never leak `|||||||` + base content into a page. Also scrub the 3-way merge
BASE markdown.
- Silent same-block loss: the block 3-way merge still resolves same-block
conflicts deterministically to git, but it is no longer silent: diff3Plan now
reports a conflict count (mergeXmlFragments3WayWithStats), gitSyncWriteBody
logs it, and the persistence boundary-snapshot now fires for git-sync writes
over a non-git-sync baseline so the human's pre-merge content is preserved in
page history (recoverable). Full both-preserved persisted-conflict UI remains
the deferred redesign.
Tests: space-lock bounded-retry (success/stuck/poll-immediate); push vault-clean
+ diff3 ||||||| strip; ensureRepo conflictStyle pin; diff3Plan/3-way conflict
counts; persistence git-sync boundary snapshot. Server tsc clean; git-sync
vitest + server collaboration/git-sync jest all green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Blocking (review id 2514):
- [security] Forbid symlinks in vaults. ensureServable now sets
core.symlinks=false in each vault's local git config (a pushed symlink is
checked out as a plain file, never a real link), and the engine cycle wraps
every read/write/mkdir in an lstat/realpath guard (new path-guard.ts) that
refuses a path that is — or traverses — a symlink, or whose realpath escapes
the vault root. Prevents a writer from publishing /etc/passwd or the server
.env, or writing outside the vault. Adds unit tests (path-guard.test.ts) +
a read-guard integration test (cycle.test.ts) + real lstat/realpath in the
roundtrip integration test.
- [simplification] Delete dead lib/diff.ts + test/diff.test.ts and drop the
now-unused @fellow/prosemirror-recreate-transform dependency.
- [documentation] Add a CHANGELOG [Unreleased] → Added entry for git-sync.
Warnings:
- [test-coverage] Cover the CREATE-branch conflict-markers guard (a new .md with
markers and no gitmost_id is recorded as a create failure, never created).
Suggestions:
- [stability] Bound each `git config` in ensureServable with a timeout.
- [authz] Trigger endpoint resolves spaceId workspace-scoped and 404s a foreign
space before any vault directory is created.
- [stability] Attribute git-initiated moves to the service account
(lastUpdatedById), via an optional actor param on PageService.movePage.
- [documentation] Document the per-space autoMergeConflicts toggle in AGENTS.md.
- [test-coverage] Cover the unterminated `:::` callout fence fallback.
- [simplification] Move test-only roundtrip-helpers.ts out of src/ into test/.
Architecture:
- Move the Yjs/ProseMirror merge primitives (yjs-body-merge, three-way-merge,
lcs + specs) into collaboration/merge/, breaking the collaboration →
integrations/git-sync dependency cycle this PR introduced.
- Port the schema-surface drift gate to packages/mcp (the mcp schema mirror had
none); pins 52 entries.
Deferred (with rationale in the review thread): the incremental-pull perf
warning (correctness-neutral; needs a high-water-mark design + its own tests on
the data-loss-critical path) and the redis-sync rolling-deploy mixed-version
edge (the deficient behavior is in already-released old-instance code; the new
code is correct on both sides; impact is a transient rollout-window artifact).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Security (must-fix):
- /git smart-HTTP gate: an authenticated NON-member of a git-sync space now gets
404 (not 403), so the 403<->404 difference can no longer be used to brute-force
which spaces exist / have git-sync enabled. 403 is reserved for a MEMBER who
lacks the required role (existence already known). New gate input
userIsSpaceMember; decision-table + service specs extended.
Config (must-fix):
- Remove the dead GIT_SYNC_SSH_KEY_PATH knob (getter + validation field + two
.env.example lines) — it had zero consumers and advertised a nonexistent push
capability.
Stability/docs (warnings):
- Wire the lost-lock AbortSignal into runReceivePack -> git http-backend so the
receive-pack child is killed if the per-space lock lapses mid-write.
- Raise the divergent-`docmost` (invariant §5) push refusal from info -> warn and
surface divergentDocmost in the run status (/status).
- Comment the stale read-after-debounced-collab-write updatedAt in
importPageMarkdown (deferred §10 loop-guard must not trust it).
- Fix the Dockerfile comment: the loader uses require.resolve + dynamic import(),
it deliberately does NOT require('@docmost/git-sync').
- Merge the two near-identical space toggle handlers into one parameterized
handler; add the 2 missing en-US i18n keys for the auto-merge switch (ru-RU not
maintained for these git-sync strings, mirrored).
Tests:
- isGitSyncHttpEnabled() default-branch (unset -> isGitSyncEnabled fallback).
- agentSourceFields 'git-sync' case (source stamped, chat key omitted).
- editor-ext name-level schema contract (vendored mirror superset of editor-ext
node/mark types) + the new shared resolver + non-member 404 gate cases.
Architecture:
- Extract resolveRequestWorkspace shared by DomainMiddleware + GitHttpService
(the two real self-hosted/cloud copies; McpService has no cloud branch).
- Document the in-process setInterval multi-replica limitation + BullMQ/fencing
future direction (deferred, not implemented).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
Addresses review 1863 (delta) on PR #119.
MUST-FIX:
- detailsToHtml (the raw-HTML path used for a details nested inside
columns/spanned cells) now emits `<details${open}>`, mirroring the
top-level case, so `open` no longer silently drops every round trip.
- Remove the dead `resolveApplyClient` delete-cap hook from the engine
`runCycle`: the orchestrator stopped passing it, so the hook + its
dry-run pass were inert. Deletes are soft (Trash) + always logged and
engine convergence is the guard, so no cap is re-added — just the dead
wiring removed.
TEST COVERAGE:
- space-lock: heartbeat refresh CAS-miss (eval -> 0) and Redis-error
(eval throws) both abort the in-flight fn's signal.
- cycle: a pre-aborted signal (and an abort during the pull read) throws
before the push apply / first destructive phase.
- converter: htmlEmbed source VALUE + height survive; encode/decode
UTF-8 symmetry and '' -> ''; footnote definition body + ref/def id
match; transclusionReference both ids survive; fix the bad
transclusionSource fixture (wrong `pageId` attr + empty content ->
schema `id` + a block child); nested details `open` parity test.
- orchestrator: autoMergeConflicts:true reaches engine settings; default
false on a missing settings row.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Callouts now export as Obsidian's blockquote-callout syntax — `> [!type]` opener
plus a `>`-prefixed body — so they render as real callouts when the vault is
opened in Obsidian, instead of `:::type` (Docusaurus-style) which Obsidian shows
as a plain blockquote.
- Export (markdown-converter `case "callout"`): `> [!type]` + each body line
blockquote-prefixed (a blank line becomes a bare `>` so the callout is not
split). Nested callouts naturally become `> > [!type]`.
- Import (preprocessCallouts): a new branch recognizes `> [!type]` openers and
the contiguous `>`-prefixed body, strips one blockquote level and recurses (so
nested callouts work), emitting the same callout div the `:::` path produces.
The legacy `:::type` parser is KEPT so existing vaults keep importing. A plain
blockquote (no `[!type]`) stays a blockquote.
Tests: 4 converter golden tests updated to the new `> [!type]` output; 4 new
import tests (simple, nested, round-trip, plain-blockquote-untouched). The §13.1
gate still round-trips callout losslessly through the real server schema.
git-sync vitest 675 (+1 expected-fail), gate 27.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Found proactively by deepening the round-trip test from node-TYPE survival to
ATTRIBUTE fidelity (distinctive attr values per node). Two real losses (the
other 3 candidates — mathInline/mathBlock/pageEmbed — were verified to be
correct; the probe had used wrong attr names):
- subpages `recursive`: the converter emitted a bare div and the schema mirror
didn't model the attr, so a recursive subpages reverted to non-recursive on a
round trip. Now emits `data-recursive="true"` and the mirror parses it back
(matching @docmost/editor-ext).
- details `open`: the `open` (collapsed/expanded) state lives on the details
node, but the converter emitted the `<details>` wrapper from the summary case
without it, so the state was dropped. The wrapper now carries `open`.
The round-trip test now also asserts attribute fidelity (12 cases) so these are
locked. Schema-surface snapshot updated for the new subpages attr.
git-sync vitest 671 (+1 expected-fail), §13.1 gate 27.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
subpages exported to the literal `{{SUBPAGES}}`, which has no markdown/HTML
inverse, so on re-import it came back as a plain paragraph holding the visible
text "{{SUBPAGES}}" — the embed rendered as that literal string on the page
after a sync (round-trip data loss, seen live). It now emits the schema-matching
`<div data-type="subpages">` like every other embed node, so the schema's
parseHTML rebuilds the subpages node. Also dropped the leaf-atom content-hole
in the subpages renderHTML.
New committed regression coverage:
- packages/git-sync/test/roundtrip-all-nodes.test.ts — exhaustive serialize ->
deserialize round trip for ALL 40 node/mark types; each asserts the node/mark
survives and no `{{...}}` literal leaks. This is the test that caught subpages.
- §13.1 gate (git-sync-converter-gate.spec.ts): subpages added to the green
corpus (round-trips through the REAL server schema).
- Corrected two PR-authored tests that asserted the old {{SUBPAGES}} loss as
"by design" — they now assert the fixed round trip.
Also folds in review #1679 coverage-gap tests (no prod change): orchestrator
pollTick/enabledSpaces, datasource 3-way merge dispatch, page.repo
last_updated_source provenance SQL.
git-sync vitest 659 (+1 expected-fail), server tsc clean, server specs green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Red-team #13 (conflict markers reaching Docmost) is now a per-space policy
exposed as a UI toggle, instead of a hardcoded behavior. New boolean
`gitSync.autoMergeConflicts` (default FALSE), mirroring the existing per-space
`gitSync.enabled` flag end-to-end (jsonb space settings -> update-space DTO ->
space.service -> client types -> space settings form switch):
- OFF (default, safe): a page whose committed body still has unresolved git
conflict markers is NOT pushed — it is recorded as a per-page push FAILURE
("unresolved conflict markers — resolve in git first"). Recording a failure
(not a soft skip) deliberately HOLDS refs/docmost/last-pushed so the conflict
commit is never marked pushed and a later pull cannot clobber the user's
in-progress resolution; the page retries until the conflict is resolved in git.
- ON: the marker lines are stripped and both sides' content is pushed (the prior
behavior), so the conflict becomes visible/fixable inside Docmost.
The engine Settings carries `autoMergeConflicts`; runPush threads it into the
update AND create paths. The orchestrator's buildSettings reads the per-space
flag from jsonb (strict opt-in like `enabled`, default false).
Tests: redteam-push-cycle #13 rewritten (default -> not pushed + failure + refs
held; ON -> strip-and-push); space.service + edit-space-form + orchestrator
specs extended. git-sync vitest 618, server jest space+git-sync 163, client
edit-space-form 11, server/client tsc clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A 10-agent red-team pass on the two-way Docmost<->git sync surfaced 16 ranked
findings (9 others triaged out as already-defended). Wrote a reproduction test
per finding (each asserts the CORRECT behavior, so it fails on the bug), then
fixed the production code so every repro goes green. All confirmed bugs:
Round-trip data loss (markdown-converter.ts + docmost-schema.ts mirror):
- #1 editor-ext node types silently dropped on export — ported the 8 missing
canon nodes (footnoteReference/footnotesList/footnoteDefinition, htmlEmbed,
status, pageEmbed, transclusionSource/Reference) into the git-sync schema
mirror and added converter cases that emit their schema-matching HTML instead
of flattening unknown nodes to '' (this was the critical data-loss flagged in
review #1679: footnotes/htmlEmbed lost on sync). Snapshot surface updated.
- #2 top-level image lost width/height/align/attachmentId — now emits an HTML
<img> (like video/diagrams) when it carries layout attrs; bare images stay
. Image node parses width/height as strings so they re-import.
- #3 code block containing a ``` fence corrupted on round-trip — outer fence is
now widened to (longest-inner-backtick-run + 1).
- #16 deep nesting threw RangeError (page never synced) — added a depth guard
(MAX_NODE_DEPTH=400) so the converter never overflows the stack.
Push/layout/cycle (engine):
- #4 disambiguation ' ~slugId' suffix corrupted Docmost titles + order-dependent
layout — deterministic, order-independent sibling disambiguation; suffix is
stripped from a path-derived title ONLY when the new name is exactly the old
title plus the suffix (never a genuine retitle ending in ' ~token').
- #6 retry-adopt by (parent,title) clobbered the wrong duplicate-title sibling —
ambiguous (parent,title) is no longer adopted (falls back to fresh create).
- #12 a new child under a new parent was created at ROOT — creates are ordered
parent-before-child with an in-memory created-id map for parent resolution.
- #13 git conflict markers could reach Docmost — bodies are scanned and the
marker lines stripped (a '=======' line is only treated as a conflict
separator inside a <<<<<<< ... >>>>>>> block, so setext headings are safe).
- #15 a divergent `docmost` mirror was escalated by runPush but dropped by
runCycle — RunCycleResult now forwards divergentDocmost to the orchestrator.
Server (merge / lock / provenance):
- #9 3-way merge lost a human's block edit when git inserted an adjacent block —
finer-grained diff3 region merge (via lcs) preserves non-overlapping human
edits; genuine same-block conflicts still resolve git-wins.
- #10 single-writer race — module-static liveLocks closes the same-process TOCTOU
window, and a heartbeat refresh that cannot confirm the lock now aborts the
cycle at its next write checkpoint (cooperative AbortSignal threaded through
runCycle). Cross-process fencing tokens remain a follow-up.
- #14 sticky-agent provenance overrode an explicit actor='git-sync' write,
blinding the listener loop-guard — resolveSource now lets an explicit actor
win over the sticky-agent fallback (explicit agent still wins).
Verified: git-sync vitest 617 pass (+1 expected-fail), server unit jest 1541
pass, server tsc clean. A review pass over the fixes caught and corrected a
title-suffix over-strip, an inert abort signal, a document-wide conflict-marker
strip, and two leaf-atom content-holes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
build/ is gitignored and compiled in CI/Docker; a few files leaked back into
the tree while replaying commits onto develop. Remove them so the package keeps
a single source of truth (src/).
Resolve the code-review findings from comment #1571 on PR #119.
Engine (packages/git-sync):
- Idempotent CREATE on retry: before createPage, look the page up in the
live Docmost tree by (parentPageId, title) and ADOPT it instead of
duplicating when a prior cycle created it but failed to persist the
pageId back to disk. Only trust a COMPLETE tree for the lookup; fall
back to createPage otherwise. Covered by new tests incl. a complete=false
regression-lock.
- Route applyPullActions diagnostics through an injected logger instead of
bare console (thread log from the cycle).
- Add a timeout to the git execFile chokepoint (runRaw) so a hung git
subprocess cannot wedge a sync cycle.
- Translate remaining Russian code comments to English.
- Remove dead standalone-CLI code (parseArgs/PushParsedArgs,
parseSettings/envSchema, loadSettingsOrExit + config-errors.ts) and the
matching index exports/specs; keep the Settings type.
- Fix the dangling docs link in package.json.
- Add a schema-surface snapshot guard so any drift in the vendored
document schema is a loud, must-review CI failure (+ provenance header).
Server (apps/server):
- Add a configurable watchdog timeout to the spawned git http-backend so a
stalled push cannot hold the per-space lock forever
(GIT_SYNC_BACKEND_TIMEOUT_MS).
- Close the in-process TOCTOU window in SpaceLockService.withSpaceLock by
reserving the slot synchronously before acquire.
- Add tests: removePage git-sync provenance (both branches), ensureServable
force-push-protection git configs, and the phase-B+ datasource methods.
Docs / build:
- AGENTS.md: list git-sync as the fifth workspace package and note the
three schema mirrors; fix the dangling git-sync-plan.md backlog link.
- pnpm-lock.yaml: add the missing @docmost/git-sync workspace link so
pnpm install --frozen-lockfile (CI default) succeeds.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes the architecture item from the #119 review: drop the "vendored from
docmost-sync" framing and the CJS↔ESM `Function('import()')` bridge so the engine
is a normal first-class gitmost package.
Part 1 — vendoring markers removed (prose only, zero behavior change): reworded
"VENDORED into gitmost" / "vendored from docmost-sync" / "Engine LOGIC is
byte-identical" / "it's a port" comments across the engine. Behavior-bearing
strings are untouched: BOT_AUTHOR_NAME/EMAIL and the `Docmost-Sync-Source:`
provenance trailers (changing them would break git authorship + the loop-guard).
Part 2 — the package is now ESM (matching the sibling @docmost/mcp): `type: module`,
tsconfig Node16, `.js` extensions on relative imports, and a static
`import { marked }` replacing the `new Function('return import(...)')` /
`loadMarked` hack — the bridge is GONE from the package. The CommonJS NestJS
server loads the now-ESM engine via a new `git-sync.loader.ts` that mirrors the
existing `docmost-client.loader.ts` mcp loader exactly (Function-indirected
dynamic import + cached promise + retry-on-reject). The 4 server consumers
(orchestrator/datasource/vault-registry/git-http-backend) call `await loadGitSync()`
for value exports; types stay `import type` (erased). The converter-gate spec —
which needs the real converter — loads the package's TS source via a jest
moduleNameMapper + isolatedModules (documented in that spec); the other git-sync
specs mock the loader.
Verified: engine builds pure ESM (no Function/require leftover), vitest 614,
editor-ext build, server + client tsc, full server jest 1397/0. Live stand
smoke-test: server starts clean on the ESM engine (no ERR_REQUIRE_ESM), a real
sync cycle runs through the loader, and the basic e2e suite is 12/12 (clone via
git-http-backend, push, pull, delete, 3-way merge — all through the new loader).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Self-review of phase 3 caught a data-corruption regression: nativeMeta always
supplies the run's spaceId, so the planner's 'create-without-spaceId' skip — which
had doubled as the only filter for non-page files — went dead. An ADDED
.obsidian/*.json, attachment, or dotfile (committed to the vault, no .gitignore)
would then be classified as a CREATE: a junk Docmost page, plus a gitmost_id
frontmatter written INTO the file, corrupting it.
Fix: isPageFile(path) — a .md file with NO dot-segment anywhere — and filter the
diff to page files at the very top of computePushActions, BEFORE any
classification, so non-page A/M/D/R are ignored (design §Адопция). 2 unit tests
pin it (.obsidian/json, attachment, dotfile, dot-segment, .md dotfile all ignored;
real pages still created). 614 engine tests green.
Also: refreshed stale docmost:meta comments to gitmost_id (review SUGGESTION), and
documented the deferred adoption frontmatter-preservation gap (review WARNING) in
page-file.ts + the design doc (do NOT roll native onto a real vault with Obsidian
properties until phase 4 round-trips them).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PUSH now consumes the native-Obsidian format end-to-end:
- identity from the gitmost_id frontmatter (parsePageFile), not docmost:meta;
- title from the FILENAME, parentPageId from the enclosing folder's folder-note
(parentFolderFile is now FOLDER-NOTE aware: a child's parent is dir/dir.md, and
a folder-note's own parent is one level up), spaceId from the run (every vault
file belongs to the vault's space);
- CREATE derives title/parent/space from path + run and writes the assigned
pageId back as gitmost_id frontmatter (serializePageFile);
- UPDATE pushes the STRIPPED body (current + 3-way-merge base), so the frontmatter
never leaks into Docmost content; the loop-guard hashes the body.
The PURE delete-sensitive classifier (computePushActions/classifyRenameMoves) is
UNCHANGED — only the injected IO resolvers (metaAt, parent, create write-back)
switched source. nativeMeta always carries the run spaceId, so the legacy
'create-without-spaceId' skip no longer fires through runPush.
Tests rewritten to native fixtures + folder-note parent paths; the noop case is
now a child under a renamed parent folder (filename=title, so a path-only-noop
needs an ancestor rename). parentFolderFile tests cover leaf/folder-note/nested/
dotted. 612 engine tests green; engine rebuilt.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PULL now serializes each page as the native-Obsidian format (serializePageFile:
a minimal gitmost_id frontmatter + the fixpoint markdown body) instead of the
heavy docmost:meta envelope. title/parent/space are derived (filename / folder /
repo), so only the pageId is persisted. readExisting recovers identity from the
gitmost_id frontmatter (parsePageFile) instead of docmost:meta.
Extracted stabilizePageBody() (the export->import->export fixpoint, no meta) so
the native writer and the legacy serializer share the same deterministic body —
re-pulls of an unchanged page stay byte-identical (loop-guard).
Tests: read-existing fixtures rewritten to gitmost_id; apply-pull asserts the
written text is native frontmatter and carries NO docmost:meta (regression
guard). 611 engine tests green.
NOTE: PUSH still reads docmost:meta — the end-to-end cycle is intentionally NOT
runnable until phase 3 (PUSH reads frontmatter + derives title/parent from path)
lands; no vault is wiped/deployed until then.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Native-Obsidian structure: a page WITH children now lives at its folder-note
<name>/<name>.md (LostPaul Folder Notes convention) with its children alongside;
a leaf stays <name>.md. Folder-notes claim their canonical path before a
same-named child, so the child (a leaf) is the one disambiguated, never the
folder-note — a folder X/ always contains its own note X.
Format-agnostic and safe in isolation: only the destination PATH changes, the
file content/serialization is untouched, so an existing parent relocates via the
move-by-id path (no delete). The frontmatter format flip (pull+push) is next.
6 new layout unit tests (leaf / parent / nested / child-named-as-parent /
twin-parents / childless). 611 engine tests green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per owner: test data, no migration. parsePageFile no longer reads the old
docmost:meta block — a file without a gitmost_id frontmatter is simply un-tracked
(adopt). Vaults are a cache: rm -rf on the transition, rebuilt native from
Docmost. Simplifies the format work (no fallback). Doc updated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pivot the thin-meta design to "the vault IS a native Obsidian vault": clean
markdown + a minimal YAML frontmatter `gitmost_id:` (the durable pageId, travels
with the file so identity survives any move); folders mirror the page tree with
the parent's body as a folder-note `<Folder>/<Folder>.md` (LostPaul Folder Notes
convention); links as `[[wikilinks]]` (basename-resolved → reparent never breaks a
link, only retitle does); collisions disambiguated Obsidian-style; `.obsidian/`
and non-page files left untouched (no .gitignore). Verified the conventions
against the Obsidian/Folder-Notes docs.
Replaces the abandoned `.gitmost/index.json` sidecar (path-keyed → fragile to
git-undetected renames; the in-file id is self-sufficient): removes vault-index.ts.
Adds lib/page-file.ts — parsePageFile/serializePageFile (frontmatter id + clean
body) with a LEGACY `docmost:meta` fallback for migration. 6 unit tests; engine
suite green. Not yet wired into pull/push — no behavior change. Design doc
rewritten to the native-Obsidian format.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pure read/write/lookup for the vault sidecar index that will hold page identity
(pageId) + collision token (slugId) keyed by file path, so the .md files can be
clean markdown. parseVaultIndex is tolerant (missing/garbage/bad entries degrade
to empty/skipped — never crashes a cycle); serializeVaultIndex is deterministic
(sorted keys -> stable diffs, no churn). Lookups (pageIdAt, pathForPageId reverse,
trackedPageIds) + mutations (set/remove/move). NOT wired into pull/push yet — no
behavior change. 5 unit tests; engine suite green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to 4376c5a6, found by a real BROWSER e2e (the flow the in-diff fix
missed). When the layout reshuffle's two halves land in SEPARATE sync cycles, the
later cycle's diff has only the DELETE of the old path — the matching add was
already pushed — so in-diff D+A coalescing can't see it, and the live page was
still trashed.
Robust fix on the identity invariant the reviewer (and the user) called out: a
page EXISTS iff its pageId is in the vault, regardless of filename. runPush now
collects the pageIds present at ANY path in the current `main` tree and passes
them to computePushActions; a deleted file whose pageId is still tracked
elsewhere is a MOVE, never a deletion. (Built only when the diff has deletes.)
Adds apps/server/test/git-sync-browser-e2e.cjs — a Playwright test that drives the
REAL Docmost web UI: log in, create several untitled pages, type a title, sync,
assert NOTHING is trashed. Reproduced the data loss before this fix; 5/5 green and
stable after. Engine suite 600 green (+2 computePushActions cases:
pageId-still-present -> skip; pageId-gone -> real delete).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CRITICAL data-loss bug: creating pages in Docmost (which start UNTITLED) and then
typing a title could soft-delete OTHER pages. Untitled pages all serialize to the
`_` fallback filename; the layout disambiguates them (`_.md`, `_ ~slug.md`).
Retitling one frees the bare `_` and another untitled page's file relocates into
it. git's rename detection (`-M`) can't see the move (the tiny meta-only files are
too dissimilar), so `git diff` reports it as DELETE(old) + ADD/MODIFY(new). The
push took the DELETE literally and trashed a live page.
Root cause is that the push trusted git's path-level rename heuristic for page
IDENTITY. Identity is the pageId. Fix: before emitting any delete, coalesce by
pageId — a pageId that is BOTH deleted (pre-image) AND present on the surviving
side (current meta of an ADD or a MODIFY, since a relocation into an occupied path
shows as M) is one page that MOVED, classified as a rename/move and NEVER a delete.
Reproduced + verified on a live stand: 4 untitled pages + retitle one trashed a
different page before; after the fix, retitling one (and stress-retitling all)
trashes nothing. Engine suite 598 green; 3 new computePushActions cases (ghost
D+A move -> rename; real delete still deletes; unrelated D+A stay delete+update).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The reconcile choreography (ensureRepo -> merge-check -> ensureBranch ->
checkout('docmost') -> pull -> push) was hand-rolled in the app orchestrator's
driveCycle, duplicating an order the vendored engine owns and could drift from on
upgrade — the failure mode is data clobber. Lift it into @docmost/git-sync as a
single entry point, `runCycle(deps)`. The orchestrator now calls runCycle and
keeps only the lock (its caller) and the gitmost-specific delete-cap POLICY,
injected as the `resolveApplyClient` hook (the engine does the dry-run, hands the
hook the planned delete count — Infinity if planning failed — and uses whatever
client it returns for the apply). driveCycle drops from ~150 lines to ~30.
Tests:
- engine test/cycle.test.ts: composition (merge-in-progress short-circuit;
ensureRepo->ensureBranch->checkout staging order before the pull; the cap hook
is consulted with the planned count; no dry-run when no hook).
- engine test/cycle-roundtrip.test.ts: runCycle against a REAL VaultGit in a temp
repo with a faked Docmost client — a git-originated CREATE flows pull->push and
the assigned pageId is written back; an unresolved merge short-circuits before
any client call.
- orchestrator spec rewired to mock runCycle and assert the wiring + the
resolveApplyClient cap policy (the engine-internal cycle-order/merge tests moved
to the engine).
Validated end to end on a live stand (real Postgres/Redis + server): a git clone
-> edit -> push over the /git remote round-trips the change into the Docmost page
through the refactored cycle.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The implementation spec docs/git-sync-plan.md was removed as completed, but ~44
code comments still cited it as "plan §N". Strip those citations (comments only),
keeping each comment grammatical. The vendored engine's own "SPEC §N" references
point at a different, still-present spec and are left untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Upgrades the 2-way body merge to a real diff3 three-way merge (review #5), so a
block ONLY the human changed is KEPT when git changed a DIFFERENT block — the
2-way merge would revert it to git's stale version.
Engine: the push update loop reads the last-synced pre-image
(`git.showFileAtRef(refs/docmost/last-pushed, path)`) and passes it as the
optional `baseMarkdown` to `client.importPageMarkdown` (the common ancestor).
Server: gitmost-datasource converts base+incoming, and writeBody runs a block-
level diff3 (new three-way-merge.ts `diff3Plan`): live-only change -> keep live,
git-only change -> take git, both-changed -> git wins (conflict policy), inserts/
deletes from either side preserved. Without a base (createPage) it falls back to
the 2-way merge. Crash-safety unchanged (docs built before the connection opens).
Tests: three-way-merge.spec.ts (14 — every diff3 case incl. the cross-block
preservation and conflict policy), yjs-body-merge 3-way (real Y.Docs: human's
block instance preserved while git's block is applied), plus an engine test that
the base is forwarded from showFileAtRef. Existing push assertions updated for the
new base arg. git-sync 589 pass; server merge/datasource/gate 62 pass; typecheck
clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review finding #2: packages/git-sync/build/ (the COMPILED engine) and the
package's node_modules/ were committed. Prod executed the committed build/ while
CI/tests ran src/ and never rebuilt it — so a fix in src/ could pass tests while
stale compiled code shipped (a silent src/prod skew). The committed node_modules
were pnpm symlinks with a baked machine-local store path (/home/claude/...),
useless and misleading for everyone else.
- git rm --cached packages/git-sync/{build,node_modules} (42 + 31 files).
- .gitignore: ignore packages/*/node_modules/ and packages/git-sync/build/.
- Build the package where it is actually consumed: apps/server `pretest` now
builds @docmost/git-sync (its suite imports the built build/index.js), and the
CI Test workflow gains an explicit "Build git-sync" step. The Dockerfile builder
already runs `pnpm build` (nx builds the package) and now COPYs the fresh build/.
Verified: wiped build/, rebuilt via `pnpm --filter @docmost/git-sync build`, then
the server converter gate (26/26, imports the rebuilt package) and the git-sync
suite (588 passed) both pass against the freshly-built, non-committed output.
NOTE: packages/mcp/ has the same committed-build/node_modules pattern (pre-existing,
out of this PR's scope) and should get the same treatment in a follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Coder↔reviewer design loop (9 rounds, reviewer verdict: exhaustive) produced
92 specs; implemented +123 tests (465 -> 588 passing). The new round-trip
coverage exposed three genuine data-loss bugs in the Markdown<->ProseMirror
converter, all now FIXED (round-trip is lossless for these):
1. pageBreak was lost on export (no converter case -> rendered to "" and the
node vanished). Now emits <div data-type="pageBreak"></div>, which the schema
parses back -> round-trips.
2. A block image between blocks left an empty <p> artifact after import-hoisting,
producing a phantom blank-gap diff on every sync. markdownToProseMirror now
strips content-less paragraphs after generateJSON — with a schema-validity
guard that keeps the obligatory single empty paragraph in `content: "block+"`
containers (tableCell/tableHeader/blockquote/column/callout/doc), so empty
cells/quotes never become an invalid `content: []`.
3. The `code` mark combined with another mark was not byte-stable (emitted nested
HTML that the schema's `code` `excludes:"_"` collapsed on import). The
converter now emits code-only when `code` co-occurs, matching the editor.
New coverage spans media/diagram/details/columns/math/mention attribute
round-trips, converter emission branches, git error paths, and engine decision
branches. A dedicated test pins the empty-container schema validity (the review
catch on the bug-2 fix).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make @docmost/git-sync natively consumable by the CommonJS server (and jest):
build to CommonJS (tsconfig module CommonJS, drop type:module, strip .js from
relative imports), and lazy-load the only ESM-only dep (marked) via the dynamic
Function('import()') trick (mirrors docmost-client.loader.ts) with a require()
fallback so vitest's evaluator works too. git-sync tests stay green (314 pass,
3 expected fail).
Add the §13.1 idempotency gate (apps/server .../git-sync-converter-gate.spec.ts):
13 editor-ext docs (paragraphs/headings, marks, links, bullet/ordered/task lists,
blockquote, callouts, code block, hr, table, nested mix) round-trip
content(editor-ext) -> convertProseMirrorToMarkdown -> markdownToProseMirror ->
TiptapTransformer.toYdoc/fromYdoc(tiptapExtensions) -> canonicalize and assert
docsCanonicallyEqual. All green => the vendored converter's docmost-schema is
schema-compatible with editor-ext (no node/mark/attr loss), which the plan §13.1
requires before Phase B. The one intrinsic markdown-image lossiness (width/height
/align can't ride plain ) is isolated in a KNOWN DIVERGENCE block, not
hidden. Server tsc clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First step of docs/git-sync-plan.md. New workspace package @docmost/git-sync
vendoring the PURE parts from docmost-sync (HEAD b03eb35):
- lib: markdown-converter, markdown-document, canonicalize, docmost-schema,
node-ops, diff, and an extracted markdown-to-prosemirror (only the pure
marked->HTML->generateJSON path from upstream collaboration.ts; no websocket).
- engine (pure, no IO): reconcile, layout, sanitize, stabilize, loop-guard.
Ported the upstream pure-module + round-trip corpus tests (vitest): 314 pass,
3 expected upstream known-limitation fails. tsc clean. No server wiring yet.
docmost-schema inlines getStyleProperty (as packages/mcp does — @tiptap/core
3.20.4 doesn't export it). IO engine (pull/push/git/settings) deferred to later
Phase A/B steps; the editor-ext idempotency gate (plan §13.1) is the next step.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
claude code agent 227
a