F1: pad-clip adjacent different-class sessions at the raw-gap midpoint so
workMs and agentOnlyMs can never double-count the same wall-clock (default
agentTGap 7m < pIn+pOut 10m made a work-session-ending-in-agent overlap a
nearby agent_only run). Reword the config guard/comment: cross-class
disjointness is now structural, tGap≥pIn+pOut is a kept sanity bound. Add a
cross-class no-double-count test and a real seeded property/fuzz test (250
random timelines × 4 tz) asserting per-class union, cross-class disjointness,
and Σ per-day activeMs == workMs.
F2: page.controller.spec — add a ForbiddenException view-gate reject test that
asserts the rejection propagates and computeWorkTime is NOT reached, locking
validateCanView before compute.
F3: WorkTimeStat renders for agent-only pages (workMs==0, agentOnlyMs>0) with
an `agent:` headline so the punch-card stays reachable.
F4: drop the dead un-bucketed `sessions` list from the PageWorkTime response
and the client work-time types (no client reads it; bucketByDay still consumes
the core sessions internally).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Only an agent-sourced idle pulse extends an agent burst; a user idle
(human supervision) now falls through to the human branch so the session
is classified `work`, not `agent_only`. Add @MaxLength(64) to tz to match
its comment.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Оценка времени работы над страницей как сессионизация истории page_history по
паузам бездействия (WakaTime-подобно), а не span между крайними правками, плюс
drill-down в суточный таймлайн 24ч×дни. Зависит от #374 (kind + idle-пульс).
Server:
- Чистая computeWorkTime(rows, config) (§5): нормализация+дедуп → коллапс
агентских всплесков по aiChatId → ОДИН проход сессионизации (порог зависит от
пары: оба agent → agentTGap, иначе tGap; последняя сессия обязательно
закрывается) → класс готовой сессии (все agent → agent_only, иначе work) →
добивка (много-сэмпл → [first−P_in, last+P_out], одиночный скаляр →
[t−P_single, t]) → метрики = union wall-clock по классу. Детерминированная,
без БД.
- Чистая bucketByDay(sessions, tz) (§6.3): union work/agent_only РАЗДЕЛЬНО, разрез
по полуночи tz через нативный Intl (DST-корректно, без «+24ч»). Инвариант
Σ activeMs == workMs держится по построению.
- PageHistoryRepo.findTimelineByPageId — лёгкая проекция без content, ASC.
- PageHistoryService.computeWorkTime + POST /pages/history/time, гейт
validateCanView как у /history; атрибуция человек/агент по lastUpdatedSource.
Client:
- usePageWorkTime(pageId) шлёт tz зрителя (Intl локаль).
- Кликабельное число «≈ 4 ч 30 мин» в шапке страницы (порог в тултипе).
- Модалка-punch-card: 24-часовые дорожки по дням, окна work/agent разным цветом,
одиночные (P_single) приглушены, сумма за день, пустой день «—», сворачивание
длинных серий пустых дней, подпись tz + T_gap. i18n ru-RU + en-US.
Tests: 22 юнита на computeWorkTime/bucketByDay (§7-фикстура ≈1ч32м vs ≈60h наив,
закрытие последней сессии, коллапс/разрыв всплеска, idle-пульс, DST 23/25ч,
полуночный разрез, инвариант, union-без-задвоения); 5 юнитов гейта контроллера;
int-spec на реальном pg (проекция + совпадение с ядром); 6 клиентских на формат.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WRITE: parameterize the canonical importer markdownToProseMirror with
{parseMath, fuzzyLinkify} (defaults true — editor/file-import/git-sync unchanged);
MCP write paths (createPage/updatePageMarkdown/patchNode/insertNode) call with both
false so $...$ stays literal text and bare domains don't autolink (explicit
https:// still links). READ: getPage format:'text' reuses the server jsonToText path
(deterministic flag) for flat machine-diffable text, [image]/[table RxC] placeholders.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getPageBreadCrumbs returns the full ancestor chain filtered only by
deletedAt; the /breadcrumbs endpoint validates validateCanView on the
target page only. Document why this is safe rather than an accepted leak:
page restrictions inherit down the tree, so viewing a page implies the
right to view every ancestor (canUserAccessPage checks the full ancestor
chain). A hidden ancestor would already hide the target itself, making
per-ancestor filtering redundant. Owner decision: document, no logic change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Аспект A — идентичность прогона реиндекса эмбеддингов.
У статуса/поллинга реиндекса не было идентичности КОНКРЕТНОГО прогона, из-за
чего класс багов «это тот же прогон или новый?» чинили дважды. Теперь каждый
прогон получает свой runId (crypto.randomUUID в start()), он хранится в Redis-
хэше рядом с total/done/startedAt и возвращается в ReindexProgress. Эндпоинт
статуса (getMasked -> MaskedAiSettings) отдаёт runId и reindexStartedAt. Клиент
кеит поллинг на (runId, startedAt): смена runId = НОВЫЙ прогон (сбрасываем
залатанное per-run состояние поллинга), тот же runId = тот же прогон. Всё
best-effort/косметика как и остальной код прогресса: пустой/отсутствующий runId
деградирует мягко и никогда не ломает реиндекс.
Аспект B — брендированные PageId/SlugId в MCP-клиенте + валидация формата в
серверных DTO (семейство инцидентов #435: двойная идентичность страницы —
внутренний id и slugId — гонялась как голая строка и молча путалась).
- packages/mcp/src/lib/page-id.ts: номинальные типы PageId/SlugId + валидирующие
конструкторы asPageId/asSlugId и гварды isPageId/isSlugId (формат UUID и
10-символьного slugId). PageId протянут через единственный узел канонизации и
записи: resolvePageId() теперь возвращает PageId, а ключ per-page лока
(withPageLock) и точки записи в collab (mutatePageContent/replacePageContent/
updatePageContentRealtime) требуют бренд — сырой slugId/непроверенный id больше
не проходит проверку типов (инвариант #260 «resolve-then-lock» теперь на уровне
компилятора). Публичный вход методов остаётся строкой (это legitimно UUID ИЛИ
slugId), брендируется каноническое значение.
- apps/server core/page/dto: PageIdDto.pageId получает валидацию формата
(@Matches, UUID или 10-символьный slugId), так что кривая/подменённая
идентичность отклоняется на границе, а не падает в repo голой строкой.
Тесты (все зелёные, с mutation-verify каждого):
- server: getMasked отдаёт runId/reindexStartedAt; стор пишет/читает runId и
мягко деградирует его до ''; DTO отклоняет кривой pageId/slugId и принимает
валидный.
- client: чистый хелпер reindexRunKey/isNewReindexRun (кеинг поллинга на runId).
- mcp: конструкторы/гварды PageId/SlugId (валидация формата).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Две дырки свипера временных заметок:
- Не было свипа на старте: заметки, протухшие во время простоя, ждали до часа
до первого тика @Interval. Добавил onApplicationBootstrap (best-effort, не
блокирует boot).
- Гонка без блокировки: re-check и removePage не были атомарны, «Сделать
постоянной» могла проскочить между ними. Теперь re-check идёт в транзакции с
FOR UPDATE SKIP LOCKED: сериализуется с toggleTemporary (тот же row-lock) и
пропускает строки, захваченные другим воркером/инстансом (без двойной
обработки). Удаление идёт ВНУТРИ этой транзакции.
removePage получил опциональный trx: чтобы удалять под блокировкой без deadlock
на вложенной независимой транзакции. Броадкаст PAGE_SOFT_DELETED при переданном
trx отложен на commit через registerAfterCommit (откат больше не рассылает
фантомное удаление); без trx поведение прежнее.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MovePageDto.position — это fractional-indexing ключ (generateJitteredKeyBetween,
тот же генератор, что в page.service). @MinLength(5)/@MaxLength(12) не совпадали
с реальным диапазоном генератора: плотные between-вставки в глубоком дереве
растят ключ далеко за 12 символов (замерено >40), и валидный ключ, который
сервер сам сгенерил, отклонялся 400 (Gitea #139, п.6).
Теперь валидируем по charset — base-62 алфавит [0-9A-Za-z] — плюс щедрый
@MaxLength(256) как чистый DoS-guard, сильно выше любого реального ключа.
test.failing (bug-lock) распинен в обычный it; добавлен кейс на отклонение
символов вне алфавита (control/separator/инъекция/пустая строка).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Нормализация чужого markdown (GFM reference-сноски [^id] -> инлайн ^[body],
срез ведущего YAML front-matter) жила только в apps/server, поэтому MCP-путь
записи страницы (updatePageMarkdown -> markdownToProseMirrorCanonical) тот же
ввод обрабатывал ИНАЧЕ, чем серверный импорт: front-matter и [^id] утекали
как литеральный текст / битая ссылка.
Перенёс normalizeForeignMarkdown в @docmost/prosemirror-markdown и вызвал его
первым шагом в MCP markdownToProseMirrorCanonical — теперь агентский
updatePageMarkdown нормализуется точно как серверный импорт. Серверные
импортёры (import.service, file-import-task.service, page.service) берут
функцию из пакета. Тест-корпус перенесён в пакет (foreign-markdown.test.ts).
Проверено: пакет (17 тестов corpus) + весь MCP unit-suite (685) зелёные,
включая reference-footnote/fence-кейсы на canonical-пути.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
One migration + targeted hot-path fixes. API behavior 1:1 (schema change = added
indexes + a byte-identical f_unaccent function-body swap, see below).
- Trigram + composite indexes (20260705T120000-perf-indexes.ts): GIN trigram on
LOWER(f_unaccent(title/name)) for pages/users/groups (the /search/suggest
leading-wildcard LIKE did a seq scan per keystroke — EXPLAIN now confirms
Bitmap Index Scan on idx_pages_title_trgm), + page_history(page_id,id DESC),
comments(page_id,id). DEVIATION (verified byte-identical): PG18 cannot inline
the two-arg f_unaccent body during index creation, so up() swaps it to the
schema-qualified single-arg `SELECT public.unaccent($1)` — same dictionary,
identical output for all inputs, so the tsvector trigger + main @@ search stay
consistent with NO reindex; down() restores the exact two-arg body.
- Auth path: jwt.strategy reuses req.raw.workspace when workspaceId matches (the
middleware already validated it) instead of re-querying; domain.middleware
caches the workspace lookup (withCache 15s, invalidated in all 8 WorkspaceRepo
mutators, with a Date reviver for the JSON-serialized cache). USER + SESSION
caching DEFERRED — the invalidation surface (role change doesn't revoke
sessions; revocation includes background jobs) can't be safely covered, and a
missed hook on a security path is worse than the win.
- AI re-embed coalescing: aiQueue.add gets {jobId: embed-<id>, delay: 30s} so
active editing collapses to one job (worker reads current page state).
- filterAccessiblePageIds: hasRestrictedPagesInWorkspace short-circuit skips the
recursive-ancestor CTE when a workspace has zero restricted pages (wired from
search/favorites/notifications/recent/created-by). EXISTS on the same pageAccess
table the CTE anti-joins → no false-positive / no access leak. Busts the cache
on insertPageAccess so a 0->1 restricted transition takes effect immediately
(review F1).
- Small: syncTransclusion guarded by a family-node probe (both old+new content, so
the removal path is preserved); mention notifications enqueue only when the set
gained a member; redis maintainLock clears a prior interval (leak fix).
Skipped as risky (flagged): global ValidationPipe transform change; a pool-wide
statement_timeout (would kill long CREATE INDEX migrations on the same pool).
NOTE: kept the trash query's `content` select — the trash UI reads page.content
for its preview modal (review F3, would have regressed).
Gate: server tsc 0; jest page-permission/auth/search/persistence 15 suites pass;
migration up+down+idempotency verified on real PG18 with EXPLAIN confirming index
use. No new deps.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move every SERVER Markdown->ProseMirror path off the editor-ext markdown layer
(`markdownToHtml`, a second marked-based parser) onto the canonical
`@docmost/prosemirror-markdown` package, and add a foreign-markdown normalizer at
the import boundary.
Code:
- `ImportService.processMarkdown` (single `.md` upload) now parses
`markdownToProseMirror(normalizeForeignMarkdown(md))` directly — no HTML hop.
- `PageService.parseProsemirrorContent` markdown case (page create/update with
`format: 'markdown'`) same.
- `FileImportTaskService` (zip import) parses markdown with the package, then
serializes to HTML (`jsonToHtml`) so the SHARED HTML attachment / internal-link
pipeline (processAttachments + formatImportHtml + processHTML) keeps handling
`.md` and `.html` imports uniformly. The markdown PARSE — the drift source — no
longer goes through editor-ext; the PM->HTML->PM hop that follows is lossless
plumbing for attachment resolution, not a second parse.
- `canonicalizeFootnotes` stays as an idempotent #228 safety net for the HTML
path (a no-op on the already-canonical markdown output).
Normalizer (`integrations/import/utils/foreign-markdown.ts`): a TEXT pre-pass,
NOT a parser fork. The strict canonical parser does not accept GFM `[^id]`
reference footnotes (and would misread `[^id]: def` as a CommonMark link-ref
definition, silently corrupting the ref into a bogus link), so the normalizer
rewrites reference footnotes into canonical inline `^[def]` before parsing.
Callout surfaces (`:::type` and `> [!type]`) are intentionally NOT touched — the
canonical parser already accepts BOTH natively, so normalizing them would be
redundant and risk degrading its nesting/code-fence-aware handling.
Fixtures-first: foreign-markdown.spec pins the normalizer and the end-to-end
acceptance (no literal `[^id]`/`:::` leaks; re-export is canonical). The two
footnote-canonicalize specs are updated to the canonical output — the parser
assigns fresh `fn-*` ids, so they now assert by definition BODY order (still
reference-ordered, deduped, orphan-free).
FINAL CHECK: `grep -rn "htmlToMarkdown\|markdownToHtml" apps/server/src` (non
-test) is now empty — both editor-ext markdown-layer functions are gone from the
server.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Must-fix:
- insertInlineFootnote could glue a footnoteReference inside an EXISTING
definition (nested footnotesList, or a bare footnoteDefinition with no list
wrapper), which canonicalize then dropped as an orphan — silently losing the
definition's prose. Now: (a) the body/notes boundary is computed from the first
top-level block that IS or CONTAINS (recursively) a footnotesList/
footnoteDefinition, not just a top-level list; and (b) the insertNodesAfterAnchor
core skips footnotesList/footnoteDefinition subtrees entirely (skipSubtreeTypes),
so an anchor whose only match is inside a definition -> inserted:false (clean
abort, no write). Added tests: nested-definition, bare-definition, and
body-before-nested-list-still-inserts.
- editor-ext footnote-canonicalize header listed `markdownToProseMirror` among the
canonicalizing MCP paths; it is the NON-canonicalizing primitive. Replaced with
`markdownToProseMirrorCanonical` (+ note that the plain primitive is for comment
bodies) and added copy_page_content.
- Client paste: canonicalizePastedFootnotes now skips a definitions-ONLY paste
(no footnoteReference anywhere) — canonicalizing it would strip the
reference-less list and yield an EMPTY paste. Added a test.
Suggestions:
- docmost_transform now runs validateDocStructure/validateDocUrls on the RAW
transform output BEFORE canonicalizeFootnotes (mirrors updatePageJson), so a
too-deep doc gives the intended max-depth error instead of a stack overflow.
- docmost_transform tool description now states the RESULT is footnote-canonical
(dryRun diff may show tidy-ups; idempotent after first run).
- insertFootnote: dropped the dead `result ? … : undefined` ternaries and the
`as any` casts (result is always set by the time we return; the not-found path
throws and aborts mutatePage). `const r = result!;`.
Tests / architecture:
- Added a LIVE-plugin golden case: the real footnoteSyncPlugin leaves a list with
non-empty content after it in place, and canonicalize agrees (placement parity
is now a driven property, not a hand-set expected).
- Added generateFootnoteId uuidv7 shape + uniqueness test.
- Item 9: added the ENFORCEMENT-RULE comments at the server parseProsemirrorContent
and the MCP canonicalizer header (any NEW full-doc persist path MUST canonicalize;
fragments/append/prepend and comment bodies MUST NOT). Kept per-call-site over a
brittle grep CI test (the replace-vs-fragment + comment-vs-page nuance makes a
single wrapper unsafe).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Must-fix:
- Move canonicalizeFootnotes OUT of parseProsemirrorContent. It now runs only
on FULL writes (createPage, updatePageContent operation==='replace'), never on
an append/prepend fragment (a fragment would lose definition-only footnotes or
synthesize a bogus empty list). Add a server binding spec.
- Match the live plugin's list PLACEMENT: a single already-canonical
footnotesList is left exactly where it sits (the plugin never repositions a
sole correct list), so the first write no longer reorders content that follows
the list. Applied to BOTH the editor-ext copy and the MCP mirror; pinned by a
shared golden corpus case with content after the list.
- Fix MCP tool count 38 -> 39 (README x3, AGENTS.md) and the transformJs param
help (add canonicalizeFootnotes/insertInlineFootnote).
Simplifications:
- Remove the dead duplicate re-id mechanism (deriveFootnoteId/suffix/occurrence)
from the PURE canonicalizer in both copies — references are never renamed, so
the derived ids were never requested; first-wins-drop is the real behaviour.
This also makes the editor-ext footnote-util note about "no cross-package copy"
true again.
- Remove the sentinel round-trip in insertInlineFootnote: a generalized
insertNodesAfterAnchor core inserts the footnoteReference node directly.
- Drop the redundant per-definition deep clone in step 4 (shallow id-normalizing
copy; out is already deep-cloned).
Docs / architecture:
- Correct the editor-ext copy's "It exists because…" header to its real
consumers (server import, page.service create/update, client paste).
- Note markdownToProseMirror reuse for create/update comment in collaboration.ts.
- A: shared golden JSON corpus exercised by BOTH the editor-ext copy and the MCP
mirror (footnote-corpus.ts / .mjs) so "the two copies behave identically" is
checkable.
- C: split the MCP canonicalizer into a pure mirror + footnote-authoring.ts.
- B: import services persist via a different path, so left one-line consolidation
comments at the call sites rather than folding (does not fall out cleanly).
Tests: insertFootnote wrapper guards + docmost_transform dryRun auto-canonicalize
(MCP mock), page.service create/update + append/prepend binding (server jest),
shared corpus incl. nested-container reference.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The footnote canonicalizer was wired into the MCP and editor-ext write paths
but NOT into the server's user-facing markdown/HTML import paths, so importing
or pasting markdown with out-of-order, reused, or orphan footnotes did not
canonicalize -- the exact trigger bug #228 fixes was still reproduced on
import. markdownToHtml -> htmlToJson builds ProseMirror JSON directly and never
runs the editor's footnoteSyncPlugin, and that plugin does not reorder an
existing list, so the stored footnotes kept the source's physical definition
order, retained orphans, and did not collapse reused references.
Wire canonicalizeFootnotes (already exported from @docmost/editor-ext) into
every server markdown/HTML -> page-JSON seam, before persisting:
- ImportService.importPage (REST single-file .md/.html import)
- FileImportTaskService (zip import worker)
- PageService.parseProsemirrorContent (API createPage / updatePageContent)
Also hook the client markdown paste: handlePaste applies a manual transaction
(returns true), bypassing transformPasted/footnoteSyncPlugin, so a pasted
out-of-order markdown footnote block would persist out of order.
canonicalizePastedFootnotes reorders a self-contained pasted block (one that
carries its own footnotesList) to reference order, deduped and orphan-free; it
is deliberately scoped to whole-block pastes so a reference-only paste that
reuses a footnote already defined in the target doc is left untouched.
canonicalizeFootnotes is pure, idempotent and shape-safe (a doc with no
footnotes is unchanged), so it is safe on every write path.
Residual: when a pasted block merges into a doc that already has footnotes,
ordering relative to the pre-existing footnotes is still governed by the live
sync plugin (which does not reorder across the boundary).
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>
"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>
attach-1: when the same attachmentId was referenced by more than one page
in a duplicated subtree, the per-attachmentId map held only a single copy
entry, so the last page processed clobbered the others. The downstream
ownership guard (`attachment.pageId !== oldPageId`) then matched at most one
page and skipped the lone DB row entirely: no blob copied, no new row, every
copy's image 404'd. Key the map to a list of entries and copy one blob/row
per referencing page; drop the now-incorrect ownership guard.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getPageBreadCrumbs (ancestor CTE) and forceDelete (descendant CTE) used
withRecursive + unionAll with no CYCLE clause or depth cap. If a parent/child
cycle already exists in the data (e.g. one slipped in via the #7 TOCTOU race),
both queries loop forever — hang / statement timeout. Worse, the move guard
itself runs the ancestor CTE, so a cycle would disable the very guard meant to
prevent it (#207#8).
Add a depth counter bounded by MAX_PAGE_TREE_DEPTH to both recursive CTEs; the
walk stops at the cap, so a cycle yields a bounded result instead of hanging.
Real page trees are only a few levels deep, so the cap never truncates a
legitimate result. getPageBreadCrumbs selects an explicit column list (not
selectAll) so the internal depth counter never leaks into the breadcrumb shape.
Adds an integration test that seeds an A<->B cycle directly and asserts both
getPageBreadCrumbs and forceDelete return bounded / complete under a short
connection-level statement_timeout instead of hanging.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The server-side move cycle guard (getPageBreadCrumbs) and the move UPDATE ran
as two separate, unlocked statements. Two concurrent moves ("A under B" and
"B under A") could each read the same pre-write acyclic snapshot, both pass the
guard, then persist A.parentPageId=B AND B.parentPageId=A — a parent/child
cycle (TOCTOU, #207#7).
Run the cycle check and the UPDATE inside one transaction (executeTx) guarded
by a per-space advisory lock (pg_advisory_xact_lock, held until COMMIT) so all
moves within a space serialize: the second mover blocks until the first commits
and then sees the freshly written parent, so its guard rejects the cycle.
getPageBreadCrumbs gains an optional trx so the check runs on the locked snapshot.
Adds an integration test driving two opposing concurrent movePage calls and
asserting no cycle ever persists and exactly one move is rejected. Updates the
movePage unit-test stubs for the new transactional path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the open items from the latest PR #143 code review:
- test(page): cover the four agentSourceFields stamp sites (create, update,
movePage, movePageToSpace) with agent + normal-user payload assertions;
add findById({ includeIsAgent: true }) wiring guards to the JWT and collab
auth-seam specs so a future drop of the option is caught.
- fix(privacy): drop `isAgent` from UserRepo.baseFields and gate it behind a
new opt-in `findById({ includeIsAgent })`, requested only by the two auth
seams that derive provenance — stops the flag leaking via the workspace
member list and generic user payloads.
- docs: correct the agentSourceFields JSDoc and the two UPDATE-site comments
to distinguish INSERT (omitted column → DB default 'user') from UPDATE
(omitted column → existing value kept, Kysely writes only present keys).
- style(page): collapse three stray double blank lines left by an earlier edit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The agent write-stamp idiom — `...(isAgent ? { <source>: 'agent', <chat>: aiChatId } : {})`
— was hand-reimplemented at every REST write site, so each new path risked a
wrong literal or a forgotten aiChatId. Extract a single
`agentSourceFields(provenance, sourceKey, chatKey)` next to AuthProvenanceData and
call it at the 5 uniform spread sites:
- comment.service create -> createdSource / aiChatId
- page.service create/update/orphan-move/move -> lastUpdatedSource / lastUpdatedAiChatId
Sites that must CLEAR the source on a non-agent action keep their own conditional
(comment un-resolve writes an explicit null), and the collab persistence path keeps
its sticky-window logic — both noted in the helper's doc.
Behavior-preserving (the helper returns the identical object/`{}`). Typecheck
clean; server comment/page/auth/collab suites 246 pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds co-located unit tests for ten targets (client → vitest *.test.ts(x),
server → jest *.spec.ts), plus minimal behavior-preserving extractions/exports
where the issue required a pure function to test:
- encode-wav: WAV header + PCM16 clamping
- editor-ext embed-provider / utils (sanitizeUrl, isInternalFileUrl) / indent
(export clampIndent)
- label.dto @Matches regex
- move-page.dto vs generateJitteredKeyBetween parity (bug locked via test.failing)
- new-note-button canCreatePage (extracted to can-create-page.ts)
- history-editor diff (extracted pure computeHistoryDiff into history-diff.ts)
- notification getTypesForTab + repo contract (direct-tab divergence locked via
test.failing)
- search buildTsQuery (extracted + sanitizes operator inputs so adversarial
queries no longer risk a to_tsquery 500)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the missing tests for the #67 guard: self-move and a destination inside the
moved page's subtree both throw BadRequestException before updatePage; a
legitimate move proceeds. Mocks pageRepo + spies getPageBreadCrumbs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reconcile the diverged develop (13 ahead / 20 behind) with gitea/develop.
Conflict resolution — html-embed: keep the local sandboxed-iframe model
(opaque-origin srcdoc, no role-gating) and supersede gitea's same-origin
strip/kill-switch hardening (#26/#28/#29/#30). The 4 conflicted html-embed
source files resolve to the local version; the 3 strip-era spec files stay
deleted. The strip apparatus (stripDisallowedHtmlEmbedNodes,
collectHtmlEmbedSources, canAuthorHtmlEmbed, htmlEmbedAllowed) is fully gone.
Integrate gitea's page-templates / page-embed work (#31-#40) cleanly.
Fix an auto-merge arity mismatch: two new gitea page-template specs
constructed TransclusionService with the pre-sandbox 11-arg signature; drop
the trailing workspaceRepo argument to match the reduced 10-arg constructor.
Verified: server + client tsc --noEmit clean; jest (html-embed + transclusion)
14 suites / 119 tests passing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The three collect*FromPmJson collectors shared the same recursion (and the #55
depth cap) but were copy-pasted, so a future edit could diverge them. Extract a
generic collectNodes(doc, {type, map, key, lastWins, skipChildrenOfType}) and
reimplement all three on it, byte-output-identical (transclusions last-wins;
references/embeds first-wins + transclusionSource skip). Documents (not removes)
the write-only page_template_references graph and the near-duplicate client
lookup-context as tracked follow-ups, per the issue's guidance.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The 4-step html-embed gate (feature-enabled AND role-allowed -> stripHtmlEmbedNodes)
was replicated across call-sites, pinned only by brittle source-regex tests. Add
stripHtmlEmbedIfNotAllowed(json, {featureEnabled, role, onStrip}) and migrate the
5 plain strip-all sites (collab handler, page create+duplicate, both import paths,
transclusion) to it, each keeping its own feature/role resolve + log via onStrip.
Left the 2 sites with different semantics: persistence.extension (#29 preserve-
admin) and share.service (feature-only kill-switch, no role gate). Real unit tests
replace the regex pins; behavior identical.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
16 suites were disabled via testPathIgnorePatterns due to two root causes: lib0
ESM not transformed (the @hocuspocus/server -> lib0/decoding.js chain) and stock
'should be defined' specs built via Test.createTestingModule without providers.
Add lib0 to transformIgnorePatterns; convert the 14 DI placeholders to direct
new X(...) instantiation with stub deps (keeping a real construct smoke test);
re-enable the suites. Also updates the public-share limiter test to assert the
fail-closed behavior from #62 (surfaced now that the suite runs). Full server
suite: 67 passed, 689 tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Assessment of the page-embed depth/cycle cap: the server /pages/template/lookup
returns FLAT single-level content and does NOT recurse into embedded pages — the
recursive expansion + the PAGE_EMBED_MAX_DEPTH cap are entirely client render
concerns, and a scripted client is already bounded by the per-user throttle
(30/min) + the ArrayMaxSize(50) per-call cap. So no server-side depth guard is
needed; documented at lookupTemplate so future readers don't add a redundant one
or assume server recursion exists.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
handleMessage became a no-op and PageWsListener intentionally ignored
PAGE_UPDATED, so a rename/icon change (client operation:updateOne) was no longer
rebroadcast -> other clients saw stale title/icon in the sidebar+breadcrumbs
until a reload (create/duplicate/restore were covered; updateOne regressed).
Add a server-authoritative onPageUpdated handler: PageService.update detects a
real title/icon change (DTO carries the field AND value differs; no-op/content-
only saves excluded) and attaches a treeUpdate snapshot to PAGE_UPDATED; the
listener broadcasts a tree updateOne via the restriction-aware emitTreeEvent
(so a restricted page's title never leaks). Content-only saves attach nothing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sidebar-pages-tree.spec tested a LOCAL COPY of the tree-shaping (so a regression
in the real getSidebarPagesTree was invisible) and justified it with a false
jest-config claim (the ^src mapping exists). Extract the pure shaping into
shapeSidebarPagesTree(); the service now calls it and the spec imports the REAL
helper. Behavior unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
collectPageEmbedsFromPmJson (and the sibling collectors/remap) recursed with no
guard, so a pathological/cyclic non-JSON input could stack-overflow (RangeError).
Add a depth cap (1000, far above any real doc nesting) so such input is handled
gracefully. Normal documents are unaffected. Updates a stale test that asserted
the old throwing behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#67: movePage didn't check the destination wasn't the page itself or inside its
own subtree, so MCP/REST/agent/fast-drag could persist+broadcast a cycle. Reject
before the update (self-parent, or moved page among the destination parent's
ancestors via getPageBreadCrumbs).
#64: movePage emitted PAGE_MOVED from a stale pre-read even when the row didn't
change / was concurrently deleted (phantom move). Gate the emit on
updateResult.numUpdatedRows !== 0n. Both are movePage hardening in one method.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the htmlEmbed node from same-origin raw-HTML execution to a sandboxed
iframe (sandbox="allow-scripts allow-popups allow-forms", no allow-same-origin,
srcdoc) with postMessage auto-resize (validated by event.source) and an optional
manual height attr. The block now runs in an opaque origin and cannot reach the
viewer's cookies/session/API, so it is safe for any member.
Because the block is now harmless, remove the entire admin/role gating apparatus:
drop htmlEmbedAllowed/canAuthorHtmlEmbed/stripDisallowedHtmlEmbedNodes/
collectHtmlEmbedSources and every role-based strip on the write paths (collab
REST/MCP + socket, page create/duplicate, import x2, transclusion unsync), along
with the now-unused WorkspaceRepo/UserRepo injections and the PageService.create
callerRole param. Keep one strip: prepareContentForShare still removes htmlEmbed
on the anonymous public-share read path when the workspace master toggle is OFF.
The workspace settings.htmlEmbed toggle is now a plain feature switch (gates the
slash-menu and share rendering); when ON the block is available to all members.
Add settings.trackerHead: an admin-only raw HTML/JS analytics snippet injected
verbatim into the <head> of public share pages only (ShareSeoController), for
trackers that genuinely need same-origin. Admin-gated via the existing CASL
Manage/Settings ability; never injected into the authenticated app shell.
Closes security-review findings #1, #2, #4, #5, #10 (and #3 as a security issue).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve the html-embed.spec.ts conflict as a union: both #46 and #49 (already in
develop) added different test cases to the same file. Keep all of them —
stripHtmlEmbedNodes gets #46's root-node case plus develop's deeply-nested,
non-object and empty-content cases; #46's collectHtmlEmbedSources and
stripDisallowedHtmlEmbedNodes suites and develop's hasHtmlEmbedNode suite all
kept; imports unioned. No production code conflicted.
Full suite green: server 651, client (16 files), editor-ext 56, mcp 247.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve conflicts from the parallel page-embed refactor that landed in develop
via #49:
- page-embed-view.tsx: keep develop's canonical decideEmbedState for the
cycle/depth/availability guard; keep #45's #39 chrome cleanup (single source
link, IconFileText fallback) and #40 refresh remount key. Drop #45's now-unused
isPageEmbedCycle/isPageEmbedTooDeep wiring.
- page-embed-picker.tsx: use develop's excludeHost util; drop #45's duplicate
filterPageEmbedOptions and its test.
- page-embed-ancestry-context.test.tsx: keep #45's superset suite.
- page-template-access.spec.ts: keep develop's constructor args; update the two
deleteByReferenceAndSources assertions to the new 4-arg workspace-scoped
signature introduced by #45 (#36 defense-in-depth).
Full suite green: server 624, client 219, editor-ext 56, mcp 247.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ~330 tests across server (Jest), client (Vitest), editor-ext (Vitest)
and packages/mcp (node:test) for the gitmost features added since
053a9c0d: AI chat, AI agent roles, public-share assistant, MCP per-user
auth, HTML embed, page templates/embed, realtime tree, tree
expand/collapse, and the AI-settings UI.
Test-tooling fixes (prerequisite, were silently hiding coverage):
- Repair 3 page-template specs broken by the 11-arg TransclusionService
constructor; they never compiled, so template access-control / content
-leak / unsync-strip coverage was fictitious.
- Build @docmost/editor-ext before server tests via a `pretest` hook;
the stale dist omitted the new HtmlEmbed/PageEmbed exports (TS2305).
- Let jest resolve the .tsx email templates: add `tsx` to
moduleFileExtensions and widen the ts-jest transform to (t|j)sx?.
Behaviour-preserving "extract pure core" refactors that the tests drive:
- server: resolveShareAssistantRequest + uiMessageTextLength
(public-share controller), decideBasicGate + mapAuthResultToResponse
(mcp), buildErrorAssistantRecord (ai-chat), jsonbObject export (roles).
- client: render-raw-html + shouldExecute/canEdit, decide-embed-state,
page-embed picker utils, tree-socket reducers, open/close branch maps,
isEndpointConfigured/resolveKeyField; buildTreeWithChildren now treats
a permission-trimmed orphan as a root instead of crashing.
Deferred (need a test DB or HTTP harness, documented in the specs):
repo-level Postgres integration tests and the public-share XFF E2E.
Pre-existing DI/lib0-ESM suite failures are untouched and out of scope.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The create/duplicate/import gate tests asserted gate presence via brittle
expect(SRC).toMatch(/regex/) over the source text plus a reimplemented
applyGate() stand-in, so a refactor could break the real gate while they still
passed. Rewrite both specs to execute the REAL methods (PageService.create /
duplicatePage; ImportService.importPage; FileImportTaskService.processGenericImport)
with each caller role and assert on the PERSISTED content via hasHtmlEmbedNode:
member -> stripped, admin/owner+toggle ON -> preserved, toggle OFF -> stripped
for everyone, unknown/missing role -> fail-closed. No source-regex assertions
remain.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- The security-relevant catch->not_found branch in lookupTemplate (returns
not_found instead of raw content when comment-mark stripping throws) is now
tested by forcing the strip to throw with a malformed text node, asserting no
content/marks leak.
- not_found for a soft-deleted source resolved through the REAL
filterViewerAccessiblePageIds (deletedAt-excluded), not the stubbed filter.
- Rename the misleading 'honours <=50 cap' test to reflect it only exercises
dedup (the cap lives in the DTO, never engaged in the service unit).
- Cover the onlyTemplates search filter (restricts to is_template=true).
Also fix two pre-existing failing 'should be defined' specs (search service +
controller) that couldn't resolve the @InjectKysely token via createTestingModule.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the per-node pageEmbed remap decision into a shared pure helper
(remapPageEmbedSourceId) and use it BOTH in PageService.duplicatePage and the
JSON walker, so the test guards the real production path (not a mirror that
could drift). Behavior is identical: source in the copied set -> new copy id;
otherwise keep the original. Add jest coverage (16 tests): the remap helper
(in-set/out-of-set/null/nested), syncPageTemplateReferences toDelete (stale refs
removed with the right workspaceId), and insertTemplateReferencesForPages
multi-workspace grouping/filtering.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The transclusion specs predated two added constructor params, so they failed to
compile (TS2554: expected 11 args, got 10) and the suites couldn't run. Add the
missing mock args: workspaceRepo (param 11) in the lookup/access specs, and
pageTemplateReferencesRepo (param 4, which had shifted pageRepo into the wrong
slot) in the unsync-html-embed spec. All three suites now compile and pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completes the workspace htmlEmbed kill-switch. The public-share path already
strips at serve time when the toggle is OFF, but the authenticated read paths
(/info and /history/info) returned page/history content with embeds intact, so
a disabled feature kept executing for in-workspace view-only viewers until the
page was next saved. Now both paths resolve the workspace toggle and run
stripHtmlEmbedNodes when it's OFF (fail-closed on a missing workspace), before
any markdown/html format conversion. Admin-authored content only — completeness,
not privilege escalation. Injects WorkspaceRepo into PageController.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consistency hardening from #17 review (not currently exploitable):
- toggleTemplate now explicitly rejects a page outside the caller's workspace
(page.workspaceId !== user.workspaceId -> NotFound, avoiding existence leak)
instead of relying solely on the space-membership model.
- PageTemplateReferencesRepo.deleteByReferenceAndSources is now workspace-scoped
(adds a workspaceId filter + param), matching the 'scope by workspaceId
everywhere' invariant; the sole caller threads its workspaceId.
The PAGE_TEMPLATE_THROTTLER limit is intentionally left as-is (the issue's
throttle item was 'consider only'; no change without usage data).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace bare //@ts-ignore (no space, no reason) with // @ts-expect-error plus a
reason on the pageEmbed sourcePageId reassignment, matching the codebase style.
ProseMirror Attrs is read-only typed, so the reassignment genuinely errors —
@ts-expect-error is valid here.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>