Ревью полной ветки #486 нашло два пункта в моих коммитах (3 и 4):
- BLOCKER (коммит 4): ai-chat.service.run-race.spec.ts (#184 F14) пинил СТАРУЮ
политику «plain begin() failure → swallow + стрим UNTRACKED» (resolves
toBeUndefined). Коммит 4 её развернул → тест падал (1 failed/6). Кейс
ИНВЕРТИРОВАН под новую политику: plain begin-failure теперь REJECTS с 503
A_RUN_BEGIN_FAILED, до первого байта, user-строка не вставлена, streamText не
вызван — путь остаётся явно запинён, а не удалён.
- NIT (коммит 3): metrics-токен сравнивался наивным !== (единственный слой auth
эндпоинта) → тайминг-утечка токена. Заменено на crypto.timingSafeEqual с
length-guard (разная длина → reject), семантика 401/200 без изменений.
Внесено отдельным fixup-коммитом (rebase -i недоступен в окружении; ветка не
запушена). Тесты: run-race 7/7 + metrics.server 7/7 зелёные.
/metrics слушал на 0.0.0.0 без какой-либо аутентификации — auth-less
эндпоинт на всех интерфейсах. Теперь дефолтный bind — loopback 127.0.0.1;
env METRICS_BIND переопределяет интерфейс (0.0.0.0 для скрейпера в
отдельном контейнере, docmost:9464); опциональный METRICS_TOKEN включает
Bearer-аутентификацию (запросы без точного токена получают 401). Доки
скрейпа в .env.example обновлены.
Тест: unit на дефолтный bind + env-переопределение + резолв токена;
интеграционный по РЕАЛЬНОМУ сокету — listener забинден на 127.0.0.1,
без токена /metrics отдаётся, с токеном без/с неверным Bearer → 401, с
верным → 200.
Правки по ревью #480.
F1 (мёртвые метрики): счётчики mcp_getpage_cache_hits_total/_misses_total
эмитились через onMetricFn, но серверный синк mcp.service.ts диспатчил по имени
через if/else-if БЕЗ default и знал только 2 имени → мои дропались молча; в
metrics.registry их вообще не было. Починка по существующему 3-частному паттерну
(как collab_connect_timeouts_total): имена-константы в metrics.constants.ts; два
Counter'а + incGetPageCacheHit/Miss в metrics.registry.ts; два else-if в
mcp.service.ts, роутящие ровно эти имена (существующие 2 ветки не тронуты).
Проверено end-to-end: скрейп prom-реестра показывает hits=2/misses=1 после
прогона роутинга.
F2 (нет теста на пропуск конверсии): добавлен overridable seam
convertPageMarkdown в read.ts (идиома проекта для юнит-тестируемости ESM-импортов);
getPage miss-ветка идёт через него. Тест мокает convertPageMarkdown и ассертит
callCount===1 через MISS→HIT одной страницы (конверсия один раз на промахе, ноль
на хите). Мутационно доказан: пропатчил hit-ветку на повторную конверсию → тест
покраснел (callCount=2), откатил → зелёный.
mcp node --test 814/814 (+1); pmd+mcp tsc чисто; серверные metrics-файлы
компилируются изолированно, runtime-тест счётчиков зелёный.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extends the #355 perf-metrics registry (all behind the METRICS_PORT hard
gate — nullable instruments, no-op helpers when unset). New families:
- collab_doc_load_duration_seconds{size_bucket} — onLoadDocument timed on
the real DB-load paths only (the already-loaded early-return is skipped).
- size_bucket added to collab_store_duration_seconds; storeDocument returns
the ydoc byte length (reusing its single Y.encodeStateAsUpdate, no second
encode) so onStoreDocument observes size without extra cost.
- collab_docs_open (gauge, read-on-scrape via collect() from
hocuspocus.getDocumentsCount() — never inc/dec'd, so it can't drift) +
collab_doc_loads_total / collab_doc_unloads_total (afterLoad/afterUnload).
- collab_connect_duration_seconds — onConnect->connected, correlated by a
WeakMap keyed on the shared request (leak-free; the current client uses
one socket per document).
- collab_auth_duration_seconds — wraps onAuthenticate (success and failure).
- sizeBucket() shared helper (lt64k|lt256k|lt1m|ge1m, 4 bounded values).
The mcp_tool_duration_seconds histogram + collab_connect_timeouts_total
counter helpers are registered here but wired in pass 2 (MCP callback).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The {search-queue} BullMQ queue had producers on every page/space/workspace change
but NO consumer/@Processor — it lives in Docmost's EE edition (external search
drivers), never in this fork. Search works independently via the pages tsvector DB
trigger; the queue was pure dead weight, growing forever (1902 stuck jobs in Redis,
the first real hit of the #355 queue-growing alert — which fired correctly).
Remove the plumbing:
- the 3 producers: drop @InjectQueue(SEARCH_QUEUE) + every searchQueue.add(...) from
page/space/workspace.listener.ts (and the isTypesense() gate that only wrapped the
search enqueue). page.listener.handlePageUpdated did ONLY the search enqueue, so its
@OnEvent(PAGE_UPDATED) handler is removed; the other handlers keep their aiQueue.add.
- the registration (queue.module.ts) and the metrics injection + 'search' depth-metric
entry (metrics-bull.service.ts).
- the constants: QueueName.SEARCH_QUEUE + the 6 unused SEARCH_INDEX_* QueueJob members
(grep-confirmed unreferenced). Shared QueueJob members (PAGE_*, SPACE_DELETED,
WORKSPACE_DELETED — used by the AI queue / embedding/history/notification processors)
are kept.
Search (tsvector trigger, search.service) and the PAGE_UPDATED websocket listener are
untouched. Verified: apps/server tsc --noEmit 0 errors; 6 spec suites / 52 tests green.
Ops (maintainer, out of code scope): one-time Redis cleanup
`redis-cli --scan --pattern "bull:{search-queue}:*" | xargs -r redis-cli del`, then
confirm bullmq_queue_depth{queue="search"} stays 0 (the metric no longer injects it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- F1: added '/icons/' to STATIC_PATH_PREFIXES — public/icons/ is copied verbatim
to client/dist (a sibling of the already-included brand/vad/locales), and
index.html references /icons/favicon-*.png on every page load, so those requests
were getting their own route labels instead of collapsing to `static`.
- F2: corrected the comment — only /assets/ is content-hashed (unbounded per
deploy); /vad//brand//locales//icons/ have stable names (repetitive, not
unbounded). Either way none belong in the API-route histogram.
- F3: the negative test now exercises the trailing-slash boundary (the actual
anti-false-collapse guard): '/assets' (no slash), '/assetsx/foo.js',
'/iconset/x.png' must NOT collapse to `static` — cases that a buggy
includes()/slashless-prefix impl would wrongly collapse. Plus '/icons/*' added
to the positive it.each.
Gate: server tsc 0; metrics.spec passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to #355: http_request_duration_seconds's `route` label captured raw
content-hashed asset filenames (route="/assets/index-CAbxDtto.js",
"/assets/chunk-*.js"). @fastify/static serves each file through a route whose
matched routeOptions.url IS the raw hashed path, so the label was unbounded — a
new set of names every deploy, growing the series forever (the exact cardinality
leak the API routes were protected against).
resolveRouteLabel now detects a static request by its path prefix (/assets/,
/vad/, /brand/, /locales/) FIRST and collapses it to a single `static` label
(query string stripped before the check); API routes still use the template and
404s still collapse to `unknown`. Static edge latency is already measured by
Traefik's traefik_router_request_duration_*.
Gate: server tsc 0; metrics.spec passes (added static-collapse + query-strip +
"real API route mentioning assets is NOT collapsed" cases).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 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>
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>
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>
agent_coder
vvzvlad
agent_coder