Батч: ai-chat/footnotes/mcp/db/tree + red-team (#163 #181 #164 #173 #168 #180 + #159 8/10) #185

Ghost · 2026-06-25T05:17:37+03:00

Ghost commented

2026-06-25 05:17:37 +03:00

Батч из заявленных issue, каждая — отдельным коммитом. #151 (реалтайм-счётчик токенов) уже был в develop (коммит 0ebb1adc), поэтому здесь его нет.

Closes #181. Closes #180. Closes #173. Closes #163. Closes #164. Closes #168.
Partially addresses #159 (5 из 10 находок; разбивка ниже).

Что сделано (по коммитам)

#163 — живой счётчик токенов тикает между шагами агента (cherry-pick фикса: liveTurnTokens покомпонентный max(authoritative, estimate)).
#181 — компактный рендеринг «Thinking»: collapseBlankLines схлопывает \n\n (сохраняя fenced-код), убран white-space: pre-wrap с markdown-<div>. Loose-списки → tight. 9 тестов.
#164 — replaceImage больше не дёргает курсор: mutateLiveContentUnlocked переведён на applyDocToFragment (structural diff #152). +2 регресс-теста (top-level + image в callout).
#173 — хвосты jsonb по ревью #172: тот же double-encoding в model_config (роли) → ::text::jsonb + self-heal на чтении; вынесен общий jsonbBind(); интеграционные round-trip тесты (jsonb_typeof + heal); fail-open warn на битый allowlist; обновлён устаревший комментарий типа; упрощён parseToolAllowlist.
#168 — мульти-бэклинки сносок: определение с N ссылками показывает ↩ a b c …, каждая скроллит к своей occurrence. computeFootnoteRefCounts + getFootnoteRefCount (кэш в numbering-плагине), scrollToReference(id, index?). Тесты на data-слой + структуру вью. (Вёрстку строки бэклинков стоит глянуть в браузере — jsdom не покрывает визуал.)
#180 — инструкция per-MCP-сервер в системном промпте агента: миграция (instructions text), репо/DTO(@MaxLength(4000))/сервис/вью, проброс через тулсет (McpServerInstruction, только для подключённых серверов с ≥1 тулом и непустым текстом), блок <mcp_tooling> внутри safety-сэндвича, реордер toolsFor→buildSystemPrompt, клиентская форма + i18n. Тесты по всем слоям (unit + integration). Делегировано + отревьюено (APPROVE).

#159 (red-team) — 5 из 10 находок

Сделано (с тестами) — 8 из 10:

#6 — patch_node/delete_node отказываются работать при дублирующихся attrs.id (абортят + ошибка модели).
#7 — интеграционный набор (*.int-spec) теперь гоняется в CI (services: postgres(pgvector)+redis + шаг test:int).
#4 — ИИ правил не ту страницу: openPage валидируется на сервере (resolveOpenPageContext), title из БД, не из клиента (+7 тестов).
#3 — SEO-роут шара утекал title restricted-страницы: гоним через resolveReadableSharePage (restricted-ancestor gate) + isSharingAllowed (+4 теста).
#10 — split-brain title/тело: тело пишется ПЕРЕД title, провал тела не оставляет новый title над старым телом.
#1 — перемещение в незагруженного/свёрнутого родителя молча теряло страницы: insertByPosition больше не материализует частичный список [node] из незагруженного (children===undefined) родителя — оставляет ветку незагруженной и лениво подтягивает ПОЛНЫЙ набор на разворот (+тесты на модель и редьюсер).
#2 — после реконнекта удалённый/перемещённый корень оставался призраком (404): mergeRootTrees теперь реконсайлит к авторитетному входящему набору корней (дропает отсутствующие), сохраняя загруженные поддеревья (+4 теста).
#8 — реконнект не обновлял загруженные дочерние узлы: treeModel.reconcileChildren + рефреш открытых загруженных веток на сокет-connect с fetchAllAncestorChildren({fresh:true}) (+тесты на reconcile и выбор веток). Живой реконнект-раунд-трип не автоматизирован в браузере (симулировать разрыв непрактично) — рендер дерева + разворот прогнаны смоук-тестом без регрессий.

Осталось 2 из 10 (низкий приоритет по модели угроз, с обоснованием):

#9 (цикл при встречных перемещениях) — требует транзакции serializable/FOR UPDATE через getPageBreadCrumbs+updatePage (риск регрессии всего перемещения) ради крайне маловероятной гонки (двое одновременно тянут X→под-Y и Y→под-X). Скажи — сделаю отдельно.
#5 (кап стоимости) — токенный/дневной бюджет для анонимного share-ассистента (по умолчанию выключен), требует аккуратной Lua-атомарности лимитера. Скажи — сделаю.

Батч из заявленных issue, каждая — отдельным коммитом. `#151` (реалтайм-счётчик токенов) уже был в develop (коммит 0ebb1adc), поэтому здесь его нет. Closes #181. Closes #180. Closes #173. Closes #163. Closes #164. Closes #168. Partially addresses #159 (5 из 10 находок; разбивка ниже). ## Что сделано (по коммитам) - **#163** — живой счётчик токенов тикает между шагами агента (cherry-pick фикса: `liveTurnTokens` покомпонентный `max(authoritative, estimate)`). - **#181** — компактный рендеринг «Thinking»: `collapseBlankLines` схлопывает `\n\n` (сохраняя fenced-код), убран `white-space: pre-wrap` с markdown-`<div>`. Loose-списки → tight. 9 тестов. - **#164** — `replaceImage` больше не дёргает курсор: `mutateLiveContentUnlocked` переведён на `applyDocToFragment` (structural diff #152). +2 регресс-теста (top-level + image в callout). - **#173** — хвосты jsonb по ревью #172: тот же double-encoding в `model_config` (роли) → `::text::jsonb` + self-heal на чтении; вынесен общий `jsonbBind()`; интеграционные round-trip тесты (`jsonb_typeof` + heal); fail-open `warn` на битый allowlist; обновлён устаревший комментарий типа; упрощён `parseToolAllowlist`. - **#168** — мульти-бэклинки сносок: определение с N ссылками показывает `↩ a b c …`, каждая скроллит к своей occurrence. `computeFootnoteRefCounts` + `getFootnoteRefCount` (кэш в numbering-плагине), `scrollToReference(id, index?)`. Тесты на data-слой + структуру вью. (Вёрстку строки бэклинков стоит глянуть в браузере — jsdom не покрывает визуал.) - **#180** — инструкция per-MCP-сервер в системном промпте агента: миграция (`instructions text`), репо/DTO(`@MaxLength(4000)`)/сервис/вью, проброс через тулсет (`McpServerInstruction`, только для подключённых серверов с ≥1 тулом и непустым текстом), блок `<mcp_tooling>` внутри safety-сэндвича, реордер `toolsFor`→`buildSystemPrompt`, клиентская форма + i18n. Тесты по всем слоям (unit + integration). Делегировано + отревьюено (APPROVE). ## #159 (red-team) — 5 из 10 находок **Сделано (с тестами) — 8 из 10:** - **#6** — `patch_node`/`delete_node` отказываются работать при дублирующихся `attrs.id` (абортят + ошибка модели). - **#7** — интеграционный набор (`*.int-spec`) теперь гоняется в CI (`services: postgres(pgvector)+redis` + шаг `test:int`). - **#4** — ИИ правил не ту страницу: `openPage` валидируется на сервере (`resolveOpenPageContext`), title из БД, не из клиента (+7 тестов). - **#3** — SEO-роут шара утекал title restricted-страницы: гоним через `resolveReadableSharePage` (restricted-ancestor gate) + `isSharingAllowed` (+4 теста). - **#10** — split-brain title/тело: тело пишется ПЕРЕД title, провал тела не оставляет новый title над старым телом. - **#1** — перемещение в незагруженного/свёрнутого родителя молча теряло страницы: `insertByPosition` больше не материализует частичный список `[node]` из незагруженного (`children===undefined`) родителя — оставляет ветку незагруженной и лениво подтягивает ПОЛНЫЙ набор на разворот (+тесты на модель и редьюсер). - **#2** — после реконнекта удалённый/перемещённый корень оставался призраком (404): `mergeRootTrees` теперь реконсайлит к авторитетному входящему набору корней (дропает отсутствующие), сохраняя загруженные поддеревья (+4 теста). - **#8** — реконнект не обновлял загруженные дочерние узлы: `treeModel.reconcileChildren` + рефреш открытых загруженных веток на сокет-`connect` с `fetchAllAncestorChildren({fresh:true})` (+тесты на reconcile и выбор веток). Живой реконнект-раунд-трип не автоматизирован в браузере (симулировать разрыв непрактично) — рендер дерева + разворот прогнаны смоук-тестом без регрессий. **Осталось 2 из 10 (низкий приоритет по модели угроз, с обоснованием):** - **#9 (цикл при встречных перемещениях)** — требует транзакции serializable/`FOR UPDATE` через `getPageBreadCrumbs`+`updatePage` (риск регрессии всего перемещения) ради крайне маловероятной гонки (двое одновременно тянут X→под-Y и Y→под-X). Скажи — сделаю отдельно. - **#5 (кап стоимости)** — токенный/дневной бюджет для анонимного share-ассистента (по умолчанию выключен), требует аккуратной Lua-атомарности лимитера. Скажи — сделаю.

Ghost added 11 commits 2026-06-25 05:17:38 +03:00

fix(ai-chat): tick the live token counter between agent steps (#163 ) 25121e269e

The header token badge (and the "Thinking… · N tokens" line) froze between
agent steps and jumped in chunks instead of ticking smoothly. liveTurnTokens
returned the authoritative server `usage` VERBATIM as soon as it appeared, but
the server only attaches usage at a step boundary and it is cumulative over
COMPLETED steps — so during the next (in-flight) step the figure stayed frozen
at the previous boundary and the running text estimate was ignored.

Combine both sources per component via max: always compute the running estimate
(chars/≈4 over the message's reasoning/text parts, which includes the in-flight
step) and take max(authoritativeBase, estimate). Between boundaries the estimate
ticks the number up; at a boundary the authoritative figure snaps it exact; and
because the server usage is cumulative and we only ever take the max, the counter
is monotonic (never drops). Reasoning/output stay split; the #151 reasoning-only
authoritative count is preserved.

Backward compatible: in every existing test the estimate is <= the authoritative
figure, so max returns the same value. +4 tests for the in-flight-step-exceeds-
base case (output + reasoning), the authoritative-wins case, and monotonicity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

feat(ai-chat): compact reasoning rendering — collapse blank lines (#181 ) 66225294b2

The "Thinking" (reasoning) block rendered with large vertical gaps: models
emit reasoning with a blank line (\n\n) between every list item and
paragraph, which `marked` turns into loose lists (each <li> wrapped in a
<p>) and separate <p> paragraphs, each carrying a margin.

- Add `collapseBlankLines(text)`: collapse 2+ newlines to one, EXCEPT inside
  fenced code blocks (``` / ~~~) where blank lines are significant. Applied
  in reasoning-block.tsx before renderChatMarkdown, so loose lists become
  tight (no <li><p>) and paragraphs join; `breaks: true` keeps single \n as
  <br>, preserving line breaks. Reasoning-only — the normal answer is
  untouched.
- Drop `white-space: pre-wrap` from `.reasoningText`: on the rendered
  markdown <div> it turned the newlines between block tags into visible
  blank lines on top of the margins. The plain-text fallback <Text> that
  needs pre-wrap already sets it inline.

Tests: collapseBlankLines unit (collapse, fence preservation incl. tilde and
unclosed fences) + rendered-HTML assertions that a blank-line-separated list
becomes a tight list and still parses as a list after a paragraph.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(mcp): replaceImage no longer yanks the cursor (#164 ) 443ad3a856

`mutateLiveContentUnlocked` — the write path used by `replaceImage` — still
did the pre-#152 destructive write (delete the whole fragment + applyUpdate a
fresh Y.Doc), discarding every Yjs node id. y-prosemirror anchors the editor
selection to those ids, so an open editor's cursor snapped to the document
end on every image swap, exactly the #152 jump that the main write path no
longer causes.

Switch it to the same `applyDocToFragment(ydoc, newDoc)` structural diff
(updateYFragment) as the main path, so unchanged nodes keep their ids and the
live cursor stays put. It runs its own atomic transact, so the old explicit
transact/delete is gone; the now-unused docmostExtensions import is dropped.

Regression tests (cursor-stability suite): a sibling paragraph's
RelativePosition survives a top-level image src/attachmentId swap, and an
image nested in a callout, matching the shapes replaceImage produces.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(db): jsonb double-encoding follow-ups from PR #172 review (#173 ) c79e7e549d

PR #172 fixed the jsonb double-encoding for `tool_allowlist` but the same
class of bug, and the same re-derived workaround, remained elsewhere.

1. model_config (agent roles): jsonbObject still used the buggy `::jsonb`
   bind, so `ai_agent_roles.model_config` round-tripped as a jsonb STRING
   SCALAR. The read-path `typeof === 'object'` check then failed and the
   model override was SILENTLY dropped (role fell back to the default model).
   Fixed to `::text::jsonb` and added `parseModelConfig` + `normalizeRow` so
   every read self-heals already-corrupted rows (no migration).

2. Centralized the write workaround as `jsonbBind()` in database/utils.ts —
   one implementation with one explanation of the quirk — replacing the
   per-repo `jsonbArray` (mcp) and `jsonbObject` (roles).

3. Integration coverage (the fix is a DB round-trip a unit test cannot see;
   the read-side parser MASKS a write regression): new
   ai-mcp-server-repo.int-spec asserts `jsonb_typeof(tool_allowlist)='array'`
   after insert + heals a seeded string-scalar row; ai-agent-roles-repo
   int-spec gains the same for `model_config` (`'object'` + heal).

4. Updated the stale `ai-mcp-servers.types.ts` comment (the driver returns a
   JSON string for legacy rows; the repo normalizes every read).

5. Fail-open logging: a corrupt tool_allowlist degrades to "no restriction"
   (agent gets ALL tools) — normalizeRow now warns (server id only, never
   contents) so the silent widening leaves a trace.

6. Simplified parseToolAllowlist (normalize the string once, then a single
   array-of-strings check) — identical behaviour, all 12 cases still pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

feat(footnotes): multi-backlinks — definition returns to ALL its references (#168 ) 4ba8991bca

After #166 a repeated `[^a]` is one footnote (reuse): one number, one
definition, N forward links. But the definition's ↩ only returned to the
FIRST reference. Now a definition with N references shows ↩ a b c …, each
backlink scrolling to its own occurrence (Pandoc/Wikipedia convention); a
single-reference footnote keeps the plain ↩ unchanged.

- editor-ext: `computeFootnoteRefCounts(doc)` (id -> occurrence count) cached
  alongside the number map in the numbering plugin state; `getFootnoteRefCount`
  getter (O(1), no per-render doc walk). `scrollToReference(id, index?)` picks
  the index-th `sup[data-footnote-ref][data-id]` occurrence (document order),
  falling back to the first.
- client: FootnoteDefinitionView renders one lettered link (a, b, c, … aa …)
  per occurrence when refCount > 1; the chrome stays after the contentDOM so
  the #146 caret invariant holds. i18n keys (ru) added.

Tests: computeFootnoteRefCounts + getFootnoteRefCount (reuse counts, unknown
id => 0); structure test gains 3 cases (N lettered links render, click jumps
to the n-th occorrence, single ref => one ↩). NOTE: the visual layout of the
backlink row needs a real browser to verify (jsdom can't); the structural and
behavioral contract is covered headless.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(mcp): refuse ambiguous patch_node/delete_node on duplicated ids (#159 ) 791bff419a

Docmost duplicates block ids on copy/paste, and copyPageContent writes the
source document verbatim with the same ids. `patchNode`/`deleteNode` address a
block by `attrs.id` via replaceNodeById/deleteNodeById, which act on EVERY node
sharing the id — so a single patch_node/delete_node could silently
replace/remove multiple unrelated blocks with no signal to the model
(red-team finding #6).

Guard both write paths: when more than one node matches the id, skip the write
entirely (the transform returns null -> no mutation) and throw a clear
"ambiguous id — N nodes share it" error so the model re-targets with a more
specific anchor. Only an unambiguous single match is written; the 0-match and
1-match behavior is unchanged.

The duplicate-count basis is covered by node-ops.test.mjs (replaceNodeById /
deleteNodeById report count===2 for a 2-duplicate doc). The end-to-end guard
is not unit-tested because patchNode/deleteNode require a live collab provider
and the test suite has no provider mock.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ci(test): run the server integration suite against real Postgres/Redis (#159 ) 6c6253caac

The only test command in CI was `pnpm -r test` (unit `.spec.ts` on mocks).
`test:int` (`.int-spec.ts`, real Postgres/Redis) ran nowhere in CI — there
were no DB `services:` — so the cost-cap, FK-cascade, jsonb round-trip and
real AI-apply integration tests never gated a PR, and regressions in those
high-severity paths stayed green (red-team finding #7).

Add `services: postgres (pgvector) + redis` and a `pnpm --filter server
test:int` step. The pgvector image is required because migrations create
vector columns and global-setup runs `CREATE EXTENSION vector`. Service
credentials/db match the defaults in apps/server/test/integration (docmost /
docmost_dev_pw, maintenance db `docmost`, redis 6379), so no TEST_*_URL
overrides are needed; global-setup drops/recreates the isolated docmost_test
DB and migrates it.

NOTE: the workflow change itself can only be validated by an actual CI run
(YAML parses locally); the int-spec suite is verified passing locally on this
branch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

feat(ai-chat): per-MCP-server instructions in the agent system prompt (#180 ) 524d69d089

Admins can now give each EXTERNAL MCP server a free-text instruction ("how/
when to use this server's tools") that the agent receives in its SYSTEM
PROMPT next to the tool descriptions — porting the built-in SERVER_INSTRUCTIONS
idea to admin-configured servers. Trusted, admin-authored text (like a system
prompt); NON-secret, so unlike headersEnc it IS returned in views/forms.

- Migration: nullable `instructions text` on ai_mcp_servers (old rows = null =
  no guidance). Table type + repo insert/update (blank/whitespace -> null via
  blankToNull). DTO `@MaxLength(4000)`. Service threads it through
  McpServerView/toView.
- mcp-clients: `McpServerInstruction { serverName, toolPrefix, instructions }`
  threaded through the toolset/cache/lease. Guidance is built ONLY for a server
  that actually connected AND contributed >=1 callable tool (the allowlist may
  filter all of them out) AND has non-blank text — so a guide never appears for
  tools the agent cannot call. Cached with the toolset, so an edit is picked up
  next turn via the existing CRUD cache invalidation.
- System prompt: `buildMcpToolingBlock` renders an <mcp_tooling> block INSIDE
  the safety sandwich (after context, before the trailing SAFETY_FRAMEWORK) so
  it informs tool choice but cannot override the rules; each section is headed
  by the server's `prefix_*` namespace. Empty/blank -> block omitted. The
  caller (ai-chat.service) now builds the external toolset BEFORE the prompt and
  passes external.instructions; client-handle lifecycle (close-once) unchanged.
- Client: instructions field in types + a Textarea (autosize, maxLength 4000)
  in the MCP-server form with a namespace-prefix hint; i18n (en/ru).

Tests across every layer (prompt block placement + both SAFETY copies; view
blank->null; buildEntry includes guidance only for connected+>=1-tool+non-blank;
DTO MaxLength; repo + integration round-trip; service wiring). Delegated impl
reviewed (APPROVE); applied the import-type follow-up.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(ai-chat): validate the open page server-side so the agent edits the right one (#159 ) 86e631034c

The client sends the "current page" as { id, title } in the request body and the
server echoed BOTH verbatim into the system prompt context and the
getCurrentPage tool. id and title are independently attacker/desync-controllable
(two tabs, stale navigation), so openPage.id could point at page B while
openPage.title said "Page A" — the model then reported "updated Page A" while it
actually edited page B (CASL still allowed it; the user has access). Red-team
finding #4.

Resolve the open page ONCE against the DB via a new `resolveOpenPageContext`:
workspace-scoped lookup + access check, returning the AUTHORITATIVE { id, title }
(title from the DB row, never the client) or null (fail-closed) for a missing /
foreign / inaccessible page. That validated value now feeds the system prompt,
the getCurrentPage tool, AND the new-chat history origin (which previously did
this validation inline, for the id only — now shared, and the title is fixed
too).

Tests: resolveOpenPageContext covers no-id, not-found, foreign-workspace,
Forbidden, non-Forbidden-fault (fail-closed), the DB-title-wins-over-client case,
and null-title coercion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(share): SEO route must not leak a restricted page's title (#159 ) 9703fc2b36

`ShareSeoController.getShare` resolved the inherited share with the RAW
`getShareForPage`, which does NOT run the restricted-ancestor gate. So for a
page shared with includeSubPages whose descendant is permission-restricted, the
SEO route served that descendant's real title in <title>/og:title/twitter:title
to anonymous visitors and crawlers — even though the content API returns 404 for
it (red-team finding #3).

Funnel the SEO path through the canonical `resolveReadableSharePage` boundary
(the single place that checks `hasRestrictedAncestor`): a non-readable page now
serves the plain SPA index with no meta. Also honour `isSharingAllowed` — a
share whose workspace/space sharing toggle was flipped off after creation no
longer leaks its title via SEO. Title comes from the server-resolved page;
`buildShareMetaHtml` already emits robots=noindex when the share opted out of
indexing.

Tests (controller routing, fs spied at call time so bcrypt's native loader is
untouched): non-readable page => plain index, no title; sharing-disabled =>
plain index; readable+indexing => title + og:title, no noindex; readable+no-
indexing => noindex. Asserts getShareForPage is never called by the SEO path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(mcp): write page body before title to avoid split-brain on failure (#159 ) a670de7638

updatePage (markdown) and updatePageJson wrote the title via REST FIRST, then
the body via collab. If the body write failed (e.g. a collab persist timeout),
the page was left with the NEW title over its OLD body — a split-brain the tool
reported as an error but never repaired (red-team finding #10).

Reorder both: write the body first, and only set the title after the body has
persisted. Now a body-write failure leaves the title untouched (no split-brain).
A title write failing after a successful body is rarer (REST is fast) and leaves
correct content under a stale title — the strictly lesser inconsistency — which
is the same trade-off the issue's "atomic, or roll back the title" intends,
without the fragility of a rollback write that could itself fail.

No unit test: both paths require a live collab provider and the suite has no
provider mock; the change is a pure reordering. All 306 mcp tests still pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost force-pushed batch/issues-2026-06-25 from a670de7638 to dd64c2ea05

2026-06-25 05:21:02 +03:00

Compare

Ghost referenced this pull request

2026-06-25 06:06:18 +03:00

feat(ai-chat): persistent history as source of truth — step durability + server export (#183) #186

Ghost added 2 commits 2026-06-25 11:15:38 +03:00

fix(tree): stop silent page loss on move-to-unloaded-parent + reconnect ghost roots (#159 ) 7b8d1649a1

Two confirmed P1 data-loss findings in the sidebar tree sync.

#1 — Move into an unloaded/collapsed parent silently dropped pages. When a
moveTreeNode (or addTreeNode) broadcast targeted a parent whose children were
NOT yet lazy-loaded, `insertByPosition` did `kids = parent.children ?? []` and
inserted the moved node, MATERIALIZING a misleading partial child list
(`[movedNode]`) out of an unloaded (`children === undefined`) parent. The
lazy-load gate fetches only when children are absent/empty, so it then refused
to fetch — leaving the parent showing ONLY the moved node and HIDING all its
other real children (and, when the parent wasn't in the tree at all, the node
was removed and never re-fetched). Fix: `insertByPosition` distinguishes
`children === undefined` (not loaded) from `[]` (loaded-empty) and, for an
unloaded parent, does NOT insert — it leaves children unloaded and just flags
`hasChildren`, so expanding fetches the FULL set (including the moved/added
node) via the existing lazy-load.

#2 — After a socket reconnect, a deleted/moved-away root lingered as a 404
"ghost". `mergeRootTrees` was append-only: it kept every previously-loaded root
and only added new ones, so a root removed during the missed-events gap was
never dropped. It runs only once all root pages are fetched, so the incoming
list is the authoritative complete root set — fix reconciles to it (drop roots
absent from incoming) while PRESERVING each surviving root's lazy-loaded
subtree and refreshing its own fields.

Tests: insertByPosition unloaded-vs-loaded-empty parent; the move reducer
keeps a collapsed destination lazy-loadable instead of partial; mergeRootTrees
drops a ghost root, preserves a surviving subtree, adds new roots, refreshes
fields. The existing "remove when parent not in tree" reducer test still holds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(tree): refresh loaded branches on reconnect so they don't go stale (#159 ) b5e1d3bcd3

Third tree-sync finding (#8). On a socket reconnect after a missed-events gap
(laptop sleep / wifi blip), the resync only invalidated the ROOT sidebar query;
a move/rename/delete that happened INSIDE an already-loaded, expanded branch was
never reflected — the branch stayed stale until the user manually interacted.
(The #2 fix reconciles the root level; this covers the deeper loaded branches.)

- `treeModel.reconcileChildren(tree, parentId, fresh)`: replace a loaded
  branch's DIRECT children with the authoritative fresh set (drop removed, add
  new, reorder to server) while PRESERVING each surviving child's already-loaded
  grandchildren, so deeper expansion is not collapsed. An unloaded branch
  (children === undefined) is left untouched (lazy-load fetches it fresh).
- `loadedOpenBranchIds(tree, openIds)`: the branches a reconnect should refresh
  (open AND loaded). `fetchAllAncestorChildren(..., { fresh: true })` bypasses
  the 30-min sidebar cache so the reconcile sees current data (handler-order
  independent).
- space-tree: on socket `connect`, re-fetch + reconcile each open loaded branch
  of the active space (space-switch-guarded; an unloaded branch is skipped).

Tests: reconcileChildren (drop/add/reorder + preserve grandchildren + unloaded
no-op) and loadedOpenBranchIds (open+loaded only, skip unloaded, nested). The
pure logic is unit-tested; the live socket-reconnect round-trip is not
browser-automated (simulating a reconnect gap is impractical) — sidebar render +
expand were smoke-tested with no regression.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost changed title from ~~Батч: ai-chat/footnotes/mcp/db + red-team (#163 #181 #164 #173 #168 #180 + #159 частично)~~ to Батч: ai-chat/footnotes/mcp/db/tree + red-team (#163 #181 #164 #173 #168 #180 + #159 8/10)

2026-06-25 11:16:36 +03:00

Ghost referenced this issue from a commit

2026-06-25 11:35:48 +03:00

refactor(review): address PR #185 review (lease leak, tests, changelog, jsonb seam)

Ghost added 1 commit 2026-06-25 11:35:48 +03:00

refactor(review): address PR #185 review (lease leak, tests, changelog, jsonb seam) 24264efa25

8-point multi-aspect review of the batch PR; security/regressions were clean.

1. Lease leak: the #180 reorder moved `toolsFor` (which leases external MCP
   clients, refCount+1) ahead of buildSystemPrompt + forUser, but the only
   release (closeExternalClients) was bound to the streamText callbacks. A throw
   in between leaked the lease (refCount stuck, undici sockets held until
   restart). Define closeExternalClients right after the lease and wrap
   buildSystemPrompt+forUser in try/catch that closes-then-rethrows.
2. Cover the patch_node/delete_node dup-id refusal (#159 #6): extract the guard
   into a pure `assertUnambiguousMatch` (node-ops) and unit-test 0/1/>1.
3. Regress the body-before-title order (#159 #10): mock-HTTP test (collab fails
   fast against a server with no WS upgrade) asserts /pages/update (title) is
   NEVER posted when the body write fails — for updatePage AND updatePageJson.
4. CHANGELOG [Unreleased]: #180, #168 (Added); #163 (Fixed).
5. Add the missing en-US i18n keys (Back to references / {{label}}).
6. Drop the duplicate content/empty/blank cases in ai-chat.prompt.spec.ts (they
   repeat the buildMcpToolingBlock unit tests); keep only sandwich placement +
   both-safety-copies.
7. CI Postgres pg16 -> pg18 (match docker-compose).
8. jsonb decode seam: shared `parseJsonbValue(value, guard)` in database/utils.ts
   holds the legacy double-encoding self-heal in one place; parseToolAllowlist /
   parseModelConfig keep only a type-guard.

Verified: server build + 124 unit + 15 integration; mcp 311; prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost force-pushed batch/issues-2026-06-25 from 24264efa25 to f80276d41a

2026-06-25 11:36:59 +03:00

Compare

Ghost referenced this issue from a commit

2026-06-25 11:37:00 +03:00

refactor(review): address PR #185 review (lease leak, tests, changelog, jsonb seam)

Ghost referenced this issue from a commit

2026-06-25 12:08:35 +03:00

test(review): add the 4 new test-coverage points from PR #185 re-review

Ghost added 1 commit 2026-06-25 12:08:35 +03:00

test(review): add the 4 new test-coverage points from PR #185 re-review 30c358a2f8

The re-review's blocking/structural points (lease leak, dup-id guard test,
body-before-title test, CHANGELOG, pg18, shared jsonb decoder) were already
addressed in commit 24264ef; this adds the 4 genuinely-new coverage requests:

- pt 6: `scrollToReference(id, index?)` exercised against a live editor DOM —
  selects the index-th `sup[data-footnote-ref][data-id]` occurrence, falls back
  to the first for out-of-range, returns false for an empty id (scrollIntoView
  stubbed). (#168)
- pt 7: export `backlinkLabel` and pin the base-26 carry boundary
  (25->z, 26->aa, 27->ab, 51->az, 52->ba). (#168)
- pt 8: integration fail-open — a PRESENT-but-corrupt tool_allowlist (jsonb
  string scalar holding non-array JSON) reads back as null ("no restriction"),
  covering normalizeRow's degrade branch. (#159 #172/#173)
- pt 9: getFootnoteRefCount cache invalidation — adding a `[^a]` reference bumps
  the cached count 2 -> 3. (#168)

Verified: editor-ext footnote 23; client structure 7 + tsc; server int 8.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost referenced this issue from a commit

2026-06-25 12:39:32 +03:00

test(review): close the two test-coverage gaps from PR #185 auto-review

Ghost added 1 commit 2026-06-25 12:39:32 +03:00

test(review): close the two test-coverage gaps from PR #185 auto-review 364838d0b2

Approve-with-comments auto-review (8 axes); no blockers. Closes the two flagged
test gaps; the two forward-looking dedup suggestions (reconcileHasChildren helper;
unifying reconcileChildren/mergeRootTrees) are non-blocking architecture notes and
left for a follow-up (as with #186's forward-looking point).

1. Ambiguous-id refusal end-to-end (#159): the patch_node/delete_node guard
   `if (replaced/deleted !== 1) return null` was only covered in pieces — the
   replaceNodeById/deleteNodeById counts and assertUnambiguousMatch in isolation —
   so loosening the guard would not have failed a test. New mock test stands up a
   REAL Hocuspocus collab server seeded (via buildYDoc, same docmost extensions)
   with a two-blocks-one-id document and drives the real client methods: both must
   reject with /ambiguous/ AND never write to collab. Tracked via Hocuspocus
   onChange (fires synchronously per update, unlike the debounced onStoreDocument)
   so a clobbering write is actually observed — verified the test FAILS when the
   guard is loosened to `< 1`.

2. scrollToReference zero-match bail: the branch "non-empty id but querySelectorAll
   returns 0 -> matches[index] ?? matches[0] is undefined -> return false" (the real
   desync: definition present, inline ref removed from the DOM) was uncovered. Added
   a footnote.test.ts case: a definition for 'ghost' with no rendered ref -> false,
   no scroll.

Verified: 313 mcp tests + 24 editor-ext footnote tests; prettier clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Ghost added 1 commit 2026-06-25 12:49:00 +03:00

Merge remote-tracking branch 'gitea/develop' into batch/issues-2026-06-25 ed3b65c36b

# Conflicts:
#	apps/server/src/core/ai-chat/ai-chat.service.spec.ts
#	apps/server/src/core/ai-chat/ai-chat.service.ts

Ghost merged commit 3ddc329bba into develop

2026-06-25 12:49:15 +03:00

Ghost referenced this issue from a commit

2026-06-25 12:49:16 +03:00

Merge pull request 'Батч: ai-chat/footnotes/mcp/db/tree + red-team (#163 #181 #164 #173 #168 #180 + #159 8/10)' (#185) from batch/issues-2026-06-25 into develop

Ghost referenced this pull request

2026-06-26 06:27:08 +03:00

fix: bug batch — #161 #190 #207 #159 + #206 findings #212

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vvzvlad/gitmost#185