Compare commits
19 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 45ff922dd4 | |||
| e3ca2dc1d5 | |||
| f919ced8c9 | |||
| 4109b2ef7f | |||
| 6b27ff0652 | |||
| e133b86982 | |||
| 579c82617b | |||
| 173f35e473 | |||
| 696b96ac18 | |||
| a96ca8e26b | |||
| f84386f24a | |||
| c0c44fddb9 | |||
| 91e58e3c9f | |||
| 8d062728e5 | |||
| f1cffc2d0f | |||
| bb9a6fd765 | |||
| 1c083fbd3d | |||
| 52763998d3 | |||
| bfb6a52eea |
@@ -124,9 +124,17 @@ jobs:
|
||||
exit "$FAILED"
|
||||
|
||||
# A GENUINE counterexample: fast-check printed a shrunk minimal case and its
|
||||
# reproducing seed into property-output.txt. File a dedup-guarded issue whose
|
||||
# title prefix is UNIQUE to counterexamples, so an infra failure (handled by
|
||||
# the next step under a different title) can never poison this dedup.
|
||||
# reproducing seed into property-output.txt. File a dedup-guarded issue.
|
||||
#
|
||||
# Dedup is keyed on a HASH of the SHRUNK COUNTEREXAMPLE (the minimal failing
|
||||
# input), NOT on the issue title prefix. Keying on the prefix would let a
|
||||
# single open issue swallow every OTHER counterexample (a different bug B whose
|
||||
# title shares the prefix would be treated as a duplicate and stay silent until
|
||||
# the first issue is closed). Hashing the shrunk example instead means two
|
||||
# DIFFERENT counterexamples get two DIFFERENT issues, while a re-find of the
|
||||
# SAME counterexample still dedupes onto the existing one. The infra-failure
|
||||
# step (below) still keys on its own distinct title, so it can never poison
|
||||
# this dedup either.
|
||||
- name: File counterexample issue
|
||||
# always() is REQUIRED: the fuzz step exits nonzero on a failing shard,
|
||||
# so a bare `if:` (implicitly success() && ...) would skip this step
|
||||
@@ -146,25 +154,48 @@ jobs:
|
||||
echo "No fast-check counterexample signature — infra failure, handled by the next step."
|
||||
exit 0
|
||||
fi
|
||||
TITLE="${TITLE_PREFIX} (seed=${FAIL_SEED})"
|
||||
# Extract the SHRUNK counterexample block: the "Counterexample:" line(s)
|
||||
# up to (but excluding) the "Shrunk N time(s)" / "Got error" line. This is
|
||||
# the minimal failing INPUT and is STABLE across the different seeds/paths
|
||||
# that reach the same bug — unlike the seed, path, or shrink count (which
|
||||
# precede/follow this block and vary run-to-run) and unlike the whole
|
||||
# output (which embeds those varying parts). Hashing THIS is what makes the
|
||||
# dedup identity the bug itself rather than an incidental run detail.
|
||||
CE_TEXT=$(awk '/Counterexample:/{c=1} /Shrunk [0-9]+ time|Got error/{c=0} c{print}' property-output.txt)
|
||||
if [ -z "$CE_TEXT" ]; then
|
||||
# No parseable shrunk block (unexpected — the signature check above
|
||||
# already confirmed fast-check output). Fall back to the reproducing
|
||||
# seed so we still emit a stable identity instead of silently deduping.
|
||||
CE_TEXT="seed:${FAIL_SEED}"
|
||||
fi
|
||||
# Stable short id: first 12 hex chars of sha256 over the counterexample.
|
||||
CE_HASH=$(printf '%s' "$CE_TEXT" | sha256sum | cut -c1-12)
|
||||
# Machine-readable marker embedded in the issue body; the open-issue search
|
||||
# below matches on it (and on the hash in the title) so identity travels
|
||||
# with the issue regardless of any human title edits.
|
||||
CE_MARKER="<!-- counterexample-hash: ${CE_HASH} -->"
|
||||
export CE_HASH CE_MARKER
|
||||
TITLE="${TITLE_PREFIX} [${CE_HASH}] (seed=${FAIL_SEED})"
|
||||
|
||||
# Best-effort dedup: skip if an open issue with the counterexample title
|
||||
# prefix already exists. A failure of this check must NOT block creation.
|
||||
# Dedup on the counterexample hash: skip only if an OPEN issue already
|
||||
# carries this exact hash (in its title or its body marker). A different
|
||||
# counterexample has a different hash and is NOT deduped. A failure of this
|
||||
# check must NOT block creation.
|
||||
EXISTING=""
|
||||
if EXISTING=$(curl -sS \
|
||||
-H "Authorization: token ${GITHUB_TOKEN}" \
|
||||
"${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}/issues?state=open&limit=100"); then
|
||||
if printf '%s' "$EXISTING" \
|
||||
| node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{let a;try{a=JSON.parse(s)}catch{process.exit(1)}if(!Array.isArray(a))process.exit(1);const p=process.env.TITLE_PREFIX;process.exit(a.some(i=>typeof i.title==="string"&&i.title.startsWith(p))?0:1)})'; then
|
||||
echo "An open '${TITLE_PREFIX}' issue already exists — skipping creation."
|
||||
| node -e 'let s="";process.stdin.on("data",d=>s+=d).on("end",()=>{let a;try{a=JSON.parse(s)}catch{process.exit(1)}if(!Array.isArray(a))process.exit(1);const h=process.env.CE_HASH,m=process.env.CE_MARKER;process.exit(a.some(i=>(typeof i.title==="string"&&i.title.includes(h))||(typeof i.body==="string"&&i.body.includes(m)))?0:1)})'; then
|
||||
echo "An open issue for counterexample ${CE_HASH} already exists — skipping creation."
|
||||
exit 0
|
||||
fi
|
||||
fi
|
||||
|
||||
# Build the JSON body with the test output SAFELY escaped (never hand-
|
||||
# interpolate the counterexample into JSON).
|
||||
BODY_TEXT=$(printf 'A nightly property fuzz SHARD failed with a fast-check counterexample.\n\n- failing shard seed: `%s`\n- NUM_RUNS (per shard): `%s`\n- run: %s\n\nReproduce locally:\n\n```\nPROPERTY_SEED=%s PROPERTY_NUM_RUNS=%s pnpm --filter @docmost/prosemirror-markdown exec vitest run test/generative/\n```\n\nfast-check shrinks the failure to a minimal counterexample. Commit it as a permanent fixture under `packages/prosemirror-markdown/test/fixtures/counterexamples/` + a case in `counterexamples.test.ts`, then FIX the converter (do not weaken a property). See `packages/prosemirror-markdown/README.md`.\n\nTail of the test output (contains the shrunk counterexample):\n\n```\n%s\n```\n' \
|
||||
"$FAIL_SEED" "$NUM_RUNS" "$RUN_URL" "$FAIL_SEED" "$NUM_RUNS" "$(tail -n 120 property-output.txt)")
|
||||
BODY_TEXT=$(printf 'A nightly property fuzz SHARD failed with a fast-check counterexample.\n\n- counterexample hash: `%s`\n- failing shard seed: `%s`\n- NUM_RUNS (per shard): `%s`\n- run: %s\n\nReproduce locally:\n\n```\nPROPERTY_SEED=%s PROPERTY_NUM_RUNS=%s pnpm --filter @docmost/prosemirror-markdown exec vitest run test/generative/\n```\n\nfast-check shrinks the failure to a minimal counterexample. Commit it as a permanent fixture under `packages/prosemirror-markdown/test/fixtures/counterexamples/` + a case in `counterexamples.test.ts`, then FIX the converter (do not weaken a property). See `packages/prosemirror-markdown/README.md`.\n\nTail of the test output (contains the shrunk counterexample):\n\n```\n%s\n```\n\n%s\n' \
|
||||
"$CE_HASH" "$FAIL_SEED" "$NUM_RUNS" "$RUN_URL" "$FAIL_SEED" "$NUM_RUNS" "$(tail -n 120 property-output.txt)" "$CE_MARKER")
|
||||
|
||||
jq -n --arg title "$TITLE" --arg body "$BODY_TEXT" \
|
||||
'{title: $title, body: $body}' > payload.json
|
||||
|
||||
@@ -471,6 +471,8 @@ Vite SPA. Code is organized by feature under `apps/client/src/features/*` (mirro
|
||||
- The version string shown in the UI comes from `APP_VERSION` (CI/Docker) or `git describe --tags --always` (local), resolved in `vite.config.ts` — not from `package.json`.
|
||||
- Server TS config is permissive (`noImplicitAny: false`, `strictNullChecks: false`, `no-explicit-any` lint disabled). Follow the existing relaxed style rather than tightening types broadly.
|
||||
- Dependency versions are heavily pinned via `pnpm.overrides` and `pnpm.patchedDependencies` (`scimmy`, `yjs`, `ai`) in the root `package.json`. Don't bump pinned/patched deps casually; the patches and overrides exist for compatibility/security reasons. The `ai@6.0.134` patch carries TWO independent server fixes, each with its own tripwire test: (1) it disables the SDK's O(n²) cumulative `partialOutput` accumulation when no output strategy is requested (server heap OOM on long agent runs, #184; tripwire: `apps/server/src/integrations/ai/ai-sdk-partial-output.patch.spec.ts`); (2) it fixes `writeToServerResponse`'s drain-hang — the loop awaited only `"drain"` under backpressure, so a mid-write client disconnect parked the pipe forever and leaked the reader/buffers until restart; it now races `"drain"` against `"close"`/`"error"`, cancels the reader on disconnect, and swallows the fire-and-forget read rejection (#486; tripwire: `apps/server/src/integrations/ai/ai-sdk-drain-hang.patch.spec.ts`). Both tripwires assert BOTH installed dist builds carry their patch marker. The patch MUST be re-created via `pnpm patch` when bumping `ai`.
|
||||
- **Upstream tracking (report the analysis upstream, don't just carry it):** both `ai` fixes and the hocuspocus one are candidates for upstreaming so we can eventually drop the local patch — the analysis is already written up in each patch's `PATCH(...)` header comments. File (a) an upstream **issue** on `vercel/ai` for the O(n²) cumulative `partialOutput` accumulation (heap OOM), (b) an upstream **issue** on `vercel/ai` for the `writeToServerResponse` drain-hang, and (c) an upstream **PR** on `@hocuspocus/server` for the connect-vs-unload race (local marker `PATCH(gitmost #401)` in `patches/@hocuspocus__server@3.4.4.patch`). Do NOT edit the patch files to add links — the patch bytes feed `patch_hash` in `pnpm-lock.yaml` (`ai@6.0.134` → `e8c599b3…`), so any content change there desyncs the lockfile pin and breaks `pnpm install`; keep upstream references here instead.
|
||||
- **`ai` version is split across the monorepo and MUST be aligned deliberately, NOT casually:** the server pins `ai@6.0.134` (patched, exact — the `patchedDependencies` key forces that version), while the client declares `ai@6.0.207` (unpatched — the server-side `writeToServerResponse`/`partialOutput` fixes are dead code in the browser, so the mismatch is currently benign but is real drift). Alignment is a **planned, install-gated step**, never a bare `package.json` edit: (1) choose the target version; (2) re-create ALL THREE patch hunks (partialOutput publish-each, the `DefaultStreamTextResult` lazy-`output` wiring, and the drain-hang race) against the target dist via `pnpm patch` — the line offsets shift between versions, so the current patch WILL fail to apply as-is; (3) run a full `pnpm install` so the lockfile + new `patch_hash` regenerate together; (4) confirm both tripwire specs still find their markers. `pnpm install` FAILS HARD on an unapplied patch — that failure is the guardrail, so treat the port as a deliberate plan rather than discovering it as a deploy-time surprise.
|
||||
- **The MCP tool inventory in `SERVER_INSTRUCTIONS` is GENERATED from the registry** (`packages/mcp/src/server-instructions.ts`: `buildToolInventory()` over `SHARED_TOOL_SPECS`) and spliced into the hand-written routing prose (`ROUTING_PROSE`). So adding/renaming/removing a **shared** spec in `packages/mcp/src/tool-specs.ts` auto-updates the `<tool_inventory>` — no manual `SERVER_INSTRUCTIONS` edit needed. Only an **inline** MCP-only tool (those registered via `server.registerTool(...)` in `index.ts`, not through the registry) needs a one-line entry in `INLINE_MCP_INVENTORY`. Enforced by `packages/mcp/test/unit/tool-inventory.test.mjs`, which fails when a registered tool is missing from the generated inventory (there is no `EXCEPTIONS` opt-out anymore — every tool must appear). Update `ROUTING_PROSE` when a tool's *intent guidance* (when-to-use) changes. `packages/mcp/build/` is gitignored and rebuilt in CI/Docker via `pnpm build` (same convention as `git-sync`/`prosemirror-markdown`) — never commit it; rebuild locally after editing to run the tests.
|
||||
|
||||
## CI / release
|
||||
|
||||
@@ -304,6 +304,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
longer controls whether a turn is a run — it now governs **only** the
|
||||
browser-disconnect semantics (ON = detached/survives a disconnect; OFF = a
|
||||
disconnect stops the run). (#487)
|
||||
- **Vendor `ai` patch: upstream-tracking + version-alignment plan documented.**
|
||||
The two local `ai@6.0.134` fixes (O(n²) `partialOutput` heap-OOM; the
|
||||
`writeToServerResponse` drain-hang) and the hocuspocus connect-vs-unload race
|
||||
now have explicit upstream-reporting and `ai`-version-alignment steps recorded
|
||||
in `AGENTS.md` (client `ai@6.0.207` vs server `ai@6.0.134`-patched drift). The
|
||||
patch bytes are unchanged — they feed the lockfile `patch_hash`, so the
|
||||
alignment is called out as an install-gated plan rather than a bare version
|
||||
bump. No runtime change.
|
||||
- **Client markdown paste/copy and AI-chat rendering now go through the canonical
|
||||
converter.** Pasting markdown into the editor, "Copy as markdown", the AI title
|
||||
generator, and the AI-chat markdown renderer all now use
|
||||
|
||||
@@ -256,6 +256,9 @@
|
||||
"Invite link": "Ссылка для приглашения",
|
||||
"Copy": "Копировать",
|
||||
"Copy to space": "Копировать в пространство",
|
||||
"Copy chat": "Копировать чат",
|
||||
"Dock to sidebar": "Закрепить в боковой панели",
|
||||
"Undock": "Открепить",
|
||||
"Copied": "Скопировано",
|
||||
"Failed to export chat": "Не удалось экспортировать чат",
|
||||
"Duplicate": "Дублировать",
|
||||
@@ -285,6 +288,9 @@
|
||||
"Alt text": "Альтернативный текст",
|
||||
"Describe this for accessibility.": "Опишите это для специальных возможностей.",
|
||||
"Add a description": "Добавить описание",
|
||||
"Caption": "Подпись",
|
||||
"Add a caption": "Добавить подпись",
|
||||
"Shown below the image.": "Отображается под изображением.",
|
||||
"Justify": "По ширине",
|
||||
"Merge cells": "Объединить ячейки",
|
||||
"Split cell": "Разделить ячейку",
|
||||
@@ -388,22 +394,6 @@
|
||||
"Quote": "Цитата",
|
||||
"Image": "Изображение",
|
||||
"Audio": "Аудио",
|
||||
"Transcribe": "Транскрибировать",
|
||||
"Transcribing…": "Транскрибация…",
|
||||
"No speech detected": "Речь не распознана",
|
||||
"Transcription failed": "Не удалось распознать речь",
|
||||
"Voice dictation is not configured": "Голосовой ввод не настроен",
|
||||
"Start dictation": "Начать диктовку",
|
||||
"Stop recording": "Остановить запись",
|
||||
"Microphone access denied": "Доступ к микрофону запрещён",
|
||||
"No microphone found": "Микрофон не найден",
|
||||
"Microphone is unavailable or already in use": "Микрофон недоступен или уже используется",
|
||||
"Could not start recording": "Не удалось начать запись",
|
||||
"Audio recording is not available in this browser/context": "Запись аудио недоступна в этом браузере/контексте",
|
||||
"Dictation": "Диктовка",
|
||||
"Dictation becomes available once the page finishes connecting": "Диктовка станет доступна после подключения к документу",
|
||||
"No connection to the collaboration server — dictation unavailable": "Нет связи с сервером совместного редактирования — диктовка недоступна",
|
||||
"This page is read-only": "Страница открыта только для чтения",
|
||||
"Embed PDF": "Встроить PDF",
|
||||
"Upload and embed a PDF file.": "Загрузите и встроите PDF-файл.",
|
||||
"Embed as PDF": "Встроить как PDF",
|
||||
@@ -419,9 +409,6 @@
|
||||
"Footnote {{number}}": "Сноска {{number}}",
|
||||
"Go to footnote": "Перейти к сноске",
|
||||
"Back to reference": "Вернуться к ссылке",
|
||||
"Back to references": "Вернуться к ссылкам",
|
||||
"Back to reference {{label}}": "Вернуться к ссылке {{label}}",
|
||||
"Empty footnote": "Пустая сноска",
|
||||
"Math inline": "Строчная формула",
|
||||
"Insert inline math equation.": "Вставить математическое выражение в строку.",
|
||||
"Math block": "Блок формулы",
|
||||
@@ -447,6 +434,9 @@
|
||||
"{{count}} command available_other": "Доступно {{count}} команд",
|
||||
"{{count}} result available_one": "Доступен 1 результат",
|
||||
"{{count}} result available_other": "Доступно {{count}} результатов",
|
||||
"{{count}} result found_one": "Найден {{count}} результат",
|
||||
"{{count}} result found_few": "Найдено {{count}} результата",
|
||||
"{{count}} result found_other": "Найдено {{count}} результатов",
|
||||
"Equal columns": "Равные столбцы",
|
||||
"Left sidebar": "Левая боковая панель",
|
||||
"Right sidebar": "Правая боковая панель",
|
||||
@@ -456,6 +446,7 @@
|
||||
"Names do not match": "Названия не совпадают",
|
||||
"Today, {{time}}": "Сегодня, {{time}}",
|
||||
"Yesterday, {{time}}": "Вчера, {{time}}",
|
||||
"now": "сейчас",
|
||||
"Space created successfully": "Пространство успешно создано",
|
||||
"Space updated successfully": "Пространство успешно обновлено",
|
||||
"Space deleted successfully": "Пространство успешно удалено",
|
||||
@@ -559,6 +550,7 @@
|
||||
"Add 2FA method": "Добавить метод 2FA",
|
||||
"Backup codes": "Резервные коды",
|
||||
"Disable": "Отключить",
|
||||
"disabled": "отключено",
|
||||
"Invalid verification code": "Недействительный код подтверждения",
|
||||
"New backup codes have been generated": "Новые резервные коды сгенерированы",
|
||||
"Failed to regenerate backup codes": "Не удалось заново сгенерировать резервные коды",
|
||||
@@ -702,62 +694,6 @@
|
||||
"AI search": "Поиск ИИ",
|
||||
"AI Answer": "Ответ ИИ",
|
||||
"Ask AI": "Спросить ИИ",
|
||||
"AI agent": "AI-агент",
|
||||
"Take a look at the current document": "Посмотри текущий документ",
|
||||
"Start automatically": "Запускать автоматически",
|
||||
"When on, picking this role sends a launch message and starts the chat. When off, the role is selected and you type the first message yourself.": "Когда включено, выбор этой роли отправляет стартовое сообщение и начинает чат. Когда выключено, роль выбирается, а первое сообщение вы вводите сами.",
|
||||
"Launch message": "Стартовое сообщение",
|
||||
"Sent automatically when this role is picked. Leave empty to use the default text. Ignored when “Start automatically” is off.": "Отправляется автоматически при выборе этой роли. Оставьте пустым, чтобы использовать текст по умолчанию. Игнорируется, когда «Запускать автоматически» выключено.",
|
||||
"AI agent is typing…": "AI-агент печатает…",
|
||||
"{{name}} is typing…": "{{name}} печатает…",
|
||||
"Thinking…": "Думаю…",
|
||||
"Thinking… · {{count}} tokens": "Думаю… · {{count}} токенов",
|
||||
"Thinking… · {{count}} tokens_one": "Думаю… · {{count}} токен",
|
||||
"Thinking… · {{count}} tokens_few": "Думаю… · {{count}} токена",
|
||||
"Thinking… · {{count}} tokens_many": "Думаю… · {{count}} токенов",
|
||||
"Thinking · {{count}} tokens": "Размышления · {{count}} токенов",
|
||||
"Thinking · {{count}} tokens_one": "Размышления · {{count}} токен",
|
||||
"Thinking · {{count}} tokens_few": "Размышления · {{count}} токена",
|
||||
"Thinking · {{count}} tokens_many": "Размышления · {{count}} токенов",
|
||||
"Agent role": "Роль агента",
|
||||
"AI chat": "AI-чат",
|
||||
"AI chat is disabled for this workspace.": "AI-чат отключён для этого рабочего пространства.",
|
||||
"Ask a question about this documentation.": "Задайте вопрос об этой документации.",
|
||||
"Ask a question…": "Задайте вопрос…",
|
||||
"Ask the AI agent anything about your workspace.": "Спросите AI-агента о чём угодно по вашему рабочему пространству.",
|
||||
"Ask the AI agent…": "Спросите AI-агента…",
|
||||
"Copy chat": "Копировать чат",
|
||||
"Dock to sidebar": "Закрепить в боковой панели",
|
||||
"Undock": "Открепить",
|
||||
"Created successfully": "Успешно создано",
|
||||
"Context size / model limit": "Размер контекста / лимит модели",
|
||||
"Context window (tokens)": "Окно контекста (токены)",
|
||||
"Shown as used / total in the chat header. Leave empty to hide the limit.": "Показывается в шапке чата как использовано / всего. Пусто — лимит скрыт.",
|
||||
"Delete this chat?": "Удалить этот чат?",
|
||||
"Deleted successfully": "Успешно удалено",
|
||||
"AI agent «{{role}}» on behalf of {{person}}": "AI-агент «{{role}}» от имени {{person}}",
|
||||
"AI agent {{name}}": "AI-агент {{name}}",
|
||||
"Failed to delete chat": "Не удалось удалить чат",
|
||||
"Failed to rename chat": "Не удалось переименовать чат",
|
||||
"Failed": "Ошибка",
|
||||
"OK · {{n}}": "OK · {{n}}",
|
||||
"Test": "Тест",
|
||||
"No tools available": "Инструменты недоступны",
|
||||
"Available tools": "Доступные инструменты",
|
||||
"Minimize": "Свернуть",
|
||||
"No chats yet.": "Чатов пока нет.",
|
||||
"Send": "Отправить",
|
||||
"Send when the agent finishes": "Отправить, когда агент закончит",
|
||||
"Queue message": "Поставить в очередь",
|
||||
"Remove queued message": "Убрать из очереди",
|
||||
"Send now": "Отправить сейчас",
|
||||
"Interrupt and send now": "Прервать и отправить сейчас",
|
||||
"Something went wrong": "Что-то пошло не так",
|
||||
"Stop": "Стоп",
|
||||
"The AI agent could not respond. Please try again.": "AI-агент не смог ответить. Попробуйте ещё раз.",
|
||||
"The AI provider is not configured. Ask an administrator to set it up.": "AI-провайдер не настроен. Попросите администратора настроить его.",
|
||||
"Universal assistant": "Универсальный ассистент",
|
||||
"You": "Вы",
|
||||
"AI is thinking...": "ИИ обрабатывает запрос...",
|
||||
"Thinking": "Думаю",
|
||||
"Ask a question...": "Задайте вопрос...",
|
||||
@@ -784,8 +720,40 @@
|
||||
"Manage API keys for all users in the workspace. View the <anchor>API documentation</anchor> for usage details.": "Управляйте API-ключами для всех пользователей в рабочем пространстве. Смотрите <anchor>документацию по API</anchor> для получения информации об использовании.",
|
||||
"View the <anchor>API documentation</anchor> for usage details.": "Смотрите <anchor>документацию по API</anchor> для получения информации об использовании.",
|
||||
"View the <anchor>MCP documentation</anchor>.": "Смотрите <anchor>документацию по MCP</anchor>.",
|
||||
"Instructions": "Инструкции",
|
||||
"AI / Models": "ИИ / Модели",
|
||||
"AI / External tools (MCP)": "ИИ / Внешние инструменты (MCP)",
|
||||
"Add server": "Добавить сервер",
|
||||
"Edit server": "Изменить сервер",
|
||||
"Delete server": "Удалить сервер",
|
||||
"Are you sure you want to delete this MCP server?": "Вы уверены, что хотите удалить этот MCP-сервер?",
|
||||
"No external servers configured": "Внешние серверы не настроены",
|
||||
"Server name": "Имя сервера",
|
||||
"Transport": "Транспорт",
|
||||
"URL": "URL",
|
||||
"Authorization header": "Заголовок авторизации",
|
||||
"Tool allowlist": "Список разрешённых инструментов",
|
||||
"Optional. Leave empty to allow all tools the server exposes.": "Необязательно. Оставьте пустым, чтобы разрешить все инструменты, которые предоставляет сервер.",
|
||||
"Optional guidance for the agent on how and when to use this server's tools. Injected into the system prompt. The server's tools are namespaced as \"<server name>_*\".": "Необязательное указание агенту, как и когда использовать инструменты этого сервера. Добавляется в системный промпт. Инструменты сервера именуются с префиксом «<имя сервера>_*».",
|
||||
"Test": "Тест",
|
||||
"Available tools": "Доступные инструменты",
|
||||
"No tools available": "Инструменты недоступны",
|
||||
"Failed": "Ошибка",
|
||||
"OK · {{n}}": "OK · {{n}}",
|
||||
"Created successfully": "Успешно создано",
|
||||
"Deleted successfully": "Успешно удалено",
|
||||
"Clear": "Очистить",
|
||||
"Provider": "Провайдер",
|
||||
"•••• set": "•••• задан",
|
||||
"Clear key": "Очистить ключ",
|
||||
"Base URL": "Базовый URL",
|
||||
"Chat model": "Модель чата",
|
||||
"Embedding model": "Модель эмбеддингов",
|
||||
"System message": "Системное сообщение",
|
||||
"A built-in safety framework is always appended.": "Встроенный набор правил безопасности всегда добавляется автоматически.",
|
||||
"Test connection": "Проверить соединение",
|
||||
"Connection successful": "Соединение установлено",
|
||||
"Connection failed": "Не удалось установить соединение",
|
||||
"Only workspace admins can manage AI provider settings.": "Управлять настройками провайдера ИИ могут только администраторы рабочего пространства.",
|
||||
"Sources": "Источники",
|
||||
"AI Answers not available for attachments": "Ответы ИИ недоступны для вложений",
|
||||
"No answer available": "Ответ недоступен",
|
||||
@@ -1013,6 +981,7 @@
|
||||
"Try again": "Попробовать снова",
|
||||
"Untitled chat": "Чат без названия",
|
||||
"No document": "Без документа",
|
||||
"You": "Вы",
|
||||
"What can I help you with?": "Чем я могу вам помочь?",
|
||||
"Are you sure you want to revoke this {{credential}}": "Вы уверены, что хотите отозвать этот {{credential}}",
|
||||
"Automatically provision users and groups from your identity provider via SCIM.": "Автоматически предоставляйте доступ пользователям и группам из вашего провайдера удостоверений через SCIM.",
|
||||
@@ -1041,6 +1010,9 @@
|
||||
"Page menu": "Меню страницы",
|
||||
"Expand": "Развернуть",
|
||||
"Collapse": "Свернуть",
|
||||
"Expand all": "Развернуть все",
|
||||
"Collapse all": "Свернуть все",
|
||||
"Couldn't expand the tree: {{reason}}": "Не удалось развернуть дерево: {{reason}}",
|
||||
"Comment menu": "Меню комментария",
|
||||
"Group menu": "Меню группы",
|
||||
"Show hidden breadcrumbs": "Показать скрытые хлебные крошки",
|
||||
@@ -1077,7 +1049,7 @@
|
||||
"Search pages and spaces...": "Поиск страниц и пространств...",
|
||||
"No results found": "Результаты не найдены",
|
||||
"You don't have permission to create pages here": "У вас нет прав на создание страниц здесь",
|
||||
"Chat menu": "Меню чата",
|
||||
"Chat menu for {{title}}": "Меню чата для {{title}}",
|
||||
"API key menu": "Меню API-ключа",
|
||||
"Jump to comment selection": "Перейти к выбору комментария",
|
||||
"Slash commands": "Команды со слешем",
|
||||
@@ -1131,6 +1103,9 @@
|
||||
"Undo": "Отменить",
|
||||
"Redo": "Повторить",
|
||||
"Backlinks": "Обратные ссылки",
|
||||
"Back to references": "Вернуться к ссылкам",
|
||||
"Back to reference {{label}}": "Вернуться к ссылке {{label}}",
|
||||
"Empty footnote": "Пустая сноска",
|
||||
"Last updated by": "Последний изменивший",
|
||||
"Last updated": "Последнее обновление",
|
||||
"Stats": "Статистика",
|
||||
@@ -1164,6 +1139,7 @@
|
||||
"Page title": "Заголовок страницы",
|
||||
"Page content": "Содержимое страницы",
|
||||
"Member actions": "Действия с участником",
|
||||
"Member actions for {{name}}": "Действия с участником {{name}}",
|
||||
"Toggle password visibility": "Переключить видимость пароля",
|
||||
"Send comment": "Отправить комментарий",
|
||||
"Token actions": "Действия с токеном",
|
||||
@@ -1183,11 +1159,187 @@
|
||||
"Removed from favorites": "Удалено из избранного",
|
||||
"Added {{name}} to favorites": "{{name}} добавлено в избранное",
|
||||
"Removed {{name}} from favorites": "{{name}} удалено из избранного",
|
||||
"Label added": "Метка добавлена",
|
||||
"Label removed": "Метка удалена",
|
||||
"Image updated": "Изображение обновлено",
|
||||
"Unsupported image type": "Неподдерживаемый тип изображения",
|
||||
"Member deactivated": "Участник деактивирован",
|
||||
"Member activated": "Участник активирован",
|
||||
"Name is required": "Укажите имя",
|
||||
"Name must be 40 characters or fewer": "Имя должно содержать не более 40 символов",
|
||||
"Group name must be at least 2 characters": "Название группы должно содержать не менее 2 символов",
|
||||
"Group name must be 100 characters or fewer": "Название группы должно содержать не более 100 символов",
|
||||
"Description must be 500 characters or fewer": "Описание должно содержать не более 500 символов",
|
||||
"Invalid invitation link": "Недействительная ссылка-приглашение",
|
||||
"Page menu for {{name}}": "Меню страницы для {{name}}",
|
||||
"Create subpage of {{name}}": "Создать подстраницу для {{name}}",
|
||||
"AI chat": "AI-чат",
|
||||
"Ask a question about this documentation.": "Задайте вопрос об этой документации.",
|
||||
"Ask a question…": "Задайте вопрос…",
|
||||
"Thinking…": "Думаю…",
|
||||
"Thinking… · {{count}} tokens": "Думаю… · {{count}} токенов",
|
||||
"Thinking… · {{count}} tokens_one": "Думаю… · {{count}} токен",
|
||||
"Thinking… · {{count}} tokens_few": "Думаю… · {{count}} токена",
|
||||
"Thinking… · {{count}} tokens_many": "Думаю… · {{count}} токенов",
|
||||
"Thinking… · {{count}} tokens_other": "Думаю… · {{count}} токенов",
|
||||
"Thinking · {{count}} tokens": "Размышления · {{count}} токенов",
|
||||
"Thinking · {{count}} tokens_one": "Размышления · {{count}} токен",
|
||||
"Thinking · {{count}} tokens_few": "Размышления · {{count}} токена",
|
||||
"Thinking · {{count}} tokens_many": "Размышления · {{count}} токенов",
|
||||
"Thinking · {{count}} tokens_other": "Размышления · {{count}} токенов",
|
||||
"The assistant is unavailable right now. Please try again.": "Ассистент сейчас недоступен. Попробуйте ещё раз.",
|
||||
"Public share assistant": "Ассистент публичного доступа",
|
||||
"Let anonymous visitors of public shares ask an AI assistant scoped to that share's pages. You pay for the tokens.": "Позвольте анонимным посетителям публичных ссылок обращаться к ИИ-ассистенту в рамках страниц этой публикации. Токены оплачиваете вы.",
|
||||
"Public assistant model": "Модель публичного ассистента",
|
||||
"Defaults to the chat model": "По умолчанию используется модель чата",
|
||||
"Optional cheaper model id for the public assistant. Empty uses the chat model above.": "Необязательный более дешёвый идентификатор модели для публичного ассистента. Если пусто, используется модель чата выше.",
|
||||
"Assistant identity": "Личность ассистента",
|
||||
"Pick an agent role whose persona the public assistant adopts. The safety rules always still apply.": "Выберите роль агента, чью личность примет публичный ассистент. Правила безопасности всегда остаются в силе.",
|
||||
"Built-in assistant persona": "Встроенная личность ассистента",
|
||||
"Minimize": "Свернуть",
|
||||
"Context size / model limit": "Размер контекста / лимит модели",
|
||||
"Context window (tokens)": "Окно контекста (токены)",
|
||||
"Shown as used / total in the chat header. Leave empty to hide the limit.": "Показывается в шапке чата как использовано / всего. Пусто — лимит скрыт.",
|
||||
"AI agent": "AI-агент",
|
||||
"Take a look at the current document": "Посмотри текущий документ",
|
||||
"AI agent is typing…": "AI-агент печатает…",
|
||||
"{{name}} is typing…": "{{name}} печатает…",
|
||||
"Send": "Отправить",
|
||||
"Send when the agent finishes": "Отправить, когда агент закончит",
|
||||
"Queue message": "Поставить в очередь",
|
||||
"Remove queued message": "Убрать из очереди",
|
||||
"Send now": "Отправить сейчас",
|
||||
"Interrupt and send now": "Прервать и отправить сейчас",
|
||||
"Stop": "Стоп",
|
||||
"Response stopped.": "Ответ остановлен.",
|
||||
"Connection lost — the answer was interrupted.": "Соединение потеряно — ответ был прерван.",
|
||||
"Response stopped (manually or the connection dropped).": "Ответ остановлен (вручную или из-за разрыва соединения).",
|
||||
"Chat menu": "Меню чата",
|
||||
"No chats yet.": "Чатов пока нет.",
|
||||
"Delete this chat?": "Удалить этот чат?",
|
||||
"Ask the AI agent…": "Спросите AI-агента…",
|
||||
"Ask the AI agent anything about your workspace.": "Спросите AI-агента о чём угодно по вашему рабочему пространству.",
|
||||
"Failed to rename chat": "Не удалось переименовать чат",
|
||||
"Failed to delete chat": "Не удалось удалить чат",
|
||||
"Something went wrong": "Что-то пошло не так",
|
||||
"AI chat is disabled for this workspace.": "AI-чат отключён для этого рабочего пространства.",
|
||||
"The AI provider is not configured. Ask an administrator to set it up.": "AI-провайдер не настроен. Попросите администратора настроить его.",
|
||||
"The AI agent could not respond. Please try again.": "AI-агент не смог ответить. Попробуйте ещё раз.",
|
||||
"Searched pages": "Поиск по страницам",
|
||||
"Read page": "Прочитана страница",
|
||||
"Created page": "Создана страница",
|
||||
"Updated page": "Обновлена страница",
|
||||
"Renamed page": "Переименована страница",
|
||||
"Moved page": "Перемещена страница",
|
||||
"Deleted page (to trash)": "Удалена страница (в корзину)",
|
||||
"Commented": "Добавлен комментарий",
|
||||
"Resolved comment": "Комментарий решён",
|
||||
"Ran tool {{name}}": "Выполнен инструмент {{name}}",
|
||||
"AI agent «{{role}}» on behalf of {{person}}": "AI-агент «{{role}}» от имени {{person}}",
|
||||
"AI agent {{name}}": "AI-агент {{name}}",
|
||||
"Endpoints": "Эндпоинты",
|
||||
"where we fetch models": "откуда мы получаем модели",
|
||||
"All endpoints are OpenAI-compatible. Point the Base URL at OpenAI, OpenRouter, a local Ollama, or any self-hosted server.": "Все эндпоинты совместимы с OpenAI. Укажите в базовом URL адрес OpenAI, OpenRouter, локального Ollama или любого self-hosted сервера.",
|
||||
"Chat / LLM": "Чат / LLM",
|
||||
"root": "корневой",
|
||||
"Semantic search": "Семантический поиск",
|
||||
"Voice / STT": "Голос / STT",
|
||||
"Voice dictation": "Голосовой ввод",
|
||||
"Streaming dictation": "Потоковый голосовой ввод",
|
||||
"Transcribe as you speak, cutting on pauses": "Транскрибирование по мере речи, с разбивкой на паузах",
|
||||
"Voice dictation is not available yet.": "Голосовой ввод пока недоступен.",
|
||||
"Test endpoint": "Проверить эндпоинт",
|
||||
"Save and test": "Сохранить и проверить",
|
||||
"Save endpoints": "Сохранить эндпоинты",
|
||||
"Configured and enabled": "Настроено и включено",
|
||||
"Configured but disabled": "Настроено, но отключено",
|
||||
"Enabled but not configured": "Включено, но не настроено",
|
||||
"Not configured": "Не настроено",
|
||||
"External tools": "Внешние инструменты",
|
||||
"Gitmost as MCP client": "Gitmost как MCP-клиент",
|
||||
"Servers the agent calls out to.": "Серверы, к которым обращается агент.",
|
||||
"MCP server": "MCP-сервер",
|
||||
"expose the workspace": "открыть доступ к рабочему пространству",
|
||||
"Enable MCP server": "Включить MCP-сервер",
|
||||
"Exposes the workspace as an MCP server at /mcp — this provides a capability, it doesn't consume a model.": "Открывает рабочее пространство как MCP-сервер по адресу /mcp — это предоставляет возможность, а не потребляет модель.",
|
||||
"Resolves to {{url}}": "Разрешается в {{url}}",
|
||||
"Model": "Модель",
|
||||
"Done": "Готово",
|
||||
"shared prompt · safety framework appended automatically": "общий промпт · правила безопасности добавляются автоматически",
|
||||
"/v1/chat/completions · root endpoint — Embeddings and Voice inherit its URL and key": "/v1/chat/completions · корневой эндпоинт — Эмбеддинги и Голос наследуют его URL и ключ",
|
||||
"/v1/embeddings · embeds pages so semantic search can find them": "/v1/embeddings · создаёт эмбеддинги страниц, чтобы их находил семантический поиск",
|
||||
"/v1/audio/transcriptions · works with local whisper (speaches / faster-whisper-server)": "/v1/audio/transcriptions · работает с локальным whisper (speaches / faster-whisper-server)",
|
||||
"Vector search · requires pgvector": "Векторный поиск · требуется pgvector",
|
||||
"Embedding API key": "API-ключ для эмбеддингов",
|
||||
"Embeddings": "Эмбеддинги",
|
||||
"Leave empty to use the chat API key": "Оставьте пустым, чтобы использовать API-ключ чата",
|
||||
"Leave empty to use the chat base URL": "Оставьте пустым, чтобы использовать базовый URL чата",
|
||||
"Reindex now": "Переиндексировать сейчас",
|
||||
"Start dictation": "Начать диктовку",
|
||||
"Stop recording": "Остановить запись",
|
||||
"Transcribing…": "Транскрибация…",
|
||||
"Microphone access denied": "Доступ к микрофону запрещён",
|
||||
"No microphone found": "Микрофон не найден",
|
||||
"Could not start recording": "Не удалось начать запись",
|
||||
"Transcription failed": "Не удалось распознать речь",
|
||||
"Transcribe": "Транскрибировать",
|
||||
"No speech detected": "Речь не распознана",
|
||||
"Voice dictation is not configured": "Голосовой ввод не настроен",
|
||||
"Microphone is unavailable or already in use": "Микрофон недоступен или уже используется",
|
||||
"Audio recording is not available in this browser/context": "Запись аудио недоступна в этом браузере/контексте",
|
||||
"Dictation": "Диктовка",
|
||||
"Dictation becomes available once the page finishes connecting": "Диктовка станет доступна после подключения к документу",
|
||||
"No connection to the collaboration server — dictation unavailable": "Нет связи с сервером совместного редактирования — диктовка недоступна",
|
||||
"This page is read-only": "Страница открыта только для чтения",
|
||||
"Request format": "Формат запроса",
|
||||
"How transcription requests are sent to the endpoint": "Как запросы на транскрибирование отправляются на эндпоинт",
|
||||
"OpenAI-compatible (multipart/form-data)": "Совместимо с OpenAI (multipart/form-data)",
|
||||
"OpenRouter (JSON, base64 audio)": "OpenRouter (JSON, аудио в base64)",
|
||||
"Dictation language": "Язык диктовки",
|
||||
"Auto-detect": "Автоопределение",
|
||||
"Spoken language hint sent to the transcription model. Auto-detect lets the model decide.": "Подсказка языка речи для модели транскрипции. «Автоопределение» оставляет выбор за моделью.",
|
||||
"Agent role": "Роль агента",
|
||||
"Universal assistant": "Универсальный ассистент",
|
||||
"Add role": "Добавить роль",
|
||||
"Edit role": "Изменить роль",
|
||||
"Role name": "Название роли",
|
||||
"e.g. Proofreader": "напр. Корректор",
|
||||
"Optional. Shown as the chat badge.": "Необязательно. Отображается как значок чата.",
|
||||
"Optional. A short note about what this role does.": "Необязательно. Краткое описание того, что делает эта роль.",
|
||||
"Instructions": "Инструкции",
|
||||
"The built-in safety framework is always added automatically.": "Встроенный набор правил безопасности всегда добавляется автоматически.",
|
||||
"Model provider override": "Переопределение провайдера модели",
|
||||
"Optional. Defaults to the workspace provider.": "Необязательно. По умолчанию используется провайдер рабочего пространства.",
|
||||
"Model override": "Переопределение модели",
|
||||
"Optional. Defaults to the workspace model.": "Необязательно. По умолчанию используется модель рабочего пространства.",
|
||||
"e.g. gpt-4o-mini": "напр. gpt-4o-mini",
|
||||
"If you choose a different provider, it must already be configured in AI settings.": "Если вы выбираете другого провайдера, он уже должен быть настроен в настройках ИИ.",
|
||||
"Start automatically": "Запускать автоматически",
|
||||
"When on, picking this role sends a launch message and starts the chat. When off, the role is selected and you type the first message yourself.": "Когда включено, выбор этой роли отправляет стартовое сообщение и начинает чат. Когда выключено, роль выбирается, а первое сообщение вы вводите сами.",
|
||||
"Launch message": "Стартовое сообщение",
|
||||
"Sent automatically when this role is picked. Leave empty to use the default text. Ignored when “Start automatically” is off.": "Отправляется автоматически при выборе этой роли. Оставьте пустым, чтобы использовать текст по умолчанию. Игнорируется, когда «Запускать автоматически» выключено.",
|
||||
"Agent roles": "Роли агента",
|
||||
"Reusable presets that shape the agent's behavior (and optionally its model). Picked when starting a new chat.": "Многоразовые пресеты, определяющие поведение агента (и, при желании, его модель). Выбираются при запуске нового чата.",
|
||||
"No roles configured": "Роли не настроены",
|
||||
"Delete role": "Удалить роль",
|
||||
"Are you sure you want to delete this role?": "Вы уверены, что хотите удалить эту роль?",
|
||||
"HTML embed": "HTML-вставка",
|
||||
"Edit HTML embed": "Изменить HTML-вставку",
|
||||
"HTML embed is disabled in this workspace": "HTML-вставки отключены в этом рабочем пространстве",
|
||||
"Click to add HTML / CSS / JS": "Нажмите, чтобы добавить HTML / CSS / JS",
|
||||
"This HTML/CSS/JS runs in a sandboxed frame and cannot access the viewer's session, cookies, or API.": "Этот HTML/CSS/JS выполняется в изолированном фрейме и не имеет доступа к сессии, cookie или API просматривающего.",
|
||||
"<script>...</script>": "<script>...</script>",
|
||||
"Height (px, blank = auto)": "Высота (px, пусто = авто)",
|
||||
"advanced": "дополнительно",
|
||||
"Enable HTML embed": "Включить HTML-вставки",
|
||||
"Allow members to insert raw HTML/CSS/JavaScript blocks. The block renders in a sandboxed frame and cannot access the viewer's session, cookies, or API. Off by default.": "Разрешить участникам вставлять блоки с необработанным HTML/CSS/JavaScript. Блок отображается в изолированном фрейме и не имеет доступа к сессии, cookie или API просматривающего. По умолчанию выключено.",
|
||||
"When enabled, any member can insert an HTML embed block. The toggle just enables or disables the block type workspace-wide.": "Когда включено, любой участник может вставить блок HTML-вставки. Переключатель просто включает или отключает этот тип блока во всём рабочем пространстве.",
|
||||
"Embeds run inside a sandboxed iframe with a separate origin, so they cannot read or modify the page they are embedded in.": "Вставки выполняются в изолированном iframe с отдельным источником, поэтому они не могут читать или изменять страницу, в которую встроены.",
|
||||
"Turning this off hides existing embeds (they render as a disabled placeholder) and stops serving them on public share pages.": "Отключение этой опции скрывает существующие вставки (они отображаются как отключённая заглушка) и прекращает их показ на публичных страницах.",
|
||||
"Analytics / tracker": "Аналитика / трекер",
|
||||
"Injected verbatim into the <head> of PUBLIC SHARE pages only (same-origin). For analytics snippets (Google Analytics, Yandex.Metrika, etc.). Admin only.": "Вставляется дословно в <head> только ПУБЛИЧНЫХ страниц (тот же источник). Для сниппетов аналитики (Google Analytics, Яндекс.Метрика и т. п.). Только для администраторов.",
|
||||
"Go to login page": "Перейти на страницу входа",
|
||||
"Move to space": "Переместить в пространство",
|
||||
"Float left (wrap text)": "Обтекание слева",
|
||||
"Float right (wrap text)": "Обтекание справа",
|
||||
"Inline (side by side)": "В ряд",
|
||||
@@ -1199,6 +1351,7 @@
|
||||
"Showing {{count}} subpages_one": "Показано {{count}} подстраница",
|
||||
"Showing {{count}} subpages_few": "Показано {{count}} подстраницы",
|
||||
"Showing {{count}} subpages_many": "Показано {{count}} подстраниц",
|
||||
"Showing {{count}} subpages_other": "Показано {{count}} подстраниц",
|
||||
"Protocol": "Протокол",
|
||||
"How chat requests are sent and how reasoning is surfaced": "Как отправляются запросы чата и как показывается reasoning",
|
||||
"OpenAI-compatible (surfaces reasoning)": "OpenAI-совместимый (показывает reasoning)",
|
||||
@@ -1268,7 +1421,6 @@
|
||||
"Retry": "Повторить",
|
||||
"The catalog is empty": "Каталог пуст",
|
||||
"No role bundles are published for this language yet. Try switching the content language.": "Для этого языка ещё не опубликовано ни одного набора ролей. Попробуйте сменить язык контента.",
|
||||
"No roles configured": "Роли не настроены",
|
||||
"Already up to date": "Уже актуальна",
|
||||
"Updated to the latest version": "Обновлено до последней версии",
|
||||
"This role is no longer in the catalog": "Эта роль больше не представлена в каталоге",
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { isChunkLoadError } from "./chunk-load-error-boundary";
|
||||
import { isChunkLoadError, shouldAutoReload } from "./chunk-load-error-boundary";
|
||||
|
||||
// The detector decides whether a caught render error is a stale-deploy chunk-404
|
||||
// (→ auto-reload to fetch the new manifest) vs a genuine app error (→ generic
|
||||
@@ -35,3 +35,31 @@ describe("isChunkLoadError", () => {
|
||||
expect(isChunkLoadError(err)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// The window gate replaces the old one-shot flag: it must permit recovery across
|
||||
// several deploys in one tab (each > window apart) while still stopping an infinite
|
||||
// reload loop when a lazy chunk is permanently broken (a second failure < window).
|
||||
describe("shouldAutoReload", () => {
|
||||
const WINDOW = 5 * 60 * 1000;
|
||||
const NOW = 1_000_000_000_000;
|
||||
|
||||
it("allows a reload when we have never auto-reloaded", () => {
|
||||
expect(shouldAutoReload(NOW, null, WINDOW)).toBe(true);
|
||||
});
|
||||
|
||||
it("allows a reload when the last one was 6 minutes ago (outside the window)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - 6 * 60 * 1000, WINDOW)).toBe(true);
|
||||
});
|
||||
|
||||
it("blocks a reload when the last one was 1 minute ago (inside the window)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - 1 * 60 * 1000, WINDOW)).toBe(false);
|
||||
});
|
||||
|
||||
it("blocks a reload exactly at the window boundary (not strictly older)", () => {
|
||||
expect(shouldAutoReload(NOW, NOW - WINDOW, WINDOW)).toBe(false);
|
||||
});
|
||||
|
||||
it("allows a reload when the stored timestamp is unparseable (NaN)", () => {
|
||||
expect(shouldAutoReload(NOW, NaN, WINDOW)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,7 +2,25 @@ import { ReactNode } from "react";
|
||||
import { ErrorBoundary } from "react-error-boundary";
|
||||
import { Button, Center, Stack, Text } from "@mantine/core";
|
||||
|
||||
const RELOAD_FLAG = "chunk-reload-attempted";
|
||||
// sessionStorage key holding the epoch-ms timestamp of the last automatic reload.
|
||||
const RELOAD_AT_KEY = "chunk-reload-at";
|
||||
// Allow at most one automatic reload per this window. A stale-deploy 404 is cured
|
||||
// by a single reload, so anything inside the window is treated as a reload loop
|
||||
// (permanently-broken chunk) and falls through to the manual UI. A window (rather
|
||||
// than a one-shot flag) lets a SECOND deploy in the same tab's lifetime recover too.
|
||||
const RELOAD_WINDOW_MS = 5 * 60 * 1000;
|
||||
|
||||
// Pure window decision, unit-tested in isolation: auto-reload only if we have never
|
||||
// auto-reloaded (lastReloadAt null/NaN) or the last one was strictly older than the
|
||||
// window. Anything inside the window is suppressed to break an infinite reload loop.
|
||||
export function shouldAutoReload(
|
||||
now: number,
|
||||
lastReloadAt: number | null,
|
||||
windowMs: number,
|
||||
): boolean {
|
||||
if (lastReloadAt === null || Number.isNaN(lastReloadAt)) return true;
|
||||
return now - lastReloadAt > windowMs;
|
||||
}
|
||||
|
||||
// Heuristic detection of a failed dynamic import. Since the code-splitting work,
|
||||
// every route (plus Aside / AiChatWindow) is React.lazy: when a new deploy
|
||||
@@ -24,12 +42,16 @@ export function isChunkLoadError(error: unknown): boolean {
|
||||
function handleError(error: unknown) {
|
||||
if (!isChunkLoadError(error)) return;
|
||||
// A stale-chunk 404 is cured by a full reload that re-fetches index.html and
|
||||
// the new chunk manifest. Auto-reload once, guarding against a reload loop
|
||||
// (e.g. a genuinely missing chunk) with a one-shot sessionStorage flag. If the
|
||||
// flag is already set we fall through to the manual recovery UI below.
|
||||
// the new chunk manifest. Auto-reload at most once per RELOAD_WINDOW_MS: this
|
||||
// recovers across multiple deploys in a single tab's lifetime, yet a
|
||||
// permanently-broken lazy chunk (which would loop) is stopped after the first
|
||||
// reload and falls through to the manual recovery UI below.
|
||||
try {
|
||||
if (sessionStorage.getItem(RELOAD_FLAG)) return;
|
||||
sessionStorage.setItem(RELOAD_FLAG, "1");
|
||||
const raw = sessionStorage.getItem(RELOAD_AT_KEY);
|
||||
const lastReloadAt = raw === null ? null : Number.parseInt(raw, 10);
|
||||
const now = Date.now();
|
||||
if (!shouldAutoReload(now, lastReloadAt, RELOAD_WINDOW_MS)) return;
|
||||
sessionStorage.setItem(RELOAD_AT_KEY, String(now));
|
||||
} catch {
|
||||
// sessionStorage unavailable (private mode / disabled): skip the automatic
|
||||
// reload rather than risk an unguarded loop; the fallback UI still recovers.
|
||||
|
||||
@@ -89,6 +89,23 @@ describe("describeChatError", () => {
|
||||
expect(view.title).not.toBe("AI provider not configured");
|
||||
});
|
||||
|
||||
it("classifies a token-degeneration abort under the SAME 'Response stopped.' marker the live view shows (#495)", () => {
|
||||
// The exact reason the server persists in metadata.error on a degeneration
|
||||
// abort (ai-chat.service OUTPUT_DEGENERATION_ERROR). Live, this event shows
|
||||
// the neutral "Response stopped." notice; the persisted banner MUST match it
|
||||
// so live and refetch never disagree.
|
||||
const view = describeChatError(
|
||||
"Output degeneration detected (repeated token loop)",
|
||||
t,
|
||||
);
|
||||
expect(view.title).toBe("Response stopped.");
|
||||
expect(view.detail).toBe(
|
||||
"The answer was stopped automatically because the model fell into a repeated output loop.",
|
||||
);
|
||||
// Regression guard: it must NOT fall through to the generic heading.
|
||||
expect(view.title).not.toBe("Something went wrong");
|
||||
});
|
||||
|
||||
it("classifies a dropped connection (ECONNRESET) as a lost-connection error", () => {
|
||||
expect(
|
||||
describeChatError("Cannot connect to API: read ECONNRESET", t).title,
|
||||
|
||||
@@ -77,6 +77,22 @@ export function describeChatError(
|
||||
};
|
||||
}
|
||||
|
||||
// Our own token-degeneration abort (#444): the server aborts a runaway
|
||||
// repetition loop and persists this exact reason in metadata.error. LIVE, the
|
||||
// same abort surfaces as the neutral "Response stopped." notice (the client
|
||||
// cannot tell it from a manual Stop mid-stream), so the persisted banner must
|
||||
// read the SAME "Response stopped." marker — otherwise the live view and a
|
||||
// later refetch show two different texts for one event. The detail explains the
|
||||
// loop-guard cause without contradicting the shared heading.
|
||||
if (/output degeneration detected|repeated token loop/i.test(msg)) {
|
||||
return {
|
||||
title: t("Response stopped."),
|
||||
detail: t(
|
||||
"The answer was stopped automatically because the model fell into a repeated output loop.",
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
if (/"statusCode"\s*:\s*403\b/.test(msg)) {
|
||||
return {
|
||||
title: t("AI chat is disabled"),
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import * as Y from "yjs";
|
||||
import { yHistoryAvailability } from "./use-toolbar-state.ts";
|
||||
|
||||
// Undo/redo availability is derived from the Yjs UndoManager's PRIVATE
|
||||
// `undoStack` / `redoStack` fields (see use-toolbar-state.ts for why we read the
|
||||
// stack lengths directly instead of the expensive `editor.can().undo()` dry-run).
|
||||
// These tests lock in the behavior AND pin the library shape so a yjs / y-undo
|
||||
// upgrade that renames/restructures those internals fails loudly here rather than
|
||||
// silently enabling/disabling the toolbar buttons in production.
|
||||
describe("yHistoryAvailability", () => {
|
||||
it("reports availability from the stack lengths", () => {
|
||||
expect(yHistoryAvailability({ undoStack: [], redoStack: [] })).toEqual({
|
||||
canUndo: false,
|
||||
canRedo: false,
|
||||
});
|
||||
expect(
|
||||
yHistoryAvailability({ undoStack: [{}], redoStack: [] }),
|
||||
).toEqual({ canUndo: true, canRedo: false });
|
||||
expect(
|
||||
yHistoryAvailability({ undoStack: [{}], redoStack: [{}, {}] }),
|
||||
).toEqual({ canUndo: true, canRedo: true });
|
||||
});
|
||||
|
||||
it("returns null when the private stack shape is unrecognized (upgrade guard)", () => {
|
||||
// Simulates a yjs / y-undo upgrade that renames or restructures the private
|
||||
// fields: the caller then falls back to the safe prosemirror-history default
|
||||
// instead of throwing on `.length` of undefined or reading garbage.
|
||||
expect(yHistoryAvailability(undefined)).toBeNull();
|
||||
expect(yHistoryAvailability(null)).toBeNull();
|
||||
expect(yHistoryAvailability({})).toBeNull();
|
||||
expect(yHistoryAvailability({ undoStack: 5, redoStack: 5 })).toBeNull();
|
||||
// Only one stack present (partial rename) is still not trusted.
|
||||
expect(yHistoryAvailability({ undoStack: [] })).toBeNull();
|
||||
});
|
||||
|
||||
it("pin-test: a real yjs UndoManager still exposes undoStack/redoStack arrays", () => {
|
||||
const doc = new Y.Doc();
|
||||
const text = doc.getText("prosemirror");
|
||||
const undoManager = new Y.UndoManager(text);
|
||||
|
||||
// Fresh manager: both stacks empty -> nothing to undo/redo.
|
||||
expect(yHistoryAvailability(undoManager)).toEqual({
|
||||
canUndo: false,
|
||||
canRedo: false,
|
||||
});
|
||||
|
||||
// A tracked edit must push onto the private undoStack. If a future yjs
|
||||
// renames these fields, yHistoryAvailability(undoManager) returns null and
|
||||
// the expectation below fails loudly.
|
||||
text.insert(0, "hello");
|
||||
undoManager.stopCapturing();
|
||||
expect(yHistoryAvailability(undoManager)).toEqual({
|
||||
canUndo: true,
|
||||
canRedo: false,
|
||||
});
|
||||
|
||||
// Undoing moves the item to the redoStack -> redo becomes available.
|
||||
undoManager.undo();
|
||||
expect(yHistoryAvailability(undoManager)).toEqual({
|
||||
canUndo: false,
|
||||
canRedo: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -35,6 +35,30 @@ export interface ToolbarState {
|
||||
// When neither history backend is installed (the pre-sync static editor —
|
||||
// mainExtensions only, undoRedo disabled), both fall through to 0 -> false,
|
||||
// matching the previous `safeCan` behavior.
|
||||
// Reads the Yjs UndoManager's undo/redo availability from its stack lengths.
|
||||
//
|
||||
// `undoStack` / `redoStack` are PRIVATE y-undo / yjs internals, so we touch them
|
||||
// defensively: a yjs or y-undo upgrade that renames or restructures these fields
|
||||
// must not silently mis-drive the toolbar buttons (nor throw on `.length` of
|
||||
// `undefined`). We only trust them when they are actually arrays; otherwise this
|
||||
// returns null and the caller falls back to a safe default. The pin-test in
|
||||
// use-toolbar-state.test.ts asserts the current library shape, so an upgrade that
|
||||
// breaks this contract fails loudly there instead of failing silently in the UI.
|
||||
export function yHistoryAvailability(
|
||||
undoManager: unknown,
|
||||
): { canUndo: boolean; canRedo: boolean } | null {
|
||||
if (!undoManager || typeof undoManager !== "object") return null;
|
||||
const { undoStack, redoStack } = undoManager as {
|
||||
undoStack?: unknown;
|
||||
redoStack?: unknown;
|
||||
};
|
||||
if (!Array.isArray(undoStack) || !Array.isArray(redoStack)) return null;
|
||||
return {
|
||||
canUndo: undoStack.length > 0,
|
||||
canRedo: redoStack.length > 0,
|
||||
};
|
||||
}
|
||||
|
||||
function historyAvailability(editor: Editor): {
|
||||
canUndo: boolean;
|
||||
canRedo: boolean;
|
||||
@@ -43,16 +67,14 @@ function historyAvailability(editor: Editor): {
|
||||
|
||||
// Collaboration history (Yjs) takes precedence when present.
|
||||
const yState = yUndoPluginKey.getState(state) as
|
||||
| { undoManager?: { undoStack: unknown[]; redoStack: unknown[] } }
|
||||
| { undoManager?: unknown }
|
||||
| undefined;
|
||||
if (yState?.undoManager) {
|
||||
return {
|
||||
canUndo: yState.undoManager.undoStack.length > 0,
|
||||
canRedo: yState.undoManager.redoStack.length > 0,
|
||||
};
|
||||
}
|
||||
const yAvail = yHistoryAvailability(yState?.undoManager);
|
||||
if (yAvail) return yAvail;
|
||||
|
||||
// Plain prosemirror-history (returns 0 when the history plugin is absent).
|
||||
// This is also the safe default when a Yjs UndoManager is present but its
|
||||
// private stack shape is no longer recognized (yHistoryAvailability -> null).
|
||||
return {
|
||||
canUndo: undoDepth(state) > 0,
|
||||
canRedo: redoDepth(state) > 0,
|
||||
|
||||
@@ -13,8 +13,7 @@ let currentAlias: IShareAlias | null = null;
|
||||
let availabilityResult: {
|
||||
valid: boolean;
|
||||
available: boolean;
|
||||
currentPageId: string | null;
|
||||
} = { valid: true, available: true, currentPageId: null };
|
||||
} = { valid: true, available: true };
|
||||
|
||||
vi.mock("@/features/share/queries/share-query.ts", () => ({
|
||||
useShareAliasForPageQuery: () => ({ data: currentAlias }),
|
||||
@@ -56,7 +55,7 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () =>
|
||||
beforeEach(() => {
|
||||
setMutateAsync.mockReset();
|
||||
currentAlias = null;
|
||||
availabilityResult = { valid: true, available: true, currentPageId: null };
|
||||
availabilityResult = { valid: true, available: true };
|
||||
});
|
||||
|
||||
it("shows a 'will move it here' HINT (not a terminal error) when the name belongs to another page, and keeps Save enabled", async () => {
|
||||
@@ -65,7 +64,6 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () =>
|
||||
availabilityResult = {
|
||||
valid: true,
|
||||
available: false,
|
||||
currentPageId: "page-X",
|
||||
};
|
||||
|
||||
renderSection("page-Y");
|
||||
@@ -97,7 +95,6 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () =>
|
||||
availabilityResult = {
|
||||
valid: true,
|
||||
available: false,
|
||||
currentPageId: "page-X",
|
||||
};
|
||||
// The server rejects the un-confirmed save asking the client to confirm.
|
||||
setMutateAsync.mockRejectedValueOnce({
|
||||
@@ -106,7 +103,6 @@ describe("ShareAliasSection — taken-name handling is never a dead end", () =>
|
||||
status: 409,
|
||||
data: {
|
||||
code: "ALIAS_REASSIGN_REQUIRED",
|
||||
currentPageId: "page-X",
|
||||
currentPageTitle: "Alias Test Page X",
|
||||
},
|
||||
},
|
||||
|
||||
@@ -48,7 +48,6 @@ export default function ShareAliasSection({
|
||||
const [availability, setAvailability] = useState<{
|
||||
valid: boolean;
|
||||
available: boolean;
|
||||
currentPageId: string | null;
|
||||
} | null>(null);
|
||||
const [reassign, setReassign] = useState<{
|
||||
alias: string;
|
||||
@@ -76,7 +75,6 @@ export default function ShareAliasSection({
|
||||
setAvailability({
|
||||
valid: res.valid,
|
||||
available: res.available,
|
||||
currentPageId: res.currentPageId,
|
||||
});
|
||||
} catch {
|
||||
setAvailability(null);
|
||||
|
||||
@@ -108,7 +108,6 @@ export interface IShareAliasAvailability {
|
||||
alias: string;
|
||||
valid: boolean;
|
||||
available: boolean;
|
||||
currentPageId: string | null;
|
||||
}
|
||||
|
||||
export interface ISharedPageTree {
|
||||
|
||||
+124
@@ -6,6 +6,8 @@ import {
|
||||
nextReindexPollInterval,
|
||||
isReindexComplete,
|
||||
isReindexButtonLoading,
|
||||
reindexRunKey,
|
||||
isNewReindexRun,
|
||||
} from './ai-provider-settings';
|
||||
|
||||
describe('resolveCardStatus', () => {
|
||||
@@ -221,6 +223,128 @@ describe('isReindexComplete', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('reindexRunKey', () => {
|
||||
it('is null when the status carries no run identity', () => {
|
||||
expect(reindexRunKey(undefined)).toBeNull();
|
||||
expect(
|
||||
reindexRunKey({ reindexing: false, indexedPages: 5, totalPages: 5 }),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('is null for a legacy/degraded record with an empty runId', () => {
|
||||
// The server sends runId='' for a record written before the field existed;
|
||||
// the client must treat that as "no identity" (fall back to prior behaviour).
|
||||
expect(
|
||||
reindexRunKey({
|
||||
reindexing: true,
|
||||
indexedPages: 0,
|
||||
totalPages: 10,
|
||||
runId: '',
|
||||
reindexStartedAt: 1000,
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('folds runId and startedAt into one stable key', () => {
|
||||
expect(
|
||||
reindexRunKey({
|
||||
reindexing: true,
|
||||
indexedPages: 0,
|
||||
totalPages: 10,
|
||||
runId: 'run-a',
|
||||
reindexStartedAt: 1000,
|
||||
}),
|
||||
).toBe('run-a:1000');
|
||||
});
|
||||
|
||||
it('changes when the runId changes for the same startedAt', () => {
|
||||
const a = reindexRunKey({
|
||||
reindexing: true,
|
||||
indexedPages: 0,
|
||||
totalPages: 10,
|
||||
runId: 'run-a',
|
||||
reindexStartedAt: 1000,
|
||||
});
|
||||
const b = reindexRunKey({
|
||||
reindexing: true,
|
||||
indexedPages: 0,
|
||||
totalPages: 10,
|
||||
runId: 'run-b',
|
||||
reindexStartedAt: 1000,
|
||||
});
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
|
||||
it('changes when the same runId restarts at a new startedAt', () => {
|
||||
const a = reindexRunKey({
|
||||
reindexing: true,
|
||||
indexedPages: 0,
|
||||
totalPages: 10,
|
||||
runId: 'run-a',
|
||||
reindexStartedAt: 1000,
|
||||
});
|
||||
const b = reindexRunKey({
|
||||
reindexing: true,
|
||||
indexedPages: 0,
|
||||
totalPages: 10,
|
||||
runId: 'run-a',
|
||||
reindexStartedAt: 2000,
|
||||
});
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isNewReindexRun (poll keying on runId)', () => {
|
||||
// Derive the status shape from the helper itself so the test needs no export
|
||||
// of the component-internal ReindexStatus type.
|
||||
type ReindexStatusLike = NonNullable<Parameters<typeof reindexRunKey>[0]>;
|
||||
const run = (runId: string, startedAt: number): ReindexStatusLike => ({
|
||||
reindexing: true,
|
||||
indexedPages: 0,
|
||||
totalPages: 10,
|
||||
runId,
|
||||
reindexStartedAt: startedAt,
|
||||
});
|
||||
|
||||
it('first identity after none latched is a NEW run', () => {
|
||||
expect(isNewReindexRun(null, run('run-a', 1000))).toBe(true);
|
||||
});
|
||||
|
||||
it('the SAME identity is not a new run (same run being watched)', () => {
|
||||
const key = reindexRunKey(run('run-a', 1000));
|
||||
expect(isNewReindexRun(key, run('run-a', 1000))).toBe(false);
|
||||
});
|
||||
|
||||
it('a DIFFERENT runId is a new run (reset per-run poll state)', () => {
|
||||
const key = reindexRunKey(run('run-a', 1000));
|
||||
expect(isNewReindexRun(key, run('run-b', 1000))).toBe(true);
|
||||
});
|
||||
|
||||
it('an identity-less poll (no runId / cleared record) is never a new run', () => {
|
||||
const key = reindexRunKey(run('run-a', 1000));
|
||||
expect(
|
||||
isNewReindexRun(key, {
|
||||
reindexing: false,
|
||||
indexedPages: 10,
|
||||
totalPages: 10,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('a legacy empty-runId poll does not spuriously reset a latched run', () => {
|
||||
const key = reindexRunKey(run('run-a', 1000));
|
||||
expect(
|
||||
isNewReindexRun(key, {
|
||||
reindexing: true,
|
||||
indexedPages: 3,
|
||||
totalPages: 10,
|
||||
runId: '',
|
||||
reindexStartedAt: 1000,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isReindexButtonLoading', () => {
|
||||
it('loads while the POST mutation is pending', () => {
|
||||
expect(
|
||||
|
||||
+54
-1
@@ -173,9 +173,43 @@ export function resolveKeyField(
|
||||
// Subset of the status payload that drives the reindex poll decisions.
|
||||
type ReindexStatus = Pick<
|
||||
IAiSettings,
|
||||
"reindexing" | "indexedPages" | "totalPages"
|
||||
"reindexing" | "indexedPages" | "totalPages" | "runId" | "reindexStartedAt"
|
||||
>;
|
||||
|
||||
/**
|
||||
* A stable per-RUN key for the reindex poll: `runId:startedAt`, or `null` when
|
||||
* the status carries no run identity (no active run, or a legacy/degraded
|
||||
* server record with an empty runId). Two polls of the SAME run share a key; a
|
||||
* new run mints a fresh runId and so a different key.
|
||||
*
|
||||
* This is the single place the client turns the server's run identity into the
|
||||
* value it keys on — it removes the "is this the same run I've been watching or
|
||||
* a brand-new one?" ambiguity that made a class of reindex-status bugs (a stale
|
||||
* pre-reindex snapshot vs a fresh run) get fixed twice (#262). `startedAt` is
|
||||
* folded in so a run that somehow reuses a runId but restarted is still new.
|
||||
*/
|
||||
export function reindexRunKey(status: ReindexStatus | undefined): string | null {
|
||||
const runId = status?.runId;
|
||||
if (!runId) return null;
|
||||
return `${runId}:${status?.reindexStartedAt ?? ""}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide whether the latest poll represents a NEW reindex run relative to the
|
||||
* run key the client last latched (`prevKey`, `null` if none yet). True only
|
||||
* when the status carries an identity AND it differs from the latched one — the
|
||||
* signal to reset any per-run poll state (the "seen active" latch / progress the
|
||||
* UI held). The same identity (or no identity) is NOT a new run, so an unchanged
|
||||
* or identity-less poll never resets mid-run.
|
||||
*/
|
||||
export function isNewReindexRun(
|
||||
prevKey: string | null,
|
||||
status: ReindexStatus | undefined,
|
||||
): boolean {
|
||||
const key = reindexRunKey(status);
|
||||
return key !== null && key !== prevKey;
|
||||
}
|
||||
|
||||
/**
|
||||
* Decide the TanStack Query `refetchInterval` while a reindex may be running.
|
||||
* Returns the poll interval (ms) to keep polling, or `false` to stop.
|
||||
@@ -320,6 +354,13 @@ export default function AiProviderSettings() {
|
||||
// counter at 0 until a manual reload. A ref (not state) because it must not
|
||||
// trigger a render and is only ever read where `reindexing` is already false.
|
||||
const reindexSeenActiveRef = useRef(false);
|
||||
// The run identity (runId:startedAt) the current poll window is keyed on. When
|
||||
// a poll reports a DIFFERENT runId the server has started a NEW run, so we
|
||||
// re-latch to it and reset `reindexSeenActiveRef` — a fresh run must never
|
||||
// inherit the previous run's "seen active"/completion state (which would stop
|
||||
// polling immediately or read the old run's counters as this run's). null =
|
||||
// no run keyed yet (steady state, or a legacy record without a runId).
|
||||
const reindexRunKeyRef = useRef<string | null>(null);
|
||||
|
||||
// Only admins may read the (masked) AI settings; the server enforces this too.
|
||||
const { data: settings, isLoading } = useAiSettingsQuery(isAdmin, (query) =>
|
||||
@@ -336,6 +377,14 @@ export default function AiProviderSettings() {
|
||||
// unmount because the deadline state goes away with the component.
|
||||
useEffect(() => {
|
||||
if (reindexDeadline === null) return;
|
||||
// Key the poll on the run identity: if this poll carries a runId different
|
||||
// from the one we latched, the server started a NEW run, so adopt it and
|
||||
// drop the per-run "seen active" latch (a fresh run must not inherit the
|
||||
// previous run's completion state). Same runId => same run, leave it alone.
|
||||
if (isNewReindexRun(reindexRunKeyRef.current, settings)) {
|
||||
reindexRunKeyRef.current = reindexRunKey(settings);
|
||||
reindexSeenActiveRef.current = false;
|
||||
}
|
||||
// Latch "we have seen the active run" the moment a poll reports it, so the
|
||||
// completion check below (and the refetchInterval's) only fires once the run
|
||||
// has genuinely started — never on the stale pre-reindex snapshot.
|
||||
@@ -1220,6 +1269,10 @@ export default function AiProviderSettings() {
|
||||
// immediately.
|
||||
onSuccess: () => {
|
||||
reindexSeenActiveRef.current = false;
|
||||
// Forget the previous run's identity so the first poll of
|
||||
// this window (carrying the new run's runId) is recognized
|
||||
// as a new run and keyed afresh.
|
||||
reindexRunKeyRef.current = null;
|
||||
setReindexDeadline(Date.now() + REINDEX_POLL_CAP_MS);
|
||||
},
|
||||
})
|
||||
|
||||
@@ -51,6 +51,14 @@ export interface IAiSettings {
|
||||
// True while a full workspace reindex is actively running; the counts above
|
||||
// then reflect the live run progress (done climbs 0 -> total).
|
||||
reindexing?: boolean;
|
||||
// Identity of the ACTIVE reindex run (present only while `reindexing`). The
|
||||
// poll keys on `runId`: a changed value means a NEW run (reset the per-run
|
||||
// poll state the UI latched), the same value is the run already being watched.
|
||||
// Absent/empty ('') => no identity available; the client keeps prior behaviour.
|
||||
runId?: string;
|
||||
// Epoch-ms the active run started; paired with `runId` so a restart with a
|
||||
// recycled id is still detected as a new run.
|
||||
reindexStartedAt?: number;
|
||||
}
|
||||
|
||||
// Update payload. Key semantics (same for `apiKey` and `embeddingApiKey`):
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { templateRoute } from "./route-template";
|
||||
import { templateRoute, KNOWN_ROUTE_TEMPLATES } from "./route-template";
|
||||
|
||||
describe("templateRoute", () => {
|
||||
it("templates a space page path (never leaks slugs)", () => {
|
||||
@@ -32,4 +32,30 @@ describe("templateRoute", () => {
|
||||
expect(templateRoute("/weird/unknown/thing")).toBe("other");
|
||||
expect(templateRoute("/s/team/p/slug/extra/segments")).toBe("other");
|
||||
});
|
||||
|
||||
// The server's /api/telemetry/vitals mirror (ALLOWED_ROUTE_TEMPLATES) drops any
|
||||
// route outside KNOWN_ROUTE_TEMPLATES, so templateRoute must NEVER emit a label
|
||||
// that is not in that dictionary — otherwise legit client metrics get dropped.
|
||||
it("only ever emits labels contained in KNOWN_ROUTE_TEMPLATES (#495)", () => {
|
||||
const samples = [
|
||||
"/",
|
||||
"/home",
|
||||
"/settings/members",
|
||||
"/settings/groups/g-1",
|
||||
"/s/team",
|
||||
"/s/team/trash",
|
||||
"/s/team/p/slug",
|
||||
"/p/slug",
|
||||
"/share/abc",
|
||||
"/share/abc/p/slug",
|
||||
"/share/p/slug",
|
||||
"/labels/urgent",
|
||||
"/invites/inv-1",
|
||||
"/weird/unknown/thing", // -> "other"
|
||||
"/deep/unmatched/x/y/z", // -> "other"
|
||||
];
|
||||
for (const path of samples) {
|
||||
expect(KNOWN_ROUTE_TEMPLATES.has(templateRoute(path))).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -44,6 +44,22 @@ const STATIC_ROUTES = new Set<string>([
|
||||
'/settings/sharing',
|
||||
]);
|
||||
|
||||
/**
|
||||
* The COMPLETE, finite vocabulary `templateRoute` can ever emit: the two
|
||||
* synthetic labels (`/` and `other`), the static routes, and the dynamic
|
||||
* templates. Exported so the public `/api/telemetry/vitals` endpoint can reject
|
||||
* any `route` outside this dictionary server-side (the endpoint is anonymous, so
|
||||
* an un-checked `route` is a free-text write surface). The server keeps a mirror
|
||||
* (`ALLOWED_ROUTE_TEMPLATES` in client-metrics.constants.ts) — this is the
|
||||
* canonical source; keep them in lockstep.
|
||||
*/
|
||||
export const KNOWN_ROUTE_TEMPLATES: ReadonlySet<string> = new Set<string>([
|
||||
'/',
|
||||
'other',
|
||||
...STATIC_ROUTES,
|
||||
...ROUTE_PATTERNS.map((p) => p.template),
|
||||
]);
|
||||
|
||||
export function templateRoute(pathname: string): string {
|
||||
// Normalise a trailing slash (except root).
|
||||
const path =
|
||||
|
||||
@@ -117,9 +117,14 @@ const FINAL_STEP_NUDGE =
|
||||
// NO text at all (#444, mitigates the "empty turn" the lockdown used to prevent
|
||||
// when the toggle is OFF). Makes the exhausted-without-answer state explicit to
|
||||
// the user and, on replay, to the model on the next turn.
|
||||
// The persisted content is the app's base locale (en-US) — which is ALSO the
|
||||
// i18n key the client localizes through `t()` — instead of a hardcoded Russian
|
||||
// string (it used to render Russian for every locale, and fed Russian back to
|
||||
// the model on replay). Keep it a plain, model-readable English sentence so the
|
||||
// next turn's replay reads cleanly; the client resolves the locale.
|
||||
const STEP_LIMIT_NO_ANSWER_MARKER =
|
||||
'(Достигнут лимит шагов — итоговый ответ не сформулирован; работа могла ' +
|
||||
'остаться незавершённой. Напишите «продолжай», чтобы агент продолжил.)';
|
||||
'(Step limit reached — no final answer was produced; the work may be ' +
|
||||
'unfinished. Reply "continue" to let the agent carry on.)';
|
||||
|
||||
// Reason recorded in ai_chat_runs.error / the assistant row when the token-
|
||||
// degeneration detector (#444) aborts a run. Distinct from a user Stop (no error)
|
||||
@@ -1514,6 +1519,13 @@ export class AiChatService implements OnModuleInit, OnModuleDestroy {
|
||||
system,
|
||||
messages,
|
||||
tools,
|
||||
// Pin the AI SDK per-request retry budget explicitly instead of relying
|
||||
// on its default (which is also 2). Connection arithmetic per turn:
|
||||
// (1 + maxRetries=2) × (1 + AI_STREAM_PRE_RESPONSE_RETRIES) network
|
||||
// connects worst-case — the two retry layers compose, so making the SDK
|
||||
// side explicit keeps that ceiling visible and pinned against SDK-default
|
||||
// drift.
|
||||
maxRetries: 2,
|
||||
// No maxOutputTokens cap on the agent: tool-call arguments (e.g. a full
|
||||
// page body for the write tools) are emitted as OUTPUT tokens, so a fixed
|
||||
// cap would truncate complex tool calls mid-argument. Let the model use its
|
||||
|
||||
@@ -9,9 +9,73 @@ import {
|
||||
DEGENERATION_CHECK_STEP,
|
||||
REPEATED_LINES_THRESHOLD,
|
||||
MIN_PERIOD_REPEATS,
|
||||
degenerationThresholds,
|
||||
} from './output-degeneration';
|
||||
import { AiChatService } from './ai-chat.service';
|
||||
|
||||
// Part A (#495 iter10): the detector thresholds are env-tunable. These drive the
|
||||
// resolver against real repeat-count shapes and mutation-verify that the env
|
||||
// override actually changes the trigger point (not a vacuous read).
|
||||
describe('degeneration thresholds are env-configurable', () => {
|
||||
const VARS = [
|
||||
'AI_CHAT_DEGENERATION_REPEATED_LINES',
|
||||
'AI_CHAT_DEGENERATION_PERIOD_MAX_LEN',
|
||||
'AI_CHAT_DEGENERATION_PERIOD_MIN_REPEATS',
|
||||
'AI_CHAT_DEGENERATION_CHECK_STEP',
|
||||
];
|
||||
const saved: Record<string, string | undefined> = {};
|
||||
beforeEach(() => {
|
||||
for (const v of VARS) saved[v] = process.env[v];
|
||||
});
|
||||
afterEach(() => {
|
||||
for (const v of VARS) {
|
||||
if (saved[v] === undefined) delete process.env[v];
|
||||
else process.env[v] = saved[v];
|
||||
}
|
||||
});
|
||||
|
||||
it('defaults to the compiled constants when unset', () => {
|
||||
for (const v of VARS) delete process.env[v];
|
||||
expect(degenerationThresholds()).toEqual({
|
||||
repeatedLines: REPEATED_LINES_THRESHOLD,
|
||||
maxPeriodLen: 150,
|
||||
minPeriodRepeats: MIN_PERIOD_REPEATS,
|
||||
checkStep: DEGENERATION_CHECK_STEP,
|
||||
});
|
||||
});
|
||||
|
||||
it('falls back to the default on blank / invalid / non-positive values', () => {
|
||||
for (const bad of ['', ' ', 'abc', '0', '-3', '1.5']) {
|
||||
process.env.AI_CHAT_DEGENERATION_REPEATED_LINES = bad;
|
||||
// '1.5' floors to 1 (still ≥1, valid); every other bad value → default.
|
||||
const expected = bad === '1.5' ? 1 : REPEATED_LINES_THRESHOLD;
|
||||
expect(degenerationThresholds().repeatedLines).toBe(expected);
|
||||
}
|
||||
});
|
||||
|
||||
it('a RAISED check-step suppresses a burst the default would have flagged', () => {
|
||||
// A ~3.3KB periodic burst is periodic-degenerate, but shouldCheckDegeneration
|
||||
// is the throttle gate. Default checkStep=2000 arms on it; raising the step
|
||||
// above the burst size means the throttle never re-fires for it.
|
||||
const burstLen = 'loadTools.\n'.repeat(300).length; // ~3300
|
||||
delete process.env.AI_CHAT_DEGENERATION_CHECK_STEP;
|
||||
expect(shouldCheckDegeneration(burstLen, 0)).toBe(true); // default 2000
|
||||
process.env.AI_CHAT_DEGENERATION_CHECK_STEP = String(burstLen + 1);
|
||||
expect(shouldCheckDegeneration(burstLen, 0)).toBe(false); // raised gate
|
||||
});
|
||||
|
||||
it('a LOWERED repeated-lines threshold trips on a shorter identical-line run', () => {
|
||||
// 8 identical lines: below the default 25 (rule 1) and below the periodic
|
||||
// rule's 20 repeats — so isDegenerateOutput is false by default.
|
||||
const shortRun = 'x\n'.repeat(8);
|
||||
delete process.env.AI_CHAT_DEGENERATION_REPEATED_LINES;
|
||||
expect(isDegenerateOutput(shortRun)).toBe(false);
|
||||
// Lower rule 1 to 5 → the 8-line run now trips.
|
||||
process.env.AI_CHAT_DEGENERATION_REPEATED_LINES = '5';
|
||||
expect(isDegenerateOutput(shortRun)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// Mock ONLY streamText so we can capture the onChunk/onStepFinish callbacks the
|
||||
// service registers and drive them by hand; every other `ai` export the service
|
||||
// uses (convertToModelMessages, stepCountIs, …) stays real.
|
||||
|
||||
@@ -23,6 +23,54 @@ export const MAX_PERIOD_LEN = 150;
|
||||
/** Rule 2: minimum number of consecutive block repeats to trigger. */
|
||||
export const MIN_PERIOD_REPEATS = 20;
|
||||
|
||||
/**
|
||||
* Read a positive-integer threshold from an env var, falling back to `fallback`
|
||||
* on unset/blank/invalid/non-positive. Mirrors the `AI_STREAM_PRE_RESPONSE_RETRIES`
|
||||
* resolver in `ai-streaming-fetch.ts`: read the RAW string first so a blank value
|
||||
* is treated as "unset" (→ fallback) rather than coercing to 0. Thresholds must
|
||||
* stay ≥ 1 — a 0/negative would make the detector fire on any text (or never), so
|
||||
* a bad value degrades to the safe compiled default instead. Env-tunable so an
|
||||
* operator can retune the anti-babble guard (#444) without a redeploy, following
|
||||
* the `AI_CHAT_FINAL_STEP_LOCKDOWN` toggle convention.
|
||||
*/
|
||||
function envThreshold(name: string, fallback: number): number {
|
||||
const rawStr = process.env[name];
|
||||
if (rawStr === undefined || rawStr.trim() === '') return fallback;
|
||||
const raw = Number(rawStr);
|
||||
return Number.isFinite(raw) && raw >= 1 ? Math.floor(raw) : fallback;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the degeneration-detector thresholds from the environment, each
|
||||
* defaulting to the compiled constant above. Read fresh per call (not cached at
|
||||
* import) so a test — or a runtime env change — takes effect deterministically.
|
||||
*/
|
||||
export function degenerationThresholds(): {
|
||||
repeatedLines: number;
|
||||
maxPeriodLen: number;
|
||||
minPeriodRepeats: number;
|
||||
checkStep: number;
|
||||
} {
|
||||
return {
|
||||
repeatedLines: envThreshold(
|
||||
'AI_CHAT_DEGENERATION_REPEATED_LINES',
|
||||
REPEATED_LINES_THRESHOLD,
|
||||
),
|
||||
maxPeriodLen: envThreshold(
|
||||
'AI_CHAT_DEGENERATION_PERIOD_MAX_LEN',
|
||||
MAX_PERIOD_LEN,
|
||||
),
|
||||
minPeriodRepeats: envThreshold(
|
||||
'AI_CHAT_DEGENERATION_PERIOD_MIN_REPEATS',
|
||||
MIN_PERIOD_REPEATS,
|
||||
),
|
||||
checkStep: envThreshold(
|
||||
'AI_CHAT_DEGENERATION_CHECK_STEP',
|
||||
DEGENERATION_CHECK_STEP,
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Rule 1 — ≥`REPEATED_LINES_THRESHOLD` consecutive IDENTICAL non-empty lines at
|
||||
* the tail. Catches the classic newline-delimited loop ("loadTools.\n" ×N).
|
||||
@@ -128,7 +176,11 @@ export function hasPeriodicTail(
|
||||
* Pure — the caller owns the abort side effect.
|
||||
*/
|
||||
export function isDegenerateOutput(text: string): boolean {
|
||||
return hasRepeatedLineRun(text) || hasPeriodicTail(text);
|
||||
const cfg = degenerationThresholds();
|
||||
return (
|
||||
hasRepeatedLineRun(text, cfg.repeatedLines) ||
|
||||
hasPeriodicTail(text, cfg.maxPeriodLen, cfg.minPeriodRepeats)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -154,7 +206,7 @@ export function shouldCheckDegeneration(
|
||||
textLen: number,
|
||||
lastCheckLen: number,
|
||||
): boolean {
|
||||
return textLen - lastCheckLen >= DEGENERATION_CHECK_STEP;
|
||||
return textLen - lastCheckLen >= degenerationThresholds().checkStep;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -307,6 +307,10 @@ export class PublicShareChatService {
|
||||
system,
|
||||
messages: modelMessages,
|
||||
tools,
|
||||
// Pin the AI SDK per-request retry budget explicitly (matches the SDK
|
||||
// default of 2). Connection arithmetic: (1 + maxRetries) × (1 +
|
||||
// AI_STREAM_PRE_RESPONSE_RETRIES) worst-case connects per turn.
|
||||
maxRetries: 2,
|
||||
// Bound the agent loop for anonymous callers.
|
||||
stopWhen: stepCountIs(5),
|
||||
// Cap per-request output so one anonymous call cannot run up the provider
|
||||
|
||||
@@ -17,9 +17,10 @@ import { MovePageDto } from './move-page.dto';
|
||||
// a valid ordering key the server itself generated would be refused on move.
|
||||
//
|
||||
// The tests below assert the CORRECT contract: any key the generator can produce
|
||||
// must satisfy the DTO. The genuinely-failing case is marked `test.failing` so the
|
||||
// suite stays green while locking the bug; it flips red (alerting us) once the DTO
|
||||
// bounds are widened to cover the generator's real range.
|
||||
// must satisfy the DTO. FIXED (#495 item 9): the DTO now validates `position` by
|
||||
// CHARSET ([0-9A-Za-z], the generator's base-62 alphabet) instead of the wrong
|
||||
// @MaxLength(12) length bound, so dense between-inserts are accepted; the former
|
||||
// `test.failing` bug-lock is now a passing assertion.
|
||||
|
||||
function constraintErrors(position: unknown) {
|
||||
const dto = plainToInstance(MovePageDto, {
|
||||
@@ -47,24 +48,33 @@ describe('MovePageDto.position vs generateJitteredKeyBetween parity', () => {
|
||||
expect(hasError(errors, 'position')).toBe(false);
|
||||
});
|
||||
|
||||
// BUG LOCK: dense between-inserts produce keys longer than 12 chars, which
|
||||
// MaxLength(12) rejects even though they are valid ordering keys. This SHOULD
|
||||
// pass; it currently fails. Flips green when the DTO bound is fixed.
|
||||
test.failing(
|
||||
'accepts dense between-inserted keys (currently rejected by MaxLength(12))',
|
||||
async () => {
|
||||
let lo = generateJitteredKeyBetween(null, null);
|
||||
let hi = generateJitteredKeyBetween(lo, null);
|
||||
// Repeatedly insert just above `lo`, shrinking the gap so the key grows.
|
||||
let longest = lo;
|
||||
for (let i = 0; i < 40; i++) {
|
||||
const mid = generateJitteredKeyBetween(lo, hi);
|
||||
if (mid.length > longest.length) longest = mid;
|
||||
hi = mid;
|
||||
}
|
||||
expect(longest.length).toBeGreaterThan(12); // sanity: we produced a long key
|
||||
const errors = await constraintErrors(longest);
|
||||
expect(hasError(errors, 'position')).toBe(false);
|
||||
},
|
||||
);
|
||||
// FIXED: dense between-inserts produce keys longer than 12 chars, which the old
|
||||
// MaxLength(12) rejected even though they are valid ordering keys. Now accepted.
|
||||
it('accepts dense between-inserted keys longer than 12 chars', async () => {
|
||||
let lo = generateJitteredKeyBetween(null, null);
|
||||
let hi = generateJitteredKeyBetween(lo, null);
|
||||
// Repeatedly insert just above `lo`, shrinking the gap so the key grows. The
|
||||
// generator is JITTERED (random), so use enough iterations that the longest
|
||||
// key reliably clears the old 12-char bound: measured min-over-50-trials is
|
||||
// ~11 at 40 iterations (flaky) but ~36 at 200 (robust margin).
|
||||
let longest = lo;
|
||||
for (let i = 0; i < 200; i++) {
|
||||
const mid = generateJitteredKeyBetween(lo, hi);
|
||||
if (mid.length > longest.length) longest = mid;
|
||||
hi = mid;
|
||||
}
|
||||
expect(longest.length).toBeGreaterThan(12); // sanity: we produced a long key
|
||||
const errors = await constraintErrors(longest);
|
||||
expect(hasError(errors, 'position')).toBe(false);
|
||||
});
|
||||
|
||||
// The charset guard replaces the length bound: reject anything outside the
|
||||
// generator's [0-9A-Za-z] alphabet (control chars, separators, injection) and
|
||||
// the empty string, while still accepting every real key.
|
||||
it('rejects a position with characters outside the fractional-index alphabet', async () => {
|
||||
for (const bad of ['a0/b', 'a b', 'a\n0', 'a.b', '', "a';--"]) {
|
||||
const errors = await constraintErrors(bad);
|
||||
expect(hasError(errors, 'position')).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import {
|
||||
IsString,
|
||||
IsOptional,
|
||||
MinLength,
|
||||
MaxLength,
|
||||
Matches,
|
||||
IsNotEmpty,
|
||||
} from 'class-validator';
|
||||
|
||||
@@ -10,9 +10,19 @@ export class MovePageDto {
|
||||
@IsString()
|
||||
pageId: string;
|
||||
|
||||
// `position` is a fractional-indexing key from `generateJitteredKeyBetween`
|
||||
// (the SAME generator page.service uses). Validate by CHARSET, not length: the
|
||||
// generator's default base-62 alphabet is [0-9A-Za-z], and DENSE between-inserts
|
||||
// legitimately grow a key well past a dozen chars (measured >40), so the old
|
||||
// @MinLength(5)/@MaxLength(12) bounds wrongly 400'd valid ordering keys the
|
||||
// server itself produced (Gitea #139 item 6). The charset regex rejects control
|
||||
// chars / separators / injection, and a generous MaxLength stays only as a
|
||||
// DoS guard — far above any realistic key, so it never rejects a real move.
|
||||
@IsString()
|
||||
@MinLength(5)
|
||||
@MaxLength(12)
|
||||
@Matches(/^[0-9A-Za-z]+$/, {
|
||||
message: 'position must be a fractional-index key ([0-9A-Za-z])',
|
||||
})
|
||||
@MaxLength(256)
|
||||
position: string;
|
||||
|
||||
@IsOptional()
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import 'reflect-metadata';
|
||||
import { plainToInstance } from 'class-transformer';
|
||||
import { validate } from 'class-validator';
|
||||
import { PageIdDto } from './page.dto';
|
||||
|
||||
// #435: PageIdDto.pageId carries a page's DOUBLE identity (internal UUID OR
|
||||
// public 10-char slugId), both as bare strings. The DTO must accept exactly
|
||||
// those two FORMATS and reject a malformed / swapped identity at the boundary.
|
||||
async function pageIdErrors(pageId: unknown) {
|
||||
const dto = plainToInstance(PageIdDto, { pageId });
|
||||
const errors = await validate(dto as object);
|
||||
return errors.some((e) => e.property === 'pageId');
|
||||
}
|
||||
|
||||
const UUID = '019f499a-9f8c-7d68-b7be-ce100d7c6c56';
|
||||
const SLUG = 'aB3xQ7kR2p';
|
||||
|
||||
describe('PageIdDto pageId format validation', () => {
|
||||
it('accepts a canonical page UUID', async () => {
|
||||
expect(await pageIdErrors(UUID)).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts a 10-char slugId', async () => {
|
||||
expect(await pageIdErrors(SLUG)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects a truncated / wrong-length slug', async () => {
|
||||
expect(await pageIdErrors('aB3xQ7kR2')).toBe(true); // 9 chars
|
||||
expect(await pageIdErrors('aB3xQ7kR2pX')).toBe(true); // 11 chars
|
||||
});
|
||||
|
||||
it('rejects a slug with an illegal character', async () => {
|
||||
expect(await pageIdErrors('aB3xQ7kR2!')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a full URL / path-shaped identity (not the bare id)', async () => {
|
||||
expect(await pageIdErrors(`my-page-title-${SLUG}`)).toBe(true);
|
||||
expect(await pageIdErrors(`https://x/p/${SLUG}`)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects a malformed UUID', async () => {
|
||||
expect(await pageIdErrors('019f499a-9f8c-7d68-b7be')).toBe(true);
|
||||
expect(await pageIdErrors('not-a-uuid-at-all-really')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects an empty string', async () => {
|
||||
expect(await pageIdErrors('')).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,39 @@
|
||||
import { applyDecorators } from '@nestjs/common';
|
||||
import { Matches, ValidationOptions } from 'class-validator';
|
||||
|
||||
/**
|
||||
* A page identity at the API boundary is EITHER the internal page UUID or the
|
||||
* public 10-char slugId (page.repo.findById matches a non-UUID input as a
|
||||
* slugId). Both arrive as bare strings, which is exactly how the two got swapped
|
||||
* silently (incident family #435). This regex pins the two accepted FORMATS so a
|
||||
* malformed / cross-wired identity (a truncated slug, a full URL, an email, an
|
||||
* id from another entity kind that isn't even shaped like either) is rejected at
|
||||
* the boundary instead of falling through to a confusing 404.
|
||||
*
|
||||
* - UUID: canonical 8-4-4-4-12 hex (version-agnostic — page ids are UUIDv7,
|
||||
* so only the shape/length is enforced, matching the MCP's UUID_RE
|
||||
* and the server's isValidUUID acceptance).
|
||||
* - slugId: exactly 10 chars over [0-9A-Za-z] (nanoid `generateSlugId`).
|
||||
*
|
||||
* The two are disjoint (a UUID is 36 chars WITH dashes, a slugId 10 chars
|
||||
* WITHOUT), so a value can only satisfy one branch.
|
||||
*/
|
||||
export const PAGE_ID_OR_SLUG_ID_REGEX =
|
||||
/^(?:[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|[0-9A-Za-z]{10})$/i;
|
||||
|
||||
/**
|
||||
* Validate that a DTO string field is a well-formed page identity (page UUID OR
|
||||
* 10-char slugId). Composed decorator so the same format rule is applied
|
||||
* consistently wherever a DTO accepts a `pageId` that may be either form.
|
||||
*/
|
||||
export function IsPageIdOrSlugId(
|
||||
validationOptions?: ValidationOptions,
|
||||
): PropertyDecorator {
|
||||
return applyDecorators(
|
||||
Matches(PAGE_ID_OR_SLUG_ID_REGEX, {
|
||||
message:
|
||||
'pageId must be a page UUID or a 10-character slugId',
|
||||
...validationOptions,
|
||||
}),
|
||||
);
|
||||
}
|
||||
@@ -9,10 +9,16 @@ import {
|
||||
import { Transform } from 'class-transformer';
|
||||
|
||||
import { ContentFormat } from './create-page.dto';
|
||||
import { IsPageIdOrSlugId } from './page-identity.validator';
|
||||
|
||||
export class PageIdDto {
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
// Format-validate the double identity (#435): accept only a page UUID or a
|
||||
// 10-char slugId so a malformed / swapped identity is rejected at the boundary
|
||||
// rather than passed to the repo as a bare string. Base for PageInfoDto,
|
||||
// DeletePageDto, BacklinksListDto, AddLabelsDto/RemoveLabelDto, etc.
|
||||
@IsPageIdOrSlugId()
|
||||
pageId: string;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,19 +1,38 @@
|
||||
import { TemporaryNoteCleanupService } from '../temporary-note-cleanup.service';
|
||||
|
||||
/**
|
||||
* Chainable Kysely stub that records every `.where(...)` call so the test can
|
||||
* assert the sweep only selects armed, expired, not-yet-trashed notes. The
|
||||
* terminal `.execute()` resolves the configured expired rows (the batch SELECT);
|
||||
* `.executeTakeFirst()` resolves the per-row deadline re-read done just before
|
||||
* each `removePage`. By default the re-read reports the note as still armed and
|
||||
* still expired (epoch deadline < now), so the sweep proceeds to delete it;
|
||||
* tests override `reReadFirst` to simulate a concurrent "Make permanent".
|
||||
* Chainable Kysely stub for the temporary-note sweep.
|
||||
*
|
||||
* `this.db` serves the non-locking candidate SELECT (selectFrom/select/where/
|
||||
* limit/execute -> the configured expired rows) AND `.transaction().execute(cb)`,
|
||||
* which runs `cb` with a separate `trx` builder. The `trx` builder serves the
|
||||
* per-row LOCKED re-check (selectFrom/select/where/forUpdate/skipLocked/
|
||||
* executeTakeFirst). `lockedRows` drives what that locked re-check returns per
|
||||
* candidate — an id/creator/workspace row means "still expired, delete it";
|
||||
* `undefined` means the predicate no longer matched (made permanent / re-armed /
|
||||
* already trashed) or the row was SKIP-LOCKED by another worker, so it is skipped.
|
||||
*/
|
||||
function makeDbStub(expiredRows: any[]) {
|
||||
function makeDbStub(expiredRows: any[], lockedRows?: any[]) {
|
||||
const whereCalls: any[][] = [];
|
||||
const reReadFirst = jest
|
||||
.fn()
|
||||
.mockResolvedValue({ temporaryExpiresAt: new Date(0), deletedAt: null });
|
||||
const locked = [
|
||||
...(lockedRows ??
|
||||
expiredRows.map((r) => ({
|
||||
id: r.id,
|
||||
creatorId: r.creatorId,
|
||||
workspaceId: r.workspaceId,
|
||||
}))),
|
||||
];
|
||||
const lockedTakeFirst = jest.fn(() => Promise.resolve(locked.shift()));
|
||||
const forUpdate = jest.fn(() => trxBuilder);
|
||||
const skipLocked = jest.fn(() => trxBuilder);
|
||||
const trxBuilder: any = {
|
||||
selectFrom: jest.fn(() => trxBuilder),
|
||||
select: jest.fn(() => trxBuilder),
|
||||
where: jest.fn(() => trxBuilder),
|
||||
forUpdate,
|
||||
skipLocked,
|
||||
executeTakeFirst: lockedTakeFirst,
|
||||
};
|
||||
const builder: any = {
|
||||
selectFrom: jest.fn(() => builder),
|
||||
select: jest.fn(() => builder),
|
||||
@@ -23,9 +42,11 @@ function makeDbStub(expiredRows: any[]) {
|
||||
}),
|
||||
limit: jest.fn(() => builder),
|
||||
execute: jest.fn().mockResolvedValue(expiredRows),
|
||||
executeTakeFirst: reReadFirst,
|
||||
transaction: jest.fn(() => ({
|
||||
execute: (cb: (trx: any) => Promise<any>) => Promise.resolve(cb(trxBuilder)),
|
||||
})),
|
||||
};
|
||||
return { builder, whereCalls, reReadFirst };
|
||||
return { builder, whereCalls, lockedTakeFirst, forUpdate, skipLocked };
|
||||
}
|
||||
|
||||
describe('TemporaryNoteCleanupService.sweepExpiredTemporaryNotes', () => {
|
||||
@@ -52,20 +73,36 @@ describe('TemporaryNoteCleanupService.sweepExpiredTemporaryNotes', () => {
|
||||
expect(builder.limit.mock.calls[0][0]).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('soft-deletes each expired note via removePage, attributed to its creator', async () => {
|
||||
it('soft-deletes each expired note via removePage under a row lock, attributed to its creator', async () => {
|
||||
const expired = [
|
||||
{ id: 'p1', creatorId: 'u1', workspaceId: 'w1' },
|
||||
{ id: 'p2', creatorId: 'u2', workspaceId: 'w1' },
|
||||
];
|
||||
const { builder } = makeDbStub(expired);
|
||||
const { builder, forUpdate, skipLocked } = makeDbStub(expired);
|
||||
const pageRepo = { removePage: jest.fn().mockResolvedValue(undefined) } as any;
|
||||
const service = new TemporaryNoteCleanupService(builder, pageRepo);
|
||||
|
||||
await service.sweepExpiredTemporaryNotes();
|
||||
|
||||
expect(pageRepo.removePage).toHaveBeenCalledTimes(2);
|
||||
expect(pageRepo.removePage).toHaveBeenNthCalledWith(1, 'p1', 'u1', 'w1');
|
||||
expect(pageRepo.removePage).toHaveBeenNthCalledWith(2, 'p2', 'u2', 'w1');
|
||||
// The 4th arg is the locking transaction — the delete runs inside it.
|
||||
expect(pageRepo.removePage).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
'p1',
|
||||
'u1',
|
||||
'w1',
|
||||
expect.anything(),
|
||||
);
|
||||
expect(pageRepo.removePage).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'p2',
|
||||
'u2',
|
||||
'w1',
|
||||
expect.anything(),
|
||||
);
|
||||
// The re-check acquired a FOR UPDATE SKIP LOCKED lock (once per candidate).
|
||||
expect(forUpdate).toHaveBeenCalledTimes(2);
|
||||
expect(skipLocked).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('continues past a failing note (one bad removePage does not abort the sweep)', async () => {
|
||||
@@ -86,60 +123,29 @@ describe('TemporaryNoteCleanupService.sweepExpiredTemporaryNotes', () => {
|
||||
service.sweepExpiredTemporaryNotes(),
|
||||
).resolves.toBeUndefined();
|
||||
expect(pageRepo.removePage).toHaveBeenCalledTimes(2);
|
||||
expect(pageRepo.removePage).toHaveBeenNthCalledWith(2, 'good', 'u2', 'w1');
|
||||
expect(pageRepo.removePage).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'good',
|
||||
'u2',
|
||||
'w1',
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('does NOT trash a note made permanent in the race window', async () => {
|
||||
// The batch SELECT saw the note as expired, but before its turn in the loop
|
||||
// the user clicked "Make permanent" (temporary_expires_at -> null). The
|
||||
// deadline re-read must catch this and skip the delete so the keep wins.
|
||||
it('does NOT trash a note made permanent / re-armed / already trashed (locked re-check returns nothing)', async () => {
|
||||
// The batch SELECT saw the note as expired, but by the time the LOCKED
|
||||
// re-check runs the row no longer matches the still-armed+expired+not-trashed
|
||||
// predicate (make-permanent, re-arm to a future deadline, or already trashed),
|
||||
// OR another worker holds the row (SKIP LOCKED). In every case the locked
|
||||
// SELECT returns nothing and the delete is skipped so the keep/other worker wins.
|
||||
const expired = [{ id: 'p1', creatorId: 'u1', workspaceId: 'w1' }];
|
||||
const { builder, reReadFirst } = makeDbStub(expired);
|
||||
reReadFirst.mockResolvedValueOnce({
|
||||
temporaryExpiresAt: null,
|
||||
deletedAt: null,
|
||||
});
|
||||
const { builder, lockedTakeFirst } = makeDbStub(expired, [undefined]);
|
||||
const pageRepo = { removePage: jest.fn() } as any;
|
||||
const service = new TemporaryNoteCleanupService(builder, pageRepo);
|
||||
|
||||
await service.sweepExpiredTemporaryNotes();
|
||||
|
||||
expect(reReadFirst).toHaveBeenCalledTimes(1);
|
||||
expect(pageRepo.removePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips a note already trashed since the batch SELECT', async () => {
|
||||
const expired = [{ id: 'p1', creatorId: 'u1', workspaceId: 'w1' }];
|
||||
const { builder, reReadFirst } = makeDbStub(expired);
|
||||
reReadFirst.mockResolvedValueOnce({
|
||||
temporaryExpiresAt: new Date(0),
|
||||
deletedAt: new Date(),
|
||||
});
|
||||
const pageRepo = { removePage: jest.fn() } as any;
|
||||
const service = new TemporaryNoteCleanupService(builder, pageRepo);
|
||||
|
||||
await service.sweepExpiredTemporaryNotes();
|
||||
|
||||
expect(pageRepo.removePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT trash a note re-armed to a future deadline in the race window', async () => {
|
||||
// The batch SELECT saw the note as expired, but before its turn in the loop
|
||||
// the user disarmed it and re-armed it to a fresh, still-future deadline
|
||||
// (temporary_expires_at -> now + 1h). The deadline re-read must catch that
|
||||
// the note is no longer expired and skip the delete so the keep wins.
|
||||
const expired = [{ id: 'p1', creatorId: 'u1', workspaceId: 'w1' }];
|
||||
const { builder, reReadFirst } = makeDbStub(expired);
|
||||
reReadFirst.mockResolvedValueOnce({
|
||||
temporaryExpiresAt: new Date(Date.now() + 60 * 60 * 1000),
|
||||
deletedAt: null,
|
||||
});
|
||||
const pageRepo = { removePage: jest.fn() } as any;
|
||||
const service = new TemporaryNoteCleanupService(builder, pageRepo);
|
||||
|
||||
await service.sweepExpiredTemporaryNotes();
|
||||
|
||||
expect(reReadFirst).toHaveBeenCalledTimes(1);
|
||||
expect(lockedTakeFirst).toHaveBeenCalledTimes(1);
|
||||
expect(pageRepo.removePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -151,4 +157,31 @@ describe('TemporaryNoteCleanupService.sweepExpiredTemporaryNotes', () => {
|
||||
await service.sweepExpiredTemporaryNotes();
|
||||
expect(pageRepo.removePage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('sweeps once on application bootstrap (catches notes expired during downtime)', async () => {
|
||||
const expired = [{ id: 'p1', creatorId: 'u1', workspaceId: 'w1' }];
|
||||
const { builder } = makeDbStub(expired);
|
||||
const pageRepo = { removePage: jest.fn().mockResolvedValue(undefined) } as any;
|
||||
const service = new TemporaryNoteCleanupService(builder, pageRepo);
|
||||
|
||||
await service.onApplicationBootstrap();
|
||||
|
||||
expect(pageRepo.removePage).toHaveBeenCalledTimes(1);
|
||||
expect(pageRepo.removePage).toHaveBeenCalledWith(
|
||||
'p1',
|
||||
'u1',
|
||||
'w1',
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('a startup-sweep failure never blocks application boot', async () => {
|
||||
const { builder } = makeDbStub([]);
|
||||
// Make the candidate SELECT throw to simulate a boot-time DB hiccup.
|
||||
builder.execute.mockRejectedValueOnce(new Error('db not ready'));
|
||||
const pageRepo = { removePage: jest.fn() } as any;
|
||||
const service = new TemporaryNoteCleanupService(builder, pageRepo);
|
||||
|
||||
await expect(service.onApplicationBootstrap()).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import {
|
||||
Injectable,
|
||||
Logger,
|
||||
OnApplicationBootstrap,
|
||||
} from '@nestjs/common';
|
||||
import { Interval } from '@nestjs/schedule';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { executeTx } from '@docmost/db/utils';
|
||||
|
||||
/**
|
||||
* Background sweeper for temporary notes ("structure or die"). A note whose
|
||||
@@ -11,7 +16,7 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
* TrashCleanupService; `@nestjs/schedule` is already enabled globally.
|
||||
*/
|
||||
@Injectable()
|
||||
export class TemporaryNoteCleanupService {
|
||||
export class TemporaryNoteCleanupService implements OnApplicationBootstrap {
|
||||
private readonly logger = new Logger(TemporaryNoteCleanupService.name);
|
||||
|
||||
// Cap a single sweep so a large backlog (e.g. many notes created during
|
||||
@@ -24,6 +29,20 @@ export class TemporaryNoteCleanupService {
|
||||
private readonly pageRepo: PageRepo,
|
||||
) {}
|
||||
|
||||
// Sweep once at startup so notes that expired during downtime are trashed
|
||||
// right away instead of waiting up to an hour for the first @Interval tick.
|
||||
// Best-effort: never let a startup-sweep failure block application boot.
|
||||
async onApplicationBootstrap() {
|
||||
try {
|
||||
await this.sweepExpiredTemporaryNotes();
|
||||
} catch (error) {
|
||||
this.logger.error(
|
||||
'Temporary-note startup sweep failed',
|
||||
error instanceof Error ? error.stack : undefined,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Hourly granularity: lifetimes are configured in hours, so a sub-hour
|
||||
// overshoot past the deadline is acceptable.
|
||||
@Interval('temporary-note-cleanup', 60 * 60 * 1000)
|
||||
@@ -31,9 +50,11 @@ export class TemporaryNoteCleanupService {
|
||||
try {
|
||||
const now = new Date();
|
||||
|
||||
// Candidate ids (non-locking). The authoritative re-check happens per row
|
||||
// under a row lock below, so this cheap pass just bounds the batch.
|
||||
const expired = await this.db
|
||||
.selectFrom('pages')
|
||||
.select(['id', 'creatorId', 'workspaceId'])
|
||||
.select(['id'])
|
||||
.where('temporaryExpiresAt', 'is not', null)
|
||||
.where('temporaryExpiresAt', '<', now)
|
||||
.where('deletedAt', 'is', null) // not already in trash
|
||||
@@ -41,50 +62,53 @@ export class TemporaryNoteCleanupService {
|
||||
.execute();
|
||||
|
||||
let trashed = 0;
|
||||
for (const page of expired) {
|
||||
for (const candidate of expired) {
|
||||
try {
|
||||
// Re-check the deadline at deletion time. The SELECT above is not
|
||||
// transactional, so a user may click "Make permanent"
|
||||
// (toggleTemporary sets temporary_expires_at = null) in the window
|
||||
// between the SELECT and this per-row removePage. removePage deletes
|
||||
// by id with only a `deletedAt IS NULL` filter and never re-reads the
|
||||
// deadline, so without this guard a concurrently-kept note would
|
||||
// still be trashed. Re-read the row and skip it unless it is still
|
||||
// armed AND still expired, so a concurrent make-permanent wins.
|
||||
const current = await this.db
|
||||
.selectFrom('pages')
|
||||
.select(['temporaryExpiresAt', 'deletedAt'])
|
||||
.where('id', '=', page.id)
|
||||
.executeTakeFirst();
|
||||
const didTrash = await executeTx(this.db, async (trx) => {
|
||||
// Re-check the row UNDER A LOCK inside the transaction. `FOR UPDATE
|
||||
// SKIP LOCKED`:
|
||||
// - serialises against a concurrent "Make permanent"
|
||||
// (toggleTemporary UPDATE takes the same row lock): if it commits
|
||||
// first, the deadline predicate below no longer matches and we
|
||||
// skip; if we lock first, it waits until this delete commits.
|
||||
// - SKIP LOCKED lets a second worker/instance skip a row another
|
||||
// sweeper already claimed instead of blocking on it (no double
|
||||
// processing, no thundering herd).
|
||||
// The predicate re-asserts still-armed AND still-expired AND
|
||||
// not-already-trashed, so a make-permanent / prior sweep drops the row.
|
||||
const locked = await trx
|
||||
.selectFrom('pages')
|
||||
.select(['id', 'creatorId', 'workspaceId'])
|
||||
.where('id', '=', candidate.id)
|
||||
.where('temporaryExpiresAt', 'is not', null)
|
||||
.where('temporaryExpiresAt', '<', now)
|
||||
.where('deletedAt', 'is', null)
|
||||
.forUpdate()
|
||||
.skipLocked()
|
||||
.executeTakeFirst();
|
||||
|
||||
if (
|
||||
!current ||
|
||||
current.deletedAt !== null ||
|
||||
current.temporaryExpiresAt === null ||
|
||||
new Date(current.temporaryExpiresAt) >= now
|
||||
) {
|
||||
// Made permanent, already trashed, or no longer expired since the
|
||||
// SELECT — leave it alone.
|
||||
continue;
|
||||
}
|
||||
if (!locked) return false;
|
||||
|
||||
// Reuse the exact soft-delete path: recursive over children, removes
|
||||
// shares in a transaction, and emits PAGE_SOFT_DELETED (tree
|
||||
// invalidation + watcher notifications). Attribute the automatic
|
||||
// deletion to the note's creator (no schema change). Both the SELECT
|
||||
// above and removePage filter `deletedAt IS NULL`, so a double sweep
|
||||
// is idempotent.
|
||||
await this.pageRepo.removePage(
|
||||
page.id,
|
||||
// creatorId is set on every created page; a temporary note always
|
||||
// has one. Cast to satisfy the non-null deletedById parameter.
|
||||
page.creatorId as string,
|
||||
page.workspaceId,
|
||||
);
|
||||
trashed++;
|
||||
// Reuse the exact soft-delete path (recursive children + share
|
||||
// removal + PAGE_SOFT_DELETED broadcast), running IN this locked
|
||||
// transaction so the delete is atomic with the re-check and cannot
|
||||
// deadlock on a nested independent transaction. The broadcast is
|
||||
// deferred by removePage to this transaction's commit. Attribute the
|
||||
// automatic deletion to the note's creator (no schema change).
|
||||
await this.pageRepo.removePage(
|
||||
locked.id,
|
||||
// creatorId is set on every created page; a temporary note always
|
||||
// has one. Cast to satisfy the non-null deletedById parameter.
|
||||
locked.creatorId as string,
|
||||
locked.workspaceId,
|
||||
trx,
|
||||
);
|
||||
return true;
|
||||
});
|
||||
if (didTrash) trashed++;
|
||||
} catch (error) {
|
||||
this.logger.error(
|
||||
`Failed to trash expired temporary note ${page.id}`,
|
||||
`Failed to trash expired temporary note ${candidate.id}`,
|
||||
error instanceof Error ? error.stack : undefined,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -133,6 +133,9 @@ describe('ShareAliasController authz gates', () => {
|
||||
creatorId: 'u-1',
|
||||
alias: 'promo',
|
||||
confirmReassign: true,
|
||||
// The requesting user is forwarded so setAlias can gate the reassign
|
||||
// 409 title disclosure on target-page view permission (#495).
|
||||
user,
|
||||
});
|
||||
expect(result).toEqual({ id: 'alias-1' });
|
||||
});
|
||||
|
||||
@@ -79,6 +79,9 @@ export class ShareAliasController {
|
||||
creatorId: user.id,
|
||||
alias: dto.alias,
|
||||
confirmReassign: dto.confirmReassign,
|
||||
// Gates whether the reassign 409 may reveal the current target's title
|
||||
// (view-permission check on that page) — see setAlias (#495).
|
||||
user,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import { BadRequestException, ConflictException } from '@nestjs/common';
|
||||
import {
|
||||
BadRequestException,
|
||||
ConflictException,
|
||||
ForbiddenException,
|
||||
} from '@nestjs/common';
|
||||
import { NoResultError } from 'kysely';
|
||||
import { ShareAliasService } from './share-alias.service';
|
||||
|
||||
@@ -7,6 +11,8 @@ import { ShareAliasService } from './share-alias.service';
|
||||
* 409 reassign guard, uniqueness-race handling, availability probe, and the
|
||||
* request-time readable-target resolution (which re-runs the share boundary).
|
||||
*/
|
||||
const USER = { id: 'u-1' } as any;
|
||||
|
||||
describe('ShareAliasService', () => {
|
||||
// Sentinel handed to repo calls so tests can assert they ran inside the tx.
|
||||
const trx = { __trx: true };
|
||||
@@ -27,6 +33,10 @@ describe('ShareAliasService', () => {
|
||||
resolveReadableSharePage: jest.fn(),
|
||||
isSharingAllowed: jest.fn(),
|
||||
};
|
||||
// Default: the requester CAN view the target page (validateCanView resolves),
|
||||
// so the reassign 409 may disclose its title. Tests override to reject to
|
||||
// assert the no-leak path.
|
||||
const pageAccessService = { validateCanView: jest.fn().mockResolvedValue(undefined) };
|
||||
// Fake kysely db: only .transaction().execute(cb) is used by setAlias.
|
||||
const db = {
|
||||
transaction: jest.fn(() => ({
|
||||
@@ -37,9 +47,10 @@ describe('ShareAliasService', () => {
|
||||
shareAliasRepo as any,
|
||||
pageRepo as any,
|
||||
shareService as any,
|
||||
pageAccessService as any,
|
||||
db as any,
|
||||
);
|
||||
return { service, shareAliasRepo, pageRepo, shareService, db };
|
||||
return { service, shareAliasRepo, pageRepo, shareService, pageAccessService, db };
|
||||
}
|
||||
|
||||
describe('setAlias', () => {
|
||||
@@ -50,6 +61,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'A', // too short + uppercase
|
||||
}),
|
||||
).rejects.toBeInstanceOf(BadRequestException);
|
||||
@@ -66,6 +78,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: ' My Page ',
|
||||
});
|
||||
|
||||
@@ -114,6 +127,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'ted',
|
||||
});
|
||||
|
||||
@@ -144,6 +158,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
});
|
||||
|
||||
@@ -179,6 +194,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'new',
|
||||
});
|
||||
|
||||
@@ -190,30 +206,77 @@ describe('ShareAliasService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('throws 409 with current target when name is taken and not confirmed', async () => {
|
||||
const { service, shareAliasRepo, pageRepo } = makeService();
|
||||
it('throws 409 with the target TITLE (never its id) when the requester CAN view it', async () => {
|
||||
const { service, shareAliasRepo, pageRepo, pageAccessService } =
|
||||
makeService();
|
||||
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
|
||||
id: 'a-1',
|
||||
alias: 'foo',
|
||||
pageId: 'p-other',
|
||||
});
|
||||
pageRepo.findById.mockResolvedValue({ id: 'p-other', title: 'Other' });
|
||||
pageAccessService.validateCanView.mockResolvedValue(undefined); // can view
|
||||
|
||||
try {
|
||||
await service.setAlias({
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
});
|
||||
fail('expected ConflictException');
|
||||
} catch (err) {
|
||||
expect(err).toBeInstanceOf(ConflictException);
|
||||
expect((err as ConflictException).getResponse()).toMatchObject({
|
||||
const body = (err as ConflictException).getResponse();
|
||||
expect(body).toMatchObject({
|
||||
code: 'ALIAS_REASSIGN_REQUIRED',
|
||||
currentPageId: 'p-other',
|
||||
currentPageTitle: 'Other',
|
||||
});
|
||||
// SECURITY (#495): the page id is NEVER disclosed, even to a viewer.
|
||||
expect(body).not.toHaveProperty('currentPageId');
|
||||
expect(pageAccessService.validateCanView).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: 'p-other' }),
|
||||
USER,
|
||||
);
|
||||
}
|
||||
expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('throws 409 WITHOUT the title or id when the requester CANNOT view the target (#495)', async () => {
|
||||
const { service, shareAliasRepo, pageRepo, pageAccessService } =
|
||||
makeService();
|
||||
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
|
||||
id: 'a-1',
|
||||
alias: 'foo',
|
||||
pageId: 'p-secret',
|
||||
});
|
||||
pageRepo.findById.mockResolvedValue({ id: 'p-secret', title: 'Secret' });
|
||||
// No view permission on the target page -> validateCanView throws.
|
||||
pageAccessService.validateCanView.mockRejectedValue(
|
||||
new ForbiddenException(),
|
||||
);
|
||||
|
||||
try {
|
||||
await service.setAlias({
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
});
|
||||
fail('expected ConflictException');
|
||||
} catch (err) {
|
||||
expect(err).toBeInstanceOf(ConflictException);
|
||||
const body = (err as ConflictException).getResponse() as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
expect(body).toMatchObject({ code: 'ALIAS_REASSIGN_REQUIRED' });
|
||||
// The enumeration hole: neither the id nor the title of a page the
|
||||
// requester cannot see may leak.
|
||||
expect(body).not.toHaveProperty('currentPageId');
|
||||
expect(body.currentPageTitle ?? null).toBeNull();
|
||||
}
|
||||
expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled();
|
||||
});
|
||||
@@ -231,6 +294,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
confirmReassign: true,
|
||||
});
|
||||
@@ -269,6 +333,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
});
|
||||
fail('expected ConflictException');
|
||||
@@ -294,6 +359,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
});
|
||||
fail('expected ConflictException');
|
||||
@@ -317,6 +383,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
});
|
||||
fail('expected ConflictException');
|
||||
@@ -346,6 +413,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
});
|
||||
fail('expected ConflictException');
|
||||
@@ -375,6 +443,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
confirmReassign: true,
|
||||
});
|
||||
@@ -406,6 +475,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'ted',
|
||||
});
|
||||
fail('expected ConflictException');
|
||||
@@ -428,6 +498,7 @@ describe('ShareAliasService', () => {
|
||||
workspaceId: 'ws-1',
|
||||
pageId: 'p-1',
|
||||
creatorId: 'u-1',
|
||||
user: USER,
|
||||
alias: 'foo',
|
||||
}),
|
||||
).rejects.toBeInstanceOf(BadRequestException);
|
||||
@@ -450,18 +521,22 @@ describe('ShareAliasService', () => {
|
||||
alias: 'free-name',
|
||||
valid: true,
|
||||
available: true,
|
||||
currentPageId: null,
|
||||
});
|
||||
// SECURITY (#495): the availability probe must NOT leak any page id.
|
||||
expect(res).not.toHaveProperty('currentPageId');
|
||||
});
|
||||
|
||||
it('reports taken with the current target page', async () => {
|
||||
it('reports taken WITHOUT leaking the current target page id (#495)', async () => {
|
||||
const { service, shareAliasRepo } = makeService();
|
||||
shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue({
|
||||
id: 'a-1',
|
||||
pageId: 'p-9',
|
||||
});
|
||||
const res = await service.checkAvailability('taken', 'ws-1');
|
||||
expect(res).toMatchObject({ available: false, currentPageId: 'p-9' });
|
||||
expect(res).toMatchObject({ available: false });
|
||||
// The row exists (available:false) but its pageId is never returned — an
|
||||
// authenticated member cannot map an alias name to a page id it can't view.
|
||||
expect(res).not.toHaveProperty('currentPageId');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -7,7 +7,8 @@ import {
|
||||
import { ShareAliasRepo } from '@docmost/db/repos/share-alias/share-alias.repo';
|
||||
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||
import { ShareService } from './share.service';
|
||||
import { Page, ShareAlias } from '@docmost/db/types/entity.types';
|
||||
import { PageAccessService } from '../page/page-access/page-access.service';
|
||||
import { Page, ShareAlias, User } from '@docmost/db/types/entity.types';
|
||||
import { isValidShareAlias, normalizeShareAlias } from './share-alias.util';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||
@@ -43,6 +44,7 @@ export class ShareAliasService {
|
||||
private readonly shareAliasRepo: ShareAliasRepo,
|
||||
private readonly pageRepo: PageRepo,
|
||||
private readonly shareService: ShareService,
|
||||
private readonly pageAccessService: PageAccessService,
|
||||
@InjectKysely() private readonly db: KyselyDB,
|
||||
) {}
|
||||
|
||||
@@ -55,9 +57,13 @@ export class ShareAliasService {
|
||||
* `/l/<old>` link survives
|
||||
* - name already points at pageId -> no-op (idempotent)
|
||||
* - name points at ANOTHER page -> the "swap". Without confirmReassign
|
||||
* we throw 409 carrying the current target so the client can confirm;
|
||||
* with it we UPDATE the single row's page_id (every /l/<alias> link
|
||||
* follows the 302 to the new page instantly — no stale cache).
|
||||
* we throw 409 so the client can confirm. SECURITY (#495): the 409 reveals
|
||||
* the current target's title ONLY when `user` may VIEW that page, and never
|
||||
* its id — otherwise any member with one editable+shared page could iterate
|
||||
* alias names with confirmReassign=false and map them to (id, title) of
|
||||
* pages they cannot see. With confirmReassign we UPDATE the single row's
|
||||
* page_id (every /l/<alias> link follows the 302 to the new page instantly
|
||||
* — no stale cache).
|
||||
*
|
||||
* To keep the invariant self-healing we DELETE every other alias row still
|
||||
* pointing at this page (a legacy duplicate, or the target page's own former
|
||||
@@ -77,8 +83,12 @@ export class ShareAliasService {
|
||||
creatorId: string;
|
||||
alias: string;
|
||||
confirmReassign?: boolean;
|
||||
// The requesting user — used ONLY to gate whether the reassign 409 may reveal
|
||||
// the current target page's title (view-permission check). Not an authz gate
|
||||
// for the write itself (the controller already validated edit on `pageId`).
|
||||
user: User;
|
||||
}): Promise<ShareAlias> {
|
||||
const { workspaceId, pageId, creatorId, confirmReassign } = opts;
|
||||
const { workspaceId, pageId, creatorId, confirmReassign, user } = opts;
|
||||
const alias = normalizeShareAlias(opts.alias);
|
||||
if (!isValidShareAlias(alias)) {
|
||||
throw new BadRequestException(
|
||||
@@ -97,14 +107,30 @@ export class ShareAliasService {
|
||||
// The name is occupied by a DIFFERENT (or dangling) target page.
|
||||
if (byName && byName.pageId !== pageId) {
|
||||
if (!confirmReassign) {
|
||||
// SECURITY (#495): only disclose the current target's TITLE, and only
|
||||
// when the requester may VIEW that page. Never disclose its id (the
|
||||
// client's confirm-reassign UX doesn't use it, and it is an enumerable
|
||||
// identity). A member with one editable+shared page must NOT be able to
|
||||
// iterate alias names and map them to (id, title) of pages they cannot
|
||||
// see. When view is denied (or the alias is dangling) the 409 is the
|
||||
// bare "occupied" fact — the client still shows a generic confirm modal.
|
||||
const currentPage = byName.pageId
|
||||
? await this.pageRepo.findById(byName.pageId)
|
||||
: null;
|
||||
let currentPageTitle: string | null = null;
|
||||
if (currentPage) {
|
||||
try {
|
||||
await this.pageAccessService.validateCanView(currentPage, user);
|
||||
currentPageTitle = currentPage.title ?? null;
|
||||
} catch {
|
||||
// No view permission on the target -> do not reveal its title.
|
||||
currentPageTitle = null;
|
||||
}
|
||||
}
|
||||
throw new ConflictException({
|
||||
message: 'Alias already in use',
|
||||
code: 'ALIAS_REASSIGN_REQUIRED',
|
||||
currentPageId: byName.pageId,
|
||||
currentPageTitle: currentPage?.title ?? null,
|
||||
currentPageTitle,
|
||||
});
|
||||
}
|
||||
// Confirmed swap. ORDER MATTERS: the partial unique index on
|
||||
@@ -223,21 +249,27 @@ export class ShareAliasService {
|
||||
alias: string;
|
||||
valid: boolean;
|
||||
available: boolean;
|
||||
currentPageId: string | null;
|
||||
}> {
|
||||
const alias = normalizeShareAlias(rawAlias);
|
||||
if (!isValidShareAlias(alias)) {
|
||||
return { alias, valid: false, available: false, currentPageId: null };
|
||||
return { alias, valid: false, available: false };
|
||||
}
|
||||
const existing = await this.shareAliasRepo.findByAliasAndWorkspace(
|
||||
alias,
|
||||
workspaceId,
|
||||
);
|
||||
// SECURITY (#495): return ONLY the boolean availability. The previous shape
|
||||
// leaked `currentPageId` — the id of whatever page the alias already targets —
|
||||
// to ANY authenticated workspace member, with no view-permission check on that
|
||||
// page. An attacker could enumerate alias names and map them to page ids they
|
||||
// have no access to. The taken/free bit is all the "is this address free"
|
||||
// probe needs. The reassign flow (setAlias 409) may surface the target's
|
||||
// TITLE, but only behind a `validateCanView` on that page (see setAlias); it
|
||||
// never returns the page id.
|
||||
return {
|
||||
alias,
|
||||
valid: true,
|
||||
available: !existing,
|
||||
currentPageId: existing?.pageId ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -24,6 +24,54 @@ export const ALLOWED_RATINGS = new Set<string>([
|
||||
'poor',
|
||||
]);
|
||||
|
||||
// The ONLY route labels accepted. The endpoint is anonymous, so an un-checked
|
||||
// `route` is a free-text write surface (arbitrary high-cardinality strings /
|
||||
// injected text into the metrics table). The client only ever sends a label from
|
||||
// a finite template dictionary (`templateRoute`), so we drop anything not in it.
|
||||
//
|
||||
// PARITY: this MUST mirror `KNOWN_ROUTE_TEMPLATES` in the client's
|
||||
// `apps/client/src/lib/telemetry/route-template.ts` (the canonical source). A
|
||||
// drift means legit client routes get dropped — keep the two in lockstep; the
|
||||
// client self-consistency test asserts `templateRoute` only emits these values.
|
||||
export const ALLOWED_ROUTE_TEMPLATES = new Set<string>([
|
||||
'/',
|
||||
'other',
|
||||
// Static routes.
|
||||
'/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',
|
||||
// Dynamic templates (slugs/ids are already collapsed to `:param`).
|
||||
'/share/:shareId/p/:slug',
|
||||
'/share/p/:slug',
|
||||
'/share/:shareId',
|
||||
'/p/:slug',
|
||||
'/s/:space/p/:slug',
|
||||
'/s/:space/trash',
|
||||
'/s/:space',
|
||||
'/labels/:label',
|
||||
'/invites/:invitationId',
|
||||
'/settings/groups/:groupId',
|
||||
]);
|
||||
|
||||
// `attr` is a web-vitals attribution TARGET: a CSS-selector-ish string (an
|
||||
// element path like `html>body>div#app>button.cta`), never free prose. Constrain
|
||||
// it to a conservative CSS-selector charset so the anonymous endpoint cannot be
|
||||
// used to write arbitrary text / PII / markup into the metrics table. A value
|
||||
// containing anything outside this set is DROPPED (-> null); the event is kept.
|
||||
export const ATTR_ALLOWED_CHARSET = /^[A-Za-z0-9#.\-_> :()[\]="'*+~,]+$/;
|
||||
|
||||
// Max events accepted per batch; the rest are ignored.
|
||||
export const MAX_EVENTS_PER_BATCH = 50;
|
||||
|
||||
@@ -77,14 +125,20 @@ export function sanitizeVitalEvent(
|
||||
? e.rating
|
||||
: null;
|
||||
|
||||
// route: accept ONLY a known template label (dictionary check), else drop to
|
||||
// null. The length cap stays as a cheap pre-guard before the Set lookup.
|
||||
let route: string | null = null;
|
||||
if (typeof e.route === 'string' && e.route.length > 0) {
|
||||
route = e.route.slice(0, MAX_ROUTE_LENGTH);
|
||||
const candidate = e.route.slice(0, MAX_ROUTE_LENGTH);
|
||||
route = ALLOWED_ROUTE_TEMPLATES.has(candidate) ? candidate : null;
|
||||
}
|
||||
|
||||
// attr: truncate, then accept ONLY if it is a CSS-selector-shaped string
|
||||
// (charset whitelist); anything with characters outside the set is dropped.
|
||||
let attr: string | null = null;
|
||||
if (typeof e.attr === 'string' && e.attr.length > 0) {
|
||||
attr = e.attr.slice(0, MAX_ATTR_LENGTH);
|
||||
const candidate = e.attr.slice(0, MAX_ATTR_LENGTH);
|
||||
attr = ATTR_ALLOWED_CHARSET.test(candidate) ? candidate : null;
|
||||
}
|
||||
|
||||
let docSize: number | null = null;
|
||||
|
||||
@@ -90,6 +90,44 @@ describe('VitalsService.buildRows', () => {
|
||||
expect(rows[0].attr).toHaveLength(MAX_ATTR_LENGTH);
|
||||
});
|
||||
|
||||
it('keeps a known route template but DROPS an unknown/free-text route (#495)', () => {
|
||||
const rows = svc.buildRows(
|
||||
{
|
||||
events: [
|
||||
{ name: 'INP', value: 1, route: '/s/:space/p/:slug' }, // known
|
||||
{ name: 'INP', value: 2, route: '/s/acme-corp/p/secret-slug' }, // raw path (slugs) — not a template
|
||||
{ name: 'INP', value: 3, route: 'DROP TABLE client_metrics;--' }, // injected free text
|
||||
{ name: 'INP', value: 4, route: '/home' }, // known static
|
||||
],
|
||||
},
|
||||
WS,
|
||||
);
|
||||
expect(rows.map((r) => r.route)).toEqual([
|
||||
'/s/:space/p/:slug',
|
||||
null, // raw path dropped
|
||||
null, // free text dropped
|
||||
'/home',
|
||||
]);
|
||||
});
|
||||
|
||||
it('DROPS an attr that is not a CSS-selector-shaped string (#495)', () => {
|
||||
const rows = svc.buildRows(
|
||||
{
|
||||
events: [
|
||||
{ name: 'INP', value: 1, attr: 'div#app>button.cta' }, // valid selector
|
||||
{ name: 'INP', value: 2, attr: 'user@example.com wrote a note' }, // free text / PII
|
||||
{ name: 'INP', value: 3, attr: '<script>alert(1)</script>' }, // markup
|
||||
],
|
||||
},
|
||||
WS,
|
||||
);
|
||||
expect(rows.map((r) => r.attr)).toEqual([
|
||||
'div#app>button.cta',
|
||||
null,
|
||||
null,
|
||||
]);
|
||||
});
|
||||
|
||||
it('caps the batch at 50 events', () => {
|
||||
const events = Array.from({ length: 200 }, () => ({ name: 'CLS', value: 1 }));
|
||||
const rows = svc.buildRows({ events }, WS);
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
import * as path from 'path';
|
||||
import { readFileSync } from 'fs';
|
||||
|
||||
// Mock ONLY kysely's `sql.raw(...).execute()` so we can observe what
|
||||
// ensureConcurrentIndexes runs and how it handles failures, without a DB.
|
||||
const execMock = jest.fn((_db: unknown) => Promise.resolve(undefined));
|
||||
const rawMock = jest.fn((_stmt: string) => ({ execute: execMock }));
|
||||
jest.mock('kysely', () => {
|
||||
const actual = jest.requireActual('kysely');
|
||||
return { ...actual, sql: { ...actual.sql, raw: rawMock } };
|
||||
});
|
||||
|
||||
import { CONCURRENT_INDEXES, ensureConcurrentIndexes } from './concurrent-indexes';
|
||||
|
||||
describe('ensureConcurrentIndexes', () => {
|
||||
const fakeDb = { __topLevelKysely: true } as never;
|
||||
|
||||
beforeEach(() => {
|
||||
execMock.mockReset().mockResolvedValue(undefined);
|
||||
rawMock.mockClear();
|
||||
});
|
||||
|
||||
it('redefines the inlinable f_unaccent BEFORE building any index, then builds each CONCURRENTLY outside a transaction', async () => {
|
||||
const onLog = jest.fn();
|
||||
await ensureConcurrentIndexes(fakeDb, onLog);
|
||||
|
||||
// One f_unaccent redefine + one statement per index.
|
||||
expect(rawMock).toHaveBeenCalledTimes(CONCURRENT_INDEXES.length + 1);
|
||||
const statements = rawMock.mock.calls.map((c) => c[0] as string);
|
||||
// ORDER: the f_unaccent redefine MUST be first — otherwise an existing
|
||||
// tenant's old 2-arg f_unaccent makes every CONCURRENTLY build fail.
|
||||
expect(statements[0]).toContain('CREATE OR REPLACE FUNCTION f_unaccent');
|
||||
expect(statements[0]).toContain('SELECT public.unaccent($1)');
|
||||
expect(statements[0]).not.toContain('CREATE INDEX');
|
||||
// The rest are the CONCURRENTLY index builds.
|
||||
for (const stmt of statements.slice(1)) {
|
||||
expect(stmt).toContain('CREATE INDEX CONCURRENTLY');
|
||||
expect(stmt).toContain('IF NOT EXISTS');
|
||||
}
|
||||
// Executed against the top-level db (a transaction would forbid CONCURRENTLY).
|
||||
for (const call of execMock.mock.calls) {
|
||||
expect(call[0]).toBe(fakeDb);
|
||||
}
|
||||
expect(onLog).toHaveBeenCalledTimes(CONCURRENT_INDEXES.length + 1);
|
||||
});
|
||||
|
||||
it('still builds the indexes when the f_unaccent redefine fails (fresh DB, best-effort)', async () => {
|
||||
// Redefine (first execute) throws; the index loop must still run.
|
||||
execMock
|
||||
.mockRejectedValueOnce(new Error('extension "unaccent" does not exist'))
|
||||
.mockResolvedValue(undefined);
|
||||
const onLog = jest.fn();
|
||||
|
||||
await expect(
|
||||
ensureConcurrentIndexes(fakeDb, onLog),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
// 1 redefine attempt + one per index.
|
||||
expect(execMock).toHaveBeenCalledTimes(CONCURRENT_INDEXES.length + 1);
|
||||
const errored = onLog.mock.calls.filter((c) => c[1] !== undefined);
|
||||
expect(errored).toHaveLength(1);
|
||||
expect(String(errored[0][1])).toContain('unaccent');
|
||||
});
|
||||
|
||||
it('is best-effort: a failing index does not abort the rest and is reported', async () => {
|
||||
// Redefine ok; fail the FIRST index; the remaining ones must still be tried.
|
||||
execMock
|
||||
.mockResolvedValueOnce(undefined) // f_unaccent redefine
|
||||
.mockRejectedValueOnce(new Error('relation "pages" does not exist'))
|
||||
.mockResolvedValue(undefined);
|
||||
const onLog = jest.fn();
|
||||
|
||||
await expect(
|
||||
ensureConcurrentIndexes(fakeDb, onLog),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(execMock).toHaveBeenCalledTimes(CONCURRENT_INDEXES.length + 1);
|
||||
const errored = onLog.mock.calls.filter((c) => c[1] !== undefined);
|
||||
expect(errored).toHaveLength(1);
|
||||
expect(String(errored[0][1])).toContain('does not exist');
|
||||
});
|
||||
});
|
||||
|
||||
// DRIFT GUARD: each CONCURRENT_INDEXES entry pre-builds an index that a plain
|
||||
// migration ALSO creates with `IF NOT EXISTS`. If the two expressions diverge,
|
||||
// Postgres would treat them as different indexes and the pre-build would NOT
|
||||
// make the migration a no-op. Assert the migration files still contain each
|
||||
// index's functional expression.
|
||||
describe('CONCURRENT_INDEXES parity with the migrations', () => {
|
||||
const migrationsDir = path.join(__dirname, 'migrations');
|
||||
const files = [
|
||||
'20260705T120000-perf-indexes.ts',
|
||||
'20260706T120000-search-lookup-trgm.ts',
|
||||
].map((f) => readFileSync(path.join(migrationsDir, f), 'utf8'));
|
||||
const allMigrationSrc = files.join('\n');
|
||||
|
||||
it.each(CONCURRENT_INDEXES.map((i) => [i.name, i]))(
|
||||
'migration source still creates %s with the same expression',
|
||||
(_name, idx) => {
|
||||
// Extract the `USING gin ((...expr...) gin_trgm_ops)` tail from the
|
||||
// canonical create and assert the migration source contains it verbatim.
|
||||
const m = (idx as { create: string }).create.match(
|
||||
/ON \w+ (USING gin .+)$/,
|
||||
);
|
||||
expect(m).not.toBeNull();
|
||||
const expr = (m as RegExpMatchArray)[1];
|
||||
expect(allMigrationSrc).toContain(expr);
|
||||
// And the migration must build it by the SAME index name.
|
||||
expect(allMigrationSrc).toContain(
|
||||
`CREATE INDEX IF NOT EXISTS ${(idx as { name: string }).name}`,
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -0,0 +1,142 @@
|
||||
import { Kysely, sql } from 'kysely';
|
||||
|
||||
/**
|
||||
* Indexes that MUST be built with `CREATE INDEX CONCURRENTLY` so an auto-deploy
|
||||
* migration never takes a `SHARE` lock that blocks writes on a hot table
|
||||
* (`pages`) for the — potentially minutes-long — GIN trigram build (#495 item 12).
|
||||
*
|
||||
* Kysely runs each migration INSIDE a transaction (Postgres has transactional
|
||||
* DDL), and `CREATE INDEX CONCURRENTLY` cannot run inside a transaction block, so
|
||||
* these cannot live in an ordinary migration. Instead {@link ensureConcurrentIndexes}
|
||||
* builds them out-of-band (no transaction) BEFORE the migrator runs; the matching
|
||||
* migrations keep a plain `CREATE INDEX IF NOT EXISTS` as a backstop, which then
|
||||
* no-ops because the index already exists. So:
|
||||
* - existing prod DB, incremental deploy: the pre-build FIRST redefines
|
||||
* `f_unaccent` to its index-inlinable 1-arg form (see below), THEN builds each
|
||||
* index CONCURRENTLY (no write lock); the migration's IF NOT EXISTS no-ops →
|
||||
* the write-blocking build is gone;
|
||||
* - fresh DB (or a DB whose `pages` / `unaccent` extension does not exist yet):
|
||||
* the pre-build fails and is swallowed (best-effort), and the migration builds
|
||||
* the index normally on an empty/small table where the lock is irrelevant.
|
||||
*
|
||||
* NOT a strict "worst case = previous behaviour":
|
||||
* - The f_unaccent form matters. The trigram expressions use `LOWER(f_unaccent(col))`.
|
||||
* The INDEX-INLINABLE 1-arg `f_unaccent(text)` (`SELECT public.unaccent($1)`)
|
||||
* is created INSIDE migration 20260705, which runs AFTER this pre-build. On an
|
||||
* existing tenant the live `f_unaccent` is still the OLD 2-arg form from
|
||||
* 20250729, which Postgres CANNOT inline into a CONCURRENTLY index expression
|
||||
* (`function unaccent(unknown, text) does not exist ... during inlining`) — the
|
||||
* build fails outright. So this pre-build redefines `f_unaccent` to the 1-arg
|
||||
* form FIRST (idempotent, output-identical, in lockstep with 20260705). Without
|
||||
* that redefine the whole feature is dead-on-arrival for the very case it
|
||||
* targets.
|
||||
* - An INTERRUPTED `CREATE INDEX CONCURRENTLY` (killed pod, cancelled query) with
|
||||
* a working `f_unaccent` leaves an INVALID index behind. A subsequent name-based
|
||||
* `IF NOT EXISTS` — in BOTH this pre-build and the migration backstop — sees the
|
||||
* name and skips, so the invalid index is NEVER repaired automatically and the
|
||||
* query keeps seq-scanning until an operator `DROP INDEX`es it. The old
|
||||
* in-transaction build could not leave an invalid index (a failed tx rolled the
|
||||
* whole index back), so this is a genuinely new failure mode, not "= previous".
|
||||
* (A future hardening could `DROP` an `indisvalid = false` index before
|
||||
* rebuilding; not done here.)
|
||||
*
|
||||
* The `create` text is the CANONICAL definition — it MUST match the migration's
|
||||
* `IF NOT EXISTS` create expression exactly (same functional expression + opclass)
|
||||
* or Postgres would treat them as two different indexes.
|
||||
*/
|
||||
export const CONCURRENT_INDEXES: ReadonlyArray<{
|
||||
name: string;
|
||||
create: string;
|
||||
}> = [
|
||||
{
|
||||
// #348 perf-indexes — pages.title trigram (coalesce-free functional expr).
|
||||
name: 'idx_pages_title_trgm',
|
||||
create:
|
||||
'CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_pages_title_trgm ' +
|
||||
'ON pages USING gin ((LOWER(f_unaccent(title))) gin_trgm_ops)',
|
||||
},
|
||||
{
|
||||
// #348 perf-indexes — users.name trigram (member search-suggest).
|
||||
name: 'idx_users_name_trgm',
|
||||
create:
|
||||
'CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_users_name_trgm ' +
|
||||
'ON users USING gin ((LOWER(f_unaccent(name))) gin_trgm_ops)',
|
||||
},
|
||||
{
|
||||
// #443 search-lookup-trgm — pages.text_content trigram (the slow, large one).
|
||||
name: 'idx_pages_text_content_trgm',
|
||||
create:
|
||||
'CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_pages_text_content_trgm ' +
|
||||
'ON pages USING gin ((LOWER(f_unaccent(text_content))) gin_trgm_ops)',
|
||||
},
|
||||
];
|
||||
|
||||
/**
|
||||
* Idempotent, output-identical redefinition of `f_unaccent` to the INDEX-INLINABLE
|
||||
* 1-arg form. MUST stay byte-for-byte in lockstep with migration
|
||||
* `20260705T120000-perf-indexes.ts` (same signature + body): the trigram indexes
|
||||
* above only build CONCURRENTLY once `f_unaccent(text)` inlines, and on an existing
|
||||
* tenant it is still the old 2-arg form until that migration runs — which is AFTER
|
||||
* this pre-build.
|
||||
*/
|
||||
const F_UNACCENT_REDEF = `
|
||||
CREATE OR REPLACE FUNCTION f_unaccent(text)
|
||||
RETURNS text
|
||||
LANGUAGE sql
|
||||
IMMUTABLE PARALLEL SAFE STRICT
|
||||
AS $func$
|
||||
SELECT public.unaccent($1);
|
||||
$func$
|
||||
`;
|
||||
|
||||
/**
|
||||
* Best-effort, non-transactional pre-build of {@link CONCURRENT_INDEXES}. Run
|
||||
* BEFORE the migrator so the blocking `CREATE INDEX` in the corresponding
|
||||
* migration becomes an `IF NOT EXISTS` no-op.
|
||||
*
|
||||
* `db` MUST be the top-level Kysely instance (NOT a transaction): each statement
|
||||
* then executes on its own connection with no surrounding `BEGIN`, which is
|
||||
* required for `CONCURRENTLY`. Every statement is independent and swallowed on
|
||||
* error: the migration backstop still builds the index, so a failure here is
|
||||
* never fatal. `onLog` reports progress/failures for the caller to route to its
|
||||
* logger.
|
||||
*
|
||||
* ORDER MATTERS: redefine `f_unaccent` to the inlinable form BEFORE the index
|
||||
* loop — otherwise an existing tenant's old 2-arg `f_unaccent` makes every
|
||||
* CONCURRENTLY build fail and the feature is a no-op (see the module docstring).
|
||||
*/
|
||||
export async function ensureConcurrentIndexes(
|
||||
db: Kysely<any>,
|
||||
onLog?: (message: string, error?: unknown) => void,
|
||||
): Promise<void> {
|
||||
// Make f_unaccent inlinable FIRST. On a fresh DB (no `unaccent` extension yet)
|
||||
// this throws harmlessly and is swallowed — the migration builds everything.
|
||||
try {
|
||||
await sql.raw(F_UNACCENT_REDEF).execute(db);
|
||||
onLog?.('f_unaccent redefined to the index-inlinable 1-arg form');
|
||||
} catch (error) {
|
||||
onLog?.(
|
||||
'f_unaccent redefine skipped (fresh DB / no unaccent extension) — ' +
|
||||
'the migration will define it and build the indexes',
|
||||
error,
|
||||
);
|
||||
}
|
||||
|
||||
for (const idx of CONCURRENT_INDEXES) {
|
||||
try {
|
||||
await sql.raw(idx.create).execute(db);
|
||||
onLog?.(`Concurrent index ensured: ${idx.name}`);
|
||||
} catch (error) {
|
||||
// Non-fatal by design — the migration's IF NOT EXISTS create is the
|
||||
// backstop. Benign on a fresh DB (the table/extension does not exist yet).
|
||||
// NOT benign, but still swallowed, on an existing tenant whose f_unaccent
|
||||
// could not be redefined above (then the migration builds the index NON-
|
||||
// concurrently under the SHARE lock this feature meant to avoid).
|
||||
onLog?.(
|
||||
`Concurrent index pre-build skipped for ${idx.name} ` +
|
||||
`(will fall back to the in-migration build)`,
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,113 @@
|
||||
import { executeTx, registerAfterCommit } from './utils';
|
||||
import { KyselyDB, KyselyTransaction } from './types/kysely.types';
|
||||
|
||||
// Post-commit hook contract (#495 item 13): a side effect registered via
|
||||
// registerAfterCommit must run ONLY AFTER the owning transaction commits, and a
|
||||
// hook registered against a passed-through existingTrx must fire at the OUTER
|
||||
// commit boundary — never inside the inner call. We fake the Kysely transaction
|
||||
// runner so the ordering is observable without a real DB.
|
||||
|
||||
/**
|
||||
* A minimal db whose `.transaction().execute(cb)` records the commit ORDER: it
|
||||
* runs `cb(trx)`, pushes 'commit' onto `log` (simulating the real commit that
|
||||
* happens after the callback resolves), then returns the callback's result.
|
||||
*/
|
||||
function fakeDb(log: string[]): { db: KyselyDB; trx: KyselyTransaction } {
|
||||
const trx = { __fakeTrx: true } as unknown as KyselyTransaction;
|
||||
const db = {
|
||||
transaction: () => ({
|
||||
execute: async (cb: (t: KyselyTransaction) => Promise<unknown>) => {
|
||||
const result = await cb(trx);
|
||||
log.push('commit');
|
||||
return result;
|
||||
},
|
||||
}),
|
||||
} as unknown as KyselyDB;
|
||||
return { db, trx };
|
||||
}
|
||||
|
||||
describe('executeTx post-commit hooks', () => {
|
||||
it('runs an afterCommit hook only AFTER the transaction commits', async () => {
|
||||
const log: string[] = [];
|
||||
const { db } = fakeDb(log);
|
||||
|
||||
await executeTx(db, async (trx) => {
|
||||
log.push('body');
|
||||
registerAfterCommit(trx, () => {
|
||||
log.push('hook');
|
||||
});
|
||||
// The hook must NOT have run yet — the tx is still open.
|
||||
expect(log).toEqual(['body']);
|
||||
});
|
||||
|
||||
// Order proves post-commit: body → commit → hook (never body → hook → commit).
|
||||
expect(log).toEqual(['body', 'commit', 'hook']);
|
||||
});
|
||||
|
||||
it('drains hooks registered against a passed-through existingTrx at the OUTER commit', async () => {
|
||||
const log: string[] = [];
|
||||
const { db, trx: outerTrx } = fakeDb(log);
|
||||
|
||||
await executeTx(db, async (outer) => {
|
||||
// Nested executeTx reuses the outer trx: it must NOT commit or drain now.
|
||||
await executeTx(
|
||||
db,
|
||||
async (inner) => {
|
||||
registerAfterCommit(inner, () => {
|
||||
log.push('inner-hook');
|
||||
});
|
||||
},
|
||||
outer,
|
||||
);
|
||||
// Still inside the outer tx — the inner hook has not fired.
|
||||
expect(log).toEqual([]);
|
||||
});
|
||||
|
||||
// The single (outer) commit drains the hook registered on the shared trx.
|
||||
expect(log).toEqual(['commit', 'inner-hook']);
|
||||
// Sanity: the trx the hooks were registered against is the outer one.
|
||||
expect(outerTrx).toBeDefined();
|
||||
});
|
||||
|
||||
it('a hook failure does not reject the already-committed executeTx', async () => {
|
||||
const log: string[] = [];
|
||||
const { db } = fakeDb(log);
|
||||
|
||||
await expect(
|
||||
executeTx(db, async (trx) => {
|
||||
registerAfterCommit(trx, () => {
|
||||
throw new Error('cache del blew up');
|
||||
});
|
||||
registerAfterCommit(trx, () => {
|
||||
log.push('second-hook-still-runs');
|
||||
});
|
||||
return 'ok';
|
||||
}),
|
||||
).resolves.toBe('ok');
|
||||
|
||||
// The throwing hook is swallowed; a later hook still runs.
|
||||
expect(log).toEqual(['commit', 'second-hook-still-runs']);
|
||||
});
|
||||
|
||||
it('does NOT run afterCommit hooks when the transaction body throws (rollback)', async () => {
|
||||
// The body rejects -> the fake transaction never pushes 'commit' and
|
||||
// db.transaction().execute() rejects, mirroring a real rolled-back tx. The
|
||||
// drain runs only AFTER the awaited (committed) transaction, so a rollback
|
||||
// must leave every registered hook UN-run — otherwise a cache-bust / event
|
||||
// would fire for a write that never landed.
|
||||
const log: string[] = [];
|
||||
const { db } = fakeDb(log);
|
||||
const hook = jest.fn();
|
||||
|
||||
await expect(
|
||||
executeTx(db, async (trx) => {
|
||||
registerAfterCommit(trx, hook);
|
||||
throw new Error('write failed -> rollback');
|
||||
}),
|
||||
).rejects.toThrow('write failed -> rollback');
|
||||
|
||||
// No commit happened, and the post-commit hook never ran.
|
||||
expect(log).toEqual([]); // no 'commit'
|
||||
expect(hook).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -33,16 +33,18 @@ import { type Kysely, sql } from 'kysely';
|
||||
* - comments: `findPageComments` does WHERE page_id ORDER BY id ASC, but only
|
||||
* `(page_id)` exists → extra sort.
|
||||
*
|
||||
* DEPLOY-TIME LOCK WARNING: these are plain (non-CONCURRENT) CREATE INDEX
|
||||
* statements — CONCURRENTLY is impossible because Kysely runs each migration in a
|
||||
* transaction. They take a SHARE lock that BLOCKS writes (INSERT/UPDATE/DELETE) on
|
||||
* pages/users/groups/comments/page_history for the duration of the build. The two
|
||||
* GIN trigram builds on pages.title / users.name are the slow ones and can take
|
||||
* minutes on a large tenant → a write-outage window during the deploy migration.
|
||||
* For large installations, run this migration in a maintenance window, or build
|
||||
* the trigram indexes out-of-band with CREATE INDEX CONCURRENTLY before deploying
|
||||
* (then this migration's `IF NOT EXISTS` is a no-op). Small/typical tenants are
|
||||
* unaffected.
|
||||
* DEPLOY-TIME LOCK: these are plain (non-CONCURRENT) CREATE INDEX statements —
|
||||
* CONCURRENTLY is impossible HERE because Kysely runs each migration in a
|
||||
* transaction. The two GIN trigram builds on pages.title / users.name are the
|
||||
* slow ones and would take a SHARE lock that BLOCKS writes on pages/users for
|
||||
* minutes on a large tenant. To avoid that, `ensureConcurrentIndexes`
|
||||
* (database/concurrent-indexes.ts) now pre-builds BOTH trigram indexes with
|
||||
* CREATE INDEX CONCURRENTLY (no transaction) BEFORE the migrator runs, so on an
|
||||
* existing DB the two `IF NOT EXISTS` trigram creates below no-op and no write
|
||||
* lock is taken. On a fresh DB the pre-build is skipped and they build on an
|
||||
* empty table. Keep the two trigram creates in lockstep with their CANONICAL
|
||||
* definitions in CONCURRENT_INDEXES. (The plain b-tree indexes further down are
|
||||
* fast metadata-only builds; they are not pre-built.)
|
||||
*/
|
||||
export async function up(db: Kysely<any>): Promise<void> {
|
||||
// Index-compatible, output-identical redefinition of f_unaccent (see header).
|
||||
|
||||
@@ -26,13 +26,16 @@ import { type Kysely, sql } from 'kysely';
|
||||
* to update than b-trees); on the small instances this fork targets that cost
|
||||
* is acceptable and the read win on agent lookups is the priority.
|
||||
*
|
||||
* DEPLOY-TIME LOCK WARNING: plain (non-CONCURRENT) CREATE INDEX — Kysely runs
|
||||
* each migration in a transaction, so CONCURRENTLY is impossible. The build takes
|
||||
* a SHARE lock that BLOCKS writes on `pages` for its duration. The text_content
|
||||
* GIN build is the slow one and can take minutes on a large tenant. For large
|
||||
* installations, run this in a maintenance window or build the index out-of-band
|
||||
* with CREATE INDEX CONCURRENTLY before deploying (then `IF NOT EXISTS` no-ops
|
||||
* here). Small/typical tenants are unaffected.
|
||||
* DEPLOY-TIME LOCK: this is a plain (non-CONCURRENT) CREATE INDEX — Kysely runs
|
||||
* each migration in a transaction, so CONCURRENTLY is impossible HERE, and the
|
||||
* build would take a SHARE lock that BLOCKS writes on `pages` for its duration
|
||||
* (the text_content GIN build can take minutes on a large tenant). To avoid that,
|
||||
* `ensureConcurrentIndexes` (database/concurrent-indexes.ts) now pre-builds this
|
||||
* index with CREATE INDEX CONCURRENTLY (no transaction) BEFORE the migrator runs,
|
||||
* so on an existing DB the `IF NOT EXISTS` below no-ops and no write lock is taken.
|
||||
* On a fresh DB the pre-build is skipped and this builds it on an empty table
|
||||
* where the lock is irrelevant. Keep this create in lockstep with the CANONICAL
|
||||
* definition in CONCURRENT_INDEXES (same expression + opclass).
|
||||
*/
|
||||
export async function up(db: Kysely<any>): Promise<void> {
|
||||
// The title predicate is served by #348's idx_pages_title_trgm — see header.
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB, KyselyTransaction } from '../../types/kysely.types';
|
||||
import { dbOrTx, executeTx } from '../../utils';
|
||||
import { dbOrTx, executeTx, registerAfterCommit } from '../../utils';
|
||||
import {
|
||||
InsertablePage,
|
||||
Page,
|
||||
@@ -349,14 +349,23 @@ export class PageRepo {
|
||||
pageId: string,
|
||||
deletedById: string,
|
||||
workspaceId: string,
|
||||
// Optional caller transaction. When passed, the reads + soft-delete run in
|
||||
// THAT transaction (so a caller holding a `FOR UPDATE` lock on the row — e.g.
|
||||
// the temporary-note sweeper — can delete under the lock without deadlocking
|
||||
// on a nested independent transaction) and the PAGE_SOFT_DELETED broadcast is
|
||||
// deferred to the caller's COMMIT via registerAfterCommit (so a rolled-back
|
||||
// delete never broadcasts). With no trx the behaviour is unchanged: own
|
||||
// transaction, broadcast right after it commits.
|
||||
existingTrx?: KyselyTransaction,
|
||||
): Promise<void> {
|
||||
const currentDate = new Date();
|
||||
const readDb = dbOrTx(this.db, existingTrx);
|
||||
|
||||
// Read the root snapshot up front so PAGE_SOFT_DELETED can carry it without
|
||||
// a post-commit DB read (variant A). Only the root of the deleted subtree is
|
||||
// needed for the tree broadcast — the client `treeModel.remove` drops all
|
||||
// descendants, so we don't snapshot/broadcast every descendant.
|
||||
const rootSnapshot = await this.db
|
||||
const rootSnapshot = await readDb
|
||||
.selectFrom('pages')
|
||||
.select([
|
||||
'id',
|
||||
@@ -371,7 +380,7 @@ export class PageRepo {
|
||||
.where('deletedAt', 'is', null)
|
||||
.executeTakeFirst();
|
||||
|
||||
const descendants = await this.db
|
||||
const descendants = await readDb
|
||||
.withRecursive('page_descendants', (db) =>
|
||||
db
|
||||
.selectFrom('pages')
|
||||
@@ -393,39 +402,60 @@ export class PageRepo {
|
||||
const pageIds = descendants.map((d) => d.id);
|
||||
|
||||
if (pageIds.length > 0) {
|
||||
await executeTx(this.db, async (trx) => {
|
||||
await trx
|
||||
.updateTable('pages')
|
||||
.set({
|
||||
deletedById: deletedById,
|
||||
deletedAt: currentDate,
|
||||
})
|
||||
.where('id', 'in', pageIds)
|
||||
.where('deletedAt', 'is', null)
|
||||
.execute();
|
||||
// Reuse the caller's transaction when given (executeTx passes it straight
|
||||
// through), else own a fresh one.
|
||||
await executeTx(
|
||||
this.db,
|
||||
async (trx) => {
|
||||
await trx
|
||||
.updateTable('pages')
|
||||
.set({
|
||||
deletedById: deletedById,
|
||||
deletedAt: currentDate,
|
||||
})
|
||||
.where('id', 'in', pageIds)
|
||||
.where('deletedAt', 'is', null)
|
||||
.execute();
|
||||
|
||||
await trx.deleteFrom('shares').where('pageId', 'in', pageIds).execute();
|
||||
});
|
||||
await trx
|
||||
.deleteFrom('shares')
|
||||
.where('pageId', 'in', pageIds)
|
||||
.execute();
|
||||
},
|
||||
existingTrx,
|
||||
);
|
||||
|
||||
this.eventEmitter.emit(EventName.PAGE_SOFT_DELETED, {
|
||||
pageIds: pageIds,
|
||||
workspaceId,
|
||||
// Root-only snapshot: one `deleteTreeNode` is enough, the client removes
|
||||
// the whole subtree. Skip if the root vanished between the two reads.
|
||||
pages: rootSnapshot
|
||||
? [
|
||||
{
|
||||
id: rootSnapshot.id,
|
||||
slugId: rootSnapshot.slugId,
|
||||
title: rootSnapshot.title,
|
||||
icon: rootSnapshot.icon,
|
||||
position: rootSnapshot.position,
|
||||
spaceId: rootSnapshot.spaceId,
|
||||
parentPageId: rootSnapshot.parentPageId,
|
||||
},
|
||||
]
|
||||
: [],
|
||||
});
|
||||
const emitSoftDeleted = () => {
|
||||
this.eventEmitter.emit(EventName.PAGE_SOFT_DELETED, {
|
||||
pageIds: pageIds,
|
||||
workspaceId,
|
||||
// Root-only snapshot: one `deleteTreeNode` is enough, the client
|
||||
// removes the whole subtree. Skip if the root vanished between reads.
|
||||
pages: rootSnapshot
|
||||
? [
|
||||
{
|
||||
id: rootSnapshot.id,
|
||||
slugId: rootSnapshot.slugId,
|
||||
title: rootSnapshot.title,
|
||||
icon: rootSnapshot.icon,
|
||||
position: rootSnapshot.position,
|
||||
spaceId: rootSnapshot.spaceId,
|
||||
parentPageId: rootSnapshot.parentPageId,
|
||||
},
|
||||
]
|
||||
: [],
|
||||
});
|
||||
};
|
||||
|
||||
if (existingTrx) {
|
||||
// Inside a caller transaction: the delete above is NOT committed yet.
|
||||
// Defer the tree broadcast to the caller's commit so a rolled-back delete
|
||||
// never broadcasts a phantom removal.
|
||||
registerAfterCommit(existingTrx, emitSoftDeleted);
|
||||
} else {
|
||||
// Own transaction already committed above — broadcast now.
|
||||
emitSoftDeleted();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import { CACHE_MANAGER } from '@nestjs/cache-manager';
|
||||
import { Cache } from 'cache-manager';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB, KyselyTransaction } from '../../types/kysely.types';
|
||||
import { dbOrTx } from '../../utils';
|
||||
import { dbOrTx, registerAfterCommit } from '../../utils';
|
||||
import {
|
||||
InsertableWorkspace,
|
||||
UpdatableWorkspace,
|
||||
@@ -80,16 +80,30 @@ export class WorkspaceRepo {
|
||||
*/
|
||||
private async bustWorkspaceCache(
|
||||
workspace?: Pick<Workspace, 'hostname'> | undefined,
|
||||
trx?: KyselyTransaction,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await this.cacheManager.del(CacheKey.WORKSPACE_SELF_HOSTED);
|
||||
if (workspace?.hostname) {
|
||||
await this.cacheManager.del(
|
||||
CacheKey.WORKSPACE_BY_HOST(workspace.hostname),
|
||||
);
|
||||
const del = async () => {
|
||||
try {
|
||||
await this.cacheManager.del(CacheKey.WORKSPACE_SELF_HOSTED);
|
||||
if (workspace?.hostname) {
|
||||
await this.cacheManager.del(
|
||||
CacheKey.WORKSPACE_BY_HOST(workspace.hostname),
|
||||
);
|
||||
}
|
||||
} catch {
|
||||
// cache is best-effort; TTL is the backstop
|
||||
}
|
||||
} catch {
|
||||
// cache is best-effort; TTL is the backstop
|
||||
};
|
||||
if (trx) {
|
||||
// Inside a caller transaction the write is NOT yet committed: busting now
|
||||
// opens a repopulation window (a concurrent reader reloads the cache with
|
||||
// the pre-commit / stale row, which then survives until TTL). Defer the del
|
||||
// to the transaction's commit (drained by the owning executeTx) (#495).
|
||||
registerAfterCommit(trx, del);
|
||||
} else {
|
||||
// No transaction: the mutation above already auto-committed, so this del is
|
||||
// already post-commit.
|
||||
await del();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -180,7 +194,7 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@@ -195,7 +209,7 @@ export class WorkspaceRepo {
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
// Bust the cached "not found" so a fresh install / new tenant is seen at once.
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@@ -249,7 +263,7 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@@ -271,7 +285,7 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@@ -326,7 +340,7 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@@ -354,7 +368,7 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@@ -376,7 +390,7 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
@@ -398,7 +412,7 @@ export class WorkspaceRepo {
|
||||
.where('id', '=', workspaceId)
|
||||
.returning(this.baseFields)
|
||||
.executeTakeFirst();
|
||||
await this.bustWorkspaceCache(workspace);
|
||||
await this.bustWorkspaceCache(workspace, trx);
|
||||
return workspace;
|
||||
}
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ import { promises as fs } from 'fs';
|
||||
import { Migrator, FileMigrationProvider } from 'kysely';
|
||||
import { InjectKysely } from 'nestjs-kysely';
|
||||
import { KyselyDB } from '@docmost/db/types/kysely.types';
|
||||
import { ensureConcurrentIndexes } from '@docmost/db/concurrent-indexes';
|
||||
|
||||
@Injectable()
|
||||
export class MigrationService {
|
||||
@@ -12,6 +13,16 @@ export class MigrationService {
|
||||
constructor(@InjectKysely() private readonly db: KyselyDB) {}
|
||||
|
||||
async migrateToLatest(): Promise<void> {
|
||||
// Build write-blocking trigram indexes CONCURRENTLY (no transaction) BEFORE
|
||||
// the migrator runs, so the corresponding in-migration `CREATE INDEX IF NOT
|
||||
// EXISTS` no-ops instead of taking a SHARE lock on `pages` during deploy
|
||||
// (#495). Best-effort: on a fresh DB (no `pages`/`f_unaccent` yet) this is a
|
||||
// no-op and the migrations build the index normally.
|
||||
await ensureConcurrentIndexes(this.db, (message, error) => {
|
||||
if (error) this.logger.warn(`${message}: ${String(error)}`);
|
||||
else this.logger.log(message);
|
||||
});
|
||||
|
||||
const migrator = new Migrator({
|
||||
db: this.db,
|
||||
provider: new FileMigrationProvider({
|
||||
|
||||
@@ -6,16 +6,76 @@ import { KyselyDB, KyselyTransaction } from './types/kysely.types';
|
||||
* If an existing transaction is provided, it directly executes the callback with it.
|
||||
* Otherwise, it starts a new transaction using the provided database instance and executes the callback within that transaction.
|
||||
*/
|
||||
/**
|
||||
* Post-commit side-effect hooks, keyed by the transaction they were registered
|
||||
* against. A WeakMap so an abandoned/never-drained transaction's entry is GC'd
|
||||
* with the trx object (no leak). Used by {@link registerAfterCommit} /
|
||||
* {@link executeTx}.
|
||||
*/
|
||||
const afterCommitHooks = new WeakMap<
|
||||
KyselyTransaction,
|
||||
Array<() => Promise<void> | void>
|
||||
>();
|
||||
|
||||
/**
|
||||
* Register a side effect to run ONLY AFTER the transaction that owns `trx`
|
||||
* commits. THE fix for "bust the cache inside the open transaction" bugs: a
|
||||
* cache-invalidation (or any read-your-write-visible side effect) done while the
|
||||
* writing transaction is still open opens a window where a concurrent reader
|
||||
* repopulates the cache with the PRE-COMMIT (stale) row, so after commit the
|
||||
* cache holds the old value until its TTL. Deferring the effect to post-commit
|
||||
* closes that window.
|
||||
*
|
||||
* The hook is drained by the OUTERMOST {@link executeTx} that actually owns
|
||||
* (created) this transaction — so registering against a passed-through
|
||||
* `existingTrx` still fires at the real commit boundary, not at the inner call.
|
||||
* NOTE: a hook registered against a transaction that was NOT created via
|
||||
* `executeTx` (untracked) will never be drained — always create transactions
|
||||
* through `executeTx` when you rely on post-commit hooks.
|
||||
*/
|
||||
export function registerAfterCommit(
|
||||
trx: KyselyTransaction,
|
||||
hook: () => Promise<void> | void,
|
||||
): void {
|
||||
const existing = afterCommitHooks.get(trx);
|
||||
if (existing) existing.push(hook);
|
||||
else afterCommitHooks.set(trx, [hook]);
|
||||
}
|
||||
|
||||
export async function executeTx<T>(
|
||||
db: KyselyDB,
|
||||
callback: (trx: KyselyTransaction) => Promise<T>,
|
||||
existingTrx?: KyselyTransaction,
|
||||
): Promise<T> {
|
||||
if (existingTrx) {
|
||||
return await callback(existingTrx); // Execute callback with existing transaction
|
||||
} else {
|
||||
return await db.transaction().execute((trx) => callback(trx)); // Start new transaction and execute callback
|
||||
// Reuse the caller's transaction. Any post-commit hooks registered here are
|
||||
// drained by the OUTER executeTx that created `existingTrx`, at the true
|
||||
// commit boundary — so we must NOT drain them now.
|
||||
return await callback(existingTrx);
|
||||
}
|
||||
// We OWN this transaction: run the body, then (only once it has COMMITTED)
|
||||
// drain the post-commit hooks registered against it during the body.
|
||||
let ownTrx: KyselyTransaction | undefined;
|
||||
const result = await db.transaction().execute((trx) => {
|
||||
ownTrx = trx;
|
||||
return callback(trx);
|
||||
});
|
||||
if (ownTrx) {
|
||||
const hooks = afterCommitHooks.get(ownTrx);
|
||||
if (hooks) {
|
||||
afterCommitHooks.delete(ownTrx);
|
||||
for (const hook of hooks) {
|
||||
// Best-effort: a failed side effect (e.g. a cache del) must not fail the
|
||||
// already-committed transaction.
|
||||
try {
|
||||
await hook();
|
||||
} catch {
|
||||
// swallow — the durable write already committed
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
@@ -99,10 +99,12 @@ describe('AiSettingsService.getMasked reindex progress', () => {
|
||||
// actually pins the progress.total branch rather than coincidentally
|
||||
// matching the DB fallback. With fix #1 the two sources agree in practice,
|
||||
// but getMasked must still return progress.total when a record is active.
|
||||
const startedAt = Date.now();
|
||||
reindexProgress.get.mockResolvedValue({
|
||||
total: 500,
|
||||
done: 120,
|
||||
startedAt: Date.now(),
|
||||
startedAt,
|
||||
runId: 'run-abc',
|
||||
});
|
||||
|
||||
const masked = await service.getMasked(WORKSPACE_ID);
|
||||
@@ -110,6 +112,10 @@ describe('AiSettingsService.getMasked reindex progress', () => {
|
||||
expect(masked.indexedPages).toBe(120); // progress.done, not DB 478
|
||||
expect(masked.totalPages).toBe(500); // progress.total, not DB 478
|
||||
expect(masked.reindexing).toBe(true);
|
||||
// The status payload must carry the run identity so the client can key its
|
||||
// poll on it (a changed runId => a NEW run).
|
||||
expect(masked.runId).toBe('run-abc');
|
||||
expect(masked.reindexStartedAt).toBe(startedAt);
|
||||
});
|
||||
|
||||
it('falls back to countIndexedPages when no reindex is active', async () => {
|
||||
@@ -121,6 +127,10 @@ describe('AiSettingsService.getMasked reindex progress', () => {
|
||||
expect(masked.indexedPages).toBe(478);
|
||||
expect(masked.totalPages).toBe(478);
|
||||
expect(masked.reindexing).toBe(false);
|
||||
// No active run -> no run identity surfaced (the client keeps its prior
|
||||
// steady-state behaviour).
|
||||
expect(masked.runId).toBeUndefined();
|
||||
expect(masked.reindexStartedAt).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -371,6 +371,12 @@ export class AiSettingsService {
|
||||
totalPages,
|
||||
// Optional hint for the client: a reindex run is currently in progress.
|
||||
reindexing: progress != null,
|
||||
// Per-run identity so the client can key its poll on a stable run id and
|
||||
// reset its per-run state when a NEW run starts. Present only while a run
|
||||
// is active; `runId` may be '' for a legacy/degraded record (the client
|
||||
// treats that as "no identity").
|
||||
runId: progress?.runId,
|
||||
reindexStartedAt: progress?.startedAt,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
// `.provider` alone cannot prove the gemini/ollama chat factories were built
|
||||
// with the instrumented streaming fetch — a regression dropping it (which drops
|
||||
// them back to the global undici fetch: no keep-alive recycle, no reset retries,
|
||||
// unbounded silence timeout; incident classes #140/#175/#310) would still pass.
|
||||
// So mock the factories and assert the exact fetch argument. jest.mock is
|
||||
// module-scoped, hence a dedicated file.
|
||||
|
||||
const mockGeminiModel = { provider: 'google.generative-ai', modelId: 'm' };
|
||||
const mockOllamaModel = { provider: 'ollama.chat', modelId: 'm' };
|
||||
|
||||
// jest allows `mock`-prefixed vars inside a jest.mock factory.
|
||||
const mockCreateGoogle = jest.fn((_settings: unknown) => () => mockGeminiModel);
|
||||
const mockCreateOllama = jest.fn((_settings: unknown) => () => mockOllamaModel);
|
||||
|
||||
jest.mock('@ai-sdk/google', () => ({
|
||||
createGoogleGenerativeAI: (settings: unknown) => mockCreateGoogle(settings),
|
||||
}));
|
||||
jest.mock('ai-sdk-ollama', () => ({
|
||||
createOllama: (settings: unknown) => mockCreateOllama(settings),
|
||||
}));
|
||||
|
||||
import { AiService } from './ai.service';
|
||||
|
||||
describe('AiService.getChatModel provider transport fetch (gemini/ollama)', () => {
|
||||
function serviceWith(cfg: Record<string, unknown>) {
|
||||
const aiSettings = {
|
||||
resolve: jest.fn().mockResolvedValue(cfg),
|
||||
};
|
||||
return new AiService(
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
aiSettings as any,
|
||||
{ find: jest.fn() } as never,
|
||||
{ decryptSecret: jest.fn() } as never,
|
||||
);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mockCreateGoogle.mockClear();
|
||||
mockCreateOllama.mockClear();
|
||||
});
|
||||
|
||||
it('builds the gemini chat model with the instrumented streaming fetch', async () => {
|
||||
await serviceWith({
|
||||
driver: 'gemini',
|
||||
chatModel: 'gemini-2.5-pro',
|
||||
apiKey: 'the-key',
|
||||
}).getChatModel('ws-1');
|
||||
expect(mockCreateGoogle).toHaveBeenCalledTimes(1);
|
||||
expect(mockCreateGoogle).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
apiKey: 'the-key',
|
||||
fetch: expect.any(Function),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('builds the ollama chat model with the instrumented streaming fetch', async () => {
|
||||
await serviceWith({
|
||||
driver: 'ollama',
|
||||
chatModel: 'llama3',
|
||||
baseUrl: 'http://localhost:11434/api',
|
||||
}).getChatModel('ws-1');
|
||||
expect(mockCreateOllama).toHaveBeenCalledTimes(1);
|
||||
expect(mockCreateOllama).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
baseURL: 'http://localhost:11434/api',
|
||||
fetch: expect.any(Function),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('reuses ONE service-lifetime fetch instance across both providers', async () => {
|
||||
const svc = serviceWith({
|
||||
driver: 'gemini',
|
||||
chatModel: 'gemini-2.5-pro',
|
||||
apiKey: 'k',
|
||||
});
|
||||
await svc.getChatModel('ws-1');
|
||||
const geminiFetch = mockCreateGoogle.mock.calls[0][0] as { fetch: unknown };
|
||||
// Same instance on a second call — the fetch is held for the service
|
||||
// lifetime to reuse the streaming dispatcher's connection pool.
|
||||
await svc.getChatModel('ws-1');
|
||||
const geminiFetch2 = mockCreateGoogle.mock.calls[1][0] as { fetch: unknown };
|
||||
expect(geminiFetch.fetch).toBe(geminiFetch2.fetch);
|
||||
});
|
||||
});
|
||||
@@ -190,10 +190,22 @@ export class AiService {
|
||||
}).chat(chatModel);
|
||||
}
|
||||
case 'gemini':
|
||||
return createGoogleGenerativeAI({ apiKey })(chatModel);
|
||||
// Route gemini through the same instrumented streaming fetch as openai
|
||||
// (finite silence timeouts + keep-alive recycling + pre-response
|
||||
// connection-reset retry). Without it the provider ran on the global
|
||||
// undici fetch — no keep-alive recycle, no reset retries, default
|
||||
// (unbounded silence) timeout — so incident classes #140/#175/#310 were
|
||||
// reproducible for gemini too.
|
||||
return createGoogleGenerativeAI({
|
||||
apiKey,
|
||||
fetch: this.aiProviderFetch,
|
||||
})(chatModel);
|
||||
case 'ollama':
|
||||
// Ollama needs no API key.
|
||||
return createOllama({ baseURL: baseUrl })(chatModel);
|
||||
// Ollama needs no API key. Same transport hardening as above (#140/#175/#310).
|
||||
return createOllama({
|
||||
baseURL: baseUrl,
|
||||
fetch: this.aiProviderFetch,
|
||||
})(chatModel);
|
||||
default:
|
||||
throw new AiNotConfiguredException();
|
||||
}
|
||||
|
||||
@@ -149,4 +149,14 @@ export interface MaskedAiSettings {
|
||||
// True while a full workspace reindex is actively running (the counts above
|
||||
// then reflect the live run progress rather than the steady-state DB count).
|
||||
reindexing?: boolean;
|
||||
// Identity of the ACTIVE reindex run (present only while `reindexing`). The
|
||||
// client keys its poll on `runId`: a changed value means a NEW run (reset the
|
||||
// per-run poll state it latched), the same value means the run it is already
|
||||
// watching — removing the "same run or a fresh one?" ambiguity a stale
|
||||
// pre-reindex snapshot otherwise causes. Absent/empty degrades gracefully.
|
||||
runId?: string;
|
||||
// Epoch-ms the active run started (present only while `reindexing`). Paired
|
||||
// with `runId` so a run that restarts with the same (recycled) id is still
|
||||
// seen as new.
|
||||
reindexStartedAt?: number;
|
||||
}
|
||||
|
||||
@@ -48,19 +48,38 @@ describe('EmbeddingReindexProgressService', () => {
|
||||
}
|
||||
|
||||
describe('get', () => {
|
||||
it('maps a valid hash to a ReindexProgress object', async () => {
|
||||
it('maps a valid hash to a ReindexProgress object (incl. the run identity)', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({ total: '478', done: '120', startedAt: '1000' });
|
||||
hgetall.mockResolvedValue({
|
||||
total: '478',
|
||||
done: '120',
|
||||
startedAt: '1000',
|
||||
runId: 'run-xyz',
|
||||
});
|
||||
const service = makeService(redis);
|
||||
|
||||
await expect(service.get(WORKSPACE_ID)).resolves.toEqual({
|
||||
total: 478,
|
||||
done: 120,
|
||||
startedAt: 1000,
|
||||
runId: 'run-xyz',
|
||||
});
|
||||
expect(hgetall).toHaveBeenCalledWith(KEY);
|
||||
});
|
||||
|
||||
it('degrades a missing runId to an empty string (legacy/partial record)', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
// A record written before runId existed: get() must still succeed and
|
||||
// report runId='' so the client treats it as "no identity", never breaks.
|
||||
hgetall.mockResolvedValue({ total: '10', done: '3', startedAt: '5' });
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toEqual({
|
||||
total: 10,
|
||||
done: 3,
|
||||
startedAt: 5,
|
||||
runId: '',
|
||||
});
|
||||
});
|
||||
|
||||
it('returns null for an empty hash (no record)', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({});
|
||||
@@ -87,11 +106,17 @@ describe('EmbeddingReindexProgressService', () => {
|
||||
|
||||
it('coerces a non-finite startedAt to 0', async () => {
|
||||
const { redis, hgetall } = makeRedis();
|
||||
hgetall.mockResolvedValue({ total: '10', done: '2', startedAt: 'nope' });
|
||||
hgetall.mockResolvedValue({
|
||||
total: '10',
|
||||
done: '2',
|
||||
startedAt: 'nope',
|
||||
runId: 'run-1',
|
||||
});
|
||||
await expect(makeService(redis).get(WORKSPACE_ID)).resolves.toEqual({
|
||||
total: 10,
|
||||
done: 2,
|
||||
startedAt: 0,
|
||||
runId: 'run-1',
|
||||
});
|
||||
});
|
||||
|
||||
@@ -115,6 +140,21 @@ describe('EmbeddingReindexProgressService', () => {
|
||||
expect(multiObj.exec).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('mints a fresh non-empty runId into the record on each start', async () => {
|
||||
const { redis, multiObj } = makeRedis();
|
||||
const service = makeService(redis);
|
||||
await service.start(WORKSPACE_ID, 1);
|
||||
await service.start(WORKSPACE_ID, 1);
|
||||
|
||||
const firstRunId = multiObj.hset.mock.calls[0][1].runId;
|
||||
const secondRunId = multiObj.hset.mock.calls[1][1].runId;
|
||||
expect(typeof firstRunId).toBe('string');
|
||||
expect(firstRunId).not.toBe('');
|
||||
// Each run gets its OWN identity so the client can tell a re-trigger apart
|
||||
// from the run it is already watching.
|
||||
expect(secondRunId).not.toBe(firstRunId);
|
||||
});
|
||||
|
||||
it('defaults the expire TTL to the full 1h record TTL', async () => {
|
||||
const { redis, multiObj } = makeRedis();
|
||||
await makeService(redis).start(WORKSPACE_ID, 478);
|
||||
|
||||
@@ -1,17 +1,28 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { RedisService } from '@nestjs-labs/nestjs-ioredis';
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import type { Redis } from 'ioredis';
|
||||
|
||||
/**
|
||||
* Live progress of an in-flight workspace embeddings reindex run.
|
||||
* `total` is the number of pages the run will process, `done` how many it has
|
||||
* already processed (success OR handled failure), `startedAt` the epoch-ms the
|
||||
* record was created.
|
||||
* record was created, and `runId` a per-run identity minted at `start()`.
|
||||
*
|
||||
* `runId` gives each reindex run a stable identity so a poller can tell "same
|
||||
* run I've been watching" from "a NEW run started" WITHOUT guessing from the
|
||||
* progress counters (the ambiguity that a stale pre-reindex snapshot vs a fresh
|
||||
* run otherwise causes — the bug class fixed twice under #262). It is best-
|
||||
* effort like the rest of this record: a record written before this field
|
||||
* existed (or a Redis hiccup) yields an empty `runId`, which the client must
|
||||
* treat as "no identity available" and degrade to its prior behaviour, never
|
||||
* break.
|
||||
*/
|
||||
export interface ReindexProgress {
|
||||
total: number;
|
||||
done: number;
|
||||
startedAt: number;
|
||||
runId: string;
|
||||
}
|
||||
|
||||
/** Redis key namespace for the per-workspace reindex-progress record. */
|
||||
@@ -86,12 +97,18 @@ export class EmbeddingReindexProgressService {
|
||||
): Promise<void> {
|
||||
const key = this.key(workspaceId);
|
||||
try {
|
||||
// A fresh identity per run so the client poll can key on it: a changed
|
||||
// runId means a genuinely NEW run (reset any latched per-run poll state),
|
||||
// the same runId means the run the client is already watching. Best-effort
|
||||
// like the counters — never surfaced to the user, only used to disambiguate.
|
||||
const runId = randomUUID();
|
||||
await this.redis
|
||||
.multi()
|
||||
.hset(key, {
|
||||
total: String(total),
|
||||
done: '0',
|
||||
startedAt: String(Date.now()),
|
||||
runId,
|
||||
})
|
||||
.expire(key, ttlSeconds)
|
||||
.exec();
|
||||
@@ -150,7 +167,15 @@ export class EmbeddingReindexProgressService {
|
||||
const done = Number(data.done);
|
||||
const startedAt = Number(data.startedAt);
|
||||
if (!Number.isFinite(total) || !Number.isFinite(done)) return null;
|
||||
return { total, done, startedAt: Number.isFinite(startedAt) ? startedAt : 0 };
|
||||
// `runId` degrades gracefully: a pre-existing record (written before this
|
||||
// field) or a stripped value reads as '' — the client treats that as "no
|
||||
// identity" and keeps its prior behaviour rather than breaking the poll.
|
||||
return {
|
||||
total,
|
||||
done,
|
||||
startedAt: Number.isFinite(startedAt) ? startedAt : 0,
|
||||
runId: typeof data.runId === 'string' ? data.runId : '',
|
||||
};
|
||||
} catch (err) {
|
||||
this.logger.warn(
|
||||
`reindex-progress read failed for workspace ${workspaceId}; ` +
|
||||
|
||||
@@ -33,6 +33,11 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
const pageRepo = {
|
||||
findById: async (id: string) => ({ id, title: `title-${id}` }),
|
||||
};
|
||||
// The requester can view the target page (permissive), so the reassign 409 may
|
||||
// include its title — these tests exercise the one-per-page invariant, not the
|
||||
// #495 disclosure gate (that is unit-tested in share-alias.service.spec.ts).
|
||||
const pageAccessService = { validateCanView: async () => {} };
|
||||
const USER = { id: 'u-int' } as any;
|
||||
|
||||
beforeAll(async () => {
|
||||
db = getTestDb();
|
||||
@@ -41,6 +46,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
repo as any,
|
||||
pageRepo as any,
|
||||
{} as any, // shareService — unused by setAlias
|
||||
pageAccessService as any,
|
||||
db as any,
|
||||
);
|
||||
wsId = (await createWorkspace(db)).id;
|
||||
@@ -188,6 +194,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId,
|
||||
creatorId,
|
||||
user: USER,
|
||||
alias: 'te',
|
||||
});
|
||||
expect(first.alias).toBe('te');
|
||||
@@ -196,6 +203,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId,
|
||||
creatorId,
|
||||
user: USER,
|
||||
alias: 'ted',
|
||||
});
|
||||
// Same row id — a RENAME, not a new insert.
|
||||
@@ -217,12 +225,14 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'hello',
|
||||
});
|
||||
const again = await service.setAlias({
|
||||
workspaceId: wsId,
|
||||
pageId,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'hello',
|
||||
});
|
||||
expect(again.id).toBe(inserted.id);
|
||||
@@ -244,6 +254,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
flakyRepo as any,
|
||||
pageRepo as any,
|
||||
{} as any,
|
||||
pageAccessService as any,
|
||||
db as any,
|
||||
);
|
||||
|
||||
@@ -252,6 +263,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'rollback-me',
|
||||
}),
|
||||
).rejects.toBeInstanceOf(BadRequestException);
|
||||
@@ -275,6 +287,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId: pageA,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'shared',
|
||||
});
|
||||
|
||||
@@ -283,6 +296,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId: pageB,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'shared',
|
||||
}),
|
||||
).rejects.toBeInstanceOf(ConflictException);
|
||||
@@ -291,6 +305,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId: pageB,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'shared',
|
||||
confirmReassign: true,
|
||||
});
|
||||
@@ -317,12 +332,14 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId: pageA,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'shared-target',
|
||||
});
|
||||
await service.setAlias({
|
||||
workspaceId: wsId,
|
||||
pageId: pageB,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'bee',
|
||||
});
|
||||
|
||||
@@ -330,6 +347,7 @@ describe('share_aliases one-per-page invariant [integration]', () => {
|
||||
workspaceId: wsId,
|
||||
pageId: pageB,
|
||||
creatorId: null as any,
|
||||
user: USER,
|
||||
alias: 'shared-target',
|
||||
confirmReassign: true,
|
||||
});
|
||||
|
||||
@@ -33,6 +33,12 @@ import { TransformsMixin, type ITransformsMixin } from "./client/transforms.js";
|
||||
export type { DocmostMcpConfig, SandboxPut } from "./client/context.js";
|
||||
export { formatDocmostAxiosError, assertFullUuid } from "./client/errors.js";
|
||||
|
||||
// Branded canonical page-identity type (#435): the internal page UUID is a
|
||||
// distinct nominal type so an unresolved raw/slug string can't be swapped into
|
||||
// the seams that require the canonical id (see lib/page-id.ts). Re-exported on
|
||||
// the package surface for hosts that type against the resolved id.
|
||||
export type { PageId } from "./lib/page-id.js";
|
||||
|
||||
// The full public + shared instance surface of the assembled client. Built by
|
||||
// INTERSECTING each domain mixin's public interface (each DERIVED from its class
|
||||
// and enforced by that class's `implements` clause — issue #446, no hand-mirror)
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
isCollabAuthFailedError,
|
||||
} from "../lib/collab-session.js";
|
||||
import { withPageLock, isUuid } from "../lib/page-lock.js";
|
||||
import type { PageId } from "../lib/page-id.js";
|
||||
import { getCollabToken, performLogin } from "../lib/auth-utils.js";
|
||||
import { formatDocmostAxiosError } from "./errors.js";
|
||||
import { GetPageConversionCache } from "./getpage-cache.js";
|
||||
@@ -715,10 +716,18 @@ export abstract class DocmostClientContext {
|
||||
* once via getPageRaw and cached (both slugId->uuid and uuid->uuid), so
|
||||
* repeated edits on the same page add no extra request.
|
||||
*/
|
||||
protected async resolvePageId(pageId: string): Promise<string> {
|
||||
if (isUuid(pageId)) return pageId;
|
||||
protected async resolvePageId(pageId: string): Promise<PageId> {
|
||||
// This is the ONE canonicalization seam, so it is where the `PageId` brand
|
||||
// is minted (#435). The value is validated here — a UUID input by isUuid, a
|
||||
// resolved id as the server's own page.id — so the downstream write path
|
||||
// (withPageLock / mutatePageContent) can require the brand and reject any
|
||||
// unresolved raw id at compile time. The brand is a pure compile-time marker
|
||||
// applied by cast (no runtime guard): the guarantee is that this seam is the
|
||||
// only place a `PageId` is produced, so every branded value went through the
|
||||
// UUID/resolve check above.
|
||||
if (isUuid(pageId)) return pageId as PageId;
|
||||
const cached = this.pageIdCache.get(pageId);
|
||||
if (cached) return cached;
|
||||
if (cached) return cached as PageId;
|
||||
const data = await this.getPageRaw(pageId);
|
||||
const uuid = data?.id;
|
||||
if (typeof uuid !== "string" || !uuid) {
|
||||
@@ -727,7 +736,7 @@ export abstract class DocmostClientContext {
|
||||
);
|
||||
}
|
||||
this.pageIdCache.set(pageId, uuid);
|
||||
return uuid;
|
||||
return uuid as PageId;
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ import { JSDOM } from "jsdom";
|
||||
import { markdownToProseMirror } from "@docmost/prosemirror-markdown";
|
||||
import { docmostExtensions, docmostSchema } from "./docmost-schema.js";
|
||||
import { withPageLock } from "./page-lock.js";
|
||||
import type { PageId } from "./page-id.js";
|
||||
import {
|
||||
sanitizeForYjs,
|
||||
findUnstorableAttr,
|
||||
@@ -250,7 +251,10 @@ export function assertYjsEncodable(doc: any): void {
|
||||
* read->write window, and it never throws (it can NEVER break a write).
|
||||
*/
|
||||
export async function mutatePageContent(
|
||||
pageId: string,
|
||||
// Canonical UUID only (#260/#435): the brand forces every caller to
|
||||
// resolvePageId() BEFORE this seam so the lock + CollabSession key can never
|
||||
// be a raw slugId.
|
||||
pageId: PageId,
|
||||
collabToken: string,
|
||||
baseUrl: string,
|
||||
transform: (liveDoc: any) => any | null,
|
||||
@@ -300,7 +304,7 @@ export async function mutatePageContent(
|
||||
* mutatePageContent.
|
||||
*/
|
||||
export async function replacePageContent(
|
||||
pageId: string,
|
||||
pageId: PageId,
|
||||
prosemirrorDoc: any,
|
||||
collabToken: string,
|
||||
baseUrl: string,
|
||||
@@ -332,7 +336,7 @@ export async function replacePageContent(
|
||||
* Tables and :::callout::: blocks survive thanks to the full schema.
|
||||
*/
|
||||
export async function updatePageContentRealtime(
|
||||
pageId: string,
|
||||
pageId: PageId,
|
||||
markdownContent: string,
|
||||
collabToken: string,
|
||||
baseUrl: string,
|
||||
|
||||
@@ -0,0 +1,18 @@
|
||||
/**
|
||||
* Branded canonical page-identity type for the MCP client (incident family #435).
|
||||
*
|
||||
* A Docmost page has TWO identities that are both plain strings: the internal
|
||||
* `page.id` (a canonical UUID the server generates as UUIDv7) and the public
|
||||
* `slugId` (a 10-char nanoid used in URLs). Because both are bare `string`s they
|
||||
* were passed around interchangeably and silently swapped — e.g. locking/keying a
|
||||
* collab doc by the slugId instead of the UUID (the #260 data-loss).
|
||||
*
|
||||
* `PageId` brands the CANONICAL id as a distinct nominal type so a raw/unresolved
|
||||
* string cannot flow into the seams that REQUIRE the canonical id (resolvePageId's
|
||||
* result, the per-page lock key, the collab write entrypoints) — those become a
|
||||
* COMPILE error, catching a swap at build time. It is still a `string` at runtime
|
||||
* (assignable INTO any `string` parameter unchanged), so branding flows outward
|
||||
* for free; the brand is minted at the single canonicalization seam
|
||||
* (`resolvePageId`, via `as PageId`).
|
||||
*/
|
||||
export type PageId = string & { readonly __brand: "PageId" };
|
||||
@@ -8,6 +8,8 @@
|
||||
* failure) before it runs. Different pages never block each other.
|
||||
*/
|
||||
|
||||
import type { PageId } from "./page-id.js";
|
||||
|
||||
const chains = new Map<string, Promise<unknown>>();
|
||||
|
||||
// Canonical UUID shape (versions 1–8, matching the `uuid` package's `validate`
|
||||
@@ -28,11 +30,14 @@ export function isUuid(value: string): boolean {
|
||||
// awaited/handled by the caller; only the internal chaining tail swallows
|
||||
// errors (purely to gate ordering).
|
||||
export function withPageLock<T>(
|
||||
pageId: string,
|
||||
pageId: PageId,
|
||||
fn: () => Promise<T>,
|
||||
): Promise<T> {
|
||||
// STRUCTURAL INVARIANT (issue #449, "resolve-then-lock"): the mutex key MUST
|
||||
// be the canonical page UUID, never a raw slugId. The whole write path relies
|
||||
// STRUCTURAL INVARIANT (issue #449/#435, "resolve-then-lock"): the mutex key
|
||||
// MUST be the canonical page UUID, never a raw slugId. The `PageId` brand now
|
||||
// enforces this at COMPILE time (a raw string / slugId no longer type-checks
|
||||
// as a key); the runtime assert below stays as a backstop for untyped (JS)
|
||||
// callers and the http/stdio transports. The whole write path relies
|
||||
// on the lock key AND the CollabSession cache key being the resolved UUID
|
||||
// (#260) — if a future write method forgot to call resolvePageId and locked
|
||||
// under a slugId, two writes to the same page would take DIFFERENT mutex keys
|
||||
|
||||
Reference in New Issue
Block a user