f3dbcec0fd9a88424c7809d8870f519c47db7220
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
b47751349f |
fix(git-sync): kill spurious marker-leaking conflict, concurrent-edit loss, flapping HEAD
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> |
||
|
|
22e3fcdeba |
fix(git-sync): address PR #119 review #2 — throttle /git Basic auth, fix mcp schema drift + warnings/tests
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>
|
||
|
|
7179f8a5b2 |
fix(git-sync): address PR #119 review — close 403/404 space-existence leak + warnings/tests/arch
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>
|
||
|
|
452a752264 |
fix(git-sync): don't run a Docmost cycle on receive-pack info/refs (fixes deterministic push 503)
A git push is a two-request exchange: GET info/refs?service=git-receive-pack (ref advertisement) then POST git-receive-pack (the pack). The git-HTTP host classified BOTH as serviceKind 'write' and routed both through ingestExternalPush, which takes the per-space lock and runs a FULL Docmost reconcile cycle. So the read-only info/refs advertisement held the lock while a cycle ran, and the client's immediately-following POST git-receive-pack collided with that still-running cycle and got 503 — deterministically, every push (and Obsidian Git's "scan" failed for the same reason, since it probes push capability via the same receive-pack info/refs). Fix: only the actual pack-receiving write (POST git-receive-pack) runs under the lock + cycle. Everything else streams the http-backend directly with no lock and no cycle — a fetch/clone (read) AND the write-AUTHORIZED but read-only info/refs?service=git-receive-pack advertisement. Authz is unchanged (the gate still requires write permission for receive-pack refs); only the side effect of running a cycle on a read-only request is removed. Verified end-to-end on a live stand: clone, then `git push` of a new file lands the page in Docmost (was 503 on every push before). Regression test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
2e83c9cebf |
fix(git-sync): git-http stream error handlers + close test gaps (#119 review)
Addresses the stability + test-coverage warnings from the #119 review: - git-http-backend.service.ts: add `'error'` handlers to child.stdout/stderr. An EventEmitter 'error' with no listener (e.g. EPIPE when the client aborts mid-response) is rethrown by Node as an uncaught exception and crashes the process; now swallowed + logged (never echoed to the client). - TEST INFRA: a jest setupFile shims `navigator`/`MessageChannel` for the `node` testEnvironment. react-dom@18 reads `navigator` at module-init (pulled in via @docmost/editor-ext -> @tiptap/react), so every spec transitively importing the conversion engine — including git-http.service.spec.ts — previously FAILED TO LOAD ("navigator is not defined") and ran ZERO tests. With the shim those specs now run (git-sync integration: 11 suites / 133 tests green). - git-http.service.spec.ts: cover the 503 lock-held push path — `ingestExternalPush` rejecting `GitSyncLockHeldError` -> 503 + Retry-After + "git-sync busy, retry", no double header write (+ the already-headers-sent no-rewrite path). - git-http-backend.service.spec.ts: unit-test run() — child 'error'/'close' before headers -> 500; normal CGI parse+stream; stdout/stderr 'error' (EPIPE) swallowed; synchronous spawn throw -> 500. - page-change.listener.ts: implement OnModuleDestroy to clearTimeout all pending debounce timers on shutdown (+ test). - .env.example: vaults are non-bare working repos, not "bare repos". (Docs deleted by the stray commit were restored in 9cdbce54.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
04032ae677 |
feat(git-sync): serve spaces over smart-HTTP (gitmost as a two-way git host)
Expose each git-sync-enabled space as a clonable/pushable git repo over HTTP, so `git clone https://<user>:<pass>@<host>/git/<spaceId>.git` works and external pushes flow back into Docmost pages — gitmost itself acts as the git host (no external GitHub/Gitea, no SSH). Transport: shell out to `git http-backend` (CGI; git is already in the runtime image) which implements the full smart-HTTP protocol (info/refs, upload-pack, receive-pack, protocol v2). A raw Fastify route `/git/*` (mounted at the root, outside the `/api` prefix) bridges the request/response to the CGI; passthrough content-type parsers for the git media types stream the raw body to stdin. Reuse the existing engine: clients push the vault's `main` branch, whose commits beyond `refs/docmost/last-pushed` the engine already reconciles into Docmost. - http/git-http.service.ts — auth (HTTP Basic -> AuthService.verifyUserCredentials), self-resolved workspace (DomainMiddleware does not run for this raw route), per-space gating (global + per-space gitSync flags, 404 hides existence), CASL authz (Read=fetch, Manage=push), dispatch. - http/git-http-backend.service.ts — spawn `git http-backend`, binary-safe CGI response parsing (Status/headers/body), stream to the socket. - http/git-http.helpers.ts — pure path parse, service->kind mapping, gate decision (unit-tested); rejects literal and percent-encoded path traversal. - orchestrator: extract reusable withSpaceLock (CAS-guarded lock heartbeat so a long push cannot let the lock expire mid-cycle) and add ingestExternalPush (receive-pack + Docmost cycle under one lock; 503 on contention). - vault-registry: ensureServable() — ensureRepo + idempotent receive.denyCurrentBranch =updateInstead / denyNonFastForwards / http.receivepack / http.uploadpack. - env: GIT_SYNC_HTTP_ENABLED (defaults to GIT_SYNC_ENABLED) + validation. - main.ts: register the /git/* route and the git content-type parsers. Tests: pure helpers, CGI parsing, and the GitHttpService handler (auth/gate/authz + workspace resolution). Server tsc + git-sync/env suites green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> |