From b3d7923b23c247707f1f1c46b768ca2c9a45a285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=98=D0=BB=D1=8C=D1=8F=D1=81=20=D0=A1=D1=83=D0=BB=D1=82?= =?UTF-8?q?=D0=B0=D0=BD=D0=BE=D0=B2?= Date: Mon, 7 Sep 2026 20:28:44 +0300 Subject: [PATCH] =?UTF-8?q?test(ci):=20=D1=81=D0=BA=D1=80=D0=B8=D0=BF?= =?UTF-8?q?=D1=82=20test,=20=D1=82=D0=B5=D1=81=D1=82=D1=8B=20=D0=B2=20pr-c?= =?UTF-8?q?heck,=20=D1=84=D0=B8=D0=BA=D1=81=20sporadic=20403=20=D0=B2=20tr?= =?UTF-8?q?ackUserActivity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Шаг A3 (из 1.6) плана production-готовности: - package.json: скрипт "test": "vitest run" - pr-check.yml: шаг Tests после type check - server/utils/userActivity.ts: try/catch в trackUserActivity — синхронная ошибка трекинга активности больше не срывает authenticateToken (ложный 403 на первом запросе пользователя за минуту) - tests/auto-transitions.test.ts: 3 устаревших теста обновлены под намеренное поведение (forward-only position, синк assignee, аудит вместо системного сообщения) 45/45 тестов зелёные, npm run check чисто. --- .github/workflows/pr-check.yml | 3 +++ IMPLEMENTATION_LOG.md | 24 ++++++++++++++++++++++++ package.json | 1 + server/utils/userActivity.ts | 29 ++++++++++++++++++----------- tests/auto-transitions.test.ts | 32 +++++++++++++++++++++----------- 5 files changed, 67 insertions(+), 22 deletions(-) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 5b5fe6c..c4f661d 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -32,6 +32,9 @@ jobs: NODE_OPTIONS: --max-old-space-size=6144 run: npm run check + - name: Tests + run: npm test + - name: Type check document worker run: | cd server/documents/worker diff --git a/IMPLEMENTATION_LOG.md b/IMPLEMENTATION_LOG.md index 46aba8e..98b7dce 100644 --- a/IMPLEMENTATION_LOG.md +++ b/IMPLEMENTATION_LOG.md @@ -23,3 +23,27 @@ - Что изменено: — (только проверка) - Как проверялось: `git fetch origin` + `git status` (чисто, синхронизировано с origin/main, HEAD `a2b231d`), `npm run check` — зелёный (оба tsc: основной и client-di2). - Влияние на поиск/UX: нет. + +--- + +## [0.15] Сквозная документация изменений (этот файл) + +- Статус: ✅ done (коммит `76a0d85` «chore(docs): add implementation log») +- Зачем: по одному файлу любой ИИ-агент понимает, что сделано, зачем, какие файлы тронуты и как проверялось. +- Что изменено: создан `IMPLEMENTATION_LOG.md` в корне репозитория (шапка-контекст + шаблон записей). +- Влияние на поиск/UX: нет. + +--- + +## [A3 / 1.6 частично] Подключение тестов + починка 5 падающих + +- Статус: ✅ done +- Зачем: тесты существовали (3 файла, vitest), но не запускались ни локально (нет скрипта), ни в CI; 5 из 45 падали. Без зелёных тестов дальнейшие изменения безопасности/производительности — вслепую. +- Что изменено: + - `package.json` — скрипт `"test": "vitest run"`. + - `.github/workflows/pr-check.yml` — шаг «Tests» (`npm test`) после type check. + - `server/utils/userActivity.ts` — **фикс реального бага**: `trackUserActivity` мог кинуть синхронно внутри `authenticateToken`, исключение попадало в catch аутентификации → ложный `403 Недействительный токен` на первом запросе пользователя за минуту. Тело обёрнуто в try/catch, throttle сбрасывается для повторной попытки (соответствие контракту fire-and-forget). + - `tests/auto-transitions.test.ts` — 3 устаревших теста обновлены под намеренное текущее поведение (forward-only по position; синк assignee в task_assignees; системное сообщение заменено аудит-записью «Автопереход»), добавлен мок `delegation.service`. +- Как проверялось: `npx vitest run` — 45/45 зелёные; `npm run check` — чисто. +- Влияние на поиск/UX: нет. +- Подводные камни: баг в `userActivity.ts` проявлялся только на первом аутентифицированном запросе в минуту (throttle) — в проде мог давать sporadic 403. diff --git a/package.json b/package.json index bdaeded..e1a3c39 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "build": "vite build && vite build -c vite.config.di2.ts && esbuild server/index.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", "start": "NODE_ENV=production node dist/index.js", "check": "tsc && tsc -p client-di2/tsconfig.json", + "test": "vitest run", "skins:generate": "tsx scripts/dev/generate-skins.ts", "db:generate": "drizzle-kit generate", "db:migrate": "node scripts/migrate-docker.js", diff --git a/server/utils/userActivity.ts b/server/utils/userActivity.ts index a5ba71a..808a685 100644 --- a/server/utils/userActivity.ts +++ b/server/utils/userActivity.ts @@ -20,15 +20,22 @@ export function trackUserActivity(userId: number): void { lastActivityUpdate.set(userId, now); // Run async without awaiting so the request is not delayed. - db.update(users) - .set({ lastActivityAt: new Date() }) - .where(eq(users.id, userId)) - .then(() => { - // No-op on success - }) - .catch((err) => { - // Reset the throttle timestamp on error so the next request retries. - lastActivityUpdate.delete(userId); - console.error("[trackUserActivity] failed to update user activity:", err); - }); + // Синхронные ошибки (например, недоступный пул соединений) тоже не должны + // срывать аутентификацию — трекинг активности некритичен. + try { + db.update(users) + .set({ lastActivityAt: new Date() }) + .where(eq(users.id, userId)) + .then(() => { + // No-op on success + }) + .catch((err) => { + // Reset the throttle timestamp on error so the next request retries. + lastActivityUpdate.delete(userId); + console.error("[trackUserActivity] failed to update user activity:", err); + }); + } catch (err) { + lastActivityUpdate.delete(userId); + console.error("[trackUserActivity] failed to update user activity:", err); + } } diff --git a/tests/auto-transitions.test.ts b/tests/auto-transitions.test.ts index 0b9d6d8..59144dd 100644 --- a/tests/auto-transitions.test.ts +++ b/tests/auto-transitions.test.ts @@ -15,7 +15,6 @@ const mockStorage = vi.hoisted(() => ({ updateTask: vi.fn(), addTaskAssignee: vi.fn().mockResolvedValue(undefined), addTaskAuditLog: vi.fn().mockResolvedValue(undefined), - createTaskMessage: vi.fn().mockResolvedValue(undefined), getForm: vi.fn().mockResolvedValue(null), })); @@ -58,6 +57,11 @@ vi.mock('../server/routes/task-helpers', () => ({ indexTaskAsync: vi.fn().mockResolvedValue(undefined), })); +vi.mock('../server/services/delegation.service', () => ({ + ensureReviewAssignees: vi.fn().mockResolvedValue(undefined), + cleanupStaleDelegates: vi.fn().mockResolvedValue(undefined), +})); + // ─── Tests ──────────────────────────────────────────────────────────────────── import { evaluateAutoTransitions } from '../server/utils/auto-transitions'; @@ -236,9 +240,10 @@ describe('evaluateAutoTransitions', () => { const result = await evaluateAutoTransitions(1, 1); expect(result.changed).toBe(true); - // 10→20→30, then 30→20 but 20 already visited → stop + // 10→20→30, дальше цепочка обрывается: автопереходы двигают задачу + // только ВПЕРЁД по position (auto-transitions.ts), поэтому отката 30→20 нет expect(result.visitedStatusIds).toEqual([10, 20, 30]); - expect(mockStorage.updateTask.mock.calls.length).toBe(3); + expect(mockStorage.updateTask.mock.calls.length).toBe(2); }); it('should stop at max depth to prevent infinite loops', async () => { @@ -261,7 +266,7 @@ describe('evaluateAutoTransitions', () => { const result = await evaluateAutoTransitions(1, 1, { maxDepth: 3 }); expect(result.changed).toBe(true); - // Should stop after maxDepth transitions (3 updates: A→B→A→B, but maxDepth=3 means 3 iterations) + // Правило «только вперёд» останавливает цепочку на B (10→20), maxDepth не достигается expect(mockStorage.updateTask.mock.calls.length).toBeLessThanOrEqual(3); }); @@ -325,6 +330,9 @@ describe('evaluateAutoTransitions', () => { mockStorage.getTaskFieldValues.mockResolvedValue([ { fieldId: 4, value: 'yes' }, ]); + // Явно пустой список переходов: правил назначения нет, поэтому + // triggeredBy не должен попадать в исполнители задачи + mockStorage.getStatusTransitions.mockResolvedValue([]); mockStorage.updateTask.mockResolvedValue(makeTask({ currentStatusId: 20 })); await evaluateAutoTransitions(1, 1, { triggeredBy: 1 }); @@ -332,8 +340,10 @@ describe('evaluateAutoTransitions', () => { expect(mockStorage.addTaskAssignee).not.toHaveBeenCalled(); }); - it('should keep authorId=0 in system message for auto-transition', async () => { - mockStorage.getTask.mockResolvedValue(makeTask({ currentStatusId: 10 })); + it('should sync resolved assignee into task_assignees on auto-transition', async () => { + // Намеренное поведение (auto-transitions.ts): основной исполнитель + // синкается в task_assignees, чтобы TaskDetail мог его отобразить + mockStorage.getTask.mockResolvedValue(makeTask({ currentStatusId: 10, assignedTo: null })); mockStorage.getFormStatuses.mockResolvedValue([ makeStatus({ id: 10, name: 'Initial', position: 0 }), makeStatus({ id: 20, name: 'Auto', position: 1, entryConditions: [{ fieldCode: 'field_4', value: 'yes' }] }), @@ -344,14 +354,14 @@ describe('evaluateAutoTransitions', () => { mockStorage.getTaskFieldValues.mockResolvedValue([ { fieldId: 4, value: 'yes' }, ]); - mockStorage.updateTask.mockResolvedValue(makeTask({ currentStatusId: 20 })); + mockStorage.getStatusTransitions.mockResolvedValue([ + { id: 1, fromStatusId: 10, toStatusId: 20, assigneeUserId: 42 }, + ]); + mockStorage.updateTask.mockResolvedValue(makeTask({ currentStatusId: 20, assignedTo: 42 })); await evaluateAutoTransitions(1, 1, { triggeredBy: 1 }); - expect(mockStorage.createTaskMessage).toHaveBeenCalledWith( - expect.objectContaining({ authorId: 0 }), - expect.any(Number) - ); + expect(mockStorage.addTaskAssignee).toHaveBeenCalledWith(1, 42, 1); }); it('should use AND logic for entryConditions and not bounce back', async () => {