Compare commits

...

16 Commits

Author SHA1 Message Date
agent_coder 5d8364bb5f fix(#355 review round-2 F9-F11): register-gate test + shutdown idle-close + DB-path metrics gate
- F10 [stability]: closeMetricsServer() now calls server.closeIdleConnections()
  + server.unref() after server.close(). server.close()'s callback doesn't fire
  until keep-alive sockets drain, and the scraper (VictoriaMetrics/vmagent) holds
  an idle keep-alive socket — so onModuleDestroy's awaited close would hang until
  the scraper disconnects or the orchestrator SIGKILLs on the kill-grace window.
  closeIdleConnections() drops idle keep-alive sockets so shutdown completes
  immediately (Node 22, per the Dockerfile base).
- F9 [test]: client-telemetry.module.spec.ts pins the E1=B register() gate — the
  core of the "public endpoint OFF by default" decision: flag unset / any non-
  "true" value ("false"/""/"0"/…) → empty controllers+providers (route absent);
  "true"/"TRUE" → registers VitalsController + VitalsService. A flag-inversion or
  truthiness regression that reopened the anonymous disk-fill surface now fails.
- F11 [regression/perf]: the db_query_duration_seconds token work (firstSqlToken
  regex + Set lookup) is now gated on isMetricsEnabled() in database.module.ts, so
  a non-metrics deployment pays NOTHING per query (previously observeDbQuery
  no-op'd but the token was still computed on every query). Also hoisted the
  13-element known-token Set to a module const (KNOWN_SQL_TOKENS) so it's built
  once, not per query.

Gate: server tsc 0; metrics + vitals + client-telemetry suites pass (incl. the
new register-gate test).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 00:20:26 +03:00
agent_coder d3209b5aab fix(#355 review E1=B + F1-F8): gate client telemetry OFF by default + throttler/lifecycle/overflow fixes
Maintainer resolved E1 as variant B: the public vitals sink + client collection
must be OFF by default (else client_metrics grows unbounded on a self-host deploy
with no external pruner, via an unauthenticated public endpoint).

- F1: new operator flag CLIENT_TELEMETRY_ENABLED (default OFF), SEPARATE from
  METRICS_PORT (Grafana reads the table directly, independent of the scrape port).
  ClientTelemetryModule.register() provides VitalsController ONLY when the flag is
  true (route absent otherwise); the flag reaches the client via window.CONFIG
  (config.ts isClientTelemetryEnabled), and initVitals() early-returns when off.
- F2/F3 [throttler]: this repo's ThrottlerGuard applies EVERY named throttler to
  every guarded route unless skipped. The new VITALS bucket therefore (a) newly
  bound collab-token → 429 behind shared/NAT IPs, and (b) the vitals route didn't
  skip the stricter public-share-ai (5/min) bucket → effective 5/min not 120.
  Fix (additive, global config unchanged): vitals.controller @SkipThrottle the
  other buckets + @Throttle VITALS 120/min; collab-token adds VITALS_THROTTLER to
  its existing @SkipThrottle (restoring its prior effectively-unthrottled state).
- F4: metrics node:http server is closed on shutdown (MetricsServerLifecycle
  OnModuleDestroy → closeMetricsServer(), fired by enableShutdownHooks).
- F5: docSize outside [0, int4-max] drops to null (keeping the event) instead of
  overflowing int4 and failing the WHOLE batch insert (+ 2 tests).
- F6: .env.example documents METRICS_PORT (no default — unset = subsystem OFF) +
  CLIENT_TELEMETRY_ENABLED; fixed the inaccurate "default 9464" wording.
- F7: disabled/non-sampled sessions install ZERO observers — isVitalsActive()
  (enabled && sampled) gates reportClientMetric AND the page-editor
  measurePageOpen + dispatchTransaction wrapping.
- F8: kept db.d.ts hand-added (wontfix) — this repo HAND-CURATES db.d.ts (verified
  across recent fork migrations a32fba63/8c5b57eb/fdeede00); codegen would be the
  deviation. The ClientMetrics interface maps the migration 1:1.

Gate: server tsc 0, client tsc 0, server metrics/vitals/telemetry/throttle 21
tests, client route-template 5. No new deps.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 00:00:03 +03:00
agent_coder b9f3de80f5 feat(observability): dev-side perf metrics — /metrics + client vitals (#355)
The metrics INFRA is already deployed (VictoriaMetrics scraping docmost:9464,
Grafana dashboards, alerts) with a target `gitmost-app` that is red because the
app half didn't exist. This is that half. The contract (metric names, port,
table, endpoint) is FIXED by the deployed infra and matched exactly.

Server (prom-client):
- A bare node:http `/metrics` server on METRICS_PORT (default 9464), SEPARATE
  from the Fastify :3000 listener so /metrics never exists publicly; the whole
  subsystem is OFF when METRICS_PORT is unset.
- collectDefaultMetrics() + http_request_duration_seconds{method,route,status}
  via a Fastify onResponse hook using the ROUTE TEMPLATE (req.routeOptions.url,
  never the raw URL — bounded cardinality; 404 -> "unknown"), EXCLUDING SSE/
  streaming responses (would record the connection lifetime and poison p95).
- db_query_duration_seconds (Kysely log callback, labelled by the leading SQL
  token), bullmq_queue_depth{queue} (getJobCounts every 15s) +
  bullmq_job_duration_seconds{queue} (worker completed/failed),
  collab_store_duration_seconds (around onStoreDocument).
- POST /api/telemetry/vitals — PUBLIC (sendBeacon) but IP-throttled; ~16KB body
  cap, <=50 events/batch, metric-name + rating whitelist, attr truncated to 120
  chars, batch insert; malformed/foreign/oversized silently dropped and 200'd (no
  browser retry). New migration `client_metrics` (schema byte-identical to the
  contract, both indexes, conditional grafana_ro GRANT; no app-side retention —
  the maintenance container prunes >90d).

Client (web-vitals):
- initVitals() decides sampling ONCE per session (25%, sessionStorage) BEFORE
  subscribing; onINP/onLCP/onCLS/onTTFB (attribution) buffered + flushed via
  navigator.sendBeacon on visibilitychange:hidden and a timer (not fetch-per-
  metric). Custom: editor_tx_ms (dispatchTransaction sync-part timer, >8ms, with
  doc_size), page_open_ms, longtask_ms. Route labels are templates only; no
  titles/slugs/text.

Gate: server + client tsc 0, frozen install 0 (added prom-client + web-vitals +
regenerated the lock), server metrics/vitals tests 11, client route-template 5,
and the migration verified valid against real Postgres.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 23:10:29 +03:00
agent_vscode 5336f06d10 Merge pull request 'fix(e2e)+ci: канон callout '> [!info]' в e2e-mcp + параллельная сборка с гейтом на publish' (#356) from fix/e2e-callout-and-gate-build into develop 2026-07-04 22:42:11 +03:00
agent_vscode 4bd579f7f6 ci(develop): build image in parallel with tests, gate only the publish
Two-phase scheme instead of the sequential gate: the build job runs in
parallel with test/e2e jobs and only warms the buildx GHA cache
(push:false, cache-to mode=max); a new publish job (needs: test,
e2e-server, e2e-mcp, build) rebuilds from the warm cache (near-instant
on hit, full rebuild on eviction — same as the old sequential timing)
and pushes :develop. GHCR login moved to publish; build-args blocks are
kept textually identical between the two jobs so the cache hits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:41:25 +03:00
agent_vscode 7bf1c91a95 ci(develop): gate the :develop image build on e2e suites
Reverse the previous policy where e2e jobs only turned the run red
without blocking the image publish: build.needs now lists test,
e2e-server and e2e-mcp, so a failing test of any kind stops the
:develop image from being built and pushed. Stale policy comments
updated accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:33:06 +03:00
agent_vscode 6c82c54470 test(mcp): expect Obsidian '> [!info]' callout export in e2e (#333 canon)
PR #333 deliberately changed the canonical markdown export of callout
nodes to the Obsidian-native format ('> [!type]' + blockquote body,
pinned by packages/prosemirror-markdown unit tests); the importer still
parses both ':::type' fences and '> [!type]'. The get_page e2e assertion
was missed in that switch and still expected ':::info', failing the
e2e-mcp job on develop since 4369bbc5.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:33:06 +03:00
agent_vscode 382e5196da Merge pull request 'fix(docker): toolchain python3/make/g++ для нативной сборки re2' (#353) from fix/docker-re2-toolchain into develop 2026-07-04 22:11:49 +03:00
agent_vscode 76e0c08cec fix(docker): install python3/make/g++ toolchain for re2 native build
The develop image build broke at `pnpm install --frozen-lockfile`: the new
native dependency re2@1.25.0 (packages/mcp, search_in_page #330) always
compiles from source under pnpm — its prebuilt-binary downloader
(install-artifact-from-github) cannot identify the GitHub repo because pnpm
does not populate npm_package_repository_*/npm_package_json env vars ("No
github repository was identified. Building locally ..."), and node:22-slim
ships no python3/make/g++ for the node-gyp fallback.

- builder stage: add a cache-friendly apt layer with python3 make g++
  before COPY; the stage is discarded so the toolchain may stay.
- installer stage: install the toolchain, run the prod install as the node
  user via `su node -c`, and purge the toolchain — all in one RUN layer so
  the final image stays slim and node_modules ownership needs no extra
  chown layer; USER node is restored right after.

Fixes the failed run 28715009124 (develop docker build); release.yml uses
the same Dockerfile and is covered too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-04 22:09:40 +03:00
vvzvlad 8978d69f3e Merge pull request 'fix(converter): стабильность round-trip image/медиа — «» ≡ absent (класс defaults-instability)' (#350) from fix/media-roundtrip-stability into develop
Reviewed-on: #350
2026-07-04 21:30:12 +03:00
agent_coder c192f2a2e1 test(prosemirror-markdown): pin the third state — explicit "" converges once, then idempotent
Reviewer addition to the round-trip stability matrix: besides "attr absent" and
"attr has a real value", a string attr in the empty-string class has a third,
degenerate state — a LITERAL "" (a user types alt/title/name in the editor then
deletes it, and Tiptap persists `attr: ""`, distinct from never-set). The fix's
`getAttribute(...) || null` coercion normalizes such a stored "" to the default
on the FIRST round-trip (a one-time "" -> null diff) and is byte-stable from the
SECOND round-trip on.

Adds a convergence contract to the reusable matrix helper (emptyStringClass flag
+ runConvergenceCase): pass 1 must converge the attr to its schema default (NOT
asserted byte-stable vs the "" input — that is the intended one-time
normalization); pass 2 must deep-equal pass 1 (idempotent thereafter). Driven for
every empty-string-class attr across image + the media family (image/drawio
alt+title, video alt via aria-label, pdf/attachment name, attachment mime).
Documents the one-time normalization so a future sync/QA diff does not flag the
single "" -> null change as converter corruption.

Gate: package suite 33 files / 682 tests passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 21:17:17 +03:00
vvzvlad d78b985062 Merge pull request 'perf(comment): статический рендер + ленивые редакторы + мемоизация панели (#340)' (#349) from fix/340-comment-panel-perf into develop
Reviewed-on: #349
2026-07-04 20:55:11 +03:00
agent_coder 2ce672709a fix(prosemirror-markdown): stabilize image round-trip — "" ≡ absent on parse (empty-string class)
A stored image authored without `alt` gained a phantom `alt: ""` on every
round-trip (`markdownToProseMirror(convertProseMirrorToMarkdown(doc))`): `marked`
renders `![](src)` as `<img alt="">`, and the stock tiptap Image `alt` parseHTML
(`getAttribute("alt")`) materialized the empty string where the original had no
attribute. That false diff is a real GS-EDIT-REVERT churn source — an agent /
git-sync touch of a page with an image mutates the stored JSON (`absent -> ""`),
producing phantom diffs that can overwrite live edits.

Fix is PARSE-SIDE ("" ≡ absent), so the RAW round-trip is idempotent — not only
the canonical form (history / stored JSON diff on the raw shape; masking it only
in canonicalize would leave that noise). `image.alt`/`title` parseHTML now coerce
`getAttribute(...) || null`, plus defense-in-depth `|| null` across the at-risk
empty-string class (video aria-label, drawio/excalidraw title+alt, pdf name,
attachment name+mime) matching the existing `image.caption || null` precedent.

NOTE — image `align` is NOT changed: it round-trips correctly (center via the
schema default "center", left/right via the `<!--img {...}-->` comment). Its
`toBeUndefined()` in the git-sync gate is canonical-form normalization, not a loss.

Intentional divergence from editor-ext: editor-ext's literal `alt` parseHTML
returns "" verbatim, but this coercion CONVERGES on editor-ext's real STORED
shape (an image inserted without alt has no `alt` attribute -> re-parses absent,
never ""), so the round-trip is idempotent and matches real documents.

Adds a reusable, node-agnostic round-trip-stability matrix helper
(test/roundtrip-stability.helper.ts) — given a node + attr spec it enumerates
default/non-default combos and asserts byte-stability of BOTH the raw and the
canonical round-trip (the documented numeric width/height→string coercion encoded
as an explicit allowed normalization) — driven over image + the whole media
family (video/audio/pdf/attachment/embed/drawio/excalidraw). The only raw
empty-string instability it found was image.alt; the family was already stable.

Gate: package suite 33 files / 672 tests passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 20:51:34 +03:00
vvzvlad c252068672 Merge pull request 'feat(ai-chat): отложенная загрузка инструментов (deferred tools + loadTools) (#332)' (#341) from fix/332-deferred-tools into develop
Reviewed-on: #341
2026-07-04 20:47:45 +03:00
agent_coder 68caf8157a test(ai-chat): document AI_CHAT_DEFERRED_TOOLS + pin ON-path & catalog completeness (#341 review F1-F3)
- F1: document AI_CHAT_DEFERRED_TOOLS in .env.example (AI_* section) — default
  ON = deferred loading (compact catalog + loadTools), =false restores the old
  "all tools always active" behavior.
- F2: integration test of the ON path in ai-chat-stream.int-spec.ts — a deferred
  tool activated via loadTools is active on the SAME turn's next step but a fresh
  turn starts cold (CORE + loadTools only), proving the per-turn activatedTools
  Set does not leak across turns/chats. Drives the real streamText loop with a
  MockLanguageModelV3 and inspects recorded per-step activeTools-filtered tools.
- F3: replace the magic toHaveLength(28) in tool-tiers.spec.ts with a two-way
  partition against the LIVE in-app toolset (AiChatToolsService.forUser keys):
  every non-core tool must appear in buildInAppDeferredCatalog and every catalog
  entry must map to a real non-core tool — so a future tool forgotten in
  INLINE_TOOL_TIERS fails the suite instead of silently vanishing from the agent.

No production logic change (mechanism was already reviewed correct).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 20:34:42 +03:00
claude code agent 227 e431b33bb1 feat(ai-chat): deferred tool loading (tiers + loadTools meta-tool) (#332)
The in-app AI agent shipped all ~41 tool schemas on every model step. This
adds a two-tier catalog: core tools (frequent or one-line) stay always-active;
the rest are advertised as a compact catalog and their full schema is fetched
on demand via the loadTools meta-tool, wired through ai@6 prepareStep's
per-step activeTools.

- tools/tool-tiers.ts: CORE_TOOL_KEYS, INLINE_TOOL_TIERS, applyLoadTools,
  catalog builders (+ tool-tiers.spec.ts, 13 cases).
- ai-chat.service.ts prepareAgentStep: returns activeTools =
  [...CORE_TOOL_KEYS, loadTools, ...activatedTools]; per-turn activated Set.
- ai-chat.prompt.ts: buildToolCatalogBlock renders the deferred catalog.
- mcp/tool-specs.ts: tier + catalogLine metadata (external snake_case /mcp
  transport unchanged).
- EnvironmentService.isAiChatDeferredToolsEnabled(): AI_CHAT_DEFERRED_TOOLS,
  default ON per issue intent (kill-switch =false restores old behavior).

Gate: server ai-chat 631/631, tool-tiers 13/13, mcp 472/472, tsc clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-04 19:57:11 +03:00
53 changed files with 3540 additions and 40 deletions
+31
View File
@@ -202,6 +202,13 @@ MCP_DOCMOST_PASSWORD=
# Default 900000 (15 min).
# AI_MCP_CALL_TIMEOUT_MS=900000
# Deferred tool loading for the in-app AI chat (#332). Default ON: the agent sees
# a compact <tool_catalog> and only CORE tools + a loadTools meta-tool are active
# each step; deferred tools (the fat/rare ones + all external MCP tools) load on
# demand. Set AI_CHAT_DEFERRED_TOOLS=false to restore the old "all tools always
# active" behavior.
# AI_CHAT_DEFERRED_TOOLS=true
# --- Anonymous public-share AI assistant ---
# Opt-in per workspace (AI settings -> "public share assistant"; off by default).
# When enabled, anonymous visitors of a published share can ask an AI about that
@@ -235,3 +242,27 @@ MCP_DOCMOST_PASSWORD=
# FAILS CLOSED if Redis is unavailable (default: 1,000,000 tokens per workspace
# per rolling day).
# SHARE_AI_WORKSPACE_TOKEN_BUDGET_PER_DAY=1000000
# --- Observability / perf metrics (#355) ---
#
# Two INDEPENDENT toggles, both OFF by default:
#
# 1) METRICS_PORT — the server-side Prometheus scrape endpoint.
# UNSET (default) => the whole prom subsystem is OFF: no registry, no
# collectors, and NOTHING is exposed on the main app port. There is NO
# default port — leaving it blank disables it. When set to a port (e.g.
# 9464), a SEPARATE bare node:http listener serves GET /metrics on that port
# only (never on the main :3000 app listener), for a scraper such as
# VictoriaMetrics/Prometheus reaching it as <host>:<port>/metrics.
# METRICS_PORT=9464
#
# 2) CLIENT_TELEMETRY_ENABLED — the public client perf-telemetry sink.
# OFF by default. When true, the unauthenticated POST /api/telemetry/vitals
# endpoint is registered and browsers collect + send web-vitals / editor
# metrics into the `client_metrics` table (read directly by Grafana, separate
# from METRICS_PORT). Leave OFF unless you actually consume this data: the
# endpoint is public and the table has NO app-side retention, so enabling it
# requires an EXTERNAL pruner to bound `client_metrics` growth (the deployed
# infra prunes rows >90d via a maintenance container). When off, the endpoint
# does not exist and the client installs no observers.
# CLIENT_TELEMETRY_ENABLED=false
+42 -11
View File
@@ -18,12 +18,48 @@ env:
IMAGE: ghcr.io/vvzvlad/gitmost
jobs:
# Run the reusable test suite first so a failing test blocks the image build.
# Run the reusable test suite. Together with the e2e jobs below it gates the
# publish job (the image push), not the build itself — build runs in parallel.
test:
uses: ./.github/workflows/test.yml
# Runs in parallel with the test/e2e jobs and only warms the buildx cache
# (GHA cache, scope develop-amd64). No push happens here — the publish job
# below is the only one that pushes the image.
build:
needs: test
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
- name: Resolve version
id: version
run: echo "value=$(git describe --tags --always)" >> "$GITHUB_OUTPUT"
- name: Build develop image (warm cache, no push)
uses: docker/build-push-action@v6
with:
context: .
platforms: linux/amd64
build-args: |
APP_VERSION=${{ steps.version.outputs.value }}
AI_AGENT_ROLES_CATALOG_URL=https://raw.githubusercontent.com/vvzvlad/gitmost/develop/agent-roles-catalog
push: false
cache-from: type=gha,scope=develop-amd64
cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
# The gate: rebuilds from the cache the build job just wrote (near-instant on
# a cache hit; worst case — cache eviction — a full rebuild, which matches the
# old sequential timing) and pushes :develop only when unit tests AND both
# e2e suites AND the build are green.
publish:
needs: [test, e2e-server, e2e-mcp, build]
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
@@ -57,13 +93,10 @@ jobs:
push: true
tags: ${{ env.IMAGE }}:develop
cache-from: type=gha,scope=develop-amd64
cache-to: type=gha,scope=develop-amd64,mode=max,ignore-error=true
# e2e jobs run on every develop push but DO NOT gate the build/publish above:
# `build` stays `needs: test` only, so the :develop image still ships even if
# e2e fails. A failing e2e job turns the run red and triggers GitHub's email
# to the pusher — that red run + email is the intended notification, not a
# deploy block.
# e2e jobs gate the publish (image push), not the build: the :develop image
# is pushed only when unit tests AND both e2e suites pass (publish.needs
# lists them all).
e2e-server:
runs-on: ubuntu-latest
# Hard cap: the full-AppModule e2e leaks open handles and hung jest to the 6h max.
@@ -124,9 +157,7 @@ jobs:
- name: Run server e2e
run: pnpm --filter ./apps/server test:e2e
# Same rationale as e2e-server: this job is intentionally NOT in
# `build.needs`. Deploy of the :develop image must not be blocked by e2e;
# a red run plus GitHub's email to the pusher is the notification mechanism.
# Gates the publish too — see the comment above e2e-server.
e2e-mcp:
runs-on: ubuntu-latest
timeout-minutes: 20
+16 -2
View File
@@ -5,6 +5,13 @@ RUN npm install -g pnpm@10.4.0
FROM base AS builder
# re2 (packages/mcp) always compiles from source under pnpm (the prebuilt-binary
# download cannot identify the GitHub repo), so node-gyp needs python3/make/g++.
# This stage is discarded, so the toolchain can stay installed.
RUN apt-get update \
&& apt-get install -y --no-install-recommends python3 make g++ \
&& rm -rf /var/lib/apt/lists/*
WORKDIR /app
COPY . .
@@ -57,9 +64,16 @@ COPY --from=builder /app/patches /app/patches
RUN chown -R node:node /app
USER node
# Toolchain is needed transiently to compile re2 during the prod install; install
# and purge it in one layer to keep the final image slim. The install itself runs
# as the node user via su to keep node_modules ownership without a costly chown layer.
RUN apt-get update \
&& apt-get install -y --no-install-recommends python3 make g++ \
&& su node -c "pnpm install --frozen-lockfile --prod" \
&& apt-get purge -y --auto-remove python3 make g++ \
&& rm -rf /var/lib/apt/lists/*
RUN pnpm install --frozen-lockfile --prod
USER node
RUN mkdir -p /app/data/storage
+1
View File
@@ -61,6 +61,7 @@
"react-clear-modal": "^2.0.18",
"react-dom": "^18.3.1",
"react-drawio": "1.0.7",
"web-vitals": "^5.1.0",
"react-error-boundary": "6.1.1",
"react-helmet-async": "3.0.0",
"react-i18next": "16.5.8",
@@ -93,6 +93,11 @@ import {
isBodyEditable,
isCollabSynced,
} from "@/features/editor/editor-sync-state";
import {
isVitalsActive,
measurePageOpen,
reportEditorTx,
} from "@/lib/telemetry/vitals";
interface PageEditorProps {
pageId: string;
@@ -351,6 +356,40 @@ export default function PageEditor({
editor.storage.pageId = pageId;
handleScrollTo(editor);
editorRef.current = editor;
// #355 — perf instrumentation. Skip ALL of it when telemetry is
// disabled (F1 flag off) or this session isn't sampled: no page-open
// measure, and crucially NO dispatch wrapping, so a non-collecting
// session pays zero per-transaction cost.
if (isVitalsActive()) {
// page_open_ms: this is the first editor-content render, so measure
// against any page-open mark set on the tree-row/link click.
measurePageOpen();
// editor_tx_ms: time the SYNCHRONOUS part of applying each
// transaction (state.apply + updateState) by wrapping the view's
// dispatch. Only slow syncs (>8ms) are reported (see reportEditorTx),
// so the common path adds just one performance.now() pair. Passive:
// the original dispatch still runs unchanged.
try {
const view = editor.view as unknown as {
dispatch: (tr: unknown) => void;
};
const originalDispatch = view.dispatch.bind(view);
view.dispatch = (tr: unknown) => {
const started = performance.now();
originalDispatch(tr);
const elapsed = performance.now() - started;
try {
reportEditorTx(elapsed, editor.state.doc.content.size);
} catch {
// never let telemetry break editing
}
};
} catch {
// if the view shape changes, skip editor_tx instrumentation
}
}
}
},
onUpdate({ editor }) {
+7
View File
@@ -47,6 +47,13 @@ export function isCompactPageTreeEnabled(): boolean {
return castToBoolean(getConfigValue("COMPACT_PAGE_TREE", "true"));
}
// #355 — operator toggle for client perf-telemetry. DEFAULT OFF: the server
// mirrors CLIENT_TELEMETRY_ENABLED into window.CONFIG; when off the client
// installs no observers and sends nothing (the sink endpoint doesn't exist).
export function isClientTelemetryEnabled(): boolean {
return castToBoolean(getConfigValue("CLIENT_TELEMETRY_ENABLED", "false"));
}
export function getAvatarUrl(
avatarUrl: string,
type: AvatarIconType = AvatarIconType.AVATAR,
@@ -0,0 +1,35 @@
import { describe, it, expect } from "vitest";
import { templateRoute } from "./route-template";
describe("templateRoute", () => {
it("templates a space page path (never leaks slugs)", () => {
const t = templateRoute("/s/engineering/p/design-doc-abc123");
expect(t).toBe("/s/:space/p/:slug");
expect(t).not.toContain("engineering");
expect(t).not.toContain("design-doc");
});
it("templates share, redirect and space paths", () => {
expect(templateRoute("/share/abc/p/xyz")).toBe("/share/:shareId/p/:slug");
expect(templateRoute("/share/p/xyz")).toBe("/share/p/:slug");
expect(templateRoute("/p/some-slug")).toBe("/p/:slug");
expect(templateRoute("/s/team")).toBe("/s/:space");
expect(templateRoute("/s/team/trash")).toBe("/s/:space/trash");
expect(templateRoute("/labels/urgent")).toBe("/labels/:label");
});
it("keeps known static routes verbatim", () => {
expect(templateRoute("/home")).toBe("/home");
expect(templateRoute("/settings/members")).toBe("/settings/members");
expect(templateRoute("/")).toBe("/");
});
it("normalises a trailing slash", () => {
expect(templateRoute("/s/team/p/slug/")).toBe("/s/:space/p/:slug");
});
it("collapses unknown paths to 'other' (bounded cardinality)", () => {
expect(templateRoute("/weird/unknown/thing")).toBe("other");
expect(templateRoute("/s/team/p/slug/extra/segments")).toBe("other");
});
});
@@ -0,0 +1,70 @@
/**
* Map a raw pathname to a BOUNDED route TEMPLATE (#355).
*
* Perf metrics must be labelled by route template only — never a raw path with
* slugs/ids — so the server-side `route` column and any downstream aggregation
* stay low-cardinality and carry NO page slugs/titles (privacy). Anything that
* does not match a known pattern collapses to `other`.
*
* The template vocabulary mirrors the issue's example (`/s/:space/p/:slug`).
*/
const ROUTE_PATTERNS: { re: RegExp; template: string }[] = [
// Share pages (public).
{ re: /^\/share\/[^/]+\/p\/[^/]+$/, template: '/share/:shareId/p/:slug' },
{ re: /^\/share\/p\/[^/]+$/, template: '/share/p/:slug' },
{ re: /^\/share\/[^/]+$/, template: '/share/:shareId' },
// Page redirect.
{ re: /^\/p\/[^/]+$/, template: '/p/:slug' },
// Space + page.
{ re: /^\/s\/[^/]+\/p\/[^/]+$/, template: '/s/:space/p/:slug' },
{ re: /^\/s\/[^/]+\/trash$/, template: '/s/:space/trash' },
{ re: /^\/s\/[^/]+$/, template: '/s/:space' },
// Misc dynamic.
{ re: /^\/labels\/[^/]+$/, template: '/labels/:label' },
{ re: /^\/invites\/[^/]+$/, template: '/invites/:invitationId' },
{ re: /^\/settings\/groups\/[^/]+$/, template: '/settings/groups/:groupId' },
];
// Static routes we accept verbatim (finite set).
const STATIC_ROUTES = new Set<string>([
'/home',
'/spaces',
'/favorites',
'/login',
'/forgot-password',
'/password-reset',
'/setup/register',
'/settings/account/profile',
'/settings/account/preferences',
'/settings/workspace',
'/settings/ai',
'/settings/members',
'/settings/groups',
'/settings/spaces',
'/settings/sharing',
]);
export function templateRoute(pathname: string): string {
// Normalise a trailing slash (except root).
const path =
pathname.length > 1 && pathname.endsWith('/')
? pathname.slice(0, -1)
: pathname;
if (path === '' || path === '/') return '/';
if (STATIC_ROUTES.has(path)) return path;
for (const { re, template } of ROUTE_PATTERNS) {
if (re.test(path)) return template;
}
return 'other';
}
/** Template for the current window location. */
export function currentRouteTemplate(): string {
try {
return templateRoute(window.location.pathname);
} catch {
return 'other';
}
}
+290
View File
@@ -0,0 +1,290 @@
import {
onCLS,
onINP,
onLCP,
onTTFB,
type CLSMetricWithAttribution,
type INPMetricWithAttribution,
type LCPMetricWithAttribution,
type TTFBMetricWithAttribution,
} from "web-vitals/attribution";
import { isClientTelemetryEnabled } from "@/lib/config";
import { currentRouteTemplate } from "./route-template";
/**
* Client perf-telemetry (#355): web-vitals + custom metrics buffered and posted
* to POST /api/telemetry/vitals via sendBeacon.
*
* Design constraints from the issue:
* - Sampling is decided ONCE per session (25%), cached in sessionStorage,
* BEFORE any observer is subscribed. Non-sampled sessions send nothing.
* - Route labels are TEMPLATES only; attr is truncated to 120 chars; no page
* titles/slugs/text ever leave the browser.
* - Observers are passive and reporting is best-effort — telemetry must not
* degrade the perf it measures.
*/
const ENDPOINT = "/api/telemetry/vitals";
const SAMPLE_RATE = 0.25;
const SAMPLE_KEY = "gm_vitals_sampled";
const FLUSH_INTERVAL_MS = 15_000;
const MAX_BUFFER = 40; // flush early if the buffer fills between timers
const MAX_ATTR_LENGTH = 120;
const EDITOR_TX_MIN_MS = 8; // only report editor transactions slower than this
const ALLOWED_NAMES = new Set([
"INP",
"LCP",
"CLS",
"TTFB",
"editor_tx_ms",
"page_open_ms",
"longtask_ms",
]);
interface VitalEvent {
name: string;
value: number;
rating?: string;
route?: string;
attr?: string;
docSize?: number;
}
let sampledCache: boolean | null = null;
let initialised = false;
let buffer: VitalEvent[] = [];
let longtaskSum = 0; // accumulated longtask duration (ms) for the current window
/**
* Decide once per session whether this session is sampled. Cached in
* sessionStorage so the choice is stable across reloads within the session and
* identical for every observer/custom-metric caller.
*/
export function isVitalsSampled(): boolean {
if (sampledCache !== null) return sampledCache;
try {
const stored = sessionStorage.getItem(SAMPLE_KEY);
if (stored === "1") return (sampledCache = true);
if (stored === "0") return (sampledCache = false);
const sampled = Math.random() < SAMPLE_RATE;
sessionStorage.setItem(SAMPLE_KEY, sampled ? "1" : "0");
return (sampledCache = sampled);
} catch {
// sessionStorage unavailable (private mode / SSR): default to not sampled.
return (sampledCache = false);
}
}
/**
* True only when telemetry is BOTH enabled by the operator (F1 flag) AND this
* session is sampled. Callers outside initVitals (e.g. the editor dispatch
* wrapper) use this to skip ALL instrumentation cost on disabled/non-sampled
* sessions — no observers, no per-transaction timing.
*/
export function isVitalsActive(): boolean {
return isClientTelemetryEnabled() && isVitalsSampled();
}
function truncateAttr(value: unknown): string | undefined {
if (typeof value !== "string" || value.length === 0) return undefined;
return value.slice(0, MAX_ATTR_LENGTH);
}
function enqueue(event: VitalEvent): void {
if (!ALLOWED_NAMES.has(event.name)) return;
if (!Number.isFinite(event.value)) return;
buffer.push(event);
if (buffer.length >= MAX_BUFFER) flush();
}
function flush(): void {
// Fold any pending longtask total into the batch first.
if (longtaskSum > 0) {
buffer.push({
name: "longtask_ms",
value: Math.round(longtaskSum),
route: currentRouteTemplate(),
});
longtaskSum = 0;
}
if (buffer.length === 0) return;
const payload = JSON.stringify({ events: buffer });
buffer = [];
try {
const blob = new Blob([payload], { type: "application/json" });
if (navigator.sendBeacon && navigator.sendBeacon(ENDPOINT, blob)) return;
// Fallback for browsers without sendBeacon: keepalive fetch.
void fetch(ENDPOINT, {
method: "POST",
body: payload,
headers: { "Content-Type": "application/json" },
keepalive: true,
}).catch(() => undefined);
} catch {
// Best-effort: never throw out of telemetry.
}
}
/**
* Report a custom client metric (editor_tx_ms, page_open_ms). No-op unless the
* session is sampled. Route is always the current TEMPLATE.
*/
export function reportClientMetric(
name: "editor_tx_ms" | "page_open_ms",
value: number,
extra?: { docSize?: number },
): void {
if (!isVitalsActive()) return;
if (!Number.isFinite(value)) return;
enqueue({
name,
value,
route: currentRouteTemplate(),
docSize: extra?.docSize,
});
}
/** Threshold-gated editor transaction reporter (only reports slow syncs). */
export function reportEditorTx(ms: number, docSize: number): void {
if (ms <= EDITOR_TX_MIN_MS) return;
reportClientMetric("editor_tx_ms", ms, { docSize });
}
const PAGE_OPEN_MARK = "gm_page_open_start";
/** Mark the start of a page-open interaction (tree-row / link click). */
export function markPageOpenStart(): void {
try {
performance.clearMarks(PAGE_OPEN_MARK);
performance.mark(PAGE_OPEN_MARK);
} catch {
// ignore
}
}
/**
* Measure page_open_ms at first editor-content render, if a start mark exists.
* Consumes the mark so a later render doesn't double-count.
*/
export function measurePageOpen(): void {
try {
const marks = performance.getEntriesByName(PAGE_OPEN_MARK, "mark");
if (marks.length === 0) return;
const started = marks[0].startTime;
const elapsed = performance.now() - started;
performance.clearMarks(PAGE_OPEN_MARK);
if (elapsed > 0 && Number.isFinite(elapsed)) {
reportClientMetric("page_open_ms", elapsed);
}
} catch {
// ignore
}
}
function attrTarget(
metric:
| INPMetricWithAttribution
| LCPMetricWithAttribution
| CLSMetricWithAttribution,
): string | undefined {
const a = metric.attribution as Record<string, unknown> | undefined;
if (!a) return undefined;
// Different vitals expose their culprit element under different keys; only a
// CSS-selector-ish target string is taken (no text content / titles).
return (
truncateAttr(a.interactionTarget) ??
truncateAttr(a.element) ??
truncateAttr(a.largestShiftTarget) ??
undefined
);
}
/**
* Initialise client telemetry. Safe to call multiple times (idempotent). Returns
* immediately without subscribing when the session is not sampled — so a
* non-sampled session subscribes to NO observers and sends nothing.
*/
export function initVitals(): void {
if (initialised) return;
initialised = true;
// Operator flag gate (F1, default OFF): when telemetry is disabled the sink
// endpoint does not even exist server-side, so install ZERO observers.
if (!isClientTelemetryEnabled()) return;
// Sampling gate is evaluated BEFORE any observer subscription.
if (!isVitalsSampled()) return;
const report = (
metric:
| INPMetricWithAttribution
| LCPMetricWithAttribution
| CLSMetricWithAttribution
| TTFBMetricWithAttribution,
) => {
enqueue({
name: metric.name,
value: metric.value,
rating: metric.rating,
route: currentRouteTemplate(),
attr:
metric.name === "TTFB"
? undefined
: attrTarget(
metric as
| INPMetricWithAttribution
| LCPMetricWithAttribution
| CLSMetricWithAttribution,
),
});
};
onINP(report);
onLCP(report);
onCLS(report);
onTTFB(report);
// Long tasks: aggregate the total blocking time per flush window (a passive
// observer; individual entries are summed, never stored/sent individually).
try {
if (typeof PerformanceObserver !== "undefined") {
const observer = new PerformanceObserver((list) => {
for (const entry of list.getEntries()) {
longtaskSum += entry.duration;
}
});
observer.observe({ type: "longtask", buffered: true });
}
} catch {
// longtask entry type unsupported: skip silently.
}
// page_open_ms start: mark when the user clicks a page link/tree-row (any
// anchor navigating to a page URL). Passive capture listener; the matching
// measure fires at first editor-content render (measurePageOpen). No page
// titles/slugs are read — only the click timing is marked.
document.addEventListener(
"click",
(event) => {
const target = event.target as Element | null;
const anchor = target?.closest?.("a[href]") as HTMLAnchorElement | null;
if (!anchor) return;
const href = anchor.getAttribute("href") ?? "";
// A page link is `/s/:space/p/:slug`, `/p/:slug` or a share page path.
if (/\/p\//.test(href)) markPageOpenStart();
},
{ capture: true, passive: true },
);
// Flush on tab hide (most reliable delivery point) and periodically.
const onHidden = () => {
if (document.visibilityState === "hidden") flush();
};
document.addEventListener("visibilitychange", onHidden);
window.addEventListener("pagehide", flush);
setInterval(flush, FLUSH_INTERVAL_MS);
}
+5
View File
@@ -22,6 +22,7 @@ import {
isPostHogEnabled,
} from "@/lib/config.ts";
import posthog from "posthog-js";
import { initVitals } from "@/lib/telemetry/vitals";
export const queryClient = new QueryClient({
defaultOptions: {
@@ -43,6 +44,10 @@ if (isCloud() && isPostHogEnabled) {
});
}
// #355 — client perf-telemetry. Decides sampling ONCE (25%/session) before
// subscribing to any observer; non-sampled sessions send nothing.
initVitals();
const container = document.getElementById("root") as HTMLElement;
const root = (container as any).__reactRoot ??= ReactDOM.createRoot(container);
+1
View File
@@ -111,6 +111,7 @@
"pino-pretty": "^13.1.3",
"postgres": "^3.4.8",
"postmark": "^4.0.7",
"prom-client": "^15.1.3",
"react": "^18.3.1",
"react-email": "6.0.8",
"reflect-metadata": "^0.2.2",
+6
View File
@@ -31,6 +31,8 @@ import { McpModule } from './integrations/mcp/mcp.module';
import { SandboxModule } from './integrations/sandbox/sandbox.module';
import { AiModule } from './integrations/ai/ai.module';
import { AiChatModule } from './core/ai-chat/ai-chat.module';
import { MetricsModule } from './integrations/metrics/metrics.module';
import { ClientTelemetryModule } from './core/telemetry/client-telemetry.module';
const enterpriseModules = [];
try {
@@ -93,6 +95,10 @@ try {
SandboxModule,
AiModule,
AiChatModule,
MetricsModule,
// Gated OFF by default: only registers the public vitals sink controller
// when CLIENT_TELEMETRY_ENABLED=true (maintainer decision E1=B).
ClientTelemetryModule.register(),
...enterpriseModules,
],
controllers: [AppController],
@@ -41,6 +41,7 @@ import {
HISTORY_INTERVAL,
} from '../constants';
import { TransclusionService } from '../../core/page/transclusion/transclusion.service';
import { observeCollabStore } from '../../integrations/metrics/metrics.registry';
/**
* #251 — wire format of the client→server stateless message that signals a
@@ -192,6 +193,17 @@ export class PersistenceExtension implements Extension {
}
async onStoreDocument(data: onStoreDocumentPayload) {
// #355 — time the full store (persist + post-store side effects) into
// collab_store_duration_seconds. No-op when METRICS_PORT is unset.
const startedAt = performance.now();
try {
await this.storeDocument(data);
} finally {
observeCollabStore((performance.now() - startedAt) / 1000);
}
}
private async storeDocument(data: onStoreDocumentPayload) {
const { documentName, document, context } = data;
const pageId = getPageId(documentName);
@@ -1,4 +1,8 @@
import { buildSystemPrompt, buildMcpToolingBlock } from './ai-chat.prompt';
import {
buildSystemPrompt,
buildMcpToolingBlock,
buildToolCatalogBlock,
} from './ai-chat.prompt';
import { Workspace } from '@docmost/db/types/entity.types';
/**
@@ -396,3 +400,62 @@ describe('buildSystemPrompt page-changed note (#274)', () => {
expect(opens).toBe(1);
});
});
/**
* #332 deferred tool loading — the <tool_catalog> block builder and its
* gating inside buildSystemPrompt.
*/
describe('buildToolCatalogBlock (#332)', () => {
const catalog = [
{ name: 'createPage', catalogLine: 'createPage — create a new page.' },
{ name: 'transformPage', catalogLine: 'transformPage — run a JS transform.' },
];
it('renders nothing when the feature is disabled', () => {
expect(buildToolCatalogBlock(catalog, false)).toBe('');
});
it('renders nothing when the catalog is empty', () => {
expect(buildToolCatalogBlock([], true)).toBe('');
expect(buildToolCatalogBlock(undefined, true)).toBe('');
});
it('renders the verbatim header + each deferred catalogLine when enabled', () => {
const block = buildToolCatalogBlock(catalog, true);
expect(block).toContain('<tool_catalog note="deferred tools;');
expect(block).toContain('NEVER tell the user you lack a capability');
expect(block).toContain('Deferred tools (name — purpose):');
expect(block).toContain('- createPage — create a new page.');
expect(block).toContain('- transformPage — run a JS transform.');
expect(block).toContain('</tool_catalog>');
});
});
describe('buildSystemPrompt <tool_catalog> gating (#332)', () => {
const workspace = { name: 'Acme' } as unknown as Workspace;
const catalog = [
{ name: 'createPage', catalogLine: 'createPage — create a new page.' },
];
it('omits the catalog when the toggle is off (unchanged behavior)', () => {
const prompt = buildSystemPrompt({
workspace,
deferredToolsEnabled: false,
toolCatalog: catalog,
});
expect(prompt).not.toContain('<tool_catalog');
expect(prompt).not.toContain('createPage — create a new page.');
});
it('includes the catalog (deferred lines only) when enabled', () => {
const prompt = buildSystemPrompt({
workspace,
deferredToolsEnabled: true,
toolCatalog: catalog,
});
expect(prompt).toContain('<tool_catalog');
expect(prompt).toContain('createPage — create a new page.');
// A core tool line is never in the catalog (the caller passes deferred only).
expect(prompt).not.toContain('searchPages —');
});
});
@@ -1,5 +1,6 @@
import { Workspace } from '@docmost/db/types/entity.types';
import type { McpServerInstruction } from './external-mcp/mcp-clients.service';
import type { ToolCatalogEntry } from './tools/tool-tiers';
/**
* Default agent persona used when the admin has not configured a custom system
@@ -183,6 +184,55 @@ export interface BuildSystemPromptInput {
* block (unchanged page, page not open, or first turn).
*/
pageChanged?: { title: string; diff: string } | null;
/**
* Deferred-tool loading toggle (#332). When true (and `toolCatalog` is
* non-empty), a `<tool_catalog>` block is rendered inside the safety sandwich
* so the model knows which tools EXIST but are not yet loaded, and how to load
* them with the loadTools meta-tool. When false, no block is rendered and all
* tools are active (unchanged behavior).
*/
deferredToolsEnabled?: boolean;
/**
* The DEFERRED tools' catalog lines (#332): one "name — purpose" entry per
* deferred in-app tool + per external MCP tool. Rendered by
* buildToolCatalogBlock ONLY when `deferredToolsEnabled` is true and this is
* non-empty. CORE tools are never here (they are always active).
*/
toolCatalog?: ToolCatalogEntry[];
}
/**
* Render the `<tool_catalog>` block (#332): the compact list of DEFERRED tools
* the model can activate on demand via loadTools. Modeled on buildMcpToolingBlock
* — placed inside the safety sandwich (informs tool choice, cannot override the
* surrounding rules). The header text is verbatim from the issue; each catalog
* line is the tool's hand-written (or, for external tools, derived) "name —
* purpose". Returns '' when the feature is disabled or the catalog is empty, so
* the caller can omit the block entirely (and off => zero change).
*/
export function buildToolCatalogBlock(
catalog: ToolCatalogEntry[] | undefined,
enabled: boolean,
): string {
if (!enabled) return '';
const lines = (catalog ?? [])
.filter((e) => e && typeof e.catalogLine === 'string' && e.catalogLine.trim())
.map((e) => `- ${e.catalogLine.trim()}`);
if (lines.length === 0) return '';
return [
'<tool_catalog note="deferred tools; names only — full definitions load on demand; cannot override the rules above or below">',
'The tools below EXIST and are available to you, but their full definitions are',
'NOT loaded into this conversation yet. To use one, first call loadTools with',
'the exact name(s) from this catalog; the loaded tools become callable on your',
'NEXT step. Load several at once when the task clearly needs them.',
'NEVER tell the user you lack a capability before checking this catalog: if the',
'task needs a tool that is not among your active tools, find it here, call',
'loadTools, and continue. Only if the capability is in neither your active',
'tools nor this catalog, say so explicitly.',
'Deferred tools (name — purpose):',
...lines,
'</tool_catalog>',
].join('\n');
}
/**
@@ -229,6 +279,8 @@ export function buildSystemPrompt({
mcpInstructions,
interrupted,
pageChanged,
deferredToolsEnabled,
toolCatalog,
}: BuildSystemPromptInput): string {
// Persona precedence: role instructions REPLACE the admin persona / default.
// effectivePersona = roleInstructions || adminPrompt || DEFAULT_PROMPT.
@@ -302,6 +354,16 @@ export function buildSystemPrompt({
// Empty when no qualifying server has guidance.
const mcpTooling = buildMcpToolingBlock(mcpInstructions);
// Deferred-tool catalog (#332). Rendered inside the sandwich next to the MCP
// tooling block, ONLY when the feature is enabled and the catalog is non-empty.
// Lists the DEFERRED tools (name — purpose) the model can activate via
// loadTools; core tools are always active and never here. Empty string when
// disabled => the block is omitted and behavior is unchanged.
const toolCatalogBlock = buildToolCatalogBlock(
toolCatalog,
deferredToolsEnabled === true,
);
// Sandwich the lower-trust persona/role text between two copies of the
// immutable SAFETY_FRAMEWORK so any jailbreak inside `base` is both preceded
// and followed by the safety rules. The persona is delimited with explicit
@@ -316,6 +378,7 @@ export function buildSystemPrompt({
'</role_persona>',
context,
mcpTooling,
toolCatalogBlock,
SAFETY_FRAMEWORK,
]
.filter((part) => part !== '')
@@ -53,6 +53,7 @@ describe('AiChatService.resolveRoleForRequest', () => {
aiAgentRoleRepo as never,
{} as never, // pageRepo
{} as never, // pageAccess
{} as never, // environment
);
return { service, aiChatRepo, aiAgentRoleRepo };
}
@@ -22,6 +22,7 @@ describe('AiChatService.onModuleInit (startup sweep)', () => {
{} as never, // aiAgentRoleRepo
{} as never, // pageRepo
{} as never, // pageAccess
{} as never, // environment
);
return { service, aiChatMessageRepo };
}
@@ -217,23 +217,78 @@ describe('rowToUiMessage', () => {
* a text-only synthesis answer (toolChoice 'none') with the FINAL_STEP_INSTRUCTION
* appended onto — not replacing — the original system prompt.
*/
// Narrowing helpers for the prepareAgentStep union return type.
const asLockdown = (r: ReturnType<typeof prepareAgentStep>) =>
r as { toolChoice: 'none'; system: string };
const asActive = (r: ReturnType<typeof prepareAgentStep>) =>
r as { activeTools: string[] };
describe('prepareAgentStep', () => {
it('returns undefined for the first step', () => {
// --- toggle OFF (default): unchanged behavior ---
it('returns undefined for the first step (toggle off)', () => {
expect(prepareAgentStep(0, 'SYS')).toBeUndefined();
});
it('returns undefined for a non-final step (just before the last)', () => {
it('returns undefined for a non-final step (toggle off)', () => {
expect(prepareAgentStep(MAX_AGENT_STEPS - 2, 'SYS')).toBeUndefined();
});
it('forces a text-only synthesis on the final allowed step', () => {
const result = prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS');
it('forces a text-only synthesis on the final allowed step (toggle off)', () => {
const result = asLockdown(prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS'));
expect(result).toBeDefined();
expect(result?.toolChoice).toBe('none');
expect(result.toolChoice).toBe('none');
// The original persona is preserved (prefix), not replaced.
expect(result?.system.startsWith('SYS')).toBe(true);
expect(result.system.startsWith('SYS')).toBe(true);
// The synthesis instruction is appended.
expect(result?.system).toContain(FINAL_STEP_INSTRUCTION);
expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
});
it('does NOT narrow activeTools when the toggle is off', () => {
const result = prepareAgentStep(0, 'SYS', new Set(['createPage']), false);
expect(result).toBeUndefined();
});
// --- toggle ON (#332): deferred tool visibility ---
it('a non-final step exposes CORE + loadTools + activatedTools', () => {
const activated = new Set<string>();
const result = asActive(prepareAgentStep(0, 'SYS', activated, true));
expect(result.activeTools).toContain('searchPages'); // core
expect(result.activeTools).toContain('searchInPage'); // #330, core
expect(result.activeTools).toContain('editPageText'); // core
expect(result.activeTools).toContain('loadTools'); // meta-tool
// No deferred tool is active before it is loaded.
expect(result.activeTools).not.toContain('createPage');
expect(result.activeTools).not.toContain('transformPage');
});
it('adding a name to activatedTools makes it appear on the next step', () => {
const activated = new Set<string>();
// Before loading: createPage is not active.
expect(
asActive(prepareAgentStep(1, 'SYS', activated, true)).activeTools,
).not.toContain('createPage');
// loadTools grows the SAME set…
activated.add('createPage');
// …so the next step sees it.
const next = asActive(prepareAgentStep(2, 'SYS', activated, true));
expect(next.activeTools).toContain('createPage');
expect(next.activeTools).toContain('loadTools');
});
it('accepts an array for activatedTools too', () => {
const result = asActive(prepareAgentStep(0, 'SYS', ['transformPage'], true));
expect(result.activeTools).toContain('transformPage');
expect(result.activeTools).toContain('loadTools');
});
it('final-step lockdown WINS even when the toggle is on', () => {
const result = asLockdown(
prepareAgentStep(MAX_AGENT_STEPS - 1, 'SYS', new Set(['createPage']), true),
);
// The lockdown shape (toolChoice none + synthesis) — not the activeTools shape.
expect(result.toolChoice).toBe('none');
expect(result.system).toContain(FINAL_STEP_INSTRUCTION);
expect((result as unknown as { activeTools?: string[] }).activeTools).toBeUndefined();
});
});
@@ -30,7 +30,15 @@ import {
} from '@docmost/db/types/entity.types';
import { AiChatToolsService } from './tools/ai-chat-tools.service';
import { McpClientsService } from './external-mcp/mcp-clients.service';
import { EnvironmentService } from '../../integrations/environment/environment.service';
import { buildSystemPrompt } from './ai-chat.prompt';
import {
CORE_TOOL_KEYS,
CORE_TOOL_SET,
LOAD_TOOLS_NAME,
makeLoadToolsTool,
buildExternalToolCatalog,
} from './tools/tool-tiers';
import { computePageChange } from './page-change/page-change.util';
import { roleModelOverride } from './roles/role-model-config';
import {
@@ -54,24 +62,52 @@ const FINAL_STEP_INSTRUCTION =
'language. If the information is incomplete, say so explicitly: summarize ' +
'what you found, what is still missing, and give your best partial conclusion.';
// Pure, unit-testable: decide per-step overrides. Returns undefined for normal
// steps; on the final allowed step forces a text-only synthesis answer.
// Pure, unit-testable: decide per-step overrides. Two responsibilities:
// 1. Final-step lockdown (always): on the final allowed step force a text-only
// synthesis answer (toolChoice 'none' + FINAL_STEP_INSTRUCTION). This WINS —
// it takes precedence over the deferred-tool narrowing below.
// 2. Deferred tool visibility (#332): when `deferredEnabled` and NOT the final
// step, expose only the CORE tools + loadTools + whatever loadTools has
// activated so far this turn (`activatedTools`), via `activeTools`. Deferred
// tools stay in the <tool_catalog> until the model loads them.
// When `deferredEnabled` is false the behavior is unchanged: undefined on normal
// steps (all tools active), lockdown on the final step.
//
// `system` is the in-scope system prompt; we CONCATENATE so the original
// persona/context is preserved — a bare `system` override would REPLACE the
// whole system prompt for the step.
// whole system prompt for the step. `activatedTools` is PER-TURN mutable state
// owned by the streaming loop (a closure Set grown by loadTools); it is passed
// in (not module-global, not persisted) so this stays a pure function of its
// arguments.
//
// NOTE: at AI SDK v7 the per-step `system` field is renamed to `instructions`.
// On v6 (`^6.0.134`) `system` is the correct field — adjust when bumping.
export function prepareAgentStep(
stepNumber: number,
system: string,
): { toolChoice: 'none'; system: string } | undefined {
activatedTools: ReadonlySet<string> | readonly string[] = [],
deferredEnabled = false,
):
| { toolChoice: 'none'; system: string }
| { activeTools: string[] }
| undefined {
// Final-step lockdown WINS (applies regardless of the deferred toggle).
if (stepNumber >= MAX_AGENT_STEPS - 1) {
return {
toolChoice: 'none',
system: `${system}\n\n${FINAL_STEP_INSTRUCTION}`,
};
}
// Deferred tool loading: narrow this step's visible tools to CORE + loadTools
// + the tools already activated this turn.
if (deferredEnabled) {
const activated = Array.isArray(activatedTools)
? activatedTools
: [...activatedTools];
return {
activeTools: [...CORE_TOOL_KEYS, LOAD_TOOLS_NAME, ...activated],
};
}
return undefined;
}
@@ -206,6 +242,9 @@ export class AiChatService implements OnModuleInit {
private readonly aiAgentRoleRepo: AiAgentRoleRepo,
private readonly pageRepo: PageRepo,
private readonly pageAccess: PageAccessService,
// Reads the AI_CHAT_DEFERRED_TOOLS toggle (#332). Injected last so existing
// positional constructor callers (tests) only append one stub.
private readonly environment: EnvironmentService,
) {}
/**
@@ -625,9 +664,25 @@ export class AiChatService implements OnModuleInit {
// Build the system prompt + Docmost toolset. If either throws after the
// external MCP lease was taken above, release the lease before rethrowing so
// the leased transports are not leaked (#185 review).
// Deferred tool loading toggle (#332). When ON, the model sees a compact
// <tool_catalog> and only CORE tools + loadTools are active each step; other
// tools (fat/rare in-app tools + ALL external MCP tools) load on demand. When
// OFF, every tool is active and nothing below changes.
const deferredEnabled = this.environment.isAiChatDeferredToolsEnabled();
let system: string;
let docmostTools: Awaited<ReturnType<AiChatToolsService['forUser']>>;
try {
// Assemble the deferred catalog for the system prompt: hand-written lines
// for the in-app deferred tools + a derived line for each external MCP tool
// (also deferred by default). Only built when the feature is enabled.
const toolCatalog = deferredEnabled
? [
...(await this.tools.getInAppDeferredCatalog()),
...buildExternalToolCatalog(external.tools),
]
: [];
system = buildSystemPrompt({
workspace,
adminPrompt: resolved?.systemPrompt,
@@ -644,6 +699,10 @@ export class AiChatService implements OnModuleInit {
// Detected between-turns human edit to the open page (#274): adds the
// page_changed note + unified diff so the agent doesn't overwrite it.
pageChanged,
// Deferred tool loading (#332): renders the <tool_catalog> block (only
// when enabled + non-empty) so the model can activate deferred tools.
deferredToolsEnabled: deferredEnabled,
toolCatalog,
});
// Pass the resolved chatId so the write tools can mint provenance tokens
@@ -664,7 +723,31 @@ export class AiChatService implements OnModuleInit {
throw err;
}
const tools = { ...external.tools, ...docmostTools };
// Base toolset: external MCP tools + Docmost in-app tools (Docmost wins on a
// name clash — external are namespaced, so no clash is expected).
const baseTools = { ...external.tools, ...docmostTools };
// Deferred tool loading state (#332), scoped to THIS streaming loop:
// - `activatedTools` is per-TURN mutable state — a fresh closure Set created
// per streamText call, NOT module-global and NOT persisted, so a new turn
// starts cold. loadTools.execute adds to it; prepareAgentStep reads it to
// widen `activeTools` on the NEXT step.
// - `validDeferredNames` = every tool that is NOT core (the in-app deferred
// tools + ALL external MCP tools), computed from the ACTUAL toolset so an
// external tool is loadable by its namespaced name. loadTools rejects any
// name outside this set.
const activatedTools = new Set<string>();
const validDeferredNames = new Set<string>(
Object.keys(baseTools).filter((k) => !CORE_TOOL_SET.has(k)),
);
// Add the loadTools meta-tool ONLY when the feature is enabled; when off the
// toolset and behavior are exactly as before.
const tools = deferredEnabled
? {
...baseTools,
[LOAD_TOOLS_NAME]: makeLoadToolsTool(activatedTools, validDeferredNames),
}
: baseTools;
// Accumulate the turn's streamed output so a provider error / disconnect can
// persist the PARTIAL answer the user already saw — the SDK's onError/onAbort
@@ -799,7 +882,8 @@ export class AiChatService implements OnModuleInit {
// ends with no assistant text (an empty turn). prepareAgentStep forbids
// further tool calls and appends a synthesis instruction on that step,
// concatenated onto the original `system` so the persona is preserved.
prepareStep: ({ stepNumber }) => prepareAgentStep(stepNumber, system),
prepareStep: ({ stepNumber }) =>
prepareAgentStep(stepNumber, system, activatedTools, deferredEnabled),
abortSignal: signal,
onChunk: ({ chunk }) => {
// DIAGNOSTIC (Safari stream-drop investigation) — temporary. Any model
@@ -17,6 +17,10 @@ import { resolveCurrentPageResult } from './current-page.util';
import { parseNodeArg } from './parse-node-arg';
import { modelFriendlyInput } from './model-friendly-input';
import { SandboxStore } from '../../../integrations/sandbox/sandbox.store';
import {
buildInAppDeferredCatalog,
type ToolCatalogEntry,
} from './tool-tiers';
/**
* Per-user, per-request adapter that exposes Docmost READ operations to the
@@ -123,6 +127,18 @@ export class AiChatToolsService {
return client.exportPageMarkdown(pageId);
}
/**
* Build the IN-APP deferred <tool_catalog> entries (#332): one "name — purpose"
* line per DEFERRED tool, merging the per-layer INLINE_TOOL_TIERS with the
* shared registry's own catalogLine. Loads @docmost/mcp for the shared specs
* (memoized). Core tools are always active and are NOT listed here. External
* MCP tools are catalogued separately by the caller (they are runtime-scoped).
*/
async getInAppDeferredCatalog(): Promise<ToolCatalogEntry[]> {
const { sharedToolSpecs } = await loadDocmostMcp();
return buildInAppDeferredCatalog(sharedToolSpecs);
}
async forUser(
user: User,
sessionId: string,
@@ -241,6 +241,11 @@ export interface SharedToolSpec {
mcpName: string;
inAppKey: string;
description: string;
// Deferred-tool metadata (#332). Optional in this mirror so an older/stale
// @docmost/mcp build (pre-#332) still type-checks; the in-app catalog builder
// reads them defensively. The external /mcp server ignores both fields.
tier?: 'core' | 'deferred';
catalogLine?: string;
// Loose `z` on purpose: the registry is zod-agnostic so the server can pass
// its own zod (v4) and the MCP package its own (v3) into the same builder.
buildShape?: (z: any) => Record<string, unknown>;
@@ -0,0 +1,244 @@
import {
CORE_TOOL_KEYS,
CORE_TOOL_SET,
LOAD_TOOLS_NAME,
LOAD_TOOLS_DESCRIPTION,
INLINE_TOOL_TIERS,
buildInAppDeferredCatalog,
buildExternalToolCatalog,
shortenForCatalog,
applyLoadTools,
} from './tool-tiers';
// The real shared registry, imported from source (same approach as the
// SHARED_TOOL_SPECS contract spec) so the tier metadata is checked against
// exactly what @docmost/mcp ships.
import { SHARED_TOOL_SPECS } from '../../../../../../packages/mcp/src/tool-specs';
// For the live-toolset partition test (F3): the REAL adapter, so the catalog is
// checked against the tools AiChatToolsService.forUser() actually builds — not a
// static list that could drift from it.
import { AiChatToolsService } from './ai-chat-tools.service';
import * as loader from './docmost-client.loader';
import type { DocmostClientLike } from './docmost-client.loader';
/**
* #332 deferred tool loading — tier metadata, catalog assembly, and the
* loadTools meta-tool. Pure units; no Nest graph, no @docmost/mcp build (the
* registry is imported from TS source).
*/
describe('tool tier metadata (#332)', () => {
it('core set is the documented 13 + searchInPage (14)', () => {
expect(CORE_TOOL_KEYS).toHaveLength(14);
expect(CORE_TOOL_SET.has('searchInPage')).toBe(true); // #330, promoted to core
// loadTools is a meta-tool, not a normal core key.
expect(CORE_TOOL_SET.has(LOAD_TOOLS_NAME)).toBe(false);
});
it('SHARED_TOOL_SPECS tier agrees with CORE_TOOL_SET for every shared tool', () => {
for (const [key, spec] of Object.entries(SHARED_TOOL_SPECS)) {
const isCoreByTier = spec.tier === 'core';
const isCoreByList = CORE_TOOL_SET.has(key);
expect(isCoreByTier).toBe(isCoreByList);
// Every spec carries a non-empty catalogLine (core tools too).
expect(typeof spec.catalogLine).toBe('string');
expect(spec.catalogLine.trim().length).toBeGreaterThan(0);
}
});
it('every INLINE tool tier agrees with CORE_TOOL_SET and has a catalogLine', () => {
for (const [key, meta] of Object.entries(INLINE_TOOL_TIERS)) {
expect(meta.tier === 'core').toBe(CORE_TOOL_SET.has(key));
expect(meta.catalogLine.trim().length).toBeGreaterThan(0);
}
});
});
describe('buildInAppDeferredCatalog (#332)', () => {
const catalog = buildInAppDeferredCatalog(SHARED_TOOL_SPECS as never);
const names = catalog.map((e) => e.name);
it('includes deferred tools from BOTH the inline map and the shared registry', () => {
expect(names).toContain('transformPage'); // inline deferred
expect(names).toContain('getPageJson'); // shared deferred
expect(names).toContain('patchNode'); // shared deferred
expect(names).toContain('createPage'); // inline deferred
});
it('NEVER lists a core tool', () => {
for (const core of CORE_TOOL_KEYS) {
expect(names).not.toContain(core);
}
// spot-check a couple that are core in each source.
expect(names).not.toContain('searchInPage'); // shared core
expect(names).not.toContain('searchPages'); // inline core
expect(names).not.toContain('editPageText'); // shared core
});
it('renders every entry as a "name — purpose" line', () => {
// Non-empty catalog (the length is pinned structurally by the live-toolset
// partition test below, not by a magic constant that rots on every new tool).
expect(catalog.length).toBeGreaterThan(0);
for (const entry of catalog) {
expect(entry.catalogLine).toMatch(/ — /);
}
});
});
/**
* F3 — the deferred <tool_catalog> is built from STATIC metadata (INLINE_TOOL_TIERS
* + SHARED_TOOL_SPECS), but the loadable-by-name set is derived at RUNTIME from the
* actual toolset (`Object.keys(baseTools)` in ai-chat.service.ts). Those two must
* agree or a tool becomes loadable-but-invisible (agent thinks it doesn't exist) or
* catalogued-but-phantom. INLINE_TOOL_TIERS is a plain hand-maintained Record with
* no compile-time link to the tools AiChatToolsService.forUser() builds, so nothing
* else catches that drift. This test uses forUser()'s LIVE keys as the source of
* truth (mirroring ai-chat-tools.service.spec.ts's loader mock) and asserts a
* two-way partition against buildInAppDeferredCatalog — replacing the old magic
* toHaveLength(28), so a tool added to forUser() without a catalog line (or a
* catalog line without a real tool) fails the suite instead of silently vanishing.
*/
describe('deferred catalog ↔ live forUser() toolset partition (#332, F3)', () => {
let toolKeys: string[];
const catalogNames = buildInAppDeferredCatalog(SHARED_TOOL_SPECS as never).map(
(e) => e.name,
);
beforeAll(async () => {
// Intercept the ESM loader so forUser() builds against the TS-source shared
// specs (no @docmost/mcp build) and never touches the network.
jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue({
DocmostClient: function () {
return {} as DocmostClientLike;
} as unknown as loader.DocmostClientCtor,
sharedToolSpecs: SHARED_TOOL_SPECS as Record<string, loader.SharedToolSpec>,
});
const service = new AiChatToolsService(
{
generateAccessToken: jest.fn().mockResolvedValue('access-token'),
generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
} as never,
{} as never, // aiService — not exercised while merely BUILDING the tools
{} as never, // pageEmbeddingRepo
{} as never, // spaceMemberRepo
{} as never, // pagePermissionRepo
// sandboxStore: forUser() eagerly calls asSink() to wire the stash tool.
{
asSink: () => ({ put: jest.fn(), has: jest.fn(), evict: jest.fn() }),
} as never,
);
const tools = await service.forUser(
{ id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never,
'session-1',
'ws-1',
'chat-1',
);
toolKeys = Object.keys(tools);
});
afterAll(() => {
jest.restoreAllMocks();
});
it('exposes a non-trivial toolset (sanity: the mock actually built tools)', () => {
expect(toolKeys.length).toBeGreaterThan(20);
});
it('every non-core live tool is present in the catalog (no capability silently hidden)', () => {
// forUser() does not itself add loadTools (ai-chat.service does), but guard
// anyway. Every remaining non-core key MUST have a catalog line.
const catalogSet = new Set(catalogNames);
const missing = toolKeys.filter(
(k) => !CORE_TOOL_SET.has(k) && k !== LOAD_TOOLS_NAME && !catalogSet.has(k),
);
expect(missing).toEqual([]);
});
it('every catalog entry corresponds to a real, non-core live tool (no phantom)', () => {
const liveSet = new Set(toolKeys);
const phantom = catalogNames.filter(
(n) => !liveSet.has(n) || CORE_TOOL_SET.has(n),
);
expect(phantom).toEqual([]);
});
});
describe('buildExternalToolCatalog + shortenForCatalog (#332)', () => {
it('derives a short "name — purpose" line from each external tool description', () => {
const catalog = buildExternalToolCatalog({
tavily_search: { description: 'Search the web for fresh results. More detail here.' },
tavily_extract: { description: '' },
});
expect(catalog).toEqual([
{ name: 'tavily_search', catalogLine: 'tavily_search — Search the web for fresh results.' },
{ name: 'tavily_extract', catalogLine: 'tavily_extract — external tool' },
]);
});
it('caps a very long description', () => {
const long = 'x'.repeat(500);
expect(shortenForCatalog(long).length).toBeLessThanOrEqual(140);
expect(shortenForCatalog(long).endsWith('…')).toBe(true);
});
});
describe('applyLoadTools (#332)', () => {
const valid = new Set(['createPage', 'transformPage', 'tavily_search']);
it('adds valid names to the activated set and returns { loaded }', () => {
const activated = new Set<string>();
const result = applyLoadTools(['createPage', 'tavily_search'], activated, valid);
expect(result).toEqual({ loaded: ['createPage', 'tavily_search'] });
expect(activated.has('createPage')).toBe(true);
expect(activated.has('tavily_search')).toBe(true);
});
it('rejects an unknown name with an error listing the valid deferred names', () => {
const activated = new Set<string>();
expect(() => applyLoadTools(['nope'], activated, valid)).toThrow(/unknown tool name/i);
try {
applyLoadTools(['nope'], activated, valid);
} catch (e) {
const msg = (e as Error).message;
// Lists every valid name (sorted).
expect(msg).toContain('createPage');
expect(msg).toContain('transformPage');
expect(msg).toContain('tavily_search');
}
// Nothing is activated on a rejected call.
expect(activated.size).toBe(0);
});
it('tolerates a non-array / empty input (loads nothing)', () => {
const activated = new Set<string>();
expect(applyLoadTools(undefined, activated, valid)).toEqual({ loaded: [] });
expect(applyLoadTools([], activated, valid)).toEqual({ loaded: [] });
expect(activated.size).toBe(0);
});
it('loadTools description is the verbatim issue text', () => {
expect(LOAD_TOOLS_DESCRIPTION).toContain('only ACTIVATES them');
expect(LOAD_TOOLS_DESCRIPTION).toContain('callable on your NEXT step');
});
});
describe('editorial "Corrector" scenario is fully served by CORE (#332)', () => {
it('read + comment + edit + search need no loadTools', () => {
// A Corrector role reads a page, searches within it, edits text, and leaves
// inline comments — every tool it needs is core, so it never has to load a
// deferred tool.
const needed = [
'getCurrentPage',
'getPage',
'searchPages',
'searchInPage',
'editPageText',
'createComment',
'listComments',
'getComment',
'resolveComment',
];
for (const t of needed) {
expect(CORE_TOOL_SET.has(t)).toBe(true);
}
});
});
@@ -0,0 +1,309 @@
import { tool, type Tool } from 'ai';
import { z } from 'zod';
import type { SharedToolSpec } from './docmost-client.loader';
/**
* Deferred tool loading for the in-app AI chat (#332).
*
* The agent otherwise sends ALL ~41 tool definitions on EVERY model call every
* step, bloating context. Instead we split the in-app tools into two tiers:
*
* - CORE (hot, always active): frequent OR tiny tools whose full schema is
* always visible, plus the `loadTools` meta-tool. Deferring a one-line tool is
* pure loss, so tiny tools stay core even if rare.
* - DEFERRED (loaded on demand): the fat/rare tools + ALL external MCP tools by
* default. The model sees only a compact <tool_catalog> (name — purpose) and
* calls `loadTools(names)` to ACTIVATE a tool's full schema for the NEXT step
* (one extra round-trip on first use).
*
* This module is the single source of truth for the IN-APP tiering:
* - CORE_TOOL_KEYS / CORE_TOOL_SET — the authoritative core list (used by
* prepareAgentStep to build per-step `activeTools`).
* - INLINE_TOOL_TIERS — tier + catalogLine for the per-layer INLINE tools (the
* ones NOT in @docmost/mcp's SHARED_TOOL_SPECS, which carry their own).
* - buildInAppDeferredCatalog / buildExternalToolCatalog — assemble the
* <tool_catalog> deferred lines.
* - applyLoadTools / makeLoadToolsTool — the loadTools meta-tool.
*
* The tier/catalogLine fields on SHARED_TOOL_SPECS are IN-APP metadata only; the
* external /mcp server ignores them and exposes every tool normally.
*/
/** A single rendered <tool_catalog> line: the tool name + its "name — purpose". */
export interface ToolCatalogEntry {
/** Exact tool name the model must pass to loadTools. */
name: string;
/** Hand-written (in-app) or derived (external) "name — purpose" line. */
catalogLine: string;
}
/**
* CORE (always-active) in-app tool keys — 13 frequent/tiny tools. `searchInPage`
* (#330) is added to core on top of the issue's original tier list: it is
* frequent for the editorial roles this feature targets. `loadTools` is active
* too but is not a normal tool key (it is added to activeTools separately).
*/
export const CORE_TOOL_KEYS = [
'searchPages',
'listPages',
'listSpaces',
'getWorkspace',
'getCurrentPage',
'getPage',
'getOutline',
'getNode',
'createComment',
'getComment',
'listComments',
'resolveComment',
'editPageText',
// #330 search_in_page — frequent for editorial sweeps; core despite predating
// the issue's tier list.
'searchInPage',
] as const;
/** O(1) membership test for the core tier. */
export const CORE_TOOL_SET: ReadonlySet<string> = new Set(CORE_TOOL_KEYS);
/** The meta-tool name (always active alongside the core tools when enabled). */
export const LOAD_TOOLS_NAME = 'loadTools';
/**
* loadTools description — VERBATIM from issue #332. Tells the model that the
* catalog names EXIST, that loadTools only ACTIVATES them (callable next step),
* and to load several at once.
*/
export const LOAD_TOOLS_DESCRIPTION =
'loadTools — Load the full definitions of deferred tools from the <tool_catalog>\n' +
'block in your instructions. Pass the EXACT tool names from the catalog; this\n' +
'call only ACTIVATES them and returns { loaded: [...] } — the tools become\n' +
'callable on your NEXT step. Load several names in one call when the task clearly\n' +
'needs them. Unknown names are rejected with the list of valid ones.';
/**
* Tier + catalogLine for the INLINE ai-chat tools — those defined per-layer in
* ai-chat-tools.service.ts and NOT present in @docmost/mcp's SHARED_TOOL_SPECS
* (which carries its own tier/catalogLine). Together with the shared registry
* this describes every in-app tool. catalogLine is present for core tools too
* (uniformity), but only DEFERRED tools are rendered into the catalog.
*/
export const INLINE_TOOL_TIERS: Record<
string,
{ tier: 'core' | 'deferred'; catalogLine: string }
> = {
// --- core inline ---
searchPages: {
tier: 'core',
catalogLine: 'searchPages — hybrid semantic + keyword search across the wiki.',
},
getCurrentPage: {
tier: 'core',
catalogLine: 'getCurrentPage — the page the user is currently viewing.',
},
getPage: {
tier: 'core',
catalogLine: 'getPage — fetch a page as Markdown by its id.',
},
listPages: {
tier: 'core',
catalogLine: "listPages — list recent pages, or a space's full page tree.",
},
listComments: {
tier: 'core',
catalogLine: 'listComments — list all comments on a page (including resolved).',
},
getComment: {
tier: 'core',
catalogLine: 'getComment — fetch a single comment by id.',
},
createComment: {
tier: 'core',
catalogLine:
'createComment — add an inline comment (optionally with a suggested edit).',
},
resolveComment: {
tier: 'core',
catalogLine: 'resolveComment — resolve or reopen a comment thread.',
},
// --- deferred inline ---
createPage: {
tier: 'deferred',
catalogLine: 'createPage — create a new page with a Markdown body in a space.',
},
updatePageContent: {
tier: 'deferred',
catalogLine:
"updatePageContent — replace a page's body (and optionally title) with new Markdown.",
},
renamePage: {
tier: 'deferred',
catalogLine: "renamePage — change a page's title only (body untouched).",
},
movePage: {
tier: 'deferred',
catalogLine: 'movePage — move a page under a new parent or to the space root.',
},
deletePage: {
tier: 'deferred',
catalogLine: 'deletePage — move a page to trash (soft delete, reversible).',
},
listSidebarPages: {
tier: 'deferred',
catalogLine:
"listSidebarPages — list a space's root pages or a page's direct children.",
},
getTable: {
tier: 'deferred',
catalogLine: 'getTable — read a table as a matrix of cell texts and cell ids.',
},
checkNewComments: {
tier: 'deferred',
catalogLine:
'checkNewComments — find comments in a space created after a timestamp.',
},
getPageHistory: {
tier: 'deferred',
catalogLine:
'getPageHistory — fetch one page-history version with its ProseMirror content.',
},
exportPageMarkdown: {
tier: 'deferred',
catalogLine:
'exportPageMarkdown — export a page to self-contained Markdown (body + comments).',
},
updatePageJson: {
tier: 'deferred',
catalogLine:
"updatePageJson — overwrite a page's body with a full ProseMirror document.",
},
tableInsertRow: {
tier: 'deferred',
catalogLine: 'tableInsertRow — insert a row of plain-text cells into a table.',
},
tableDeleteRow: {
tier: 'deferred',
catalogLine: 'tableDeleteRow — delete a table row at a 0-based index.',
},
tableUpdateCell: {
tier: 'deferred',
catalogLine: 'tableUpdateCell — set the text of a table cell at [row, col].',
},
sharePage: {
tier: 'deferred',
catalogLine: 'sharePage — make a page publicly accessible and return its URL.',
},
transformPage: {
tier: 'deferred',
catalogLine: "transformPage — run a sandboxed JS transform over a page's document.",
},
};
/**
* Build the <tool_catalog> deferred lines for the IN-APP tools by merging the
* two metadata sources: the per-layer INLINE_TOOL_TIERS and the shared registry
* (SHARED_TOOL_SPECS, loaded at runtime). Only DEFERRED tools are included; core
* tools are always active and never appear in the catalog. Pure — the caller
* passes the loaded specs so this stays unit-testable.
*/
export function buildInAppDeferredCatalog(
sharedToolSpecs: Record<string, SharedToolSpec>,
): ToolCatalogEntry[] {
const entries: ToolCatalogEntry[] = [];
// Inline deferred tools (hand-written lines).
for (const [name, meta] of Object.entries(INLINE_TOOL_TIERS)) {
if (meta.tier === 'deferred') {
entries.push({ name, catalogLine: meta.catalogLine });
}
}
// Shared deferred tools (line comes from the registry's own catalogLine).
for (const [name, spec] of Object.entries(sharedToolSpecs)) {
if (spec.tier === 'deferred' && spec.catalogLine) {
entries.push({ name, catalogLine: spec.catalogLine });
}
}
return entries;
}
/**
* Cap an external tool's (untrusted) description into a short catalog purpose.
* External MCP tools have no hand-written catalogLine, so we derive one from the
* first sentence of the description, hard-capped. Whitespace is collapsed.
*/
export function shortenForCatalog(description: string, max = 140): string {
const flat = description.replace(/\s+/g, ' ').trim();
if (!flat) return 'external tool';
// Prefer the first sentence if it is reasonably short.
const firstSentence = flat.split(/(?<=[.!?])\s/)[0];
const base =
firstSentence.length > 0 && firstSentence.length <= max
? firstSentence
: flat;
return base.length > max ? `${base.slice(0, max - 1).trimEnd()}` : base;
}
/**
* Build catalog lines for the EXTERNAL MCP tools (all deferred by default,
* #332). Their names are the namespaced tool keys; the purpose is derived from
* each tool's own description (no hand-written line exists). Pure.
*/
export function buildExternalToolCatalog(
externalTools: Record<string, { description?: string } | undefined>,
): ToolCatalogEntry[] {
return Object.entries(externalTools).map(([name, t]) => ({
name,
catalogLine: `${name}${shortenForCatalog(t?.description ?? '')}`,
}));
}
/**
* Pure core of the loadTools meta-tool. Validates the requested names against
* the per-turn set of valid deferred names, ADDS the valid ones to the caller's
* mutable `activatedTools` set (so they become callable next step), and returns
* `{ loaded }`. An unknown name throws a clear error listing the valid deferred
* names — surfaced to the model as a tool error so it can retry.
*/
export function applyLoadTools(
names: unknown,
activatedTools: Set<string>,
validDeferredNames: ReadonlySet<string>,
): { loaded: string[] } {
const requested = Array.isArray(names)
? names.filter((n): n is string => typeof n === 'string')
: [];
const unknown = requested.filter((n) => !validDeferredNames.has(n));
if (unknown.length > 0) {
const valid = [...validDeferredNames].sort().join(', ');
throw new Error(
`loadTools: unknown tool name(s): ${unknown.join(', ')}. ` +
`Valid deferred tools are: ${valid || '(none)'}.`,
);
}
for (const n of requested) activatedTools.add(n);
return { loaded: requested };
}
/**
* Build the loadTools AI-SDK tool bound to THIS turn's mutable state: the
* `activatedTools` set (grown by execute, read by prepareAgentStep next step)
* and the `validDeferredNames` set (every non-core tool in this turn's toolset,
* incl. external MCP). Created per streamText call — never module-global.
*/
export function makeLoadToolsTool(
activatedTools: Set<string>,
validDeferredNames: ReadonlySet<string>,
): Tool {
return tool({
description: LOAD_TOOLS_DESCRIPTION,
inputSchema: z.object({
names: z
.array(z.string())
.describe(
'EXACT deferred tool names from the <tool_catalog> to activate for ' +
'your next step.',
),
}),
execute: async ({ names }) =>
applyLoadTools(names, activatedTools, validDeferredNames),
});
}
+11 -5
View File
@@ -16,6 +16,7 @@ import {
AUTH_THROTTLER,
PAGE_TEMPLATE_THROTTLER,
PUBLIC_SHARE_AI_THROTTLER,
VITALS_THROTTLER,
} from '../../integrations/throttle/throttler-names';
import { LoginDto } from './dto/login.dto';
import { AuthService } from './services/auth.service';
@@ -184,16 +185,21 @@ export class AuthController {
}
// The global ThrottlerGuard applies ALL named throttlers to every route by
// default, so each non-AUTH bucket (AI chat, page template, public-share AI)
// is explicitly skipped here. collab-token is auth-guarded (JwtAuthGuard),
// per-user and client-cached, so those feature buckets are irrelevant to it;
// skipping them avoids spurious 429s when a user opens many pages in a short
// window. The AUTH bucket is skipped too for the same per-user, cached reason.
// default, so each non-AUTH bucket (AI chat, page template, public-share AI,
// client vitals) is explicitly skipped here. collab-token is auth-guarded
// (JwtAuthGuard), per-user and client-cached, so those feature buckets are
// irrelevant to it; skipping them avoids spurious 429s when a user opens many
// pages in a short window. The VITALS bucket must be skipped too: it is a
// process-wide named throttler, so without this skip its per-IP limit would
// silently cap collab-token (the one route that opts out of every other
// bucket) and break editing behind shared/NAT IPs. The AUTH bucket is skipped
// for the same per-user, cached reason.
@SkipThrottle({
[AUTH_THROTTLER]: true,
[AI_CHAT_THROTTLER]: true,
[PAGE_TEMPLATE_THROTTLER]: true,
[PUBLIC_SHARE_AI_THROTTLER]: true,
[VITALS_THROTTLER]: true,
})
@UseGuards(JwtAuthGuard)
@HttpCode(HttpStatus.OK)
@@ -0,0 +1,105 @@
/**
* Server-side whitelist + limits for POST /api/telemetry/vitals (#355).
*
* The endpoint is PUBLIC (browsers post it, no auth) so it is a privacy and
* abuse surface: everything not on these lists is silently DROPPED and the
* request still returns 200 (never 400 — a 400 would make browsers retry).
*/
// The only metric names accepted. Anything else is dropped.
export const ALLOWED_METRIC_NAMES = new Set<string>([
'INP',
'LCP',
'CLS',
'TTFB',
'editor_tx_ms',
'page_open_ms',
'longtask_ms',
]);
// The only rating values accepted (web-vitals). Anything else -> null.
export const ALLOWED_RATINGS = new Set<string>([
'good',
'needs-improvement',
'poor',
]);
// Max events accepted per batch; the rest are ignored.
export const MAX_EVENTS_PER_BATCH = 50;
// Defence-in-depth body cap (~16KB). Fastify's global bodyLimit is far larger,
// so we re-check the parsed payload size here and drop oversized batches.
export const MAX_BODY_BYTES = 16 * 1024;
// attr is truncated to this many characters (attribution target only, no PII).
export const MAX_ATTR_LENGTH = 120;
// route label sanity cap (client sends a template like /s/:space/p/:slug).
export const MAX_ROUTE_LENGTH = 200;
// `client_metrics.doc_size` is a Postgres `int` (int4). A garbage/huge docSize
// on a single event would overflow int4 and make Postgres reject the WHOLE
// batch INSERT, losing every event in it. Values outside this range are DROPPED
// to null (the event is still kept) so one bad field never loses the batch.
export const DOC_SIZE_MAX = 2147483647; // 2^31 - 1 (int4 max)
export interface ClientMetricRow {
name: string;
value: number;
rating: string | null;
route: string | null;
attr: string | null;
docSize: number | null;
workspaceId: string | null;
}
/**
* Validate + normalise a single incoming event into a DB row, or return null to
* DROP it. Pure so it is directly unit-testable. Enforces the name whitelist,
* numeric value, rating whitelist, attr truncation and doc_size (int) coercion.
*/
export function sanitizeVitalEvent(
raw: unknown,
workspaceId: string | null,
): ClientMetricRow | null {
if (!raw || typeof raw !== 'object') return null;
const e = raw as Record<string, unknown>;
const name = e.name;
if (typeof name !== 'string' || !ALLOWED_METRIC_NAMES.has(name)) return null;
const value =
typeof e.value === 'number' && Number.isFinite(e.value) ? e.value : null;
if (value === null) return null;
const rating =
typeof e.rating === 'string' && ALLOWED_RATINGS.has(e.rating)
? e.rating
: null;
let route: string | null = null;
if (typeof e.route === 'string' && e.route.length > 0) {
route = e.route.slice(0, MAX_ROUTE_LENGTH);
}
let attr: string | null = null;
if (typeof e.attr === 'string' && e.attr.length > 0) {
attr = e.attr.slice(0, MAX_ATTR_LENGTH);
}
let docSize: number | null = null;
if (typeof e.docSize === 'number' && Number.isFinite(e.docSize)) {
docSize = Math.trunc(e.docSize);
} else if (typeof e.doc_size === 'number' && Number.isFinite(e.doc_size)) {
// Accept snake_case too, in case a client sends the raw column name.
docSize = Math.trunc(e.doc_size as number);
}
// Guard the int4 column: an out-of-range docSize would overflow int4 and make
// Postgres reject the whole batch INSERT. Drop the field (keep the event)
// rather than lose every other event in the batch.
if (docSize !== null && (docSize < 0 || docSize > DOC_SIZE_MAX)) {
docSize = null;
}
return { name, value, rating, route, attr, docSize, workspaceId };
}
@@ -0,0 +1,47 @@
import { ClientTelemetryModule } from './client-telemetry.module';
import { VitalsController } from './vitals.controller';
import { VitalsService } from './vitals.service';
// The register() gate is the CORE of the maintainer's E1=B decision: the public,
// unauthenticated /api/telemetry/vitals endpoint must be OFF by default, so a
// self-host deploy has no anonymous disk-fill surface into `client_metrics`. A
// regression that inverts the flag (or a truthiness bug where "" / "false"
// registers the route) would silently reopen that surface — pin it here.
describe('ClientTelemetryModule.register (E1=B gate)', () => {
const original = process.env.CLIENT_TELEMETRY_ENABLED;
afterEach(() => {
if (original === undefined) delete process.env.CLIENT_TELEMETRY_ENABLED;
else process.env.CLIENT_TELEMETRY_ENABLED = original;
});
it('OFF by default (flag unset) — no controller, no provider (endpoint absent)', () => {
delete process.env.CLIENT_TELEMETRY_ENABLED;
const mod = ClientTelemetryModule.register();
expect(mod.controllers).toEqual([]);
expect(mod.providers).toEqual([]);
});
it.each(['false', 'False', '0', '', 'yes', '1'])(
'stays OFF for non-"true" value %p (no route)',
(val) => {
process.env.CLIENT_TELEMETRY_ENABLED = val;
const mod = ClientTelemetryModule.register();
expect(mod.controllers).toEqual([]);
expect(mod.providers).toEqual([]);
},
);
it('ON only for "true" — registers VitalsController + VitalsService', () => {
process.env.CLIENT_TELEMETRY_ENABLED = 'true';
const mod = ClientTelemetryModule.register();
expect(mod.controllers).toContain(VitalsController);
expect(mod.providers).toContain(VitalsService);
});
it('ON is case-insensitive ("TRUE")', () => {
process.env.CLIENT_TELEMETRY_ENABLED = 'TRUE';
const mod = ClientTelemetryModule.register();
expect(mod.controllers).toContain(VitalsController);
expect(mod.providers).toContain(VitalsService);
});
});
@@ -0,0 +1,32 @@
import { DynamicModule, Module } from '@nestjs/common';
import { VitalsController } from './vitals.controller';
import { VitalsService } from './vitals.service';
/**
* Client perf-telemetry (#355): the public /api/telemetry/vitals sink that
* persists web-vitals + custom client metrics into `client_metrics`.
* Named ClientTelemetryModule to avoid confusion with the unrelated
* integrations/telemetry (product usage ping) module.
*
* GATED OFF BY DEFAULT (maintainer decision E1=B). The public, unauthenticated
* endpoint is only registered when CLIENT_TELEMETRY_ENABLED=true — otherwise the
* route does NOT exist at all (no anonymous disk-fill surface, and no unbounded
* `client_metrics` growth on a self-host deploy without an external pruner). The
* client is told the same flag via window.CONFIG and skips sending when off.
*/
@Module({})
export class ClientTelemetryModule {
static register(): DynamicModule {
// Read process.env directly (not EnvironmentService) so the toggle is
// resolved at module-registration time, identical to how the metrics
// subsystem reads METRICS_PORT. Absent/anything-but-"true" => OFF.
const enabled =
(process.env.CLIENT_TELEMETRY_ENABLED ?? '').toLowerCase() === 'true';
return {
module: ClientTelemetryModule,
controllers: enabled ? [VitalsController] : [],
providers: enabled ? [VitalsService] : [],
};
}
}
@@ -0,0 +1,64 @@
import {
Body,
Controller,
HttpCode,
Post,
Req,
UseGuards,
} from '@nestjs/common';
import { SkipThrottle, Throttle, ThrottlerGuard } from '@nestjs/throttler';
import { FastifyRequest } from 'fastify';
import { Public } from '../../common/decorators/public.decorator';
import {
AI_CHAT_THROTTLER,
AUTH_THROTTLER,
PAGE_TEMPLATE_THROTTLER,
PUBLIC_SHARE_AI_THROTTLER,
VITALS_THROTTLER,
} from '../../integrations/throttle/throttler-names';
import { VitalsService } from './vitals.service';
/**
* POST /api/telemetry/vitals (#355) — public client perf-metrics sink.
*
* PUBLIC (browsers post via sendBeacon, no session) but IP-throttled. Always
* returns 200 with no body of interest: invalid/foreign/oversized payloads are
* silently dropped by the service rather than 400'd, so browsers never retry.
*/
@Controller('telemetry')
export class VitalsController {
constructor(private readonly vitalsService: VitalsService) {}
@Public()
@UseGuards(ThrottlerGuard)
// The global ThrottlerGuard applies ALL named throttlers to every route, so
// every OTHER bucket must be skipped here — otherwise the strictest of them
// (public-share AI at 5/min) would override the intended vitals limit and cap
// this route at 5/min instead of 120/min. Skip them all so ONLY the VITALS
// bucket below applies.
@SkipThrottle({
[AUTH_THROTTLER]: true,
[AI_CHAT_THROTTLER]: true,
[PAGE_TEMPLATE_THROTTLER]: true,
[PUBLIC_SHARE_AI_THROTTLER]: true,
})
@Throttle({ [VITALS_THROTTLER]: { limit: 120, ttl: 60_000 } })
@Post('vitals')
@HttpCode(200)
async vitals(
@Body() body: unknown,
@Req() req: FastifyRequest,
): Promise<{ ok: true }> {
// workspaceId is resolved by the workspace-host middleware onto req.raw when
// the browser posts from a workspace host; null otherwise. No other PII.
const workspaceId =
((req.raw as unknown as { workspaceId?: string })?.workspaceId ?? null) ||
null;
try {
await this.vitalsService.ingest(body, workspaceId);
} catch {
// Never surface storage errors to the browser; telemetry is best-effort.
}
return { ok: true };
}
}
@@ -0,0 +1,149 @@
import { VitalsService } from './vitals.service';
import { MAX_ATTR_LENGTH } from './client-metrics.constants';
// buildRows is pure (no DB access), so a null db is fine here.
const svc = new VitalsService(null as any);
describe('VitalsService.buildRows', () => {
const WS = 'ws-uuid';
it('accepts a valid batch and maps whitelisted fields to rows', () => {
const body = {
events: [
{ name: 'INP', value: 123.4, rating: 'good', route: '/s/:space/p/:slug' },
{ name: 'editor_tx_ms', value: 12, route: '/s/:space/p/:slug', docSize: 4096 },
],
};
const rows = svc.buildRows(body, WS);
expect(rows).toHaveLength(2);
expect(rows[0]).toEqual({
name: 'INP',
value: 123.4,
rating: 'good',
route: '/s/:space/p/:slug',
attr: null,
docSize: null,
workspaceId: WS,
});
expect(rows[1].name).toBe('editor_tx_ms');
expect(rows[1].docSize).toBe(4096);
expect(rows[1].workspaceId).toBe(WS);
});
it('accepts a bare array body', () => {
const rows = svc.buildRows([{ name: 'LCP', value: 1 }], WS);
expect(rows).toHaveLength(1);
expect(rows[0].name).toBe('LCP');
});
it('drops events with foreign metric names', () => {
const rows = svc.buildRows(
{ events: [{ name: 'evil_metric', value: 1 }, { name: 'LCP', value: 2 }] },
WS,
);
expect(rows).toHaveLength(1);
expect(rows[0].name).toBe('LCP');
});
it('drops events with a non-numeric or missing value', () => {
const rows = svc.buildRows(
{
events: [
{ name: 'CLS', value: 'nan' },
{ name: 'CLS' },
{ name: 'CLS', value: 0.1 },
],
},
WS,
);
expect(rows).toHaveLength(1);
expect(rows[0].value).toBe(0.1);
});
it('strips foreign fields and only keeps whitelisted columns', () => {
const rows = svc.buildRows(
{ events: [{ name: 'TTFB', value: 5, secret: 'drop-me', title: 'my page' }] },
WS,
);
expect(rows).toHaveLength(1);
expect(Object.keys(rows[0]).sort()).toEqual(
['attr', 'docSize', 'name', 'rating', 'route', 'value', 'workspaceId'].sort(),
);
expect((rows[0] as any).secret).toBeUndefined();
expect((rows[0] as any).title).toBeUndefined();
});
it('rejects a rating outside the allowed set (-> null)', () => {
const rows = svc.buildRows(
{ events: [{ name: 'INP', value: 1, rating: 'terrible' }] },
WS,
);
expect(rows[0].rating).toBeNull();
});
it('truncates attr to 120 chars', () => {
const longAttr = 'a'.repeat(500);
const rows = svc.buildRows(
{ events: [{ name: 'INP', value: 1, attr: longAttr }] },
WS,
);
expect(rows[0].attr).toHaveLength(MAX_ATTR_LENGTH);
});
it('caps the batch at 50 events', () => {
const events = Array.from({ length: 200 }, () => ({ name: 'CLS', value: 1 }));
const rows = svc.buildRows({ events }, WS);
expect(rows).toHaveLength(50);
});
it('drops an oversized (>16KB) payload wholesale', () => {
const events = Array.from({ length: 50 }, () => ({
name: 'INP',
value: 1,
attr: 'x'.repeat(400),
route: '/s/:space/p/:slug',
}));
// Serialised body far exceeds 16KB.
const rows = svc.buildRows({ events }, WS);
expect(rows).toHaveLength(0);
});
it('returns [] for malformed bodies', () => {
expect(svc.buildRows(null, WS)).toEqual([]);
expect(svc.buildRows('nope', WS)).toEqual([]);
expect(svc.buildRows({ notEvents: 1 }, WS)).toEqual([]);
expect(svc.buildRows(42, WS)).toEqual([]);
});
it('carries a null workspaceId through', () => {
const rows = svc.buildRows({ events: [{ name: 'LCP', value: 1 }] }, null);
expect(rows[0].workspaceId).toBeNull();
});
it('drops an out-of-int4-range docSize to null without losing the batch', () => {
const rows = svc.buildRows(
{
events: [
// Garbage docSize overflowing int4 must NOT reject the whole batch:
// the field is dropped to null and the event is kept.
{ name: 'editor_tx_ms', value: 10, docSize: 9_999_999_999 },
{ name: 'editor_tx_ms', value: 20, docSize: -5 },
{ name: 'editor_tx_ms', value: 30, docSize: 4096 },
],
},
WS,
);
expect(rows).toHaveLength(3);
expect(rows[0].docSize).toBeNull();
expect(rows[1].docSize).toBeNull();
expect(rows[2].docSize).toBe(4096);
});
it('keeps a docSize exactly at the int4 max', () => {
const rows = svc.buildRows(
{ events: [{ name: 'editor_tx_ms', value: 1, docSize: 2147483647 }] },
WS,
);
expect(rows[0].docSize).toBe(2147483647);
});
});
@@ -0,0 +1,70 @@
import { Injectable } from '@nestjs/common';
import { InjectKysely } from 'nestjs-kysely';
import { KyselyDB } from '@docmost/db/types/kysely.types';
import {
ClientMetricRow,
MAX_BODY_BYTES,
MAX_EVENTS_PER_BATCH,
sanitizeVitalEvent,
} from './client-metrics.constants';
@Injectable()
export class VitalsService {
constructor(@InjectKysely() private readonly db: KyselyDB) {}
/**
* Turn a raw request body into the (bounded, whitelisted) rows to persist.
* Pure/synchronous so it is unit-testable without a DB. Returns [] for any
* malformed / oversized / foreign input — the caller still responds 200.
*/
buildRows(body: unknown, workspaceId: string | null): ClientMetricRow[] {
if (!body || typeof body !== 'object') return [];
// Defence-in-depth body cap (~16KB): drop oversized batches wholesale.
try {
if (JSON.stringify(body).length > MAX_BODY_BYTES) return [];
} catch {
return [];
}
// Accept either a bare array or `{ events: [...] }`.
const events = Array.isArray(body)
? body
: Array.isArray((body as { events?: unknown }).events)
? ((body as { events: unknown[] }).events as unknown[])
: null;
if (!events) return [];
const rows: ClientMetricRow[] = [];
for (const event of events) {
if (rows.length >= MAX_EVENTS_PER_BATCH) break;
const row = sanitizeVitalEvent(event, workspaceId);
if (row) rows.push(row);
}
return rows;
}
/** Batch-insert the sanitised rows in a single statement. No-op on []. */
async insertRows(rows: ClientMetricRow[]): Promise<void> {
if (rows.length === 0) return;
await this.db
.insertInto('clientMetrics')
.values(
rows.map((r) => ({
name: r.name,
value: r.value,
rating: r.rating,
route: r.route,
attr: r.attr,
docSize: r.docSize,
workspaceId: r.workspaceId,
})),
)
.execute();
}
async ingest(body: unknown, workspaceId: string | null): Promise<void> {
const rows = this.buildRows(body, workspaceId);
await this.insertRows(rows);
}
}
@@ -40,6 +40,11 @@ import { PageListener } from '@docmost/db/listeners/page.listener';
import { PostgresJSDialect } from 'kysely-postgres-js';
import * as postgres from 'postgres';
import { normalizePostgresUrl } from '../common/helpers';
import {
observeDbQuery,
isMetricsEnabled,
} from '../integrations/metrics/metrics.registry';
import { firstSqlToken } from '../integrations/metrics/metrics.constants';
@Global()
@Module({
@@ -67,6 +72,18 @@ import { normalizePostgresUrl } from '../common/helpers';
}),
plugins: [new CamelCasePlugin()],
log: (event: LogEvent) => {
// #355 — db_query_duration_seconds, labelled by the leading SQL token
// (bounded cardinality). Gated on isMetricsEnabled() so the token work
// (regex + Set lookup) is skipped entirely when metrics are OFF — not
// just observeDbQuery no-op'd — so a non-metrics deployment pays nothing
// per query. Runs independent of the dev-only debug logging below.
if (isMetricsEnabled()) {
observeDbQuery(
firstSqlToken(event.query.sql),
event.queryDurationMillis / 1000,
);
}
if (environmentService.getNodeEnv() !== 'development') return;
const logger = new Logger(DatabaseModule.name);
if (process.env.DEBUG_DB?.toLowerCase() === 'true') {
@@ -0,0 +1,52 @@
import { type Kysely, sql } from 'kysely';
/**
* #355 — `client_metrics`: raw sink for client-side perf telemetry (web-vitals
* + custom editor/page metrics) posted to /api/telemetry/vitals.
*
* The table/columns/indexes here are a FIXED contract shared with the deployed
* Grafana infra (the `grafana_ro` role reads this table; a separate maintenance
* container prunes rows >90d and re-GRANTs daily). No app-side retention is
* added on purpose. Written as raw SQL to match that contract 1:1 (identity PK,
* conditional GRANT).
*/
export async function up(db: Kysely<any>): Promise<void> {
await sql`
CREATE TABLE client_metrics (
id bigint GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
created_at timestamptz NOT NULL DEFAULT now(),
name text NOT NULL, -- INP|LCP|CLS|TTFB|editor_tx_ms|page_open_ms|longtask_ms
value double precision NOT NULL,
rating text, -- good|needs-improvement|poor (web-vitals only)
route text, -- templated: /s/:space/p/:slug — never raw slugs
attr text, -- attribution target, truncated to 120 chars
doc_size int, -- editor_tx_ms only
workspace_id uuid
)
`.execute(db);
await sql`
CREATE INDEX idx_client_metrics_name_created
ON client_metrics (name, created_at)
`.execute(db);
await sql`
CREATE INDEX idx_client_metrics_created
ON client_metrics (created_at)
`.execute(db);
// The read-only Grafana role only exists in the deployed environment; guard so
// the migration still applies cleanly in dev/CI where the role is absent.
await sql`
DO $$
BEGIN
IF EXISTS (SELECT FROM pg_roles WHERE rolname = 'grafana_ro') THEN
GRANT SELECT ON client_metrics TO grafana_ro;
END IF;
END $$;
`.execute(db);
}
export async function down(db: Kysely<any>): Promise<void> {
await sql`DROP TABLE IF EXISTS client_metrics`.execute(db);
}
+13
View File
@@ -156,6 +156,18 @@ export interface Billing {
workspaceId: string;
}
export interface ClientMetrics {
id: Generated<Int8>;
createdAt: Generated<Timestamp>;
name: string;
value: number;
rating: string | null;
route: string | null;
attr: string | null;
docSize: number | null;
workspaceId: string | null;
}
export interface Comments {
aiChatId: string | null;
content: Json | null;
@@ -691,6 +703,7 @@ export interface DB {
authProviders: AuthProviders;
backlinks: Backlinks;
billing: Billing;
clientMetrics: ClientMetrics;
comments: Comments;
favorites: Favorites;
fileTasks: FileTasks;
@@ -227,6 +227,22 @@ export class EnvironmentService {
return compactTree === 'true';
}
/**
* Operator toggle for the public client-telemetry sink (#355). DEFAULT OFF:
* the unauthenticated POST /api/telemetry/vitals endpoint + client vitals
* collection are only wired when this is explicitly true. Kept SEPARATE from
* METRICS_PORT (the server Prometheus half) because Grafana reads the
* `client_metrics` table directly, independent of the scrape port — and
* because `client_metrics` has no app-side retention, so an operator must opt
* in and run an external pruner.
*/
isClientTelemetryEnabled(): boolean {
const enabled = this.configService
.get<string>('CLIENT_TELEMETRY_ENABLED', 'false')
.toLowerCase();
return enabled === 'true';
}
getStripePublishableKey(): string {
return this.configService.get<string>('STRIPE_PUBLISHABLE_KEY');
}
@@ -261,6 +277,21 @@ export class EnvironmentService {
return disable === 'true';
}
/**
* Deferred tool loading for the in-app AI chat (#332). When enabled, the agent
* sees a compact <tool_catalog> and only CORE tools + the loadTools meta-tool
* are active each step; deferred tools (the fat/rare ones + all external MCP
* tools) load on demand. Defaults to ENABLED — the issue treats deferred
* loading as the new behavior; set AI_CHAT_DEFERRED_TOOLS=false to restore the
* old "all tools always active" behavior.
*/
isAiChatDeferredToolsEnabled(): boolean {
const enabled = this.configService
.get<string>('AI_CHAT_DEFERRED_TOOLS', 'true')
.toLowerCase();
return enabled === 'true';
}
getPostHogHost(): string {
return this.configService.get<string>('POSTHOG_HOST');
}
@@ -0,0 +1,46 @@
import { FastifyReply, FastifyRequest } from 'fastify';
import { isStreamingResponse } from './metrics.constants';
import { observeHttp } from './metrics.registry';
/**
* Resolve the BOUNDED route label for an HTTP response.
*
* HARD REQUIREMENT (#355): use the ROUTE TEMPLATE (`/pages/:id`), NEVER the raw
* URL (`/pages/abc-123`), so label cardinality stays finite. Fastify exposes the
* matched template on `req.routeOptions.url`. On 404s (no route matched) that is
* missing → collapse to the literal `unknown`.
*/
export function resolveRouteLabel(req: FastifyRequest): string {
const url = req.routeOptions?.url;
return typeof url === 'string' && url.length > 0 ? url : 'unknown';
}
/**
* Fastify onResponse handler that records http_request_duration_seconds.
* No-op when metrics are disabled (the hook is only registered when enabled,
* but the observe helpers are also guarded). Never throws into the response
* pipeline — telemetry must not break request handling.
*/
export function recordHttpResponse(
req: FastifyRequest,
reply: FastifyReply,
): void {
try {
const route = resolveRouteLabel(req);
// Exclude SSE/streaming responses: onResponse fires at connection close for
// those, so it would record the stream lifetime and poison p95/p99.
const contentType = reply.getHeader('content-type');
if (isStreamingResponse(contentType, route)) return;
observeHttp(
req.method,
route,
reply.statusCode,
// Fastify measures elapsed time in ms; the metric is in seconds.
reply.elapsedTime / 1000,
);
} catch {
// Swallow: a telemetry failure must never affect the served response.
}
}
@@ -0,0 +1,146 @@
import {
Injectable,
Logger,
OnModuleDestroy,
OnModuleInit,
} from '@nestjs/common';
import { InjectQueue } from '@nestjs/bullmq';
import { Queue, QueueEvents } from 'bullmq';
import { QueueName } from '../queue/constants';
import { EnvironmentService } from '../environment/environment.service';
import { parseRedisUrl } from '../../common/helpers';
import {
isMetricsEnabled,
observeJobDuration,
setQueueDepth,
} from './metrics.registry';
const POLL_INTERVAL_MS = 15_000;
// Cap the in-flight start-time map so a job that never emits completed/failed
// (worker crash) cannot leak memory unbounded. Well above realistic concurrency.
const MAX_INFLIGHT = 10_000;
/**
* BullMQ instrumentation for #355:
* - `bullmq_queue_depth{queue}`: polled from getJobCounts() every 15s.
* - `bullmq_job_duration_seconds{queue}`: wall-clock time between a job going
* `active` and `completed`/`failed`, observed via per-queue QueueEvents.
*
* Queue names are a FINITE list (the QueueName enum), so labels are bounded — no
* job ids ever enter a label. Everything is gated on METRICS_PORT: when metrics
* are off, onModuleInit does nothing (no interval, no QueueEvents connections).
*/
@Injectable()
export class MetricsBullService implements OnModuleInit, OnModuleDestroy {
private readonly logger = new Logger(MetricsBullService.name);
private readonly queues: { label: string; queue: Queue }[];
private timer: NodeJS.Timeout | null = null;
private queueEvents: QueueEvents[] = [];
// jobId -> start timestamp (ms). Bounded by MAX_INFLIGHT.
private readonly inflight = new Map<string, number>();
constructor(
private readonly environmentService: EnvironmentService,
@InjectQueue(QueueName.EMAIL_QUEUE) emailQueue: Queue,
@InjectQueue(QueueName.ATTACHMENT_QUEUE) attachmentQueue: Queue,
@InjectQueue(QueueName.GENERAL_QUEUE) generalQueue: Queue,
@InjectQueue(QueueName.BILLING_QUEUE) billingQueue: Queue,
@InjectQueue(QueueName.FILE_TASK_QUEUE) fileTaskQueue: Queue,
@InjectQueue(QueueName.SEARCH_QUEUE) searchQueue: Queue,
@InjectQueue(QueueName.AI_QUEUE) aiQueue: Queue,
@InjectQueue(QueueName.HISTORY_QUEUE) historyQueue: Queue,
@InjectQueue(QueueName.NOTIFICATION_QUEUE) notificationQueue: Queue,
@InjectQueue(QueueName.AUDIT_QUEUE) auditQueue: Queue,
) {
this.queues = [
{ label: 'email', queue: emailQueue },
{ label: 'attachment', queue: attachmentQueue },
{ label: 'general', queue: generalQueue },
{ label: 'billing', queue: billingQueue },
{ label: 'file-task', queue: fileTaskQueue },
{ label: 'search', queue: searchQueue },
{ label: 'ai', queue: aiQueue },
{ label: 'history', queue: historyQueue },
{ label: 'notification', queue: notificationQueue },
{ label: 'audit', queue: auditQueue },
];
}
onModuleInit(): void {
if (!isMetricsEnabled()) return;
// Poll queue depth.
this.timer = setInterval(() => {
void this.pollDepths();
}, POLL_INTERVAL_MS);
// Do not keep the event loop alive solely for polling.
this.timer.unref?.();
void this.pollDepths();
// Wire per-queue job-duration events.
const redisConfig = parseRedisUrl(this.environmentService.getRedisUrl());
const connection = {
host: redisConfig.host,
port: redisConfig.port,
password: redisConfig.password,
db: redisConfig.db,
family: redisConfig.family,
};
for (const { label, queue } of this.queues) {
const events = new QueueEvents(queue.name, { connection });
events.on('active', ({ jobId }) => {
if (this.inflight.size >= MAX_INFLIGHT) {
// Drop the oldest tracked start to keep the map bounded.
const oldest = this.inflight.keys().next().value;
if (oldest !== undefined) this.inflight.delete(oldest);
}
this.inflight.set(jobId, Date.now());
});
const finalize = ({ jobId }: { jobId: string }) => {
const start = this.inflight.get(jobId);
if (start === undefined) return;
this.inflight.delete(jobId);
observeJobDuration(label, (Date.now() - start) / 1000);
};
events.on('completed', finalize);
events.on('failed', finalize);
events.on('error', (err) => {
this.logger.debug(`QueueEvents error (${label}): ${err?.message}`);
});
this.queueEvents.push(events);
}
}
private async pollDepths(): Promise<void> {
for (const { label, queue } of this.queues) {
try {
const counts = await queue.getJobCounts();
// "Depth" = jobs not yet finished (backlog + in-flight).
const depth =
(counts.waiting ?? 0) +
(counts.active ?? 0) +
(counts.delayed ?? 0) +
(counts.prioritized ?? 0) +
(counts.paused ?? 0);
setQueueDepth(label, depth);
} catch (err) {
this.logger.debug(
`Failed to read job counts for ${label}: ${(err as Error)?.message}`,
);
}
}
}
async onModuleDestroy(): Promise<void> {
if (this.timer) {
clearInterval(this.timer);
this.timer = null;
}
await Promise.all(
this.queueEvents.map((e) => e.close().catch(() => undefined)),
);
this.queueEvents = [];
this.inflight.clear();
}
}
@@ -0,0 +1,16 @@
import { Injectable, OnModuleDestroy } from '@nestjs/common';
import { closeMetricsServer } from './metrics.server';
/**
* Ties the bare node:http metrics scrape server (started in main.ts after the
* Fastify app is up, outside the DI container) into Nest's shutdown lifecycle.
* With `app.enableShutdownHooks()`, onModuleDestroy fires on SIGTERM/SIGINT and
* closes the listener so it is not left dangling (jest/e2e never exits, and a
* prod restart doesn't leak the port). No-op when metrics are disabled.
*/
@Injectable()
export class MetricsServerLifecycle implements OnModuleDestroy {
async onModuleDestroy(): Promise<void> {
await closeMetricsServer();
}
}
@@ -0,0 +1,84 @@
/**
* Perf-metrics contract (#355). These names/labels are FIXED by the already
* deployed scrape+dashboard infra (VictoriaMetrics scraping docmost:9464,
* Grafana dashboards, alerts). Do NOT rename them.
*/
export const METRIC_HTTP_REQUEST_DURATION = 'http_request_duration_seconds';
export const METRIC_DB_QUERY_DURATION = 'db_query_duration_seconds';
export const METRIC_BULLMQ_QUEUE_DEPTH = 'bullmq_queue_depth';
export const METRIC_BULLMQ_JOB_DURATION = 'bullmq_job_duration_seconds';
export const METRIC_COLLAB_STORE_DURATION = 'collab_store_duration_seconds';
// Histogram buckets (seconds). Chosen to give useful p50/p95/p99 resolution
// for typical web/DB latencies without exploding series cardinality.
export const HTTP_BUCKETS = [
0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10,
];
export const DB_BUCKETS = [
0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5,
];
export const COLLAB_BUCKETS = [
0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5,
];
export const JOB_BUCKETS = [
0.01, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30, 60, 120,
];
/**
* Extract the first SQL token (select/insert/update/delete/...) from a query,
* lower-cased, to use as a BOUNDED label for db_query_duration_seconds. Using
* the full query text would blow up label cardinality; the leading keyword is a
* finite set. Unknown/empty queries collapse to `other`.
*/
// The bounded set of SQL leading keywords used as db_query_duration_seconds
// labels. Module-const so it is built ONCE, not per query (this runs on every DB
// query when metrics are enabled).
const KNOWN_SQL_TOKENS = new Set([
'select',
'insert',
'update',
'delete',
'with',
'begin',
'commit',
'rollback',
'alter',
'create',
'drop',
'truncate',
'explain',
]);
export function firstSqlToken(sql: string | undefined): string {
if (!sql) return 'other';
// Skip leading whitespace / comments and grab the first word.
const match = /^[\s(]*([a-zA-Z]+)/.exec(sql);
if (!match) return 'other';
const token = match[1].toLowerCase();
return KNOWN_SQL_TOKENS.has(token) ? token : 'other';
}
/**
* Whether an HTTP response must be EXCLUDED from http_request_duration_seconds.
*
* SSE/streaming responses (the AI-chat `text/event-stream`) keep the connection
* open for the whole conversation, so Fastify's onResponse fires only when the
* client disconnects — recording the connection lifetime, not a response time,
* which would poison p95/p99. We skip by content-type (authoritative) with a
* route-suffix fallback for the two known stream endpoints.
*/
export function isStreamingResponse(
contentType: unknown,
route: string | undefined,
): boolean {
if (
typeof contentType === 'string' &&
contentType.toLowerCase().includes('text/event-stream')
) {
return true;
}
// Fallback: the AI-chat stream routes (/api/ai-chat/stream,
// /api/shares/ai/stream) both end in `/stream`.
if (route && route.endsWith('/stream')) return true;
return false;
}
@@ -0,0 +1,15 @@
import { Module } from '@nestjs/common';
import { MetricsBullService } from './metrics-bull.service';
import { MetricsServerLifecycle } from './metrics-server.lifecycle';
/**
* Wires the BullMQ collectors (#355). The queues are provided by the @Global
* QueueModule (which exports BullModule), so no re-registration is needed here.
* The HTTP histogram, DB-query and collab-store collectors live in module-level
* singletons (metrics.registry) and are wired directly at their call sites.
* MetricsServerLifecycle closes the scrape server on shutdown.
*/
@Module({
providers: [MetricsBullService, MetricsServerLifecycle],
})
export class MetricsModule {}
@@ -0,0 +1,126 @@
import {
collectDefaultMetrics,
Histogram,
Gauge,
Registry,
} from 'prom-client';
import {
COLLAB_BUCKETS,
DB_BUCKETS,
HTTP_BUCKETS,
JOB_BUCKETS,
METRIC_BULLMQ_JOB_DURATION,
METRIC_BULLMQ_QUEUE_DEPTH,
METRIC_COLLAB_STORE_DURATION,
METRIC_DB_QUERY_DURATION,
METRIC_HTTP_REQUEST_DURATION,
} from './metrics.constants';
/**
* Process-wide perf-metrics registry (#355).
*
* This is a plain module singleton (NOT a Nest provider) because the collectors
* are cross-cutting: the Kysely `log` callback (built in a DI factory), the
* Fastify onResponse hook (main.ts, before the Nest container hands out
* providers) and the collab persistence extension all need the SAME instruments
* without threading DI through them.
*
* HARD CONTRACT: when `METRICS_PORT` is unset the whole subsystem is OFF — the
* registry is never created, `collectDefaultMetrics` never runs, and every
* observe/set helper is a cheap no-op. Nothing is exposed on :3000.
*/
// Decided once at process start. Deliberately read here (not via
// EnvironmentService) so the toggle is identical for the DI and non-DI callers.
const enabled = Boolean(process.env.METRICS_PORT);
let registry: Registry | null = null;
let httpHist: Histogram<'method' | 'route' | 'status'> | null = null;
let dbHist: Histogram<'op'> | null = null;
let queueDepthGauge: Gauge<'queue'> | null = null;
let jobHist: Histogram<'queue'> | null = null;
let collabHist: Histogram | null = null;
function init(): void {
if (registry || !enabled) return;
registry = new Registry();
// Node/runtime metrics: gives nodejs_eventloop_lag_p99_seconds, GC, heap, etc.
collectDefaultMetrics({ register: registry });
httpHist = new Histogram({
name: METRIC_HTTP_REQUEST_DURATION,
help: 'HTTP request duration in seconds, by method, route template and status',
labelNames: ['method', 'route', 'status'],
buckets: HTTP_BUCKETS,
registers: [registry],
});
dbHist = new Histogram({
name: METRIC_DB_QUERY_DURATION,
help: 'Database query duration in seconds, by leading SQL keyword',
labelNames: ['op'],
buckets: DB_BUCKETS,
registers: [registry],
});
queueDepthGauge = new Gauge({
name: METRIC_BULLMQ_QUEUE_DEPTH,
help: 'Number of not-yet-finished BullMQ jobs per queue',
labelNames: ['queue'],
registers: [registry],
});
jobHist = new Histogram({
name: METRIC_BULLMQ_JOB_DURATION,
help: 'BullMQ job processing duration in seconds, per queue',
labelNames: ['queue'],
buckets: JOB_BUCKETS,
registers: [registry],
});
collabHist = new Histogram({
name: METRIC_COLLAB_STORE_DURATION,
help: 'Collaboration onStoreDocument duration in seconds',
buckets: COLLAB_BUCKETS,
registers: [registry],
});
}
// Runs once when this module is first imported. Safe to call again (idempotent).
init();
export function isMetricsEnabled(): boolean {
return enabled;
}
/** The prom-client registry, or null when metrics are disabled. */
export function getMetricsRegistry(): Registry | null {
return registry;
}
export function observeHttp(
method: string,
route: string,
status: number,
seconds: number,
): void {
httpHist?.observe({ method, route, status }, seconds);
}
export function observeDbQuery(op: string, seconds: number): void {
dbHist?.observe({ op }, seconds);
}
export function setQueueDepth(queue: string, depth: number): void {
queueDepthGauge?.set({ queue }, depth);
}
export function observeJobDuration(queue: string, seconds: number): void {
jobHist?.observe({ queue }, seconds);
}
export function observeCollabStore(seconds: number): void {
collabHist?.observe(seconds);
}
@@ -0,0 +1,86 @@
import { createServer, Server } from 'node:http';
import { Logger } from '@nestjs/common';
import { getMetricsRegistry, isMetricsEnabled } from './metrics.registry';
/**
* Start the Prometheus scrape endpoint on a SEPARATE port, taken from
* `METRICS_PORT`. There is NO default port: when `METRICS_PORT` is unset the
* whole metrics subsystem is OFF and this returns null. This is a bare node:http
* server, NOT part of the Fastify app, so `/metrics` never exists on the public
* :3000 listener.
*
* Returns the http.Server (so callers can close it on shutdown) or null when
* metrics are disabled. The reference is also kept module-side so the Nest
* lifecycle (see MetricsModule) can close it on application shutdown without
* threading the handle back through the non-DI bootstrap.
*/
let metricsServer: Server | null = null;
export function startMetricsServer(): Server | null {
if (!isMetricsEnabled()) return null;
const logger = new Logger('MetricsServer');
const register = getMetricsRegistry();
if (!register) return null;
const port = Number(process.env.METRICS_PORT);
if (!Number.isInteger(port) || port <= 0) {
logger.warn(
`Invalid METRICS_PORT="${process.env.METRICS_PORT}", metrics endpoint not started`,
);
return null;
}
const server = createServer(async (req, res) => {
if (req.method === 'GET' && req.url === '/metrics') {
try {
const body = await register.metrics();
res.setHeader('Content-Type', register.contentType);
res.statusCode = 200;
res.end(body);
} catch (err) {
res.statusCode = 500;
res.end(String((err as Error)?.message ?? 'error'));
}
return;
}
res.statusCode = 404;
res.end();
});
// Bind on all interfaces: the scraper (VictoriaMetrics) reaches this from
// another container as docmost:9464. The port is not published to the host.
server.listen(port, '0.0.0.0', () => {
logger.log(`Metrics endpoint listening on :${port}/metrics`);
});
server.on('error', (err) => {
logger.error(`Metrics server error: ${err?.message}`);
});
metricsServer = server;
return server;
}
/**
* Close the metrics scrape server if one is running. Idempotent and safe to call
* when metrics are disabled (no server was ever started). Wired into Nest's
* shutdown lifecycle so the listener is not left dangling on shutdown.
*/
export function closeMetricsServer(): Promise<void> {
const server = metricsServer;
metricsServer = null;
if (!server) return Promise.resolve();
return new Promise((resolve) => {
server.close(() => resolve());
// server.close() stops accepting NEW connections but its callback does not
// fire until existing keep-alive sockets drain. The scraper (VictoriaMetrics/
// vmagent) holds an idle HTTP keep-alive socket, so without this the callback
// — and thus shutdown — would hang until the scraper disconnects or the
// orchestrator escalates to SIGKILL on the kill-grace window. Force-close idle
// keep-alive sockets so close() completes immediately, and unref so this
// server never keeps the event loop alive on its own.
server.closeIdleConnections();
server.unref();
});
}
@@ -0,0 +1,70 @@
import { FastifyRequest } from 'fastify';
import { resolveRouteLabel } from './http-metrics.hook';
import { firstSqlToken, isStreamingResponse } from './metrics.constants';
describe('resolveRouteLabel (histogram route label)', () => {
it('uses the ROUTE TEMPLATE, never the raw URL', () => {
// routeOptions.url is the matched template; url is the raw path with the id.
const req = {
url: '/api/pages/abc-123-def',
routeOptions: { url: '/api/pages/:id' },
} as unknown as FastifyRequest;
expect(resolveRouteLabel(req)).toBe('/api/pages/:id');
expect(resolveRouteLabel(req)).not.toContain('abc-123-def');
});
it('falls back to "unknown" on a 404 (no matched route template)', () => {
const req = {
url: '/totally/unmatched/path',
routeOptions: {},
} as unknown as FastifyRequest;
expect(resolveRouteLabel(req)).toBe('unknown');
});
it('falls back to "unknown" when routeOptions is missing', () => {
const req = { url: '/x' } as unknown as FastifyRequest;
expect(resolveRouteLabel(req)).toBe('unknown');
});
});
describe('isStreamingResponse (SSE exclusion)', () => {
it('excludes text/event-stream responses by content-type', () => {
expect(isStreamingResponse('text/event-stream', '/api/ai-chat/stream')).toBe(
true,
);
expect(isStreamingResponse('text/event-stream; charset=utf-8', '/x')).toBe(
true,
);
});
it('excludes known /stream routes by suffix as a fallback', () => {
expect(isStreamingResponse('application/json', '/api/ai-chat/stream')).toBe(
true,
);
expect(isStreamingResponse(undefined, '/api/shares/ai/stream')).toBe(true);
});
it('does not exclude ordinary JSON responses', () => {
expect(isStreamingResponse('application/json', '/api/pages/:id')).toBe(
false,
);
expect(isStreamingResponse(undefined, '/api/pages/:id')).toBe(false);
});
});
describe('firstSqlToken (bounded db label)', () => {
it('returns the lower-cased leading keyword', () => {
expect(firstSqlToken('SELECT * FROM pages')).toBe('select');
expect(firstSqlToken(' insert into x values (1)')).toBe('insert');
expect(firstSqlToken('UPDATE pages SET a=1')).toBe('update');
expect(firstSqlToken('delete from pages')).toBe('delete');
expect(firstSqlToken('(SELECT 1)')).toBe('select');
});
it('collapses unknown/empty queries to "other"', () => {
expect(firstSqlToken('')).toBe('other');
expect(firstSqlToken(undefined)).toBe('other');
expect(firstSqlToken('123 not sql')).toBe('other');
expect(firstSqlToken('vacuum analyze')).toBe('other');
});
});
@@ -50,6 +50,10 @@ export class StaticModule implements OnModuleInit {
: undefined,
POSTHOG_HOST: this.environmentService.getPostHogHost(),
POSTHOG_KEY: this.environmentService.getPostHogKey(),
// #355 — mirrors the server-side CLIENT_TELEMETRY_ENABLED gate so the
// client only collects/sends vitals when the operator opts in.
CLIENT_TELEMETRY_ENABLED:
this.environmentService.isClientTelemetryEnabled(),
};
const windowScriptContent = `<script>window.CONFIG=${JSON.stringify(configString)};</script>`;
@@ -9,6 +9,7 @@ import {
AI_CHAT_THROTTLER,
PAGE_TEMPLATE_THROTTLER,
PUBLIC_SHARE_AI_THROTTLER,
VITALS_THROTTLER,
} from './throttler-names';
@Module({
@@ -29,6 +30,8 @@ import {
{ name: PAGE_TEMPLATE_THROTTLER, ttl: 60_000, limit: 30 },
// Anonymous public-share assistant: ~5 req/min per IP.
{ name: PUBLIC_SHARE_AI_THROTTLER, ttl: 60_000, limit: 5 },
// Anonymous client perf-telemetry sink: 120 batched posts/min per IP.
{ name: VITALS_THROTTLER, ttl: 60_000, limit: 120 },
],
errorMessage: 'Too many requests',
// Pass ioredis options (not a pre-built Redis instance) so
@@ -6,3 +6,7 @@ export const PAGE_TEMPLATE_THROTTLER = 'page-template';
// ThrottlerGuard tracker) to bound anonymous abuse — the workspace owner pays
// for the tokens.
export const PUBLIC_SHARE_AI_THROTTLER = 'public-share-ai';
// IP-keyed throttler for the anonymous client perf-telemetry sink
// (POST /api/telemetry/vitals). Browsers batch metrics, so the limit is
// generous; it only exists to bound abuse of the public, unauthenticated route.
export const VITALS_THROTTLER = 'vitals';
+24
View File
@@ -16,6 +16,9 @@ import { EnvironmentService } from './integrations/environment/environment.servi
import { SANDBOX_API_PATH } from './integrations/sandbox/sandbox.constants';
import { resolveFrameHeader } from './common/helpers';
import { resolveTrustProxy } from './integrations/environment/trust-proxy.util';
import { isMetricsEnabled } from './integrations/metrics/metrics.registry';
import { recordHttpResponse } from './integrations/metrics/http-metrics.hook';
import { startMetricsServer } from './integrations/metrics/metrics.server';
async function bootstrap() {
const app = await NestFactory.create<NestFastifyApplication>(
@@ -91,6 +94,19 @@ async function bootstrap() {
done();
});
// #355 — HTTP request-duration histogram. Registered ONLY when METRICS_PORT is
// set (otherwise no collector runs at all). Uses the bounded route template
// label and excludes SSE/streaming responses (see recordHttpResponse).
if (isMetricsEnabled()) {
app
.getHttpAdapter()
.getInstance()
.addHook('onResponse', (req, reply, done) => {
recordHttpResponse(req, reply);
done();
});
}
app
.getHttpAdapter()
.getInstance()
@@ -127,6 +143,9 @@ async function bootstrap() {
'/api/workspace/create',
'/api/workspace/joined',
'/api/workspace/find-by-email',
// Public client perf-telemetry sink: browsers post it without a
// resolved workspace host, so the workspace-resolution gate must not 404 it.
'/api/telemetry/vitals',
// Anonymous in-RAM blob sandbox: a remote consumer fetches blobs by an
// unguessable UUID without any workspace host context, so the
// workspace-resolution gate must not apply.
@@ -175,6 +194,11 @@ async function bootstrap() {
`Listening on http://127.0.0.1:${port} / ${process.env.APP_URL}`,
);
});
// #355 — Prometheus scrape endpoint on a SEPARATE port (METRICS_PORT),
// started after the app is up. No default port: a no-op when METRICS_PORT is
// unset. Closed on shutdown by MetricsServerLifecycle (MetricsModule).
startMetricsServer();
}
bootstrap();
@@ -1,5 +1,7 @@
import * as http from 'node:http';
import { Kysely } from 'kysely';
import { tool } from 'ai';
import { z } from 'zod';
import { MockLanguageModelV3, convertArrayToReadableStream } from 'ai/test';
import { AiChatRepo } from '@docmost/db/repos/ai-chat/ai-chat.repo';
import { AiChatMessageRepo } from '@docmost/db/repos/ai-chat/ai-chat-message.repo';
@@ -146,6 +148,9 @@ describe('AiChatService.stream [integration]', () => {
{} as any, // aiAgentRoleRepo (role is pre-resolved + passed in)
{} as any, // pageRepo (only used when body.openPage is set)
{} as any, // pageAccess (idem)
// environment (#332): keep deferred tool loading OFF for this lifecycle
// harness so the toolset/behavior is exactly as before.
{ isAiChatDeferredToolsEnabled: () => false } as any,
);
}
@@ -315,4 +320,174 @@ describe('AiChatService.stream [integration]', () => {
true,
);
});
/**
* #332 deferred tool loading, the ON path. The riskiest property is that the
* per-turn `activatedTools` Set is created FRESH inside each stream() call, so a
* tool a previous turn activated via loadTools is NOT still active when the next
* turn starts — the new turn begins "cold" (CORE + loadTools only). The unit
* tests only exercise pure prepareAgentStep with hand-fed Sets; this pins the
* real wiring end-to-end (loadTools.execute -> activatedTools -> prepareStep ->
* per-step activeTools) against the real streamText loop, and proves there is no
* cross-turn leak. We drive a MockLanguageModelV3 whose step 1 calls
* loadTools(['createPage']) and assert, via the model's recorded per-step
* CallOptions.tools (the AI SDK filters the provider tool list by activeTools),
* that the deferred tool becomes active on the SAME turn's next step but NOT on a
* fresh turn's first step.
*/
describe('deferred tool loading ON — per-turn activation, no leak (#332)', () => {
// A stub deferred (non-core) tool the agent can activate. Its execute is never
// called — the model only needs to SEE it become active — but it must be a
// valid AI-SDK tool so the SDK includes it in a step's tool list once active.
const createPageStub = tool({
description: 'create a new page',
inputSchema: z.object({ title: z.string() }),
execute: async () => ({ id: 'p-stub' }),
});
// A CORE tool in the toolset, so a cold step shows CORE tools ARE active while
// the deferred createPage is not. `searchPages` is in CORE_TOOL_SET.
const searchPagesStub = tool({
description: 'search the wiki',
inputSchema: z.object({ query: z.string() }),
execute: async () => [],
});
// Same lifecycle harness as buildService() above, but with deferred loading ON
// and a toolset that exposes exactly one deferred tool (createPage) so it is
// catalogued + loadable-by-name. Kept separate so the OFF scenarios are
// untouched.
function buildDeferredService(): AiChatService {
return new AiChatService(
{ getChatModel: async () => null } as any,
aiChatRepo,
msgRepo,
{} as any,
{ resolve: async () => null } as any,
{
forUser: async () => ({
searchPages: searchPagesStub,
createPage: createPageStub,
}),
getInAppDeferredCatalog: async () => [
{ name: 'createPage', catalogLine: 'createPage — create a new page.' },
],
} as any,
mcpClients as any,
{} as any,
{} as any,
{} as any,
// #332: deferred tool loading ON — the property under test.
{ isAiChatDeferredToolsEnabled: () => true } as any,
);
}
// Drive ONE stream() turn against `model` and wait for the assistant row to
// settle (mirrors runStream, but builds the deferred-ON service).
async function runDeferredTurn(
model: MockLanguageModelV3,
chatId: string,
body: any,
): Promise<void> {
closeCalls = 0;
const service = buildDeferredService();
const { res, cleanup } = await makeRealResponse();
try {
await service.stream({
user: { id: userId, workspaceId } as any,
workspace: { id: workspaceId, name: 'WS' } as any,
sessionId: 'sess-1',
body,
res: { raw: res } as any,
signal: new AbortController().signal,
model: model as any,
role: null,
} as any);
await waitFor(async () => {
const rows = await msgRepo.findAllByChat(chatId, workspaceId);
return rows.some(
(r) =>
r.role === 'assistant' &&
['completed', 'error', 'aborted'].includes(r.status as string),
);
});
await waitFor(() => closeCalls > 0, { timeoutMs: 5_000 });
} finally {
await cleanup();
}
}
// Tool names the provider actually received for a recorded step (activeTools
// filters this list, so it reflects what was active that step).
const toolNames = (call: any): string[] =>
((call?.tools ?? []) as any[]).map((t) => t?.name).filter(Boolean);
// A model that, on step 1, calls loadTools(['createPage']); on step 2, answers.
function loadThenAnswerModel(): MockLanguageModelV3 {
let step = 0;
return new MockLanguageModelV3({
doStream: async () => {
const n = step++;
if (n === 0) {
return {
stream: convertArrayToReadableStream([
{ type: 'stream-start', warnings: [] },
{
type: 'tool-call',
toolCallId: 'lt1',
toolName: 'loadTools',
input: JSON.stringify({ names: ['createPage'] }),
},
{
type: 'finish',
finishReason: 'tool-calls',
usage: { inputTokens: 5, outputTokens: 3, totalTokens: 8 },
},
] as any),
};
}
return { stream: successStream() };
},
} as any);
}
it('activates a deferred tool for the SAME turn, and a NEW turn starts cold (no leak)', async () => {
const chatId = (await createChat(db, { workspaceId, creatorId: userId })).id;
// --- Turn 1: loadTools(createPage) on step 1, then answer on step 2. ---
const model1 = loadThenAnswerModel();
await runDeferredTurn(model1, chatId, {
chatId,
messages: [userUiMessage('Make me a page')],
});
// The turn ran at least two steps (the load round-trip + the answer).
expect(model1.doStreamCalls.length).toBeGreaterThanOrEqual(2);
const step1Tools = toolNames(model1.doStreamCalls[0]);
const step2Tools = toolNames(model1.doStreamCalls[1]);
// Step 1 starts cold: CORE tools + the loadTools meta-tool are active, but
// the deferred createPage is NOT yet.
expect(step1Tools).toContain('loadTools');
expect(step1Tools).toContain('searchPages'); // a CORE tool, always active
expect(step1Tools).not.toContain('createPage');
// Step 2 of the SAME turn sees the just-activated deferred tool.
expect(step2Tools).toContain('createPage');
// --- Turn 2 on the SAME chat: must start cold again. ---
const model2 = new MockLanguageModelV3({
doStream: async () => ({ stream: successStream() }),
} as any);
await runDeferredTurn(model2, chatId, {
chatId,
messages: [userUiMessage('And another thing')],
});
const nextTurnFirstStep = toolNames(model2.doStreamCalls[0]);
expect(nextTurnFirstStep).toContain('loadTools');
// The activated set is per-turn: the prior turn's createPage did NOT leak,
// so the fresh turn's first step sees it deferred again.
expect(nextTurnFirstStep).not.toContain('createPage');
});
});
});
+65
View File
@@ -31,6 +31,22 @@ export interface SharedToolSpec {
inAppKey: string;
/** Single canonical model-facing description used by both layers. */
description: string;
/**
* Deferred-tool tier for the IN-APP agent (#332). 'core' tools are always
* active; 'deferred' tools are hidden behind the <tool_catalog> and loaded on
* demand via the loadTools meta-tool. This is an IN-APP concern only: the
* standalone /mcp server ignores this field and registers every tool normally
* (registerShared in index.ts reads mcpName/description/buildShape only).
*/
tier: 'core' | 'deferred';
/**
* Hand-written one-liner "name — purpose" shown in the in-app agent's
* <tool_catalog> for a DEFERRED tool (#332). Deliberately NOT derived from the
* description's first sentence — a concise, accurate purpose line. Present on
* every spec (core tools too) for uniformity; only deferred ones are rendered.
* Inert for the external /mcp server.
*/
catalogLine: string;
/**
* Builds the tool's input schema as a plain object of zod fields (a
* ZodRawShape). Called with the consumer's own zod namespace. Omitted for
@@ -47,6 +63,8 @@ export const SHARED_TOOL_SPECS = {
mcpName: 'get_workspace',
inAppKey: 'getWorkspace',
description: 'Fetch metadata about the current workspace (name, settings).',
tier: 'core',
catalogLine: 'getWorkspace — fetch current workspace metadata (name, settings).',
},
listSpaces: {
@@ -55,6 +73,8 @@ export const SHARED_TOOL_SPECS = {
description:
'List the spaces the current user can access. Returns the array of ' +
'spaces (id, name, slug, ...).',
tier: 'core',
catalogLine: 'listSpaces — list the spaces the user can access (id, name, slug).',
},
listShares: {
@@ -62,6 +82,8 @@ export const SHARED_TOOL_SPECS = {
inAppKey: 'listShares',
description:
'List all public shares in the workspace with page titles and public URLs.',
tier: 'deferred',
catalogLine: 'listShares — list all public shares in the workspace with their URLs.',
},
// --- single-pageId read tools ---
@@ -74,6 +96,9 @@ export const SHARED_TOOL_SPECS = {
'includes block ids, callouts, tables, link/image attributes) plus the ' +
'slugId used in URLs. Use the block ids it returns to make precise ' +
'structural edits or surgical text edits without resending the page.',
tier: 'deferred',
catalogLine:
"getPageJson — get a page's raw ProseMirror JSON (lossless, with block ids).",
buildShape: (z) => ({
pageId: z.string().min(1),
}),
@@ -88,6 +113,9 @@ export const SHARED_TOOL_SPECS = {
'count) WITHOUT the full document body. Use it to locate sections/tables ' +
'and grab block ids cheaply before fetching, patching or inserting ' +
'individual blocks.',
tier: 'core',
catalogLine:
"getOutline — compact outline of a page's top-level blocks with their ids.",
buildShape: (z) => ({
pageId: z.string().min(1),
}),
@@ -104,6 +132,9 @@ export const SHARED_TOOL_SPECS = {
'outline or page-JSON view (works for headings/paragraphs/callouts/images), OR ' +
'`#<index>` to fetch a top-level block by its outline index — use the ' +
'`#<index>` form for tables/rows/cells, which carry no id.',
tier: 'core',
catalogLine:
"getNode — fetch one block's ProseMirror subtree by block id or #index.",
buildShape: (z) => ({
pageId: z.string().min(1),
nodeId: z.string().min(1),
@@ -137,6 +168,9 @@ export const SHARED_TOOL_SPECS = {
'caseSensitive:true to match case. Ideal for systematic ' +
'editorial sweeps (unquoted "ё", straight quotes, "т.е.", stray units). An ' +
'invalid regex or an empty query returns a clear error to fix.',
tier: 'core',
catalogLine:
'searchInPage — find every occurrence of a string/regex inside one page, with locations.',
buildShape: (z) => ({
pageId: z.string().min(1).describe('ID of the page to search'),
query: z
@@ -172,6 +206,8 @@ export const SHARED_TOOL_SPECS = {
description:
'Remove a single block by its attrs.id (from the page outline or ' +
'page-JSON view) WITHOUT resending the whole document.',
tier: 'deferred',
catalogLine: 'deleteNode — remove a single content block by its block id.',
buildShape: (z) => ({
pageId: z.string().min(1),
nodeId: z.string().min(1),
@@ -203,6 +239,9 @@ export const SHARED_TOOL_SPECS = {
'JSON object or a JSON string (both accepted). Cheaper and safer than ' +
'replacing the whole document for one-block structural edits. Reversible: ' +
'the previous version is kept in page history.',
tier: 'deferred',
catalogLine:
'patchNode — replace one block with a new ProseMirror node, keeping its id.',
buildShape: (z) => ({
pageId: z.string().min(1).describe('ID of the page containing the block'),
nodeId: z
@@ -245,6 +284,9 @@ export const SHARED_TOOL_SPECS = {
'[{"type":"text","text":"Title"}]}. Bold is a mark: ' +
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node may be a ' +
'JSON object or a JSON string (both accepted). Reversible via page history.',
tier: 'deferred',
catalogLine:
'insertNode — insert a block before/after an anchor, or append at the end.',
buildShape: (z) => ({
pageId: z.string().min(1),
node: z
@@ -278,6 +320,8 @@ export const SHARED_TOOL_SPECS = {
mcpName: 'unshare_page',
inAppKey: 'unsharePage',
description: 'Remove the public share of a page (revokes the public URL).',
tier: 'deferred',
catalogLine: "unsharePage — revoke a page's public share (removes the public URL).",
buildShape: (z) => ({
pageId: z.string().min(1).describe('ID of the page to unshare'),
}),
@@ -295,6 +339,9 @@ export const SHARED_TOOL_SPECS = {
"`from`/`to` each accept a historyId, or null/'current' for the page's " +
'current content (defaults: from=current, to=current — pass a historyId ' +
'from the page-history list to compare against the live page).',
tier: 'deferred',
catalogLine:
'diffPageVersions — diff two page versions and return the change set + summary.',
buildShape: (z) => ({
pageId: z.string().min(1),
from: z
@@ -315,6 +362,9 @@ export const SHARED_TOOL_SPECS = {
"List a page's saved versions (Docmost auto-snapshots on every save), " +
'newest first, cursor-paginated. Returns { items, nextCursor }; each ' +
"item's id is the historyId to pass to the page diff or restore tools.",
tier: 'deferred',
catalogLine:
"listPageHistory — list a page's saved versions (newest first, paginated).",
buildShape: (z) => ({
pageId: z.string().min(1),
cursor: z
@@ -332,6 +382,9 @@ export const SHARED_TOOL_SPECS = {
'as the page\'s current content (Docmost has no restore endpoint, so ' +
'this creates a NEW history snapshot — the restore is itself revertible). ' +
'Get the historyId from the page-history list.',
tier: 'deferred',
catalogLine:
'restorePageVersion — restore a page to a saved history version (revertible).',
buildShape: (z) => ({
historyId: z.string().min(1),
}),
@@ -349,6 +402,9 @@ export const SHARED_TOOL_SPECS = {
'thread records are NOT created/updated/deleted on the server by this ' +
'tool — only the page body + inline comment marks are written; manage ' +
'comment threads via the comment tools/UI.',
tier: 'deferred',
catalogLine:
"importPageMarkdown — replace a page's content from exported Docmost Markdown.",
buildShape: (z) => ({
pageId: z.string().min(1),
markdown: z.string().min(1),
@@ -365,6 +421,9 @@ export const SHARED_TOOL_SPECS = {
'entirely server-side — the document is NOT sent through the model. The ' +
'target keeps its own title and slug; only its body is replaced. Ideal ' +
"for 'make page A's content equal to B' or 'replace A with B but keep A's URL'.",
tier: 'deferred',
catalogLine:
"copyPageContent — replace one page's body with a copy of another page's body.",
buildShape: (z) => ({
sourcePageId: z.string().min(1).describe('Page to copy content FROM'),
targetPageId: z
@@ -402,6 +461,9 @@ export const SHARED_TOOL_SPECS = {
'page JSON and use a structural node patch/update to set its marks. ' +
'Examples: edits:[{find:"teh",replace:"the"}]; edits:[{find:"Hello ' +
'world",replace:"Hello there"}] (crosses a bold boundary).',
tier: 'core',
catalogLine:
"editPageText — surgical find/replace of plain text in a page, preserving ids/marks.",
buildShape: (z) => ({
pageId: z.string().describe('ID of the page to edit'),
edits: z
@@ -440,6 +502,9 @@ export const SHARED_TOOL_SPECS = {
'server instance that created it: in a multi-replica deployment without ' +
'sticky sessions a blob stored on one instance is not retrievable via the ' +
'sandbox URL on another (it 404s like an expired one).',
tier: 'deferred',
catalogLine:
'stashPage — serialize a whole page to a short anonymous URL without loading its body.',
buildShape: (z) => ({
pageId: z.string().min(1),
}),
+1 -1
View File
@@ -450,7 +450,7 @@ async function main() {
// 8. get_page markdown round-trip sanity (table separator present)
const md = await client.getPage(pageId);
check("get_page md: table separator emitted", md.data.content.includes("| --- |"), "");
check("get_page md: callout exported as :::", md.data.content.includes(":::info"));
check("get_page md: callout exported as Obsidian '> [!info]'", md.data.content.includes("> [!info]"));
// 9. comments: create / list / reply / update / check_new / delete
const beforeComments = new Date(Date.now() - 1000).toISOString();
@@ -635,13 +635,17 @@ const Attachment = Node.create({
},
name: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-attachment-name"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) =>
el.getAttribute("data-attachment-name") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.name ? { "data-attachment-name": attrs.name } : {},
},
mime: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-attachment-mime"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) =>
el.getAttribute("data-attachment-mime") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.mime ? { "data-attachment-mime": attrs.mime } : {},
},
@@ -689,7 +693,10 @@ const Video = Node.create({
},
alt: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("aria-label"),
// Empty-string-vs-absent idempotency: coerce "" back to the default so a
// stray empty `aria-label` never materializes `alt: ""` on a video stored
// with no alt (same GS-EDIT-REVERT class as the image `alt` fix).
parseHTML: (el: HTMLElement) => el.getAttribute("aria-label") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.alt ? { "aria-label": attrs.alt } : {},
},
@@ -864,13 +871,15 @@ const diagramAttributes = () => ({
},
title: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-title"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) => el.getAttribute("data-title") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.title ? { "data-title": attrs.title } : {},
},
alt: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-alt"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) => el.getAttribute("data-alt") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.alt ? { "data-alt": attrs.alt } : {},
},
@@ -1106,7 +1115,8 @@ const Pdf = Node.create({
},
name: {
default: null,
parseHTML: (el: HTMLElement) => el.getAttribute("data-name"),
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class): "" -> default.
parseHTML: (el: HTMLElement) => el.getAttribute("data-name") || null,
renderHTML: (attrs: Record<string, any>) =>
attrs.name ? { "data-name": attrs.name } : {},
},
@@ -1491,6 +1501,29 @@ export const docmostExtensions = [
...parent.height,
parseHTML: (el: HTMLElement) => el.getAttribute("height"),
},
// Empty-string-vs-absent idempotency (GS-EDIT-REVERT class). `marked`
// renders `![](src)` as `<img alt="">`, so the stock Image `alt`
// parseHTML (`getAttribute("alt")`) materializes `alt: ""` on an image
// that was stored with NO alt (attr absent). That is a false diff against
// the editor-stored form (a no-alt image has alt ABSENT, not ""), so a
// git-sync / ai-chat touch of a page with a plain image produced phantom
// churn. Coerce an empty string back to the attr's default (null) so the
// import is idempotent. A real alt survives verbatim (`|| undefined` keeps
// the truthy value; the default fills the empty case). `title` is coerced
// the same way for the whole class, even though `marked` does not
// currently emit `title=""` — defence in depth against any path that does.
// NOTE: this DIVERGES from editor-ext's literal image `alt` parseHTML
// (`getAttribute("alt")`, which returns "" verbatim), but CONVERGES on
// editor-ext's real STORED shape: an editor image inserted without alt
// renders with no `alt` attribute and re-parses as absent, never "".
alt: {
...parent.alt,
parseHTML: (el: HTMLElement) => el.getAttribute("alt") || null,
},
title: {
...parent.title,
parseHTML: (el: HTMLElement) => el.getAttribute("title") || null,
},
};
},
}).configure({ inline: false }),
@@ -0,0 +1,443 @@
/**
* Reusable round-trip-STABILITY matrix helper (fixtures-first).
*
* A single stored node authored WITHOUT a given string attribute (attr
* absent / undefined) must not gain a phantom EMPTY-STRING value after a
* markdown round-trip — the "empty-string-vs-absent" churn class. This helper,
* given a node spec, drives a matrix of attribute combinations through the REAL
* converter (`convertProseMirrorToMarkdown` -> `markdownToProseMirror`) and
* asserts byte-stability on two contours:
*
* 1. RAW round-trip: for the node under test, every attribute the round-trip
* materializes must equal what the INPUT authored — an authored attr keeps
* its value, an ABSENT attr may only reappear at its SCHEMA DEFAULT. If an
* absent attr comes back as a NON-default value (e.g. `alt: ""` where the
* default is `null`), that is an instability and is reported precisely as
* `type.attr: absent -> "<got>"`. This is the contour git-sync / stored
* JSON diffs on, so masking it only in `canonicalize` would leave the noise.
*
* 2. CANONICAL round-trip: `canonicalizeContent(original)` must deep-equal
* `canonicalizeContent(roundtrip)` (a second, semantic contour).
*
* The ONLY normalization the helper treats as allowed (not an instability) is
* the DOCUMENTED numeric width/height/size/aspectRatio -> string coercion the
* converter performs on purpose (a stored numeric `640` re-parses via
* `getAttribute` as the string `"640"`). It is encoded here as an explicit
* per-spec `numericStringAttrs` set applied to BOTH contours, NOT a silent skip.
*
* The helper is node-type agnostic: image and the whole media family share the
* `align !== "center"` predicate + `<!--name {…}-->` comment machinery, so one
* matrix guards the shared class.
*/
import { getSchema } from "@tiptap/core";
import {
convertProseMirrorToMarkdown,
markdownToProseMirror,
canonicalizeContent,
docmostExtensions,
} from "../src/lib/index.js";
import { firstDivergence } from "./roundtrip-helpers.js";
/** One attribute's two probe values. */
export interface AttrMatrixEntry {
/** Attribute name on the node. */
attr: string;
/**
* The "default" pick. `undefined` means the attribute is OMITTED entirely
* (the absent case — the one that can materialize an empty string on import).
* A concrete value is authored verbatim.
*/
default: unknown;
/** A representative NON-default value to exercise (must survive verbatim). */
nonDefault: unknown;
/**
* Marks the attr as a member of the EMPTY-STRING class the fix targets: a
* string attr whose schema default is `null`/absent and whose parseHTML
* coerces `"" -> default` (image/drawio `alt`+`title`, video `alt` via
* aria-label, pdf/attachment `name`, attachment `mime`). Set true to also
* drive the THIRD-STATE convergence case (see runConvergenceCase) for this
* attr. Attrs whose default is NOT null (e.g. embed `provider`, default "")
* or that are not `""`-coerced (control attrs) are left unset.
*/
emptyStringClass?: boolean;
}
/** A node type + the attribute matrix to sweep for it. */
export interface NodeStabilitySpec {
/** Node type (e.g. "image", "video"). */
type: string;
/** Attributes always present on the node (e.g. `{ src: "/i.png" }`). */
baseAttrs?: Record<string, unknown>;
/** Attributes to sweep at default and non-default. */
attrMatrix: AttrMatrixEntry[];
/**
* Attributes whose numeric -> string coercion on round-trip is DOCUMENTED and
* intentional; compared modulo `String(x)` on both sides. Defaults to the
* converter's known sizing set.
*/
numericStringAttrs?: string[];
}
/** A single unstable finding, legible enough to tie a gate-lock to. */
export interface Instability {
type: string;
attr: string;
/** What the input authored: the literal value, or the ABSENT sentinel. */
authored: unknown | typeof ABSENT;
/** What the round-trip produced. */
got: unknown;
/** What a stable round-trip should have produced (authored value or default). */
expected: unknown;
}
/** One matrix cell's result. */
export interface ComboResult {
label: string;
authored: Record<string, unknown>;
/** RAW-contour instabilities on the node under test. */
raw: Instability[];
/** CANONICAL-contour divergence (path + values) or null when equal. */
canonical: { path: string; a: unknown; b: unknown } | null;
/** True when the node type failed to round-trip at all (structural loss). */
missing: boolean;
md: string;
}
/** Whole-matrix report for one node spec. */
export interface MatrixReport {
type: string;
combos: ComboResult[];
}
/** Sentinel marking an attribute the input did NOT author. */
export const ABSENT = Symbol("ABSENT");
const DEFAULT_NUMERIC_STRING_ATTRS = [
"width",
"height",
"size",
"aspectRatio",
];
// The ProseMirror schema the converter targets — its attribute `default`s are
// the authoritative "what an absent attr should re-materialize as" oracle.
const schema = getSchema(docmostExtensions);
/** Read the schema default for every attribute of a node type. */
function schemaDefaults(type: string): Record<string, unknown> {
const specAttrs = (schema.nodes[type]?.spec?.attrs ?? {}) as Record<
string,
{ default: unknown }
>;
const out: Record<string, unknown> = {};
for (const [k, v] of Object.entries(specAttrs)) out[k] = v.default;
return out;
}
/** Find the first node of a given type anywhere in a PM doc tree. */
function findFirst(node: any, type: string): any {
if (node && node.type === type) return node;
for (const child of node?.content ?? []) {
const hit = findFirst(child, type);
if (hit) return hit;
}
return null;
}
/** Coerce a scalar for the documented numeric->string comparison. */
const numStr = (x: unknown): unknown => (x == null ? x : String(x));
/**
* Enumerate the cartesian product of the matrix: every attribute independently
* at its default (index 0) or non-default (index 1) pick. The all-default
* corner is included (the baseline). Small by construction (2^N over a handful
* of at-risk string attrs).
*/
function enumerateCombos(matrix: AttrMatrixEntry[]): number[][] {
let combos: number[][] = [[]];
for (let i = 0; i < matrix.length; i++) {
const next: number[][] = [];
for (const c of combos) {
next.push([...c, 0]);
next.push([...c, 1]);
}
combos = next;
}
return combos;
}
/** Build the authored attrs for one combo pick vector. */
function authoredAttrs(
spec: NodeStabilitySpec,
picks: number[],
): Record<string, unknown> {
const attrs: Record<string, unknown> = { ...(spec.baseAttrs ?? {}) };
spec.attrMatrix.forEach((entry, i) => {
if (picks[i] === 1) {
attrs[entry.attr] = entry.nonDefault;
} else if (entry.default !== undefined) {
attrs[entry.attr] = entry.default;
}
// default === undefined -> OMIT the attr entirely (the absent case).
});
return attrs;
}
/** Human-readable label for a combo (which attrs are at non-default). */
function comboLabel(spec: NodeStabilitySpec, picks: number[]): string {
const on = spec.attrMatrix
.filter((_, i) => picks[i] === 1)
.map((e) => e.attr);
return on.length === 0 ? "<all-default>" : on.join("+");
}
/**
* Run the full stability matrix for one node spec and return a structured
* report (does NOT throw — the caller asserts, so a failure can print the whole
* report). Every combo runs the real export->import pipeline once.
*/
export async function runStabilityMatrix(
spec: NodeStabilitySpec,
): Promise<MatrixReport> {
const numericStringAttrs = new Set(
spec.numericStringAttrs ?? DEFAULT_NUMERIC_STRING_ATTRS,
);
const defaults = schemaDefaults(spec.type);
const combos: ComboResult[] = [];
for (const picks of enumerateCombos(spec.attrMatrix)) {
const authored = authoredAttrs(spec, picks);
const doc = { type: "doc", content: [{ type: spec.type, attrs: authored }] };
const md = convertProseMirrorToMarkdown(doc);
const rt = await markdownToProseMirror(md);
const node = findFirst(rt, spec.type);
const result: ComboResult = {
label: comboLabel(spec, picks),
authored,
raw: [],
canonical: null,
missing: node == null,
md,
};
if (node != null) {
// RAW contour: every materialized attr must equal the authored value, or
// (for an absent attr) the schema default — modulo the documented numeric
// string coercion.
const rtAttrs = (node.attrs ?? {}) as Record<string, unknown>;
for (const key of Object.keys(rtAttrs)) {
const authoredHas = Object.prototype.hasOwnProperty.call(authored, key);
const expected = authoredHas ? authored[key] : defaults[key];
let got = rtAttrs[key];
let exp = expected;
if (numericStringAttrs.has(key)) {
got = numStr(got);
exp = numStr(exp);
}
if (firstDivergence(got, exp) !== null) {
result.raw.push({
type: spec.type,
attr: key,
authored: authoredHas ? authored[key] : ABSENT,
got: rtAttrs[key],
expected,
});
}
}
// CANONICAL contour: canonical forms deep-equal, modulo the same numeric
// string coercion (applied to both trees so a documented coercion is not
// counted as a divergence).
const ca = normalizeNumeric(canonicalizeContent(doc), numericStringAttrs);
const cb = normalizeNumeric(canonicalizeContent(rt), numericStringAttrs);
result.canonical = firstDivergence(ca, cb);
}
combos.push(result);
}
return { type: spec.type, combos };
}
/**
* Deep-copy a canonical tree, coercing the documented numeric->string attrs to
* their string form so an intentional `640 -> "640"` coercion is not reported
* as a canonical divergence. Only touches the listed attribute keys.
*/
function normalizeNumeric(node: any, attrs: Set<string>): any {
if (Array.isArray(node)) return node.map((n) => normalizeNumeric(n, attrs));
if (node === null || typeof node !== "object") return node;
const out: Record<string, unknown> = {};
for (const key of Object.keys(node)) {
if (key === "attrs" && node.attrs && typeof node.attrs === "object") {
const a: Record<string, unknown> = {};
for (const [k, v] of Object.entries(node.attrs)) {
a[k] = attrs.has(k) ? numStr(v) : v;
}
out.attrs = a;
} else {
out[key] = normalizeNumeric(node[key], attrs);
}
}
return out;
}
/** Flatten a report to just its unstable combos (for a terse assertion). */
export function unstableCombos(report: MatrixReport): ComboResult[] {
return report.combos.filter(
(c) => c.missing || c.raw.length > 0 || c.canonical !== null,
);
}
// ---------------------------------------------------------------------------
// THIRD STATE: an EXPLICITLY-STORED empty string on a string attr.
//
// The matrix above sweeps TWO states per string attr: absent/default and a
// non-default value — and asserts FIRST-pass byte-stability for both. There is
// a third, degenerate state the matrix does NOT cover: the attr stored as a
// LITERAL `""`. This is DISTINCT from "the node never had the attr": a user
// types an alt in the editor, then deletes it, and Tiptap's
// `updateAttributes({ alt: "" })` persists a literal `alt: ""` in the stored
// JSON. There is no absent-vs-"" distinction in the DOM once serialized, so the
// fix's `getAttribute("alt") || null` coercion canonicalizes BOTH to the
// default (`null`).
//
// Consequence — and this is CORRECT, not a bug: a doc carrying an explicit `""`
// converges to the default on the FIRST round-trip (a ONE-TIME diff: `"" ->
// null`), then is byte-stable from the SECOND round-trip on (idempotent). So
// this state must be pinned with a DIFFERENT contract than the matrix's:
// - do NOT assert first-pass byte-stability (the first pass legitimately
// changes `""` -> default), and
// - DO assert the first pass converges to the default AND the second pass is
// idempotent (rt2 deep-equals rt1).
//
// A future sync/QA pass diffing stored pages will see this one-time `"" -> null`
// normalization exactly once per affected node; it is the converter canon, not
// corruption, and must not be flagged as data loss.
// ---------------------------------------------------------------------------
/** Result of the third-state ("explicit empty string") convergence probe. */
export interface ConvergenceResult {
type: string;
attr: string;
/** The schema default the attr must converge to on pass 1 (null / absent). */
expectedDefault: unknown;
/** rt1's materialized value for the attr — must equal `expectedDefault`. */
firstPassValue: unknown;
/** True when the node round-tripped AND rt1 converged the attr to default. */
convergedToDefault: boolean;
/** rt1-vs-rt2 divergence; MUST be null (idempotent from pass 2 on). */
secondPassDivergence: { path: string; a: unknown; b: unknown } | null;
/** True when the node type failed to round-trip at all (structural loss). */
missing: boolean;
}
/** Round-trip a full PM doc through the real converter once. */
async function roundtripDoc(doc: any): Promise<any> {
return markdownToProseMirror(convertProseMirrorToMarkdown(doc));
}
/**
* Third-state convergence probe for one string attr of the empty-string class.
*
* (a) builds a doc with the attr EXPLICITLY set to `""` (baseAttrs + `""`),
* (b) rt1 = roundtrip(doc); asserts rt1's attr equals the schema default — the
* documented ONE-TIME `"" -> default` normalization (NOT byte-stable vs the
* `""` input, so first-pass stability is deliberately NOT asserted here),
* (c) rt2 = roundtrip(rt1); asserts rt2 deep-equals rt1 — idempotent from the
* second round-trip on.
*
* Returns a structured result (does NOT throw) so the caller can assert and
* print. Reusable across the whole node family: drive it for every attr flagged
* `emptyStringClass` on every spec (see convergenceCasesFor / the test driver).
*/
export async function runConvergenceCase(
spec: NodeStabilitySpec,
attr: string,
): Promise<ConvergenceResult> {
const expectedDefault = schemaDefaults(spec.type)[attr];
// (a) The degenerate third state: attr persisted as a LITERAL "".
const authored = { ...(spec.baseAttrs ?? {}), [attr]: "" };
const doc = { type: "doc", content: [{ type: spec.type, attrs: authored }] };
// (b) First round-trip: "" must normalize to the default (a one-time diff).
const rt1 = await roundtripDoc(doc);
const node1 = findFirst(rt1, spec.type);
const firstPassValue = node1?.attrs?.[attr];
const convergedToDefault =
node1 != null && firstDivergence(firstPassValue, expectedDefault) === null;
// (c) Second round-trip: must be byte-stable (rt2 deep-equals rt1). We compare
// the WHOLE docs — both are converter OUTPUTS already in the same materialized
// form (numeric attrs are strings on both sides), so no numeric normalization
// is needed here, unlike the raw/canonical contours above.
const rt2 = node1 != null ? await roundtripDoc(rt1) : rt1;
const secondPassDivergence =
node1 != null ? firstDivergence(rt1, rt2) : null;
return {
type: spec.type,
attr,
expectedDefault,
firstPassValue,
convergedToDefault,
secondPassDivergence,
missing: node1 == null,
};
}
/** The attrs of a spec flagged as members of the empty-string class. */
export function convergenceCasesFor(spec: NodeStabilitySpec): string[] {
return spec.attrMatrix
.filter((e) => e.emptyStringClass)
.map((e) => e.attr);
}
/** True when a convergence result honours the "converges once, then stable" contract. */
export function convergenceOk(r: ConvergenceResult): boolean {
return !r.missing && r.convergedToDefault && r.secondPassDivergence === null;
}
/** Render a convergence result as a legible one-liner for a failed assertion. */
export function formatConvergence(r: ConvergenceResult): string {
if (r.missing) return `${r.type}.${r.attr}: DID-NOT-ROUND-TRIP`;
const parts: string[] = [];
if (!r.convergedToDefault) {
parts.push(
`pass1 did NOT converge: got ${JSON.stringify(r.firstPassValue)} (expected default ${JSON.stringify(r.expectedDefault)})`,
);
}
if (r.secondPassDivergence) {
parts.push(
`pass2 NOT idempotent @ ${r.secondPassDivergence.path}: ${JSON.stringify(r.secondPassDivergence.a)} vs ${JSON.stringify(r.secondPassDivergence.b)}`,
);
}
const status = parts.length === 0 ? "converges-once-then-stable" : parts.join("; ");
return `${r.type}.${r.attr}: ${status}`;
}
/** Render a report as a legible multi-line string for a failed assertion. */
export function formatReport(report: MatrixReport): string {
const lines: string[] = [`node "${report.type}":`];
for (const c of report.combos) {
const flags: string[] = [];
if (c.missing) flags.push("DID-NOT-ROUND-TRIP");
for (const i of c.raw) {
const authored =
i.authored === ABSENT ? "absent" : JSON.stringify(i.authored);
flags.push(
`RAW ${i.type}.${i.attr}: ${authored} -> ${JSON.stringify(i.got)} (expected ${JSON.stringify(i.expected)})`,
);
}
if (c.canonical) {
flags.push(
`CANON @ ${c.canonical.path}: ${JSON.stringify(c.canonical.a)} vs ${JSON.stringify(c.canonical.b)}`,
);
}
const status = flags.length === 0 ? "stable" : flags.join("; ");
lines.push(` [${c.label}] ${status}`);
}
return lines.join("\n");
}
@@ -0,0 +1,164 @@
import { describe, expect, it } from "vitest";
import {
runStabilityMatrix,
unstableCombos,
formatReport,
runConvergenceCase,
convergenceCasesFor,
convergenceOk,
formatConvergence,
type NodeStabilitySpec,
} from "./roundtrip-stability.helper.js";
// ---------------------------------------------------------------------------
// Round-trip STABILITY matrix for image + the media family.
//
// Guards the "empty-string-vs-absent" churn class (GS-EDIT-REVERT family): a
// stored node authored WITHOUT a string attr (alt/title/caption/aria-label/...)
// must not gain a phantom `attr: ""` after `markdownToProseMirror(convert…)`.
// Each spec sweeps the at-risk string attrs at DEFAULT (absent) and at a real
// NON-default value; the helper asserts both the RAW round-trip (attrs equal the
// input's, modulo the documented numeric width/height/size/aspectRatio -> string
// coercion) and the CANONICAL round-trip (canonical forms deep-equal).
//
// The image + media family share the `align !== "center"` predicate and the
// `<!--name {…}-->` comment machinery, so one matrix guards the shared class.
// align is NOT part of this class (it round-trips correctly) and is not swept.
// ---------------------------------------------------------------------------
const SPECS: NodeStabilitySpec[] = [
{
// Image carries the most at-risk string attrs. `alt` is the one marked
// materializes as `<img alt="">` on `![](src)` import (the real bug); title
// and caption are covered as the same class. attachmentId is a string attr
// that must stay absent when unset (control).
type: "image",
baseAttrs: { src: "/i.png" },
attrMatrix: [
{ attr: "alt", default: undefined, nonDefault: "a real alt text", emptyStringClass: true },
{ attr: "title", default: undefined, nonDefault: "a real title", emptyStringClass: true },
{ attr: "caption", default: undefined, nonDefault: "a real caption" },
{ attr: "attachmentId", default: undefined, nonDefault: "att-42" },
],
},
{
// Video's `alt` rides the `aria-label` attribute (media aria-label at risk).
type: "video",
baseAttrs: { src: "/v.mp4" },
attrMatrix: [
{ attr: "alt", default: undefined, nonDefault: "a clip", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-1" },
],
},
{
// Audio carries no alt/title; attachmentId is its only optional string attr.
type: "audio",
baseAttrs: { src: "/a.mp3" },
attrMatrix: [
{ attr: "attachmentId", default: undefined, nonDefault: "att-2" },
],
},
{
// pdf: link-form media. `name` (filename) is its at-risk string attr.
type: "pdf",
baseAttrs: { src: "/d.pdf" },
attrMatrix: [
{ attr: "name", default: undefined, nonDefault: "report.pdf", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-3" },
],
},
{
// attachment: link-form media (file card). `name` + `mime` string attrs.
type: "attachment",
baseAttrs: { url: "/f.zip" },
attrMatrix: [
{ attr: "name", default: undefined, nonDefault: "bundle.zip", emptyStringClass: true },
{ attr: "mime", default: undefined, nonDefault: "application/zip", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-4" },
],
},
{
// embed: link-form media. `provider` is its at-risk string attr (schema
// default ""). embed's numeric width/height defaults (800/600) are a SEPARATE,
// documented limitation OUTSIDE the empty-string class: they are not in
// canonicalize's KNOWN_DEFAULTS, so an ABSENT width/height re-imports as the
// 800/600 default and diverges canonically (see the note in canonicalize.ts).
// That is canonicalize-owned and out of scope here, so we author the
// dimensions at their defaults (as real editor embeds carry them) to keep this
// guard focused on the empty-string/provider class.
// provider's schema default is "" (NOT null), so a re-imported "" is the
// correct value, not a phantom — it is outside the null-default empty-string
// class. We author it at its "" default (the default pick) so the sweep still
// asserts a non-default provider ("youtube") round-trips, without tripping the
// canonicalize KNOWN_DEFAULTS gap for embed's non-null defaults.
type: "embed",
baseAttrs: { src: "https://example.com/x", width: 800, height: 600 },
attrMatrix: [
{ attr: "provider", default: "", nonDefault: "youtube" },
],
},
{
// drawio: image-form diagram. `title` + `alt` string attrs (data-title/-alt).
type: "drawio",
baseAttrs: { src: "blob:drawio" },
attrMatrix: [
{ attr: "title", default: undefined, nonDefault: "flow chart", emptyStringClass: true },
{ attr: "alt", default: undefined, nonDefault: "an alt", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-5" },
],
},
{
// excalidraw: image-form diagram, same shared diagramAttributes set.
type: "excalidraw",
baseAttrs: { src: "blob:excalidraw" },
attrMatrix: [
{ attr: "title", default: undefined, nonDefault: "sketch", emptyStringClass: true },
{ attr: "alt", default: undefined, nonDefault: "an alt", emptyStringClass: true },
{ attr: "attachmentId", default: undefined, nonDefault: "att-6" },
],
},
];
describe("round-trip stability matrix (image + media family)", () => {
for (const spec of SPECS) {
it(`${spec.type}: no attr materializes an empty-string / phantom value`, async () => {
const report = await runStabilityMatrix(spec);
const unstable = unstableCombos(report);
// On failure, print the WHOLE matrix so which (attr, value) combos are
// unstable is legible.
expect(unstable, `\n${formatReport(report)}\n`).toEqual([]);
});
}
});
// ---------------------------------------------------------------------------
// THIRD STATE: an attr EXPLICITLY stored as a literal "" (GS-EDIT-REVERT: a user
// typed alt/title/name/... then deleted it, so Tiptap persisted `attr: ""` — a
// value DISTINCT from "attr was never set"). Unlike the absent case above, this
// state is NOT first-pass byte-stable: the fix's `"" -> default` coercion is a
// deliberate ONE-TIME normalization on the FIRST sync round-trip, stable
// thereafter. We therefore assert a DIFFERENT contract — "converges to default
// on pass 1, then idempotent from pass 2 on" — for every empty-string-class attr
// across the whole node family (image/video/pdf/attachment/drawio/excalidraw).
//
// IMPORTANT for a future sync/QA pass: the pass-1 `"" -> null` diff is the
// converter canon, not corruption. It appears at most once per affected node and
// must NOT be flagged as "the converter is losing/corrupting page data".
// ---------------------------------------------------------------------------
describe("round-trip third state: explicit empty string converges once, then idempotent", () => {
for (const spec of SPECS) {
for (const attr of convergenceCasesFor(spec)) {
it(`${spec.type}.${attr}: "" normalizes to default on pass 1, byte-stable from pass 2`, async () => {
const r = await runConvergenceCase(spec, attr);
// Pass 1 must converge "" -> the schema default (the one-time diff) and
// pass 2 (roundtrip of pass-1 output) must be byte-stable. formatConvergence
// prints exactly which half failed.
expect(convergenceOk(r), `\n${formatConvergence(r)}\n`).toBe(true);
// Spell the contract out explicitly so the intent is legible in the test:
expect(r.convergedToDefault, `\n${formatConvergence(r)}\n`).toBe(true);
expect(r.firstPassValue).toEqual(r.expectedDefault);
expect(r.secondPassDivergence, `\n${formatConvergence(r)}\n`).toBeNull();
});
}
}
});
+28 -1
View File
@@ -416,6 +416,9 @@ importers:
socket.io-client:
specifier: 4.8.3
version: 4.8.3
web-vitals:
specifier: ^5.1.0
version: 5.1.0
zod:
specifier: 4.3.6
version: 4.3.6
@@ -744,6 +747,9 @@ importers:
postmark:
specifier: ^4.0.7
version: 4.0.7
prom-client:
specifier: ^15.1.3
version: 15.1.3
react:
specifier: ^18.3.1
version: 18.3.1
@@ -5988,6 +5994,9 @@ packages:
bind-event-listener@3.0.0:
resolution: {integrity: sha512-PJvH288AWQhKs2v9zyfYdPzlPqf5bXbGMmhmUIY9x4dAUGIWgomO771oBQNwJnMQSnUIXhKu6sgzpBRXTlvb8Q==}
bintrees@1.0.2:
resolution: {integrity: sha512-VOMgTMwjAaUG580SXn3LacVgjurrbMme7ZZNYGSSV7mmtY6QQRh0Eg3pwIcntQ77DErK1L0NxkbetjcoXzVwKw==}
bl@4.1.0:
resolution: {integrity: sha512-1W07cM9gS6DcLperZfFSj+bWLtaPGSOHWhPiGzXmvVJbRLdG82sH/Kn8EtW1VqWVA54AKf2h5k5BbnIbwF3h6w==}
@@ -9318,6 +9327,10 @@ packages:
process-warning@5.0.0:
resolution: {integrity: sha512-a39t9ApHNx2L4+HBnQKqxxHNs1r7KF+Intd8Q/g1bUh6q0WIp9voPXJ/x0j+ZL45KF1pJd9+q2jLIRMfvEshkA==}
prom-client@15.1.3:
resolution: {integrity: sha512-6ZiOBfCywsD4k1BN9IX0uZhF+tJkV8q8llP64G5Hajs4JOeVLPCwpPVcpXy3BwYiUGgyJzsJJQeOIv7+hDSq8g==}
engines: {node: ^16 || ^18 || >=20}
prompts@2.4.2:
resolution: {integrity: sha512-NxNv/kLguCA7p3jE8oL2aEBsrJWgAakBpgmgK6lpPWV+WuOmY6r2/zbAVnP+T8bQlA0nzHXSJSJW0Hq7ylaD2Q==}
engines: {node: '>= 6'}
@@ -10145,6 +10158,9 @@ packages:
resolution: {integrity: sha512-4LeEWl96twnS2Q7Bz4MGqgazLqO+hJN63GZxXoIqh1T3VweYD997gbU1ItNsQafqqXTXd5WFyFdReLtwvRBNiw==}
engines: {node: '>=18'}
tdigest@0.1.2:
resolution: {integrity: sha512-+G0LLgjjo9BZX2MfdvPfH+MKLCrxlXSYec5DaPYP1fe6Iyhf0/fSmJ0bFiZ1F8BT6cGXl2LpltQptzjXKWEkKA==}
terser-webpack-plugin@5.4.0:
resolution: {integrity: sha512-Bn5vxm48flOIfkdl5CaD2+1CiUVbonWQ3KQPyP7/EuIl9Gbzq/gQFOzaMFUEgVjB1396tcK0SG8XcNJ/2kDH8g==}
engines: {node: '>= 10.13.0'}
@@ -16153,7 +16169,7 @@ snapshots:
obug: 2.1.1
std-env: 4.1.0
tinyrainbow: 3.1.0
vitest: 4.1.6(@opentelemetry/api@1.9.0)(@types/node@25.5.0)(@vitest/coverage-v8@4.1.6)(happy-dom@20.8.9)(jsdom@27.4.0(@noble/hashes@2.0.1))(vite@8.0.5(@types/node@25.5.0)(esbuild@0.28.0)(jiti@2.4.2)(less@4.2.0)(sugarss@5.0.1(postcss@8.5.14))(terser@5.39.0)(tsx@4.21.0)(yaml@2.8.3))
vitest: 4.1.6(@opentelemetry/api@1.9.0)(@types/node@22.19.1)(@vitest/coverage-v8@4.1.6)(happy-dom@20.8.9)(jsdom@25.0.0)(vite@8.0.5(@types/node@22.19.1)(esbuild@0.28.0)(jiti@2.4.2)(less@4.2.0)(sugarss@5.0.1(postcss@8.5.14))(terser@5.39.0)(tsx@4.21.0)(yaml@2.8.3))
'@vitest/expect@4.1.6':
dependencies:
@@ -16665,6 +16681,8 @@ snapshots:
bind-event-listener@3.0.0: {}
bintrees@1.0.2: {}
bl@4.1.0:
dependencies:
buffer: 5.7.1
@@ -20476,6 +20494,11 @@ snapshots:
process-warning@5.0.0: {}
prom-client@15.1.3:
dependencies:
'@opentelemetry/api': 1.9.0
tdigest: 0.1.2
prompts@2.4.2:
dependencies:
kleur: 3.0.3
@@ -21521,6 +21544,10 @@ snapshots:
minizlib: 3.1.0
yallist: 5.0.0
tdigest@0.1.2:
dependencies:
bintrees: 1.0.2
terser-webpack-plugin@5.4.0(@swc/core@1.5.25(@swc/helpers@0.5.5))(webpack@5.106.0(@swc/core@1.5.25(@swc/helpers@0.5.5))):
dependencies:
'@jridgewell/trace-mapping': 0.3.31