WIP: feat(git-sync): native two-way Docmost↔git Markdown sync (Phases A–D, live-verified) #119
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#119
Reference in New Issue
Block a user
Delete Branch "feat/git-sync"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Фича: нативный двусторонний git-sync (страницы Docmost ↔ git-папка Markdown)
Отчёт по скиллу архитектора. Реализация спеки
docs/git-sync-plan.md(Фазы A→D), движок вендорен изdocmost-sync(HEADb03eb35).1. Задача
Встроить прямо в gitmost двусторонний синк страниц спейса ↔ локальный git-репозиторий Markdown: правки в Docmost доезжают в файлы и обратно, нативно (без REST-к-себе и сервис-юзер-токенов), с провенансом и безопасным single-writer при репликах.
2. Архитектура (что собрано)
Новый пакет
@docmost/git-sync— вендоринг чистого конвертера (ProseMirror↔Markdown) + чистого и IO-движка (reconcile/layout/sanitize/stabilize/loop-guard + pull/push/VaultGit/settings) из docmost-sync. Пакет ESM-only; CJS-сервер грузит его целиком через мост динамическогоimport()(git-sync.loader.ts, копированиеbuild/в образ — см. Dockerfile/AGENTS.md), а не черезrequire(). Движок взаимодействует с Docmost через узкий интерфейсGitSyncClient— единственный нетривиальный шов.Сервер (
apps/server/src/integrations/git-sync):GitmostDataSourceService— нативная реализацияGitSyncClient: чтение черезPageRepo/SpaceRepo; запись тела через collabopenDirectConnection('page.'+id, {actor:'git-sync'})(зеркалитwithYdocConnection'replace': drop fragment +TiptapTransformer.toYdoc(tiptapExtensions)+Y.applyUpdate); структурные мутации черезPageService(create/soft-delete/move с вычислением fractional-позиции/rename/restore).GitSyncOrchestrator— per-space Redis leader-lock (SET NX PX+ CAS-Lua release) + in-process мьютекс; цикл PULL→PUSH через движок; динамический poll-интервал из конфига (SchedulerRegistry).PageChangeListener—@OnEventPAGE_* → дебаунс → цикл, с loop-guard (lastUpdatedSource==='git-sync').'git-sync'(PersistenceExtension: приоритет agent>git-sync>user, debounced история; PageService штампует строку; AuthProvenanceData расширен).GIT_SYNC_*(EnvironmentService + валидация),GitSyncController(admin trigger/status),GitSyncModuleв app.module.http/git-http*.ts+ raw-route вmain.ts, вне/api-пайплайна Nest):git clone/fetch/push http://.../git/<spaceId>.git— собственный Basic-auth +FailedLoginLimiter(throttle до bcrypt, 429 без сигнала существования), резолюция воркспейса, CASL (read → fetch / Manage → push), security-гейт 404-не-403 (существование/enabled-статус спейса не палится не-участнику), push исполняется черезgit http-backendпод space-lock с abort по потере лока.settings.gitSync.enabled, jsonb-merge не затирает соседей, CASL Manage/Settings) + Switch в форме настроек спейса + i18n.gitв runtime-образе Dockerfile.Гейт §13.1 (блокировал Фазу B): 13 editor-ext документов идемпотентно проходят
content → convertProseMirrorToMarkdown → markdownToProseMirror → toYdoc/fromYdoc(tiptapExtensions) → canonicalize(docsCanonicallyEqual). Доказывает схема-совместимость вендорного конв��ртера с реальной схемой editor-ext.3. Безопасность (ключевые инварианты, проверены ревью)
Строгий per-space opt-in (НЕТ фоллбэка «все спейсы»); Redis-lock + мьютекс (single-writer); delete-cap (dry-run считает удаления, при превышении нейтрализует deletePage — fail-safe); soft-delete (не forceDelete); запись только через collab (не в колонку content); провенанс создаётся только in-process (не из клиентского токена); admin-trigger под JWT+CASL.
4. Найденные баги — статистика тестирования
Циклы живого e2e (autonomous, реальный стенд + Postgres/Redis, нашёл КРИТИЧЕСКИЕ баги):
UNDEFINED_VALUE, vault-папка названаundefined@IsUUIDна DTO (whitelist-pipe стрипал spaceId)checkout('docmost')перед pull → Docmost-контент писался в main и затирал правки; добавлен checkout + merge-guardЦикл комплексного ревью (review-субагент, весь серверный интегрейшн): APPROVE WITH SUGGESTIONS, критичных data-loss/security нет. Найдено 3 WARNING + 3 SUGGESTION → исправлено 5 (1 косметика пропущена):
Браузерная проверка (Фаза C): субагент — тоггл присутствует и персистит
settings.gitSync.enabled(reload + API), PASS.5. Тесты
@docmost/git-sync: 431 pass + 3 expected-fail (upstream known-limitations).nest buildок.6. НЕ вошло (осознанно отложено — честно)
GitmostDataSourceпротив тестовой БД — фундамент (харнесс F5) есть, сами тесты ��� следующий шаг.🤖 Generated with Claude Code
Closes #194
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>Native data plane for git-sync (plan §3, §8.1): - provenance: widen actor to 'user'|'agent'|'git-sync' (jwt-payload, auth-provenance decorator); PersistenceExtension resolves lastUpdatedSource with precedence agent > git-sync > user, debounced history (like a human edit, not the agent's immediate snapshot). - GitmostDataSourceService implements @docmost/git-sync's GitSyncClient natively: reads via PageRepo/SpaceRepo (listSpaceTree complete:true, getPageJson), writes via PageService (create/removePage soft-delete/movePage with computed fractional position/update-rename/restore) + the writeBody linchpin through collab openDirectConnection('page.'+id, {actor:'git-sync'}) mirroring collaboration.handler withYdocConnection 'replace'. bind({workspaceId,userId}) returns the context-bound client for the orchestrator. - 10 unit/contract tests (mapping + soft-delete + move-position), tsc clean. Known gap (closed in A.4b): PageService.create/update/movePage only branch on actor==='agent'; git-sync provenance is already passed through so the row source marker propagates once PageService honors 'git-sync'. Module/orchestrator/config come next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Comprehensive-review follow-ups (APPROVE WITH SUGGESTIONS; no critical issues): - poll interval is now actually configurable: replaced the hardcoded @Interval('git-sync-poll', 15000) with a dynamic SchedulerRegistry interval registered in onModuleInit from getGitSyncPollIntervalMs() (cleared in onModuleDestroy); /status and the real cadence now share one config source. Boots logging 'poll interval registered (Nms)'. - loop-guard now ALWAYS applies: the lastUpdatedSource==='git-sync' skip was nested inside the !spaceId/!workspaceId branch, so structural self-writes (CREATE/MOVE/RESTORE/SOFT_DELETE, which carry spaceId+workspaceId) bypassed it and re-triggered cycles. Fetch the page row once, guard unconditionally, then resolve space/workspace. - remove the dead PAGE_CONTENT_UPDATED subscription (it's a BullMQ job, never an EventEmitter event; body edits arrive via PAGE_UPDATED). - fix the stale datasource comment (PageService DOES stamp 'git-sync' now). - env getters: parseInt radix 10 + NaN/<=0 fallback for poll/debounce (+ max deletes), with 6 new environment.service.spec tests. tsc clean; jest 723 pass; live cycle re-verified post-refactor (ran, push applied, unflagged 92-page space untouched). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Code review — нативный двусторонний Docmost↔git Markdown-синк
Мульти-аспектное ревью (8 направлений: security, stability, conventions, documentation, regressions, test coverage, simplification, architecture). База
develop(merge-basee5bc82c7), head39edf911, объём 168 файлов (+26462/−548). Вендорный движокpackages/git-sync/src(≈8500 строк, 431 тест) проверялся в точках сопряжения с интеграцией, не построчно.Вердикт: Request changes (запросить доработку).
Фича архитектурно крепкая, граница доверия провенанса держится (проверено:
actor:'git-sync'минтуется только in-process, клиентский JWT его не подделает — REST-декоратор коэрсит всё кроме'agent'в'user', а сервер'git-sync'-токены не подписывает). Но: (1) самая критичная склейка — delete-cap, CAS Redis-лока, loop-guard листенера — поставляется без единого автотеста (подтверждено листингом каталога; PR прямо признаёт, что §13.3/§13.4 проверялись только вживую); (2) реальный зазор живучести лока (фиксированный TTL 5 мин без продления) → два писателя на один vault в multi-replica, ради которого лок и существует; (3) операторская конфигурация не задокументирована. Подтверждённой потери данных на happy-path нет, поэтому не «Block merge».Critical
Нет. Зазор лока ниже — серьёзный, но условный warning, а не подтверждённый дефект happy-path.
Warnings
[stability] Redis leader-lock: фиксированный TTL 5 мин без продления/watchdog —
git-sync.constants.ts:41(использованиеservices/git-sync.orchestrator.ts:96-124)GIT_SYNC_LOCK_TTL_MS = 5*60*1000захардкожен (в отличие от poll/debounce/max-deletes, у которых есть env-override), продления внутриdriveCycleнет. Полный цикл PULL→dry-PUSH→PUSH не ограничен по времени: на первичной синхронизации крупного спейса или большом батче он реально может превысить 5 мин. Тогда лок авто-истекает, следующий poll-tick другой реплики делаетacquire()на тот же спейс и запускает конкурентныйdriveCycleпо тому же vault (<DATA_DIR>/git-sync/<spaceId>, общий том — ровно сценарий, ради которого лок и нужен). In-process мьютекс (running) видит только свой процесс. Два писателя на один git-репозиторий → гонкиindex.lock, частичные коммиты, затёртое состояние веток. CAS-release защищает от удаления чужого лока, но не от самого конкурентного запуска. В single-replica деплое не воспроизводится; в multi-replica + крупный спейс приближается к critical.Fix: heartbeat-продление лока (
PEXPIREпо CAS-Lua, пока цикл наш, на ~TTL/3) + сделать TTL конфигурируемым.[test coverage] Delete-cap fail-safe не покрыт тестами —
services/git-sync.orchestrator.ts:305-338Самая safety-критичная новая логика (именно она не даёт несходящемуся vault массово soft-удалить реальные страницы) не тестируется ничем: ветки над/под порогом, нейтрализующий
deletePageвраппер и fail-safe при выбросе dry-run — ни одна не прогоняется. Off-by-one в сравнении или провал ветки выброса молча вернёт массовое удаление, и CI этого не поймает.Fix: unit-spec для
driveCycleс замоканнымrunPush/vault/client на три ветки.[test coverage] Acquire/release CAS лока + skip-лестница
runOnceне покрыты —services/git-sync.orchestrator.ts:96-124acquire(SET NX PX), CAS-Luareleaseи пять ранних возвратовrunOnce(disabled→no-service-user→in-process mutex→lock-held) — нетривиальная конкурентная логика без тестов. CAS-release — единственное, что мешает реплике удалить чужой лок после хэндовера по TTL.Fix: тест с фейковым Redis: значение лока = instanceId, no-op release при чужом значении, каждая ветка skip (включая release в
finallyпри выбросеdriveCycle).[test coverage] page-change листенер (loop-guard + debounce) не покрыт —
listeners/page-change.listener.tsРанний возврат при
lastUpdatedSource === 'git-sync'— единственное, что рвёт эхо-петлю write→event→sync. Извлечение spaceId/workspaceId из разнородных событий и debounce-схлопывание бёрста — всё без тестов. Регрессия, уронившая loop-guard, создаст бесконечный цикл синхронизации, и CI смолчит.Fix: spec с fake timers на (a) git-sync-строка не планирует цикл, (b) обычная — планирует, (c) бёрст схлопывается в один
runOnce, (d) disabled/missing short-circuit.[documentation] 8 новых
GIT_SYNC_*env-переменных отсутствуют в.env.example—integrations/environment/environment.validation.ts.env.example— де-факто операторская справка по конфигу (там документированыMCP_TOKEN,TRUST_PROXY,SHARE_AI_*). Новые переменные туда не добавлены, включаяGIT_SYNC_SERVICE_USER_ID— обязательную приGIT_SYNC_ENABLED=true(иначе падение на старте) — иGIT_SYNC_SSH_KEY_PATH/GIT_SYNC_REMOTE_TEMPLATE. Оператор не узнает о них, не читая исходники.Fix: блок «Git sync» в
.env.exampleс дефолтами и пометкой про обязательность service-user-id.Suggestions
driveCycleигнорируетrunPush(...).aborted === 'merge-in-progress'—orchestrator.ts:340-352. Конфликтный pull оставляет vault в mid-merge, push возвращаетabortedбезapplied/failures, но статус читает толькоmode/failures→ застрявший на конфликте спейс выглядит как успешный push с 0 ошибок. Fix: проверятьpushResult.aborted, подниматьskipped:'merge-in-progress'.pollTickбез re-entrancy-guard —orchestrator.ts:374-385.setIntervalбьёт каждые 15с независимо от завершения прошлого тика; самоограничивается (per-spacerunningбыстро скипает), но под медленными циклами копит лишниеenabledSpaces()-сканы БД и шум в логах. Fix: булевpollInFlightв try/finally.gitSyncпроверяется только на границе мока —database/repos/space/space.repo.ts:114-134. Инвариант «мердж не затирает соседниеsettings» живёт в сырой SQL, ноspace.service.spec.tsмокает репозиторий и проверяет лишь факт вызова. Fix: repo-тест на строке{ sharing:{…}, gitSync:{ other:1 } }.agent > git-sync > userне тестируется —collaboration/extensions/persistence.extension.ts:134-146. Если git-sync-запись ошибочно пометится'user', loop-guard примет её за правку человека → эхо-цикл. Fix: вынести резолв в чистый хелпер и покрыть три случая.getGitSyncDataDirиisGitSyncEnabledбез тестов —environment.service.ts. Числовые геттеры покрыты, а композиция data-dir (override vsDATA_DIR, strip хвостового слеша) и.toLowerCase()==='true'— нет; регрессия в strip переместит все vault'ы.GIT_SYNC_SSH_KEY_PATH/getGitSyncSshKeyPath()—environment.service.ts+environment.validation.ts. Ноль потребителей (remote-push пути нет). Удалить до появления фичи, что его использует.gitRemoteмёртв —orchestrator.ts:153-158. Движок на текущем путиsettings.gitRemoteне читает; шаблон + подстановка{spaceId}выглядят как живая фича, но не работают.buildSettings(docmostApiUrl/Email/Password) — мёртвая поверхность ради вендорного типа; «настоящие» на вид литералы провоцируют путаницу. Сделать optional/инертными сентинелами.GIT_SYNC_DEBOUNCE_MS_DEFAULT/POLL_INTERVAL_MS_DEFAULT—git-sync.constants.ts:7-8,44,47. Нигде не импортируются; реальные дефолты — литералы вenvironment.service.ts. Комментарий утверждает, что они «back» поведение — неправда.plan §Nповисают — по всему интеграционному шву. Финальный коммит PR удаляетdocs/git-sync-plan.md, на который ссылаютсяorchestrator.ts,gitmost-datasource.service.ts,git-sync.constants.tsи др. Проза корректна, но якорь удалён. Fix: сохранить урезанные «design notes» или заменить ссылки на самодостаточное обоснование.packages/git-sync/node_modules/.vite/vitest/<hash>/results.json(+ реальные блобы.bin/*). Закоммиченныеbuild/**и symlink-node_modulesсами по себе совпадают с прецедентомpackages/mcp, но.vite-кэш — машинно-локальный мусор без аналога в прецеденте. Fix: рекурсивныйnode_modules/иpackages/*/node_modules/.viteв.gitignore, удалитьresults.jsonиз индекса.handleGitSyncToggleглотает пойманную ошибку безconsole.error—apps/client/src/features/space/components/edit-space-form.tsx:47-54. Нарушает правило обработки ошибок из AGENTS.md (нотификацию покажетonErrorмутации, но raw-ошибка не логируется). Fix:console.error("Failed to toggle git sync for space", err).updateGitSyncSettingsсплайситprefKeyчерезsql.raw—database/repos/space/space.repo.ts:114-134. Латентный SQL-инъекционный footgun: значение рядом корректно черезsql.lit, а ключ — черезsql.rawбез экранирования. Не эксплуатируется (единственный вызов передаёт хардкод'enabled') и это клон существующихupdateCommentSettings/sharing-методов — то есть следование принятой конвенции, не новый паттерн. Но вызов с ключом из запроса откроет дыру. Fix: захардкодить путь внутри метода или allow-list дляprefKey.Test coverage (итог)
gitmost-datasource.service.spec.tsсодержательно покрывает рискованные пути (writeBody-провенанс,computeMovePosition, soft-delete, importPageMarkdown); числовые env-геттеры (NaN/zero/negative).GitSyncOrchestratorцеликом (delete-cap, CAS-лок, suppress-on-dry-run-throw, pollTick/dynamic interval) иPageChangeListener(loop-guard + debounce) — ровно те узлы, что PR признаёт проверенными «только вживую». Плюс не исполняются: jsonb-мерджgitSync(только мок), приоритет провенанса,getGitSyncDataDir/isGitSyncEnabled. §13.3 и §13.4 сознательно отложены.Regressions
Чисто. Все правки разделяемых файлов (
persistence.extension.ts,auth-provenance.decorator.ts,jwt-payload.ts,page.service.ts,space.service.ts/space.repo.ts/update-space.dto.ts,environment.*,app.module.ts,Dockerfile,history-item.tsx) аддитивны и обратносовместимы —'user'/'agent'пути байт-идентичны базе, новые поля@IsOptional, jsonb-мердж сохраняет соседей,ScheduleModule.forRootне дублируется.Architecture & design (forward-looking, не блокирует merge)
Опции сохранены намеренно — это выбор для вас, не готовое решение.
1. Стратегия вендоринга: закоммиченные
build/**+node_modules/**vs обычный workspace-пакет. Компилят (42 файла) иnode_modules(31 запись) закоммичены;node_modulesпросочился, т.к. root.gitignoreимеет только корневой/node_modules. Dockerfile копирует свежиеpackages/mcp/buildиeditor-ext/dist, но неgit-sync/build→ два источника правды + риск дрейфаbuild/**отsrc/**. CJS/ESM реален (module:CommonJSпотребляет ESM-firstmarked@17).build/**+node_modules/**, рекурсивный gitignore, сборка через nx^build, копироватьgit-sync/buildв Dockerfile как mcp/editor-ext. Pros: единый источник правды, нет дрейфа. Cons: проверить nx build-граф и marked-interop под пайплайновым tsc.build/**, добавить CI-проверкуtscна дрейф; gitignore толькоnode_modules/**(effort: s). Pros: «замороженный артефакт» + страховка. Cons: всё ещё два источника правды, асимметрия Dockerfile.node_modules/**стоит убрать в любом случае.2. Модель single-writer: TTL лока vs длина цикла, без продления (пересекается с warning'ом выше).
PEXPIREпо CAS, пока цикл наш, ~TTL/3. Pros: прямо закрывает зазор, краш-холдер истекает по TTL. Cons: гасить таймер на всех путях выхода.docmost/mid-merge (ловится guard'ом).3. Размещение инварианта delete-safety: «два прохода
runPush» в оркестраторе vs в движке. Абсолютный cap — в оркестраторе через dry-run-подсчёт + monkey-patch{ ...client, deletePage: noop }; движок владеет другим (дробнымMASS_DELETE_FRACTION) guard'ом. Политика расщеплена на два слоя, второй проход опирается на тонкий side-effect (dry-run коммитит рабочее дерево вmain).maxDeletesPerCycleуrunPush) (effort: m). Pros: один дом safety, один проход, нет monkey-patch, тестируется в vitest. Cons: шире форк от upstream.runPush(deps,{ suppressDeletes:true })(effort: s). Pros: убирает хрупкий спред-клиент, явный контракт. Cons: всё ещё два прохода.4. Связанность с внутренностями collab/persistence (
writeBody,context: any). Нативная запись тела вручную реализует операцию'replace'коллаб-хэндлера и зависит от трёх внутренних контрактов из других модулей, скоупленных только формой и комментарием;context— нетипизированныйany. Самый хрупкий стык: правка full-body replace редактора или ключей провенанса вPersistenceExtensionмолча рассинхронит git-sync.replaceDocumentBody(name, json, { actor, userId })на CollaborationGateway, общий для редактора и git-sync (effort: m). Pros: одна реализация replace, типизированный провенанс. Cons: трогает collab за пределами git-sync.CollabWriteContext) без консолидации (effort: s). Pros: дёшево, silent-break → ошибка компиляции. Cons: дублирование replace остаётся.(Пятое предложение — движок уронил
main(), иdriveCycleвручную сшивает 9-шаговую pull-последовательность — слабейшее: один потребитель, хорошо закомментировано. Имеет смысл только если будете править движок по п.3 — тогда симметричныйrunPullв движке.)Снятый ложный риск: «недетерминизм layout при коллизии имён → фантомные удаления» —
layout.tsуже делает слоистую детерминированную дизамбигуацию (~slugIdв рамках сиблингов, финальный full-path проход, глобально-уникальныйpageIdкрайним средством); delete-cap — defense-in-depth поверх уже детерминированного layout.🤖 Автоматическое мульти-аспектное ревью (code-review-orchestrator). Все находки сверены с исходниками; ключевые claim'ы (TTL лока, отсутствие spec оркестратора/листенера,
sql.raw) перепроверены вручную.Отчёт по тест-стратегии — gitmost PR #119 (feat/git-sync) — 2026-06-21
Область анализа — только изменения PR #119 (нативный двусторонний синк страниц Docmost ↔ git-папка
Markdown), а не весь монорепозиторий. PR: 168 файлов, +26462/−548. Код проанализирован в изолированном
git-worktree на HEAD ветки
feat/git-sync(39edf91), база сравнения —develop(d4658d4).По каждому из 5 модулей запускался отдельный субагент
module-testability-analyst; все утверждения«уже покрыто» перепроверены прогоном тестов и измерением покрытия (см. §7).
1. Исполнительное резюме
lib, движокengine, серверная NestJS-интеграция,серверные точки-стыковки, клиентские точки-стыковки).
Пирамида соблюдена: unit ≈77 %, integration ≈19 %, contract 2 шт., E2E 0.
i18n-JSON, тривиальные env-геттеры, декоратор провенанса с недостижимым входом — см. §2 «НЕ тестировать»).
@docmost/git-sync— 82.5 % строк / 81.7 % statements / 68.6 % branch (431 тест зелёный +3 ожидаемых
it.fails); движок 88–93 %, ноlibпроседает:docmost-schema.ts47 %,markdown-converter.ts71 %,markdown-to-prosemirror.ts77 %,roundtrip-helpers.ts31 %.gitmost-datasource.service.ts; оркестратор,листенер, контроллер и vault-registry = 0 % (спеков нет вовсе).
оркестратора/листенера/контроллера; провенанс-маркировка и валидация конфигурации защищены тестами.
pnpm install --frozen-lockfile(дефолт CI) на PRпадает —
pnpm-lock.yamlне содержит зависимость@docmost/git-sync, хотяapps/server/package.jsonеё объявляет. Без фикса лок-файла CI не соберётся и тесты не запустятся.
2. Рекомендации по модулям
Модуль 1 —
packages/git-sync/src/lib(pure-конвертер ProseMirror↔Markdown)Нижний слой пирамиды, без IO. Покрыт богато (canonicalize/diff/node-ops/markdown-document ≈95–100 %),
но измерение подтвердило бреши в конвертере и импорт-пути.
markdown-converter.ts:convertProseMirrorToMarkdown—pageBreakне имеет case → узел, легальносуществующий в схеме (
docmost-schema.tsL1009), молча исчезает при экспорте (потеря данных). Зафиксироватькак round-trip-репро (при политике «документируем» —
it.fails).markdown-converter.ts:case "subpages"— эмитится литерал{{SUBPAGES}}, на импорте превращается впараграф текста (lossy round-trip); текущий golden-тест проверяет только строку эмиссии.
markdown-to-prosemirror.ts:preprocessCallouts— вложенные callout'ы и:::внутри fenced-code(две ветки, не покрытые ни corpus, ни property-генератором).
markdown-to-prosemirror.ts:bridgeTaskLists— нумерованный чеклист<ol>→taskList (фантомный пустойorderedList) и негатив: смешанный список НЕ конвертится.
markdown-converter.ts:case "column"/column.widthparseHTML — дробная ширина: дрейф number↔string наround-trip;
case "details"с пустымdetailsContent(схема разрешаетblock*).index.ts(barrel); определения TipTap-расширений вdocmost-schema.ts(framework wiring — покрыто через round-trip converter-gate);
sanitizeCssColor/clampCalloutType(косвенно покрыты); canonicalize/diff/node-ops/markdown-document (уже покрыты).
Модуль 2 —
packages/git-sync/src/engine(reconcile/layout/pull/push/git)Чистое ядро + IO-оболочка вокруг git CLI. Покрытие очень высокое; предлагаются только остаточные бреши
в высокорисковых классах дефектов.
push.ts:parentFolderFile(линчпин классификации move↔rename, тестируется лишь косвенно);roundtrip-helpers.ts:firstDivergence— экспортируется, но не используется (строки 45–76, 0 % покрытия):либо покрыть, либо удалить как мёртвый экспорт (открытый вопрос §7).
parentFolderFile(корень/глубина/точки в имени);planReconciliationпри коллизии цели одного move со старым путём другого (chained/swap-move, защита от потери данных);
buildVaultLayoutветка last-resort по id (L135–139);firstDivergence(равенство/неравенство по путям);applyPushActionsизоляция сбоя prefetch move (try/catch L644–672);applyPullActionsс несколькимиremoveOldPathгде один write падает (корректный ключfailedPageIdsпо pageId).VaultGit.diffNameStatusс rename, перемешаннымс add/modify в одном диффе (выравнивание
-z-токенов — иначе сдвиг путей и misclassify move→delete);парсинг
C(copy)-строк (зависит от R1);VaultGit.commit— committer-identity не утекает в repo/global(
GIT_COMMITTER_*, основа провенанса/loop-guard).GitSyncClient(createPage → {data:{id}},importPageMarkdown → {updatedAt?}) — ловит дрейф API серверного адаптера, из-за которогоassignedPageIdстал бы undefined и создания зациклились.index.ts(barrel),client.types.ts(типы),loop-guard.bodyHash/vaultGitEnv/buildCommitMessage/settings/stabilize(уже покрыты), внутренностиrecreateTransform(third-party).Модуль 3 —
apps/server/src/integrations/git-sync(NestJS-обвязка) — НАИВЫСШИЙ приоритетСвязывает движок с сервером: событие →
PageChangeListener(debounce) →GitSyncOrchestrator.runOnce(Redis-лидер-лок + in-proc mutex) → PULL/PUSH через
GitmostDataSourceService+VaultRegistryService.Покрыт только datasource; оркестратор/листенер/контроллер/registry — 0 % (подтверждено: §7).
runOnce/driveCycle: short-circuit при выключенном флаге; нет service-user; in-procmutex (повторный вызов →
in-progress); Redis-лок не взят →lock-heldи mutex очищен; снятие лока +очистка mutex при throw в
driveCycle(защита от «отравленного» спейса); guard merge-in-progress;порядок
ensureRepo→ensureBranch('docmost')→checkout→applyPullActions(write-back не должен затиратьлокальные правки на
main); delete-cap: превышение лимита → реальныйrunPush({apply})сno-op
deletePage(маркерный anti-data-loss тест); fail-safe при throw в dry-run; pass-through подлимитом; подстановка
{spaceId}в remote-шаблон; идемпотентныйonModuleInit/Destroy(поллинг-таймер).handlePageEvent: gate по флагу; loop-guardlastUpdatedSource==='git-sync'→ непланировать (ключевой тест против бесконечного эха); отсутствующая страница; приоритет резолва
spaceId/pageId по вариантам формы события; debounce-коалесинг (всплеск → один
runOnce); глотаниеошибок (assert логирование, не тавтология).
vaultPath; ленивый кэш (одинVaultGitна спейс).triggerблокирует не-админа (ForbiddenException,runOnceне вызван) — противнеавторизованного ручного синка; admin использует workspace из контекста, не из тела;
statusпод админом.writeBody(datasource) на реальном Yjs-доке + реальныхtiptapExtensions— веткаfragment.length>0(очистка перед записью), которую unit-спек намеренно неисполняет; ловит дублирование тела при write-back. (Связка с editor-ext требует тех же моков, что в
существующих спеках.)
git-sync.module.ts(DI wiring),git-sync.constants.ts(константы),GitmostDataSourceService.bind(замыкание-обвязка; ветки в делегатах уже в спеке),TriggerGitSyncDto@IsUUID(class-validator), сами функции движка (third-party-вендор), примитивы ioredis.Модуль 4 — серверные точки-стыковки (
page.service,persistence.extension,space,environment)lastUpdatedSource:'git-sync'вpage.service.tsв трёх ветках —
create,update/rename,movePage(datasource-спек доказывает лишь передачуаргумента, но не что PageService на него реагирует; ошибка тут = бесконечный цикл синка);
приоритет провенанса
agent > git-sync > userвpersistence.extension.ts+ негатив: git-sync непишет boundary-снапшот истории;
environment.service:getGitSyncDataDir(DATA_DIR-fallback + striptrailing-slash + суффикс
/git-sync).environment.validationчерезvalidateSync—GIT_SYNC_ENABLED='true'безGIT_SYNC_SERVICE_USER_ID→ ошибка (главная непокрытая валидация: синквключён без атрибутируемого автора),
@IsInотвергает'maybe';space.service.updateSpace— audit-дельтатолько при реальном изменении флага, no-op не пишется; расширить corpus converter-gate (contract) —
добавить узлы editor-ext, отсутствующие в текущем корпусе (mention, math, details, highlight, sub/sup,
цвет/выравнивание) — гейт сейчас «слеп» к узлу, которого нет в фикстурах.
jwt-payload.ts(расширение union — type-only),update-space.dto.ts(class-validator),auth-provenance.decorator.ts(резолвит толькоuser/agent; входgit-syncчерез токен недостижим —тест был бы тавтологией),
app.module.ts(wiring), тривиальные env-геттеры-passthrough.Модуль 5 — клиентские точки-стыковки (
edit-space-form,history-item,space.types)Реальный diff содержит только тумблер «Enable Git sync» (без полей repo URL/branch, вопреки описанию PR —
открытый вопрос §7) и значок провенанса в истории.
history-itemпоказывается толькопри
lastUpdatedSource==='git-sync'и НЕ приagent/user/undefined; провенанс атрибутируется верномуавтору и значок не кликабелен (в отличие от
AiAgentBadge— частая регрессия copy-paste); начальноесостояние тумблера из
space.settings.gitSync.enabled ?? false; оптимистичный апдейт + корректный payload{spaceId, gitSyncEnabled}; откат при ошибке мутации (самый ценный — иначе пользователь думает, чтосинк включён); тумблер disabled при
readOnly/isPending.isGitSyncEdit(source)/resolveProvenance(source)изhistory-item.tsxL156–157 — позволит заменить часть рендер-тестов дешёвым табличным unit (опционально).
translation.json(i18n-данные),space.types.ts(типы), рендер MantineSwitch/Badge/Tooltip/иконок (third-party-презентация), точный текст перевода.3. Сквозные аспекты
GitSyncClientмежду движком и серверным адаптером (Модуль 2);(б) расширение corpus converter-gate как контракт «схема editor-ext ↔ vendored-схема» (Модуль 4).
markdown-roundtrip.property.test.ts, fast-check). Расширитьарбитрари на
pageBreak/subpages/вложенные callout'ы — именно туда, где property сейчас не доходит.VaultGitи fake-GitSyncClient(минимальные
Pick<>-подмножества), плюс билдер событий page-change по всем вариантам формы.4. Обнаруженные антипаттерны и находки
pnpm-lock.yamlне содержит@docmost/git-sync, аapps/server/package.json:43объявляет"@docmost/git-sync":"workspace:*"→pnpm install --frozen-lockfileпадает (ERR_PNPM_OUTDATED_LOCKFILE). Проверено в чистом worktree. Предусловие длялюбого тест-прогона в CI.
@docmost/editor-extи@docmost/git-sync(committedbuild/), а не исходники. CI обязан билдить оба пакета перед jest, иначегарантия совместимости схемы устаревшая.
markdown-to-prosemirror.tsL66–71 присваиваетglobal.window/documentкак side-effect импорта (риск перекрёстного загрязнения тестов; причина, почемуюниты импортируют конвертер напрямую, минуя barrel).
process.exitв коде под тестом:environment.validation.ts(validate()),engine/config-errors.ts(
loadSettingsOrExit) — failure-путь невозможно ассертить in-process (см. R: тестироватьvalidateSync).running:Set,debounce:Map,vaults:Map,last-pushedref — каждый тест должен строить свежий инстанс (иначе order-dependent проход).листенер (
handlePageEvent), клиентскийcatchотката вedit-space-form.tsx:53.sql.raw(prefKey)вspace.repo.ts:updateGitSyncSettings— сейчасprefKey= константа'enabled'(безопасно), но паттерн инъекционно-подобный: зафиксировать вызов на allowlist/константе.
it.failsв пакете — намеренныезадокументированные баги round-trip, не флаки).
5. Необходимые рефакторинги перед написанием тестов
Все — опциональные (блокирующих нет; тесты пишутся и на текущем коде), кроме лок-файла из §4.
pnpm-lock.yaml(pnpm installна ветке) — блокирует ВЕСЬ серверныйтест-прогон в CI.
engine/git.ts: вынести-z-парсерdiffNameStatusв чистуюparseDiffNameStatusZ(raw)— разблокируетдетерминированные тесты
C-строк и malformed-tail (Модуль 2, integration).engine/roundtrip-helpers.ts: решить судьбуfirstDivergence(покрыть или удалить).git-sync.orchestrator.ts: вынестиdecideSuppressDeletes(planned, max)(чистая) — упростит 3 delete-capтеста; опционально инъектировать функции движка/
node:fsвместо module-level import.persistence.extension.ts: вынестиresolveSource(agentTouched, actor)— чистый unit вместо DB-сетапа.environment.validation.ts: тестировать черезvalidateSync(plainToInstance(...)), минуяprocess.exit.history-item.tsx: вынестиisGitSyncEdit(source)(клиентский unit вместо рендера).6. План внедрения (по фазам, ROI-приоритет)
debounce; (2) оркестратор delete-cap + lock/mutex + порядок checkout/pull; (3)
page.serviceпровенанс в3 ветках + приоритет в
persistence.extension; (4)environment.validationenabled-без-service-user.pageBreak/subpagesround-trip; веткиpreprocessCallouts/bridgeTaskLists;parentFolderFile;planReconciliationswap-move;applyPushActions/applyPullActionsсбой-изоляция.GitSyncClient; расширение corpus converter-gate;writeBody-integration; контроллер authz;VaultRegistry; клиентские 6 тестов; remote-шаблон.diffNameStatusrename+add/modify иC; committer-identity;getGitSyncDataDir;audit-дельта
space.service; решение поfirstDivergence.7. Источники и верификация
module-testability-analyst(по одному на модуль; прочитаны полностью).@docmost/git-sync(vitest 4.1.6): 30 файлов, 431 passed + 3 expected-fail — зелёный.gitmost-datasource,git-sync-converter-gate,space.service,environment.service— 5 suites, 38 passed — зелёный.docmost-schema.ts47 %,markdown-converter.ts71 %,markdown-to-prosemirror.ts77 %,roundtrip-helpers.ts31 %, движок 88–93 %. Сервер: оркестратор/листенер/контроллер/vault-registry —0 ссылок из спеков (подтверждённый 0 %); покрыт только datasource.
pageBreakсведён к unit-уровнюlib(U1),серверный converter-gate оставлен как комплементарный контракт схемы; шаг 2 (skip-list) — отброшено ≈18
целей (wiring/константы/типы/DTO/i18n/тривиальные геттеры/недостижимый декоратор); шаг 3 (бюджет пирамиды) —
reclass: engine I1/I2 и серверные unit оставлены в unit, реальные git/Yjs/рендер — в integration
(unit 77 % ≥ 70 %, integration 19 % ≤ 20 %, E2E 0 ≤ 10); шаг 6 (adversarial) — каждый тест сформулирован
так, чтобы падать при реалистичной незаметной поломке.
Открытые вопросы
runPush, но нетrunPull— где живёт preflightpull-цикла (refuse-on-merge-in-progress перед pull) и протестирован ли он на уровне сервера?
firstDivergence— есть ли продакшен-потребитель, или это мёртвый экспорт?Если валидатор git-URL появится — это тест №1 по ROI на клиенте.
C(copy) вdiffNameStatus— достижим ли в проде (rename-детект только-M)? Если нет, тестыC—защитные, низкий приоритет.
Конечно блядь не коммитить build, что за чушь блядь.
запись делать через мерж
надо поддержать вообще все, блядь, что за хуйня? конвертер туда-обратно не должен ничего портить, сука. написать на это тесты, проверить ВСЕ элементы
c30b771c58to58498cf231The 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>Ghost referenced this pull request2026-06-24 04:48:57 +03:00
b4bb7a538ato9f6478a0eda093bc8331to46109ad4a8Ghost referenced this pull request2026-06-25 23:40:09 +03:00
feat(git-sync): native two-way Docmost↔git Markdown sync (Phases A–D, live-verified)to WIP: feat(git-sync): native two-way Docmost↔git Markdown sync (Phases A–D, live-verified)Адресовал ревью #1571 — коммит
68515ef9(запушен вfeat/git-sync)Все полные сьюты зелёные: движок 605 passed (+1 expected-fail, 0 type errors), сервер 193 passed / 15 suites. Каждое изменение прошло отдельный ревью-цикл (вердикт APPROVE).
🛑 Must fix
createPageтеперь читаю живое дерево Docmost и при совпадении по(parentPageId, title)адоптирую существующую страницу (write-back id +importPageMarkdown) вместо повторного создания — сбой write-back междуcreatePageи коммитом больше не плодит дубликаты. Лукап только поcomplete:trueдереву (иначе fallback наcreatePage; нативный клиент всегда complete). Покрыто: retry-adopt тест + regression-lock на веткуcomplete:false(мутационно проверен — без guard'а падает ровно он).notesHeading = "Примечания переводчика"вlib/diff.ts:255— доменное, не трогал.packages/git-sync/package.jsondescription → ссылка переуказана наdocs/backlog/git-sync-thin-meta.md.docs/backlog/ai-chat-tool-definitions-duplicated.md→git-sync-thin-meta.md.AGENTS.md: git-sync добавлен пятым workspace-пакетом (таблица + счётчик) и заметка про синхронизацию схемы обновлена на три зеркала (editor-extканоничен +mcp+git-sync).Warnings
git http-backend. Watchdog (GIT_SYNC_BACKEND_TIMEOUT_MS, дефолт 120с) убивает зависший дочерний процесс и шлёт чистый 500; таймер чистится вclose/error,resolve()ровно один раз — зависший/заглохший клиент больше не держит space-lock бесконечно. Заодно добавил таймаут на единый git-чокпоинт движка (runRawвengine/git.ts).consoleвapplyPullActions(пробрасывается изcycle) — критичные для потери данных диагностики pull снова идут через структурированный Nest-логгер соspaceId.Suggestions
parseArgs/PushParsedArgs,parseSettings/envSchema,loadSettingsOrExit+ файлconfig-errors.ts+ мёртвые спеки и экспорты изindex.ts(типSettings,BOT_AUTHOR_*и трейлерыDocmost-Sync-Sourceсохранены).listRecentSince/listTrash/restorePage) — оставил в контрактеGitSyncClient, закрыл юнит-тестами через client-seam.space-lock:running.addтеперь синхронно доawait acquire(in-process слот резервируется раньше Redis-acquire; release только при успешном acquire, cleanup во внешнем finally).Test coverage
removePage— обе ветки: git-sync → source'git-sync', обычный юзер →undefined.ensureServable— тест на все 4git configзаписи, включая security-критичнуюreceive.denyNonFastForwards=true(защита от force-push наmain).Architecture (forward-looking)
@docmost/editor-ext; + provenance-заголовок вdocmost-schema.ts. Сделал самостоятельный snapshot (а не кросс-импорт editor-ext — кросс-репрезентационная сверка двух разных форматов хрупка); цель «превратить молчаливый инвариант в громкий CI-fail» достигнута.Бонус
pnpm-lock.yaml: добавлен недостающий workspace-линк@docmost/git-syncподapps/server—pnpm install --frozen-lockfile(дефолт CI) снова проходит.Отчёт по тест-стратегии — gitmost / PR #119 (git-sync) — 2026-06-26
1. Исполнительное резюме
module-testability-analystна модуль).@docmost/git-sync(vitest): 604 pass + 1 expected-fail, 42 файла — подтверждено локальным прогоном. (Цифра в описании PR «431 pass» устарела.)jest; React не слинкован в client → даже существующий тестai-agent-badge.test.tsxпадает сnull (reading 'useState')). Это артефакт окружения, не дефект тестов PR. Источник истины по этим слоям — CI.2. Рекомендации по модулям
M1.
packages/git-sync/src/lib— чистые конвертеры (ProseMirror↔Markdown)Покрыт лучше всех (605 тестов). Шов схемы §13.1.
markdown-converter.ts:convertProseMirrorToMarkdown— top-levelimageтеряет width/height/align (внутри columnsimageToHtmlих сохраняет — асимметрия). Зафиксироватьmd1иdocsCanonicallyEqual===false. Ловит «починили один путь — забыли второй».default(L584-586): неизвестный узел верхнего уровня молча сводится к тексту детей. Pin деградации без падения.RangeError(~5000 уровней), нет depth-guard/try-catch (в отличие отdiffDocs). Зафиксировать границу либо завести guard (R7).math) на 3 циклах — нет накопления&→&.node-ops.ts:insertNodeRelative— дваthrow(appendtableRowна верхнем уровне; anchor внеtableRow) не покрыты.node-ops.ts:replaceNodeById/deleteNodeById— дублирующийсяattrs.id: оба заменяются/удаляются, без рекурсии в подставленный узел.diff.ts:diffDocs— детерминизм порядкаchanges(иначе фантомный git-churn).docmost-schemaparseHTML/renderHTML (третья сторона — TipTap);index.tsре-экспорт;sanitizeCssColor/clampCalloutType(уже исчерпывающе);page-filedeterminism-by-double-call (тавтология).M2.
packages/git-sync/src/engine— reconcile/layout/pull/push/cycle/VaultGitcycle.ts:runCycle— fail-safe delete-cap: при исключении в dry-run в hook должен уйтиInfinity(отложить удаления). Сейчас тестируется толькоdeletes===0. Это ровно та ветка, что защищает от инцидента «9 реальных страниц soft-удалены».layout.ts:buildVaultLayout— детерминизм при перестановке входа (два сиблинга с одинаковым заголовком; orphan-bucket; folder-note vs child). Корневая причина «фантомных absence-удалений» (автор её НЕ чинил) — pin инвариант «стабильно под перестановкой» (не «правильное имя»).push.ts:isPageFile—.MD(верхний регистр) → false: намеренно ли (case-insensitive ФС)? 1 кейс + open-question.run-push-realgit.test.ts,git.test.ts).VaultGit.run/isRepo;loop-guard.bodyHash(обёртка SHA, покрыта);stabilize.*(композиция конвертеров, покрыта ниже). Политика cap (значение, off-by-one, cap=0) живёт в оркестраторе — см. M3, не дублировать здесь.M3.
apps/server/.../git-sync/services+listeners— оркестрация и доменные сервисыlcs.ts:buildLcsTable— нет своего спека (off-by-one таблицы DP).three-way-merge.ts:diff3Plan— провенанс/якоря изlive(сохранение инстансов блоков) + дублирующиеся ключи блоков (повторяющиеся пустые абзацы — реальный случай).yjs-body-merge.ts:mergeXmlFragments3Way— base==live==target (0 операций), вложенные блоки (list>item), стабильностьserializeXmlNode.git-sync.orchestrator.ts:enabledSpaces/pollTick— строгий per-space opt-in (НЕТ all-spaces fallback) + изоляция: падение одного спейса не ломает остальные. Регрессия здесь — тихая (всё вcatch).page-change.listener.ts— независимость таймеров по спейсам + перевзвод debounce после срабатывания.GitmostDataSourceService.writeBodyпротив реального collab + БД: провенансgit-sync/serviceUser, корректность тела, 3-way: правка человека выживает. Текущий unit-спек тавтологичен (мокаетTiptapTransformer, ассертlength>0).createPage(атрибуция serviceUser),deletePage(soft, не force),move/rename(fractional position после последнего сиблинга),restore.listRecentSince(>а не>=),listTrash(deletedAt is null),computeMovePosition(collate('C')— сейчас даже не ассертится).SpaceLockService— контеншн двух «реплик» против реального Redis/ioredis-mock: CAS-release НЕ удаляет чужой лок; потеря лока по TTL. Центральный инвариант single-writer.GitSyncClient(bind()) ↔ интерфейс движка (ловит дрейф шва).bind()closure-passthrough;onModuleInitрегистрация интервала (framework wiring, один тест есть); REST-плейсхолдерыbuildSettings.as anyкастUser/DTO вrenamePage(L314) прячет дрейф контракта PageService;catch-проглатывание вpollTick/listener.M4.
apps/server/.../git-sync/http— Git Smart-HTTP backend (безопасность!)parseGitPath— encoded/NUL traversal в сегменте spaceId: regex%2e|%2fприменяется только кsubpath, НЕ кfirst/spaceId (а именно он выбирает репозиторий и идёт вfindById/PATH_INFO). HIGH.GitHttpService.handle— BOLA: кросс-spaces/кросс-workspace:findByIdвернул null/другой spaceId → 404, backend не запущен, наружу уходит resolvedspace.id, не сырой из URL. HIGH (нужен R2).handle—CASL createForUserбросает / не-credential ошибка auth → 403/401, не 500 и не fail-open.resolveServiceKind— спуфингservice/регистр метода: write не должен классифицироваться как read.decideGitHttpGate—serviceKind=nullбез креденшелов → 401 раньше 400 (оракул статуса).buildGitBackendCgiEnv+parseCgiResponse— инъекция в CGI-env/заголовки (CR/LF, response splitting).run— POST-тело стримится в stdin + client-abort/EPIPE (весь stdin-путь не тестируется — все тесты GET).git http-backendв temp-репо — path-scoping/LFI (краденый PATH_INFO не выходит за GIT_PROJECT_ROOT); round-tripinfo/refs; большой pack стримом; watchdog (SIGTERM→SIGKILL).headerValuepassthrough; DI-обвязка;mcp-auth.helpers(свой спек).findByIdигнорирует аргументы → изоляция «проверяется» только по call-args (тавтология); непокрытыеcatch-ветки безопасности.M5.
apps/server/.../git-syncwiring +environment(конфиг/контроллер)isGitSyncHttpEnabled(ветка наследования от master-switch);getGitSyncBackendTimeoutMs(NaN/0/отриц → дефолт — тот же класс бага parseInt/NaN, иначе watchdog не сработает и лок держится вечно);GIT_SYNC_HTTP_ENABLED@IsIn(опечатка булевого env у оператора).GitSyncControllerчерез реальныйJwtAuthGuard+ValidationPipe(whitelist)— 401 без JWT / 403 не-админ / 200 админ;@IsUUID() spaceIdне вырезается whitelist-пайпом (прежний баг «vault назван undefined»). Unit-спек контроллера хорош, но guard'ы он не проверяет.git-sync.module.ts(DI wiring);git-sync.constants.ts(литералы — change-detector); приватныйassertAdmin(покрыт через публичные); passthrough-геттеры без ветвления.loadGitSync(memo/retry) — опционально, и только если шов импорта тестируем без рефактора R6.M6. Серверные core-правки (provenance / page·space services·repos / collab persistence)
auth-provenance.decorator.ts:agentSourceFields— ветка'git-sync'опускает chat-ключ (иначе фантомныйlastUpdatedAiChatId, ломается loop-guard) — это буквально новая строка PR, на уровне хелпера не покрыта.space.service.ts:updateSpace— gitSync пишется независимо от соседних флагов (sharing/comments) в одной транзакции.space.repo.ts:updateGitSyncSettings— jsonb-merge НЕ затирает соседей (sharing, вложенныйgitSync.*); текущий спек лишьtoContain("COALESCE")(change-detector, не доказывает merge).page.repo.ts:removePage/restorePage—lastUpdatedSource='git-sync'проставляется на всё поддерево (рекурсивный UPDATE) — иначе echo-цикл в git.attachment/audio/video/youtube/drawio/excalidraw/embed/pdf(HTML-путь сdata-width/align),columns/column,subpages/pageBreak/hardBreak/underline/comment. Узлы editor-exthtmlEmbed/status/footnote/page-embed/transclusionконвертер даже не упоминает — задокументировать их статус (зелёный / known-divergence / намеренно не экспортируется). Высший приоритет для контракта схемы.withSpace/withCreator/...);jwt-payload.ts/update-space.dto.ts(типы/декораторы — третья сторона; поведение whitelist живёт вValidationPipe, покрывается интеграцией M5).space.repo.spec(ломаются на апгрейде Kysely, merge не доказывают);jest.spyOn(require(...))безrestoreAllMocks.M7. Клиент (React) — тоггл per-space + бейдж истории
Оба новых теста (
history-item.test.tsx,edit-space-form.test.tsx) — поведенческие (ассертят payload и условный рендер по source, есть adversarial-контраст), не тавтологичные снапшоты. Главный пробел — минимальность payload.{spaceId, gitSyncEnabled}без name/slug/description (риск затирания настроек); тоггл OFF шлётfalse;git-sync-badgeсauthorName=undefinedне рендерит литерал "undefined".EditSpaceForm+ реальныйuseUpdateSpaceMutation+QueryClientProvider—onSuccessmerge кэша ({...space, ...data}) не теряетsettings.sharing/comments..toBeTruthy()послеgetByText(предпочестьtoBeInTheDocument); связка с CSS-классом Mantine.M8. E2E-харнесс / CI / runtime
3 скрипта (
git-sync-e2e.sh,…-advanced.sh,…-browser-e2e.cjs) ассертят, но не CI-runnable (живой стенд,sleep 2, доступ к host-$VAULT_DIR, Redis-инъекция лока) и не вызываются ни из одного workflow — это и есть §13.4. Готовые «кирпичи» уже есть:test/integration/{global-setup,db}.ts+jest-integration.json(миграцииdocmost_test, pgvector) и real-git паттерн в пакете.test.ymlгоняет толькоpnpm -r test(unit/spec). Нетservices: postgres/redis→ 5 существующих.int-spec.tsтоже не идут в CI; нет шагаtest:int/test:e2e; нет gate по покрытию.jest.setup.ts,.env.example,Dockerfile apt-get git(smoke — в релиз-пайплайн);…browser-e2e.cjsоставить ручным (покрыто детерминированно на data-слое).3. Сквозные аспекты
GitSyncClient↔ движок (M3). Между сервисами per se контрактов нет — «контракт» здесь это git Smart-HTTP wire-протокол (покрывается e2e ниже).markdown-roundtrip.property.test.ts, fast-check) — образцово; расширять только при добавлении новых видов узлов.git clone→ правка.md→git push→ тело страницы обновилось. Главный незакрытый пробел.<<<<<<<никогда не доезжают в Docmost (push.ts:1199-1209).test/integration/db.ts(createWorkspace/User/Space/Page); добавить сидер дляGIT_SYNC_SERVICE_USER_IDи хелпер «сконструировать конфликтный vault» (паттерн изgit.test.ts:371).4. Обнаруженные антипаттерны
gitmost-datasource.service.spec(тело/SQL не доказываются — мок возвращает ассертируемое);space.repo.spec(SQL-toContain, change-detector);git-http.service.spec(findByIdигнорирует аргументы → BOLA-слепота).catch:git-http.service(CASL throw, не-credential auth) — тихий fail-open-риск.convertProseMirrorToMarkdown(~860 строк, нет depth-limit →RangeError).layout.buildVaultLayout(«первый побеждает») детерминирован только при стабильном порядке входа отlistSpaceTree— корневая причина фантомных удалений.as any-касты, прячущие контракт:gitmost-datasource.service.ts:314(User/DTO в PageService).jest.spyOn(require(...))безrestoreAllMocks;sleep/waitForTimeoutи привязка к host-FS в e2e-скриптах (flaky, не CI-runnable).git-sync-e2e.shшаг merge зависит от предыдущего шага push (общие маркеры).5. Необходимые рефакторинги перед написанием тестов
TiptapTransformer.toYdocв unit-спеке datasource; ассерты тела перенести в integration (I1), на unit оставить только выбор ветки merge. Блокирует: M3-unitwriteBody, I1.spaceRepo.findByIdдолжен учитывать(spaceId, workspaceId)(фикс харнеса, не кода). Блокирует: M4 BOLA-тест.GIT_SYNC_DATA_DIR+docmost_testна эфемерном порту. Блокирует: E1–E3.services: postgres + redisвtest.yml, подключитьtest:int(и e2e-lane). Разблокирует и 5 уже существующих.int-spec.ts.extractRest/extractQueryStringв helpers (фаззинг URL); вынести шов импортаloadGitSync.convertProseMirrorToMarkdown(если решено делать конвертер устойчивым, а не только задокументировать предел).6. План внедрения (по фазам, ROI-обоснование)
Engine fail-safe delete-cap (
Infinity);layoutдетерминизм под перестановкой; orchestrator strict-opt-in/изоляция;agentSourceFields 'git-sync'; клиентский minimal-payload; HTTP-security unit'ы (traversal в spaceId, authz fail-open; BOLA после R2); конфиг-геттеры NaN/@IsIn. Плюс расширение §13.1 gate (контракт схемы). ROI: ловит именно классы багов, уже воспроизведённые вживую (потеря 9 страниц, vault «undefined», feedback-loop), правкой только тестов.R5 (CI: PG+Redis) → datasource↔БД: writeBody+3-way, CRUD (soft-delete/position/restore), SQL-семантика,
space.repojsonb non-clobber,page.reposource на поддереве,SpaceLockпротив реального Redis, контроллер под реальным guard+ValidationPipe. ROI: выводит из «слепых» единственный нетривиальный шов и инвариант single-writer.R3/R4 → E1 committed round-trip (§13.4), E2 conflict-marker gating (§9), E3 git-wire authz; HTTP real-git path-scoping (LFI); параллельный push→503 (integration); добор lib/converter-unit'ов (image-asymmetry, depth, escaping, node-ops, diff order).
7. Источники
module-testability-analyst(M1–M8), прочитаны и сверены.pnpm --filter @docmost/git-sync test→ 604 pass + 1 expected-fail (42 файла). Серверный jest и клиентский vitest в worktree не запускаются (нет полной установки зависимостей: нетjest-бинаря; React не слинкован → падает и существующийai-agent-badge.test.tsx) — это окружение, не дефект; источник истины — CI.page-filedeterminism-by-double-call иremovePage-mapping (уже покрыт). Итог: ~71 «сырых» → ≈46 выживших.8. Открытые вопросы (требуют подтверждения у автора PR)
baseMarkdown-путь?importPageMarkdown(…, baseMarkdown)объявлен иwriteBodyна нём ветвится, но ни один вызов в области PR не передаёт base. Если движок его не подаёт — весь 3-way (включаяmergeXmlFragments3Way) сейчас мёртв в проде, хотя юнит-тестирован.listSpaceTree— от него зависит корректностьbuildVaultLayout(иначе нужна сортировка по slugId/id в layout).parseGitPath: намеренно ли guard%2e|%2fне применяется к сегменту spaceId? Как выглядит уже декодированный Fastify путь, доходящий до парсера?htmlEmbed/status/footnote/page-embed/transclusion) — намеренно не экспортируются в Markdown или просто не гейтятся?services: postgres/redis? Без этогоtest:int(и новые datasource-тесты, и 5 существующих) в CI не идут.68515ef947to142ed3a825subpages 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>> [!type]) instead of:::type937f04b735When a git-sync body write (gitSyncWriteBody) is routed to the collab instance that owns the doc, the handler runs remotely inside handleRedisMessage and CAN throw (markdown->ProseMirror transform). Previously the throw was uncaught: the customEventComplete reply was never published, so the origin's writePageBody promise only rejected after customEventTTL (~30s) as a generic 'TIMEOUT', and an unhandledRejection escaped the async messageBuffer listener on the owning instance. Now the owner wraps handleEventLocally in try/catch and, on throw, publishes a customEventComplete carrying an `error` field on the same correlation channel. The origin's pendingReplies holds {resolve, reject} and rejects promptly with the real Error. The TTL TIMEOUT remains as the fallback for a genuinely lost reply. The no-throw and local (same-instance) paths are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Code review (delta) — PR #119: native two-way Docmost↔git Markdown sync
Вердикт: Approve with comments. Скоуп-дельта чистая по security/stability; блокеров для слияния нет. Главное к исправлению — две асимметрии потери данных в конвертере (open у вложенного details; см. ниже) и пробелы тестов на новую safety-логику (abort-on-lost-lock) и на round-trip значений htmlEmbed/footnote/transclusion — это WIP, поэтому помечаю как сильные рекомендации, а не как Block.
Скоуп: ДЕЛЬТА-ревью. DELTA review. The last multi-aspect review (Request changes) was against head
68515ef9. Since then the branch was rebased and gained ~65 new commits. This review is SCOPED to the new work in the git-sync ENGINE + server integration seam (packages/git-sync/src + apps/server/src/integrations/git-sync), excluding develop-merge noise in unrelated paths. Review ONLY these scoped changes. (~23 файлов, +2372/−324). Аспекты: security, stability, regressions, test-coverage (параллельные ревьюеры + judge).Статус прошлых блокеров
htmlEmbed+ base64-хелперыencode/decodeHtmlEmbedSource. Но фактическое сохранение ЗНАЧЕНИЯ не покрыто тестом (см. Test coverage).openstate — частично: top-level case теперь несётopen, но raw-HTML путь для вложенного details — нет (новая асимметрия, см. Must fix).driveCycle; задокументирован как намеренный (удаления soft/Trash, всегда логируются). Engine-конвергенция остаётся единственной защитой (см. Non-blocking).Must fix before merge
[regressions] Пробросить
openчерез raw-HTML путьdetailsToHtml—packages/git-sync/src/lib/markdown-converter.ts:858-860top-level case теперь эмитит<details${open}>и схема парситopenобратно (docmost-schema.ts:471-476), ноdetailsToHtml(используется для details внутриcolumns/column/spanned-ячейки) эмитит голый<details>—open: trueу вложенного details молча теряется на каждом round-trip. Это та же потеря данных, что только что починили сверху, теперь рассогласованная между двумя путями. Fix: вdetailsToHtmlэмитить атрибут изnode.attrs?.open(<details${node.attrs?.open ? " open" : ""}>…), как в top-level case.**[regressions] warning — per-cycle delete-cap зафиксировать отказ ** —
apps/server/src/integrations/git-sync/services/git-sync.orchestrator.ts:253-300дельта убрала хукresolveApplyClient, который прошлый ревью добавил как защиту от bulk фантомных удалений при неконвергентном vault; engine хук всё ещё объявляет (cycle.ts:48,154-170), но оркестратор его не передаёт — кэп инертен. Изменение задокументировано и осознанно (удаления soft→Trash, всегда логируются), поэтому не блокер. удалить мёртвую обвязкуresolveApplyClientиз engine, чтобы не подразумевать контроль, который никогда не срабатывает.Test coverage
space-lock.service.ts:105-118,160-171+cycle.ts:128,174новая safety-логика без единого теста:refreshLock()вызываетcontroller?.abort()на CAS-miss (res !== 1) И на ошибке Redis (catch);withSpaceLock()создаётAbortControllerи прокидывает signal вfn;cycle.tsзовётsignal?.throwIfAborted()перед ОБЕИМИ деструктивными фазами. Heartbeat-тест проверяет только happy-path (что refresh eval отработал). Регрессия, тихо убравшаяabort()(вернувшая blind-write race), оставила бы тесты зелёными. Fix: тест, где heartbeat-refresh CAS-промахивается (fake redis eval → 0) покаfnв полёте → assert signal aborted; sibling-кейс, где eval бросает; кейс вcycle.test.tsс pre-aborted signal → assertrunCycleбросает доapplyPullActions/runPush.markdown-converter.ts:686-707roundtrip-all-nodes.test.tsпроверяет толькоtypes.has('htmlEmbed'), redteam — толькоnot.toBe(''). Что<b>hi</b>реально возвращается после convert→import — не утверждается; хелперы без прямого unit-теста. Баг, эмитящий пустой/битыйdata-source, прошёл бы. Fix: round-trip doc с htmlEmbedsource: '<b>hi</b>'(+ height), импорт обратно, assertattrs.source === '<b>hi</b>'и height сохранён; отдельный unit на UTF-8 симметрию и'' → ''.markdown-converter.ts:708-714единственный фикстур (roundtrip-all-nodes.test.ts:70-74) строитfootnotesList+footnoteDefinition, но утверждает толькоprimary='footnoteReference'. Не проверяется, что узлы определения переживают импорт, что текст заметки'note'сохранён, чтоreference.idссылается наdefinition.id. Fix: расширить round-trip — после import убедиться, что естьfootnotesList/footnoteDefinition, текст'note'присутствует, и id совпадают.markdown-converter.ts:731-746transclusionReference(атом, несущий потеряемые id) не имеет round-trip теста вовсе; единственный фикстур используетtransclusionSourceс НЕВЕРНЫМ именем атрибута (pageIdвместо схемногоid) и пустым content (а схема требует+, т.е. может не пере-распарситься). Fix: round-trip дляtransclusionReferenceс{ sourcePageId, transclusionId }(assert оба переживают); починить фикстурtransclusionSourceнаattrs.idхотя бы с одним блочным потомком.git-sync.orchestrator.ts:116-138buildSettingsдобавил per-space readsettings->'gitSync'->>'autoMergeConflicts' = 'true'(всё кроме литерала'true'→ false) вsettings.autoMergeConflicts. Spec хардкодит stub{ autoMergeConflicts: false }и не утверждает поле наdeps.settings; SQL-тест проверяет только предикатenabled. Downstream skip/strip покрыт в redteam-push-cycle, так что пробел умеренный. Fix: тест со stubexecuteTakeFirst → { autoMergeConflicts: true }(assertdeps.settings.autoMergeConflicts === true) и кейс с missing row (assert default false).open; drop dead delete-cap wiring; cover lost-lock abort + lose-prone atom round-trips 4b3153f2d2openas a boolean so open state survives render/round-trip c1c87c21c3c1c87c21c3to5141279e425141279e42toa358d96a80Heads-up: в develop/main сноски стали first-class — конвертеру git-sync их надо поддержать
В
develop(и в релизной ветке) сноски — полноценная фича: узлыfootnoteReference/footnotesList/footnoteDefinition(схема@docmost/editor-ext, канонизацияfootnoteSyncPlugin), и канонический конвертерpackages/mcp/src/lib/markdown-converter.tsгоняет их round-trip как[^id]/[^id]: text(адресация поid).Вендорный конвертер git-sync их не знает. В
packages/git-sync/build/lib/markdown-converter.jsнетcase "footnoteReference"/"footnotesList"/"footnoteDefinition", аcanonicalize.jsих игнорирует. Неизвестный узел уходит вdefault(оборачивание детей в<div>), поэтому на export→import:footnoteReference(инлайн-атом без детей) → теряется (пустой маркер);footnoteDefinition/footnotesList→ текст определений утекает в тело страницы как<div>.Итог: любая страница со сносками через git-sync round-trip ломается. Это надо поддержать — желательно по той же модели (id-keyed
[^id]/[^id]: text) и в идеале поверх общего канонизатора. Модель, канонизатор и анализ совместимости расписаны в #228 — там же предложено вынестиcanonicalizeFootnotesв общий код, чтобы git-sync переиспользовал его, а не заводил свою копию.Заодно — просьба про аудит покрытия конвертера vs «апстрим»
Прогони, пожалуйста, сверку покрытия узлов/марок вендорного конвертера против канонического (
packages/mcp/src/lib/markdown-converter.ts) и зафиксируй реальные потери на round-trip. Я сделал быстрый diff как стартовую точку:footnoteReference,footnotesList,footnoteDefinition— больше по спискуcaseничего не отсутствует.case "pageBreak"(стр. ~519), которого нет в каноническом конвертере. Стоит сверить и эту сторону — схемы немного разъехались.и одинаково теряют выравнивание/ширину (align/width), потому что markdown-картинка их не несёт. Если важно сохранять выравнивание/ширину через round-trip — это правка для обоих конвертеров (например HTML-fallback<img …>по аналогии с тем, как уже сделан<video>с сохранением data-атрибутов), и её надо оценить отдельно.Просьба: гонять converter-gate / round-trip на документах со сносками, выровненными картинками и pageBreak, и приложить список фактических потерь (что дропается, что мутирует). Сноски из этого списка — блокер для страниц с ними; остальное — по приоритету.
a358d96a80to7c535f0165Security (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>Must-fix: - Throttle the raw /git HTTP-Basic path: it bypasses Nest/ThrottlerGuard, so verifyUserCredentials (bcrypt) ran unthrottled. Wrap it in the SAME FailedLoginLimiter the /mcp path uses (5/60s; per-IP, per-IP+email, global per-email keys; atomic tryReserve BEFORE bcrypt; success resets, non-credential errors release). The (threshold+1)-th attempt now gets 429 pre-bcrypt. Sweep timer + onModuleDestroy mirror McpService. - Fix the mcp schema mirror drift: packages/mcp details `open` attr now reads via hasAttribute (matches editor-ext canon + git-sync copy); getAttribute dropped a bare `<details open>` state. (build/ is gitignored — rebuilt locally.) Tests added: - /git brute-force throttle: pre-bcrypt 429 on the 6th failure; success resets; non-credential error releases the budget. - git-http-backend lost-lock AbortSignal: already-aborted -> no spawn + 500; live abort mid-request -> SIGTERM + response closed. - orchestrator divergentDocmost -> WARN + flag surfaced in status (+ clean case). - pollTick re-entrancy guard skips an overlapping tick. - datasource NotFound early-throws (getPageJson/move/rename) + updatedAt:undefined stale-read branch (importPageMarkdown/createPage). Suggestions: - space.repo updateGitSyncSettings: parameterize the jsonb key (`${prefKey}::text`) instead of sql.raw (latent-injection footgun); value stays sql.lit. Spec updated. - pollTick re-entrancy guard (private `polling` flag). - page-change.listener docstring: honest about the move/rename/delete over-skip (loop-guard keys only on lastUpdatedSource) -> ~poll-interval latency, not loss. - AGENTS.md: document the root /git smart-HTTP route + GitSyncModule. - Remove redundant redteam-provenance.spec.ts (covered e2e in persistence.extension.spec.ts:145). - Extract the duplicated SIGTERM->SIGKILL+finish block (watchdog + abort) into terminateChild; centralize watchdog-timer teardown in done(). Architecture (deferred, documented): mcp schema header now carries the three-copy keep-in-sync + schema-core note; the editor-ext contract test documents that the mcp copy and attribute-behaviour drift (details `open`) are not mechanically covered yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>1. gitRemote is NOT yet consumed (the vendored engine has no remote-push path, SPEC §7). Corrected the buildSettings docstring (it wrongly called gitRemote "load-bearing") and marked the env -> validation -> getter -> buildSettings chain as inert SCAFFOLDING for the deferred remote-push feature at all three sites. Kept the wiring (harmless; removing only churns). 2. .env.example: document that GIT_SYNC_REMOTE_TEMPLATE substitutes the literal "{spaceId}" per-space (with the example), so an operator doesn't point every space at one remote. 3. Extracted the copy-pasted CJS->ESM dynamic-import bridge (`new Function('s','return import(s)')`) into one shared common/helpers/esm-import.ts; git-sync.loader, docmost-client.loader and mcp.service now import it and keep their own typed loadX() wrappers. Deferred (notes only, not implemented): - lcs.ts + three-way-merge.ts could move into packages/git-sync, but that engine is vendored (manual re-sync) — added a one-line note at three-way-merge.ts to revisit once the re-sync story is settled. - schema-core single source + BullMQ/fencing remain documented from prior rounds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>9345396bb5to906733b5c8F4 [blocking] (critical)
packages/git-sync/src/engine/push.ts:403-424(+ ghost-move ветки A:312-320, M:345-353) — при переименовании/перемещении файла, ОДНОВРЕМЕННО отредактированного в том же диффе, правка ТЕЛА теряется безвозвратно.Подтверждено чтением кода (судья):
computePushActionsдляcase "R"/"C"кладёт страницу ТОЛЬКО вactions.renamesMoves, никогда вactions.updates. То же в ghost-move ветках A (312) и M (349): любой переименованный/перемещённый pageId уходит в renamesMoves.applyPushActionsдля renamesMoves (900-976) вызывает лишьclient.movePage/client.renamePage.client.importPageMarkdown(тело) вызывается ТОЛЬКО дляactions.updates(682-716) и creates (841) — для renamesMoves НЕ вызывается никогда.-Mрепортит rename+правку однимR-row (или, при крупной правке, D+A, который ghost-move-коалесинг сводит в renamesMoves). В обоих случаях тело в Docmost не уходит.Хуже: потеря НЕ откладывается, а становится постоянной. После move push продвигает refs (LAST_PUSHED_REF → main, docmost-mirror ff → main), т.е. зеркало docmost заявляет «тело = новое», хотя в БД Docmost тело старое. На следующем pull читается старое тело из Docmost, пишется в ветку docmost поверх нового, merge docmost→main фаст-форвардит main к старому — правка затирается и в git.
Реалистично: в Obsidian/git переименовали заметку (или перенесли в папку) и в том же коммите поправили текст → правка тела молча и необратимо потеряна. Все прошлые ревью #119 это пропустили.
Fix: в computePushActions для R/C (403-424) и ghost-move A/M (312-320, 345-353) дополнительно эмитить
actions.updates.push({ pageId, path: <newPath> })рядом с записью в renamesMoves, чтобы тело прогонялось через importPageMarkdown (idempotent 3-way merge — для чистого переименования без правки это near-no-op). Обновить compute-push-actions.test.ts (ожидание updates:[] для R/C заменить) и добавить кейс «rename+edit → и move, и update тела».F5 [warning]
packages/git-sync/src/engine/push.ts:803-811— гард «конфликт-маркеры никогда не попадают в Docmost» на CREATE-пути (autoMergeConflicts ON) не покрыт тестом. При ON ветка вызывает stripConflictMarkers(rawBody) и затем createPage(title, body) с очищенным телом. Покрыты только: CREATE при OFF (redteam-push-cycle.test.ts:221 — createPage НЕ вызван) и UPDATE при ON (другой code-path через importPageMarkdown). Регрессия, передающая rawBody вместо body в createPage, опубликует сырые <<<<<<< / >>>>>>> в новую страницу — ни один тест не поймает. Сами авторы отмечают важность симметрии в комментарии redteam-push-cycle.test.ts:222-225, но закрыли только OFF-половину.Fix: добавить в redteam-push-cycle.test.ts тест CREATE при autoMergeConflicts:true — новый .md (status 'A', без gitmost_id) с конфликт-маркерами; проверить, что createPage вызван и его body не содержит <<<<<<</=======/>>>>>>>, но сохраняет контент обеих сторон, а файл переписан чистым телом.
F6 [warning]
.env.example:219-224— блок GIT_SYNC_REMOTE_TEMPLATE описан как рабочая фича («Optional remote URL template to mirror each space's vault to… each space mirrors to its OWN remote… Leave unset to keep vaults local-only»), но это инертный scaffolding: значение ничем не потребляется. Все внутренние места говорят обратное: environment.validation.ts:193-195 («SCAFFOLDING for the deferred remote-push feature… currently inert»), environment.service.ts:370-379 («currently inert»), git-sync.orchestrator.ts:129-142 («gitRemote is NOT yet consumed… inert SCAFFOLDING»), pull.ts:534 логирует «git push to remote is DEFERRED». Оператор, выставивший переменную в расчёте на зеркалирование на git-хост, получает молчаливый no-op, поданный как рабочая ручка.Fix: переписать комментарий GIT_SYNC_REMOTE_TEMPLATE в .env.example — указать, что remote push ещё не реализован (deferred, SPEC §7) и переменная сейчас инертный scaffolding без эффекта, по формулировке из environment.validation.ts:193-195.
Ревью
fbaaa8441— раунд 3 (ПОЛНЫЙ РАЗДЕЛЬНЫЙ веер, 8 отдельных субагентов: security/stability/regressions/test-coverage/conventions/documentation/simplification/architecture). Вердикт: CHANGES.ВАЖНО: полный раздельный веер поймал CRITICAL, который пропустили round-1 (stability=LGTM) и объединённый round-2.
Открыто: F4 (blocking/critical, stability — rename/move + правка тела в одном диффе теряет прав484 тела безвозвратно; подтверждено судьёй чтением кода), F5 (warning, test-coverage — CREATE-путь strip конфликт-маркеров не покрыт), F6 (warning, docs — .env.example врёт про GIT_SYNC_REMOTE_TEMPLATE как рабочую фичу).
Прошлые F1/F2/F3 подтверждены закрытыми. security / regressions / conventions / simplification / architecture — LGTM (git через execFile, path-guard, фича opt-in, чистый seam пакета, Yjs=авторитет).
F4 — НЕ эскалация: фикс конкретный (эмитить updates рядом с renamesMoves), кодер применит.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.