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>
The shell e2e suites defaulted to the General space and created/edited pages
there, polluting real content (and, when several enabled spaces raised poll
contention, flaking on 503s). Now each suite creates its OWN throwaway,
git-sync-enabled space at setup, runs everything against it, and deletes the
space (+ its vault) on exit. Set SPACE_ID explicitly to opt into an existing
space. Also gives the basic suite the 503-retry push helper the advanced one
already had. Verified isolated: basic 12/12, advanced 23/23, no spaces/users/
pages left behind, the real space untouched.
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>
Reproduces the browser bug at the API level: create several untitled pages (all
collapse to the `_` fallback name), retitle one, sync — assert NO page is
trashed and all survive. Caught the data-loss bug fixed in 4376c5a6.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The push / 3-way-merge cases edited the FIRST real `.md` in the vault, leaving
`E2E-PUSH-*` / `E2E-MERGE-*` marker headings accumulating in a real page, and the
Docmost->git case left its created page in the Trash. Now the suite creates a
dedicated `E2E-SyncTarget-*` page and targets only that, and a teardown
hard-deletes every `E2E-*` fixture page and converges the vault on exit — so runs
never mutate real content and leave the stand clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Output of a generate→critique subagent pass on "what the feature's tests do NOT
cover", implemented + verified against the live stand (20/20). Complements the
basic two-way suite. Covers:
- protocol shape: unknown service subpath -> 400; unknown content-type -> 415
(global allowlist); PUT/DELETE on pack endpoints -> 400;
- path-traversal: `..%2f..`, `%2e%2e%2f`, bare `.git` space-id -> 400/404, no
escape, never a file leak;
- authz boundaries: a gitSync-DISABLED space -> 404 (existence hidden) and flips
to 200 when enabled; a READER member can fetch (200) but is FORBIDDEN to push
(403); a NON-member of an enabled space gets 403 (NOT 404 — the critic caught a
wrong generator assumption here; pinned as a contract);
- concurrency: a push while the per-space Redis lock is held -> 503 + Retry-After,
and the receive-pack does NOT mutate the vault;
- idempotency: repeated no-op cycles never churn `main` / `refs/docmost/last-pushed`;
- data-loss guard (PR #119): deleting MORE than GIT_SYNC_MAX_DELETES_PER_CYCLE is
HELD — none trashed AND last-pushed does not advance past the delete commit
(retry-safe, not silently dropped).
Auto-creates/tears down its fixtures (reader/non-member users, a 2nd space) and
resets the vault cache on exit so re-runs and the basic suite stay green. Needs
the vault dir + Redis container reachable (see header). A structural rename/move
case was intentionally left to the engine unit suite (git rename-similarity on
meta-only fixture pages is a fixture artifact, not a feature bug).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A runnable end-to-end suite that drives a LIVE git-sync stand over the real /git
remote — the integration counterpart to the unit tests. 10 checks across the full
feature:
- the auth/authz gate: no creds -> 401, wrong password -> 401, unknown space ->
404 (existence never revealed), valid creds on a sync space -> 200;
- fetch: git clone over HTTP returns the vault markdown;
- push: a git-side edit propagates into the Docmost page;
- Docmost -> git: a page created via the API materializes as a vault file;
- delete: `git rm` + push soft-deletes the Docmost page (Trash);
- 3-way merge: a new git edit is added without clobbering prior page content.
Parameterized via env (SERVER/SPACE_ID/EMAIL/PASSWORD/DB_CONTAINER) and isolates
its own test page. It boots nothing — see the header for the stand prerequisites
(GIT_SYNC_ENABLED + a per-space gitSync flag + a service user). This is the suite
that caught the smart-HTTP PATH_INFO 404 bug.
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 /git smart-HTTP host 404'd EVERY fetch and push: PATH_INFO was built as
`/<spaceId>.git/<subpath>`, so `git http-backend` resolved the repo at
`<GIT_PROJECT_ROOT>/<spaceId>.git` — which does not exist. The vault is a NON-bare
working repo (the engine needs a working tree) at `<dataDir>/<spaceId>`, so the
CGI repo path must be `<spaceId>` (git http-backend serves the `.git` inside).
The URL's conventional `.git` suffix is already stripped to `spaceId` by
parseGitPath; re-appending it for PATH_INFO was the bug.
Found by standing up a full e2e stand (real Postgres/Redis + server + a real git
clone/push over the /git remote): clone and push both 404'd until this fix, after
which a clone → edit → push round-trips the change all the way into the Docmost
page.
Also extracts the CGI-env construction into a pure, exported `buildGitBackendCgiEnv`
and adds unit tests (the env build was previously untested — the gap this bug hid
in): a regression guard pinning PATH_INFO to `/<spaceId>/<subpath>` (no `.git`),
plus method/query/content-type/remote-user forwarding and the conditional
GIT_PROTOCOL.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Closes the test-coverage warning that the smart-HTTP push ingest path was
unexercised. Adds 5 cases: receive-pack streams BEFORE the Docmost cycle; a
held lock throws GitSyncLockHeldError and runs neither the receive-pack nor the
cycle; a post-push cycle error is swallowed (the push is durable, poll retries)
while the lock is still released; a missing service user runs the receive-pack
but skips the immediate cycle; and a globally-disabled git-sync refuses without
touching the lock.
(The 503/Retry-After mapping in git-http.service is the sibling warning; its spec
is in the repo's pre-existing set of jest suites that can't load locally via the
react-dom/tiptap transform chain, so that case is left for CI.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The per-space single-writer lock — Redis CAS leader lock (SET NX PX, DEL-CAS and
PEXPIRE-CAS Lua), the in-process mutex, the per-process instanceId and the
heartbeat — lived inline in GitSyncOrchestrator. Extract it into a dedicated
@Injectable() SpaceLockService exposing one narrow surface, withSpaceLock(spaceId,
fn), so the lock is the orchestrator's only Redis-lock touch-point and is testable
in isolation. The orchestrator now injects SpaceLockService and both consumers
(runOnce, ingestExternalPush) go through spaceLock.withSpaceLock — behavior
unchanged (same sentinel returns, same 503-on-lock-held contract). Orchestrator
drops 591→472 lines.
Adds space-lock.service.spec.ts asserting the lock SEMANTICS against a fake Redis
(the test-coverage warning from the review): the SET NX/PX args, the DEL-CAS and
PEXPIRE-CAS Lua + ARGV[1]=instanceId, plus the lock-held / in-progress / throw-
still-releases paths. The orchestrator spec is unchanged in count and stays green
(it now builds the real SpaceLockService over its mock Redis).
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>
The debounce map value carried `workspaceId`, but the scheduled cycle closes over
the `workspaceId` argument directly — the field was written and never read.
Replace the entry struct with `Map<string, NodeJS.Timeout>` (the timer handle is
all the map tracks). No behavior change. (page-change.listener.spec is in the
repo's pre-existing set of jest suites that can't load locally via the
react-dom/tiptap transform chain — unaffected by this change; tsc clean.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two-way block diff (yjs-body-merge.diffBlocks) and the three-way merge
planner (three-way-merge.lcsPairs) built the identical backward-filled LCS DP
table inline. Extract it to lcs.ts (buildLcsTable); each caller keeps its own
traceback. No behavior change — merge specs unchanged and green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two stability warnings from the #119 review:
1. delete-cap no longer drops deletions forever. When planned deletes exceed
GIT_SYNC_MAX_DELETES_PER_CYCLE the apply client's deletePage now THROWS
instead of resolving to a no-op. A throw is recorded by the engine as a
per-page failure, so `refs/docmost/last-pushed` is NOT advanced past the
commit that dropped the files — the next cycle re-diffs from the un-advanced
ref and re-plans the same deletes (a transient over-cap is retried, not
silently dropped and then recreated by the next pull). Previously a resolving
no-op let the engine count `deleted++` with no failure, advance the ref, and
never replay the deletions.
2. git-sync soft-delete and restore now stamp provenance. deletePage routes
GIT_SYNC_PROVENANCE through pageService.removePage, and restorePage stamps
lastUpdatedSource='git-sync' on the restore update — so the page-change
listener's loop-guard (skip when lastUpdatedSource==='git-sync') recognizes
both as its own writes instead of scheduling a wasted echo cycle. Done via a
backward-compatible optional `lastUpdatedSource` param on
pageRepo.removePage/restorePage (omitted for ordinary user deletes/restores).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the documentation/convention warnings from the #119 review:
- .env.example: add the GIT-SYNC block (9 GIT_SYNC_* vars with defaults), noting
GIT_SYNC_SERVICE_USER_ID is required when sync is enabled.
- yjs-body-merge.ts: translate the Russian review note in the docstring to
English (comments-only-in-English rule).
- persistence.extension.ts: correct the stale "git-sync writes are full-body
replaces" rationale — a git-sync write is now a block-level merge into the live
doc, which is why it is debounced like a human edit rather than snapshotted.
- history-item.tsx: the GitSyncBadge version is created on the PUSH path (writing
the git body back into the doc), not by the pull — fix the comment.
- edit-space-form.tsx: log the raw error in the git-sync toggle catch instead of
swallowing it (AGENTS.md).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Same hygiene fix as git-sync (review #2), applied to packages/mcp which had the
identical pre-existing problem: committed build/ (20 files) + node_modules (28,
pnpm symlinks with a baked /home/claude store path).
- git rm --cached packages/mcp/{build,node_modules}.
- .gitignore: add packages/mcp/build/ (packages/*/node_modules/ already covers it).
- Build where consumed: apps/server `pretest` and the CI Test workflow now build
@docmost/mcp too. The Dockerfile builder already runs `pnpm build` (nx builds
mcp) and already COPYs packages/mcp/build into the runtime image.
Verified: wiped build/, rebuilt via `pnpm --filter @docmost/mcp build`; the mcp
server suites (96 tests) pass against the freshly-built, non-committed output.
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>
Supersedes the active-session "defer" guard with a real merge (review #5 —
"запись делать через мерж", not skip-while-editing).
writeBody no longer does delete-all + re-insert (which discarded a concurrent
editor's in-flight changes on every sync). It now diffs the live body against the
incoming git body at TOP-LEVEL BLOCK granularity (LCS over a canonical structural
serialization) and applies only the minimal inserts/deletes:
- a block a human is editing is left UNTOUCHED when git changed a DIFFERENT block;
- an unchanged resync is a complete 0-op write;
- Yjs CRDT-merges the minimal ops with concurrent edits.
New yjs-body-merge.ts (mergeXmlFragments + cloneXmlNode + diffBlocks) is pure-Yjs
and unit-tested with real Y.Docs (8 tests): identical->0 ops, edit-one-block keeps
the other block instances, append/delete keep neighbours, marks survive the
cross-doc clone. Crash-safety kept: the incoming doc is built before the
connection opens, so a transform failure can't empty the body.
Removed: the ActiveEditSessionError defer path and the now-unused
CollaborationGateway.getActiveEditorCount.
Honest limitation: this is a 2-way merge — for a block BOTH sides changed since the
last sync, git wins (no common ancestor to decide). A full 3-way merge would need
the last-synced base plumbed from the engine; the dominant cases (unchanged
resync, edits to different blocks) are now lossless.
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>
Review finding #5: the git -> page body write (writeBody) did a full-body replace
(delete-all + re-insert) on the shared Yjs doc. Applied while a human is editing
the page, it discarded their in-flight changes; and TiptapTransformer.toYdoc ran
AFTER the fragment was cleared, so a conversion failure could leave the page with
an empty body.
Fixes:
- Active-session guard: CollaborationGateway.getActiveEditorCount(documentName)
reports live human (websocket) editor sessions for a doc, excluding server-side
direct connections. writeBody now throws ActiveEditSessionError when an editor
is connected. The engine's push loop already isolates each importPageMarkdown in
try/catch and does not advance the loop-guard on failure, so the write is simply
retried on the next poll once the editor disconnects — never a clobber.
- Crash-safe conversion: build the replacement Yjs update BEFORE opening the
connection / clearing the fragment, so a transform failure can never leave the
body empty.
Also updates the server-side converter gate spec to the corrected round-trip
shape: the block-image hoist no longer leaves a leading empty paragraph (the
git-sync converter fix in 7d39c16b, now reaching the built package).
A true merge of git content into a live Yjs session is out of scope (it needs a
real 3-way text merge with no shared update lineage); deferring the write while a
page is being edited is the safe, owner-approved minimum.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
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>
- page-history history-item: a lastUpdatedSource==='git-sync' version renders a
neutral gray 'Git sync' badge (git-merge icon), NOT the agent badge/deep-link
(it is not an agent edit). +2 i18n keys.
- Dockerfile: install git in the installer (runtime) stage — VaultGit shells out
to git, so assertGitAvailable() needs the binary at runtime.
Client tsc clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
UI opt-in for git-sync, mirroring the existing sharing/comments settings pattern
(no new endpoint, no new mechanism; orchestrator read query untouched):
- UpdateSpaceDto.gitSyncEnabled?: boolean.
- SpaceRepo.updateGitSyncSettings: jsonb-merge into settings.gitSync.<key>
(COALESCE || jsonb_build_object — never clobbers sibling sharing/comments);
stored as a real jsonb boolean so the orchestrator's
settings->'gitSync'->>'enabled' = 'true' matches.
- SpaceService.updateSpace handles the flag (audit diff) via the existing
CASL-guarded space update path (Manage/Settings).
- client: Switch in edit-space-form (optimistic mutate + revert-on-error,
readOnly-aware) + space types + 2 i18n keys.
- space.service.spec extended (calls updateGitSyncSettings; no-op when undefined).
tsc clean (server+client); jest src/core/space 4 pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes found by the live pull/push e2e:
- CRITICAL: driveCycle never checked out the 'docmost' branch before
applyPullActions, so Docmost content was written straight onto 'main',
clobbering local file edits before push could diff them. Now checkout
'docmost' before pull (applyPullActions commits there then checks out main +
merges) — mirrors the engine's pull main(). Round-trip now works both ways.
- add an unresolved-merge guard (SPEC §9): skip the cycle if the vault is
mid-merge instead of failing on checkout.
- SAFETY: enabledSpaces() is now STRICT opt-in — only spaces with
settings.gitSync.enabled===true; removed the all-spaces fallback that synced
every space (incl. a 92-page one) the moment GIT_SYNC_ENABLED flipped.
- SAFETY: per-cycle delete cap (GIT_SYNC_MAX_DELETES_PER_CYCLE, default 5):
dry-run the push, and if planned deletes exceed the cap, run the apply with
deletePage neutralized — phantom absence-deletions from a non-convergent vault
can't soft-delete real pages. Fails safe if the dry-run throws.
- fix manual trigger: TriggerGitSyncDto.spaceId needs @IsUUID or the global
whitelist ValidationPipe strips it (arrived undefined -> vault 'undefined').
Live-verified on an isolated flagged space: push (vault file edit -> Docmost
content, stamped lastUpdatedSource='git-sync') and pull (Docmost rename -> vault
file + meta) both work; an unrelated 92-page space stayed untouched throughout.
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>
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>
The "Custom address" slug field sized its leftSection with a
character-count heuristic (label.length * 7 + 12), which over-estimated
the real width of the small dimmed domain prefix and left an ugly empty
gap between "docs.../l/" and the input text.
- Measure the real prefix width via a ref + useLayoutEffect (scrollWidth)
and feed it to leftSectionWidth so the slug sits flush against the
prefix, regardless of host length or font metrics.
- Restyle the prefix as an attached addon: subtle background, a right
divider border and input-matching left corner radii.
- Minor spacing tidy: description mb 4->6, action buttons mt xs->sm.
No behavior change: validation, availability probe, save/remove and the
reassign modal are untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
develop merged 20260626T130000-share-aliases; rename this PR's migration to
20260626T140000 so the two no longer share a timestamp prefix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Port the only substantive fix#211 was missing relative to #203 (which is
being closed): the "Send now" handler branched on the closure-captured
isStreaming, but a turn can finish between render and click. In that window
stop() is a no-op, so arming flushOnAbortRef/interruptNextSendRef would strand
those one-shot flags and leak into a later, unrelated Stop (auto-sending a
queued message the user never asked to send).
- Mirror the live useChat status in statusRef (updated each render) and branch
sendNow on it instead of isStreaming, so the not-streaming path runs when the
turn has already ended and the interrupt flags are never armed against a
no-op stop().
- Belt-and-suspenders: clear flushOnAbortRef/interruptNextSendRef when a new
turn starts streaming, defusing the sub-render-tick window where a flag could
still be armed but the expected abort never fired. No-op for the legit
interrupt path (both refs are consumed synchronously beforehand).
Keeps #211's existing structure and its flushNext-returns-boolean fix. The
rest of #203's divergence is comment rewording, a server-side rename of the
same pure interrupt-gate, and fewer tests — nothing else to port.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Register the new AI-chat keys "Send now" and "Interrupt and send now" in
both en-US and ru-RU catalogs so the UI never renders mixed-language
tooltip/aria-label (i18n policy).
- Make INTERRUPT_NOTE module-private (drop the unused re-export), matching the
module's private DEFAULT_PROMPT/SAFETY_FRAMEWORK siblings.
- Reset interruptNextSendRef in the flush-on-abort branch when nothing is
actually sent, so a stuck one-shot interrupt flag cannot tag the next
unrelated send; flushNext now reports whether it sent.
- Add a CHANGELOG [Unreleased]/Added entry for #198.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The deletion guard skips a note when its re-read deadline is still in the
future (user disarmed-then-re-armed in the race window between the batch
SELECT and the per-row re-read). The default stub returns an epoch deadline
(always < now), so the existing race tests never exercised the
`new Date(temporaryExpiresAt) >= now` branch; a regression dropping it or
inverting the comparison would pass unnoticed. Add a test that re-reads a
fresh future deadline and asserts removePage is not called.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Must-fix:
- CHANGELOG: add [Unreleased]/Added entry for temporary notes (#201).
- temporary-note-cleanup: re-check temporary_expires_at at deletion time so a
concurrent "Make permanent" (sets it NULL) between the batch SELECT and the
per-row removePage wins the race and the note is not trashed. Add unit tests
for the make-permanent and already-trashed race windows.
Non-blocking review items:
- temporary-note-cleanup: cap the sweep batch (LIMIT 500) so a large backlog is
not loaded into memory; remainder drains on the next hourly run.
- client: extract duplicated post-toggle cache sync into
syncTemporaryExpiresInCache() shared by the header menu and the banner.
- Remove the tautological migration spec that mocked the whole Kysely builder.
- Tests: cover create() frozen temporaryExpiresAt (workspace override + NULL
default fallback + non-temporary skips lookup) and restorePage disarming the
timer (temporaryExpiresAt: null).
Deferred (forward-looking, non-blocking): extract
PageService.computeTemporaryExpiresAt() to dedupe the deadline formula and drop
the @InjectKysely from PageTemplateController; replace migration unit test with
a real Postgres up/down integration test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address the PR #202 review (approve-with-comments). The only actionable
non-blocking item was the test-coverage suggestion: the source switch in
AiChatService.handle from findRecent(chatId, ws, 50) to findAllByChat(chatId,
ws) was not pinned by a test. handle() is a streaming method the project marks
as not unit-testable, so cover the behavioral guarantee it now relies on at the
repo/integration level — seed a chat of 60 messages and assert the default
findAllByChat (exactly how handle calls it) returns the FULL transcript in
chronological order, including the first turn the old 50-window would have
dropped.
Also document the behavior change under CHANGELOG [Unreleased] -> Changed.
The two stability items (token-budget trim before streamText; O(N) history
rebuild per turn) are deferred: the reviewer flagged both as non-blocking
conscious trade-offs aligned with the PR's stated goal, and the trim is a
larger architecture change out of scope for this follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-turn model conversation was rebuilt via findRecent(chatId, ws, 50),
a sliding window that dropped the beginning of any chat longer than ~50 stored
rows. Switch streamChat to the existing findAllByChat, which loads the full
non-deleted transcript chronologically with a 5000-row memory-safety backstop
(keeps the newest rows + logs a warning on overflow) — a safety net, not a
conversational limit. Remove the now-unused findRecent method and update the
comments/log text that referenced it (findAllByChat now feeds both the Markdown
export and the model history).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add coverage for the two untested branches in useGeneratePageTitle's
post-generation write: suppressing setContent when the live title editor
is focused (DB write + broadcast still happen, only the visible field
write is skipped), and the early return when the page editor is
destroyed (model never called).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- CHANGELOG: add an [Unreleased]/Added bullet documenting the
"generate title from content" byline button (reads live editor
content, generates via the workspace AI provider, applies through
/pages/update, gated by settings.ai.generative, throttled per user).
- use-generate-page-title: guard the visible title write against page
navigation during generation. The mutation awaits the model for 1-3s;
its closure captures the editors from the starting render, but the
global page/title atoms re-point on navigation. We now keep a live ref
to the current editors and skip setContent unless the live page editor
still belongs to the page the title was generated for
(editor.storage.pageId === pageId, mirroring TitleEditor's
activePageId guard). The DB write stays correct (keyed by the captured
pageId) and the websocket broadcast is unchanged, so only the wrong-page
field write is suppressed.
- Add a vitest suite for the hook: empty content, empty model response,
happy path, the navigation guard, and 403/503/429/other onError mapping.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an AI button in the page byline that generates a note's title from the
live editor content (including unsaved edits) and applies it immediately.
Server: one-shot, non-streaming POST /ai-chat/generate-page-title mirroring
the chat generateTitle path — gated by settings.ai.generative, throttled via
AI_CHAT_THROTTLER, resolves the workspace chat model and returns { title }.
The endpoint never touches the page; the client applies the title through the
existing /pages/update route (which enforces edit permission).
Client: ai-chat-service.generatePageTitle, a useGeneratePageTitle hook that
converts the editor HTML to markdown, calls the endpoint, applies the title
via updateTitle + updatePageData, reflects it in the unfocused title editor,
and broadcasts the UpdateEvent (mirroring TitleEditor.saveTitle). A sparkles
button (GenerateTitleGroup) renders next to dictation, edit-mode + flag gated.
Tests: pure cleanGeneratedTitle helper + controller gate/delegation/error-map.
i18n: en-US + ru-RU strings.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
"Temporary notes" with a death timer: created via a dedicated hourglass button
in the space-tree header, a note auto-moves to Trash after a configurable X
hours (default 24) unless explicitly made permanent ("structure or die").
Reuses existing mechanisms, mirroring is_template and the trash-cleanup job:
- New nullable column pages.temporary_expires_at (NULL = permanent; non-NULL =
frozen deadline) + partial index for the sweep; workspace column
temporary_note_hours (default via DEFAULT_TEMPORARY_NOTE_HOURS = 24).
- create-page DTO `temporary` flag; the deadline is frozen at creation so later
setting changes never reschedule existing notes.
- POST /pages/toggle-temporary (mirror of toggle-template): arm/clear the timer,
CASL-guarded via validateCanEdit, cross-workspace NotFound defense-in-depth.
- TemporaryNoteCleanupService: hourly @Interval sweep that soft-deletes expired
notes through the exact PageRepo.removePage path (recursive over children,
emits PAGE_SOFT_DELETED), attributed to the creator; idempotent via
deletedAt IS NULL filters.
- restorePage clears temporary_expires_at so a restored note can't be re-trashed.
- Workspace setting temporary_note_hours (audit-tracked) + a hours editor in
workspace General settings.
- Client: second create button, orange tree icon, tree + page-header menu toggle
("Make temporary"/"Make permanent"), an open-note banner with a rescue action,
and en/ru i18n.
Tests (unit): toggle-temporary controller (toggle/explicit/permission/cross-ws +
DTO validation), cleanup-job sweep (selection filters, per-note removePage,
error isolation), and a migration up/down sanity. Server tsc, client tsc -b,
and the page+workspace jest suites are green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a "send now" button to queued AI-chat messages: it interrupts the
running agent and immediately sends that message, while the agent's
partial output at interruption is kept in history and the next turn is
marked as a user interrupt.
Client:
- queue-helpers: pure `promoteToHead` to move a queued message to the head.
- chat-thread: `sendNow` (promote head + abort + flush-on-abort), one-shot
`flushOnAbortRef`/`interruptNextSendRef`, `interrupted` flag in the
request body, and the "send now" ActionIcon in the queued list.
Server:
- `interrupted` on AiChatStreamBody; pure `isInterruptResume` confirms the
client hint against persisted history (prev assistant turn aborted/
streaming) before honouring it.
- prompt: INTERRUPT_NOTE injected in the context section only on a
confirmed interrupt-resume turn so the model treats the partial answer
above as incomplete.
Tests: promoteToHead, chat-thread send-now (abort + resend + one-shot
interrupt flag + non-streaming immediate send), isInterruptResume, and
the prompt interrupt-note injection.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The #205 share-aliases feature placed share-aliases.migration.spec.ts
inside src/database/migrations/. Kysely's FileMigrationProvider loads
EVERY file in that folder as a migration, so `migration:latest` imported
the test file and crashed with "ReferenceError: describe is not defined"
(no Jest globals under tsx). That broke the migration step shared by the
e2e-server, e2e-mcp and integration-test (test/test) jobs.
Move the spec one level up to src/database/ (matching the existing
src/database/jsonb-bind.spec.ts convention) so the migration runner no
longer sees it, and fix its relative imports
(./migrations/... and ./types/...). Jest still picks it up via the
src/**/*.spec.ts test glob. Verified locally: 3 passed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The e2e transform matches .js (required so ESM-only node_modules like
nanoid/@sindresorhus get transpiled), which also sweeps in editor-ext's
prebuilt CommonJS dist/*.js. ts-jest then warns "Got a .js file to
compile while allowJs is not set to true" for each footnote file. The
.js match cannot be dropped without reintroducing the ESM load errors, so
enable allowJs for ts-jest via an inline tsconfig override (merged with
apps/server/tsconfig.json — decorators/paths/module stay intact).
Verified locally: 0 allowJs warnings, app still compiles and boots to the
Redis connection (no DI/metadata regressions).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
