No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#197
Reference in New Issue
Block a user
Delete Branch "batch/issues-189-187-170"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Батч из трёх задач, каждая — отдельным коммитом.
#189— feat(ai-chat): бейдж контекста «текущий / максимум» в шапкеБейдж в шапке плавающего окна AI-чата больше не «прыгает» между live-счётчиком хода и размером контекста — теперь он всегда показывает
текущий / максимум(например572 / 200k). Максимум берётся из новой настройки AI «Context window (tokens)»: сервер резолвит её и кладётmaxContextTokensв метаданные завершённого ассистентского хода (рядом сcontextTokens), поэтому клиенту не нужно резолвить модель, и это переживает публичные шары / пер-ролевые модели. Live-фидбек «Thinking · N tokens» в теле чата сохранён; из шапки убран только дублирующий live-счётчик. Подводный камень: провайдерные настройки хранятся как::text, поэтому значение коэрсится обратно в положительное целое вresolve()/getMasked().Коммит:
feat(ai-chat): header badge shows current/max context, max from AI settings (#189)#187— ci(develop): e2e на каждый push в develop, без блокировки деплояВ
develop.ymlдобавлены два независимых job —e2e-serverиe2e-mcp(pgvector + redis, миграции, для MCP — сборка, старт прод-сервера с REST +/collab, seed через/api/auth/setup).buildостаётсяneeds: testи не зависит от e2e: падение e2e не блокирует публикацию образа:develop, а делает run красным и шлёт письмо GitHub автору пуша.Коммит:
ci(develop): run server + mcp e2e on every develop push without blocking deploy (#187)#170— feat(ai-chat): кнопка «Test» на каждой строке списка MCP-серверовInline-проверка подключения прямо из строки списка внешних MCP-серверов, без попапов. Строка вынесена в
AiMcpServerRowсо своим инстансом мутации (изоляция состояния — нет глобального мигания). Состояния: покой (Test), проверка (loading), успех (зелёная,OK · Nпо числу инструментов), ошибка (красная,Failed); тултип показывает список инструментов или текст ошибки. Результат сбрасывается при смене url/transport/headers. Бэкенд/сервис/мутация не менялись.Коммит:
feat(ai-chat): inline Test button per external MCP server row (#170)Closes #189
Closes #187
Closes #170
🤖 Generated with Claude Code
Add a per-row Test button to the external MCP servers list that shows the connection result inline (no toasts). Extract the row into AiMcpServerRow so each row owns its own useTestAiMcpServerMutation instance — independent loading and result, no cross-row flicker. States: idle (Test), pending (loading), success (green, "OK · N" with the tool count), failure (red, "Failed"); a tooltip shows the tool list or the error. The result resets when url/transport/headers change (the row is keyed by id, so it does not remount). Backend, service and mutation are unchanged. - ai-mcp-servers.tsx: AiMcpServerRow + Test button + reset effect + tooltip. - i18n: add Failed / "OK · {{n}}" (en, ru) and ru Test / tool-list keys. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Code review — батч #197 (бейдж контекста #189 + e2e в CI #187 + inline-тест MCP #170)
Вердикт: Approve with comments (одобрено с замечаниями). Блокирующих проблем нет. Удаление старого live-счётчика токенов из шапки выполнено чисто (ни одной висячей ссылки на удалённые символы), новое поле
chatContextWindowвалидируется и проброшено консистентно во все слои, новые CI-джобы аддитивны и не гейтят деплой. Замечания ниже некритичные; главное из них — отсутствие тестов на парсинг::textдляchatContextWindow.Ревью провёл в 8 аспектов параллельно (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) против merge-base с
develop. Объём: 18 файлов, +481/−390. Аспекты security, conventions, documentation, regressions и architecture — чисто (LGTM), все находки верифицированы по веткеbatch/issues-189-187-170.Must fix before merge
Нет.
Test coverage
chatContextWindowиз::textвresolve()иgetMasked()—apps/server/src/integrations/ai/ai-settings.service.ts:162-176, 235-241. БлокNumber(provider.chatContextWindow); Number.isFinite && >0 ? Math.floor : undefined— реальный парсинг с 4 ветками, не покрыт ни одним тестом: отдельногоai-settings.service.spec.tsнет, и ни один спек не инстанцируетAiSettingsService/не вызываетresolve()/getMasked()(проверено по ветке). Непокрытые ветки: валидная строка"200000"→200000; нечисловая/пустая""/"abc"→undefined;"0"/отрицательная→undefined; дробная"1.5"→Math.floor→1. Блок продублирован вgetMasked()с теми же непокрытыми ветками. Fix: добавитьai-settings.service.spec.ts, который стабаетreadProviderпод каждую форму и проверяетresolve().chatContextWindow/getMasked().chatContextWindow(200000/undefined/undefined/1).maxContextTokensи ветки рендера бейджа —apps/client/src/features/ai-chat/components/ai-chat-window.tsx:304-323.useMemo(скан строк с конца до самой свежей с положительнымmaxContextTokens, пропуск null-метаданных) и рендер бейджа (только числитель /current / max/ скрыт) не покрыты. Важные кейсы: свежая строка сmaxContextTokens: 0не должна перекрывать более старую положительную;current > maxпоказывается как есть (без clamp, по комментарию в коде). Проект тестирует подобные компоненты через RTL (role-cards,reasoning-block) — то есть это в рамках его планки.apps/client/src/features/workspace/components/settings/components/ai-mcp-servers.tsx:167-280. Четыре состояния кнопки (idle/loading/ok/failed), текст тултипа (tools.join/ "No tools available" /error) иuseEffect, сбрасывающий результат при сменеurl/transport/hasHeaders, не покрыты. Легче всего незаметно сломать веткуok && tools.length === 0→ "No tools available" и сброс по сменеurl.Покрыто адекватно (замечаний нет):
flushAssistant+maxContextTokens(set / unset / явный 0 → ключ опущен), DTOchatContextWindow(accept 0/200000, reject −1, reject 1.5, accept omitted),estimateTokens.Non-blocking findings
e2e-mcpдля диагностируемости —.github/workflows/develop.yml:132-145.start:prod &запускается без редиректа stdout/stderr. Механика верна: фоновый процесс переживает переход между шагами джобы, а упавший старт ловится health-loop'ом (exit 1после 60 попыток). Но причина падения (ошибка бинда/стек/несоответствие миграций) нигде не сохраняется — падение CI оказывается недиагностируемым, что подрывает сам смысл джобы (красный run как уведомление). Fix:pnpm --filter ./apps/server start:prod > /tmp/server.log 2>&1 &+ шаг сif: failure()иcat /tmp/server.log.apps/client/.../ai-mcp-servers.tsx:816-819. Эффект сброса зависит от[url, transport, hasHeaders]; сами значения заголовков на клиент не отдаются (только булевhasHeaders), поэтому при смене, например, токена без смены url/transport/hasHeadersостаётся устаревший зелёный «OK · N». Это stale-positive, пользователь может перенажать Test — влияние низкое; это ограничение write-only модели заголовков, а не баг эффекта (render-loop/stale-closure нет). Опционально: сбрасывать результат при любом сохранении формы редактирования.::text→положительный int в хелпер —apps/server/src/integrations/ai/ai-settings.service.ts:162-176, 235-241. Идентичный 4-строчный блок вresolve()иgetMasked(); в этом же модуле уже есть именованные хелперы такой формы (positiveEnvвai-streaming-fetch.ts,embeddingTimeoutMsвai.service.ts), так что хелпер — норма модуля, а не новая абстракция, и устраняет риск расхождения двух копий. Бонус: один хелпер проще покрыть тестом (см. Test coverage). Fix:function positiveIntOrUndefined(raw: unknown): number | undefined { const n = Number(raw); return Number.isFinite(n) && n > 0 ? Math.floor(n) : undefined; }..github/workflows/develop.yml:15-158. Пять шагов (Checkout / pnpm / Node / install / build-editor-ext) повторяются вe2e-server,e2e-mcpи уже в reusabletest.yml— фактически трижды. Оговорка: GitHub Actions не поддерживает YAML-якоря в workflow-файлах, аservices:нельзя вынести в composite action, так что блокиenv:/services:останутся продублированными per-job; выносится только последовательность шагов в.github/actions/setup. Если дублирование сочтено приемлемым — можно пропустить.AiMcpServerRowк одной деривации —apps/client/.../ai-mcp-servers.tsx:821-848.tooltipLabelи четыреbutton*-переменные вычисляются двумя отдельными цепочкамиif/else ifпо одному и тому же тристейту (нет результата / ok / failed). Одна деривация (объектviewс{color, variant, icon, label, tooltip}) читается легче и не даёт цепочкам разъезжаться. Чисто читаемость.Architecture & design (forward-looking, non-blocking)
Нет. Фан-аут добавления одной настройки по ~7 спискам/типам (DTO,
UpdateAiSettingsInput,AiProviderSettings,ResolvedAiConfig,MaskedAiSettings,PROVIDER_SETTINGS_KEYS,AI_PROVIDER_SETTINGS_ALLOWED) — осознанный type-safe дизайн:PROVIDER_SETTINGS_KEYS(сsatisfies) — канонический источник, копияAI_PROVIDER_SETTINGS_ALLOWEDзащищена parity-тестом в CI, а раздельные view-типы (resolved добавляет расшифрованные ключи; masked заменяет их наhasApiKey-флаги) несут разные формы. Структура впитала изменение штатно — предложений нет.Code review — PR #197 «Батч: бейдж контекста (#189) + e2e в CI (#187) + inline-тест MCP (#170)»
Вердикт: Approve with comments (одобрить с замечаниями).
Diff (
develop…batch/issues-189-187-170, 18 файлов, +481/−390) разобран восемью специализированными ревью-агентами: security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture. Блокирующих проблем нет. Удаление live-счётчика токенов из шапки выполнено полностью — висячих ссылок наliveTurnTokens/onLiveTurnTokensне осталось,estimateTokensи in-body «Thinking · N tokens» сохранены; новое полеchatContextWindowконсистентно проведено через все слои (DTO → service →resolve/getMasked→ типы → клиентская форма) и оба allow-list, i18n-ключи добавлены вen-USиru-RU; CI-джобы чисто аддитивны и не гейтят публикацию образа:develop. Замечания ниже — некритичные.[warning][test-coverage] Покрыть юнит-тестом коэрцию
chatContextWindow(::text→ int) на пути чтения —apps/server/src/integrations/ai/ai-settings.service.ts:162-176(и дубль вgetMasked():235-243). Провайдерные настройки хранятся как::text, поэтому значение читается строкой и сворачивается в положительный int:Number(...) → Number.isFinite && > 0 ? Math.floor : undefined. Это чистая ветвистая логика ("200000"→200000,"1.9"→1,"0"→undefined,"-5"→undefined,""/"abc"/undefined→undefined), и она не покрыта ничем —ai-settings.service.spec.tsотсутствует. Добавленный в PR DTO-тест проверяет только путь записи (@IsInt @Min(0)), а не round-trip на чтении. Регрессия (например, потеряMath.floorили> 0→>= 0) уйдёт молча. Fix: вынести коэрцию в маленький экспортируемый helperparsePositiveInt(raw): number | undefinedи протестировать его (заодно убирает дублирование — см. архитектурную заметку), либо добавить spec наresolve()/getMasked()с замоканнымreadProvider.[suggestion][stability] В e2e-mcp не захватывается вывод прод-сервера и падение всплывает только через ~120 с —
.github/workflows/develop.yml:132-145. ШагStart server (prod)бэкграундит сервер (pnpm … start:prod &) без перенаправления stdout/stderr; шаг завершается мгновенно, поэтому краш на старте не попадает ни в один лог, а единственный сигнал — общий «Server did not become healthy in time» после полного цикла поллинга (60×2 с). Fix: перенаправить вывод в файл (… start:prod > /tmp/server.log 2>&1 &) иcat-ить его в шаге health-check при неуспехе; опционально раньше выходить, если процесс умер.[suggestion][simplification] Свести два обратных прохода по
messageRowsв один —apps/client/src/features/ai-chat/components/ai-chat-window.tsx:303-319. Новый useMemo дляmaxContextTokensповторяет форму соседнего useMemo дляcontextTokens(!meta-skip /typeof === "number" && > 0, та же зависимость[activeChatId, messageRows]). Их можно слить в один проход, возвращающий{ contextTokens, maxContextTokens }. Важно сохранить семантику «каждое значение берётся из самого свежего ряда, где есть именно оно» (числитель и знаменатель могут лежать в разных рядах) — см. архитектурную заметку про десинхронизацию.Главный пробел — серверная коэрция
chatContextWindowвai-settings.service.ts(см. warning выше): чистая, ветвистая, легко тестируемая, но без тестов.Что покрыто в PR (ок):
flushAssistantприкрепляет/опускаетmaxContextTokens(ai-chat.service.spec.ts); DTO-валидацияchatContextWindow(ai-provider-settings-keys.spec.ts).Удаление тестов
liveTurnTokensизcount-stream-tokens.test.tsоправдано — сама утилита и её единственный потребитель удалены; оставшиеся тестыestimateTokensпокрывают выживший экспорт.Низкий приоритет (опционально): клиентская коэрция
""→0вbuildPayload(ai-provider-settings.tsx:370-376) и выборmaxContextTokensв useMemo (ai-chat-window.tsx) — по конвенции проекта «тестируем вынесенные чистые хелперы, а не inline-мемо», ценность ниже.Шов «штамповать
maxContextTokensв метаданные каждого завершённого хода» vs живой конфиг. Лимит модели — это одна воркспейс-настройка (resolved.chatContextWindow), но она копируется в метаданные каждого ряда (ai-chat.service.ts:622) и восстанавливается на клиенте отдельным обратным сканом (ai-chat-window.tsx:307-320). Два следствия (ни одно не баг для v1): (1) значение «замораживается» на момент хода — при смене лимита старые чаты показывают прежний знаменатель до следующего хода; (2) числитель (contextTokens) и знаменатель (maxContextTokens) восстанавливаются двумя независимыми сканами и могут попасть на разные ряды (например, свежий error-ряд несётcontextTokens, но неmaxContextTokens).отдавать
maxContextTokensклиенту как живой конфиг* (из уже загружаемых AI-настроек /getMasked().chatContextWindow), аcontextTokensоставить per-row (effort: small). Плюсы: единый источник правды, всегда актуально, уходит per-row штамп и второй скан, рассинхронизация невозможна. Минусы: связывает окно чата с settings-запросом; знаменатель становится «текущим лимитом», а не «лимитом на момент хода» (поведенческое изменение — нужно подтвердить желаемость); экспортированные транскрипты перестают самоописывать лимит.Дублирование коэрции в
ai-settings.service.ts. Один и тот же блок (Number(...)→Number.isFinite && > 0 ? Math.floor : undefined) повторён дословно вresolve()иgetMasked(). Вариант: вынести приватный/экспортируемый helperparsePositiveInt(value): number | undefined(effort: small) — единственное определение правила, тривиально тестируется (см. warning по покрытию), готово к будущим числовым::text-настройкам. Минус — лёгкая over-DRY при всего двух местах вызова. Рекомендация: склоняюсь вынести.The per-row MCP Test button derived its presentation solely from the test mutation's data ({ ok, tools } | { ok, error }). When the request itself rejected (401/403/500/network) there is no payload, so the row silently spun back to the idle "Test" instead of reporting the failure. Feed the mutation error into mcpTestButtonView so a reject also renders a red "Failed", with the tooltip taken from the server message (error.response.data.message) or a generic i18n fallback. Enable the tooltip for any non-idle state. Cover the reject branch (with and without a server message) in the helper unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Code review (re-review) — PR #197: батч context badge (#189) + e2e в CI (#187) + inline MCP test (#170)
Вердикт: Approve. Дельта — это чистый test-coverage + поведенчески-нейтральный рефакторинг: две вынесенные pure-функции (
selectContextBadge,mcpTestButtonView) и серверный хелперparsePositiveInt, каждый теперь покрыт юнит-тестами, плюс захват лога сервера в CI. Новых блокеров нет, прошлые замечания закрыты.Ре-ревью дельты
9b61024b..ba5cd024(9 файлов, +357/−85). Аспекты: security, stability, conventions, documentation, regressions, test-coverage (параллельные ревьюеры + judge).Статус прошлых блокеров
context-badge.test.ts(8 кейсов),ai-mcp-server-test-view.test.ts(4 состояния),ai-settings.service.spec.ts(8 кейсов дляparsePositiveInt).chatContextWindow(::text→ число): семантикаNumber.isFinite && > 0+Math.floorсохранена дословно, продублированный inline-код заменён единымparsePositiveInt, дрейф> 0→>= 0теперь ловится тестом. Закрыто.IAiMcpServerTestResult(errorуже санитизирован сервером) не изменён. Не затронуто."OK · {{n}}","No tools available","Failed","Test"проходят через тот жеt()с уже существующими ключами. Закрыто.Must fix before merge
Нет.
Non-blocking
Нет. (Проверено против исходников:
selectContextBadgeточно воспроизводит прежний двойной backward-scan с независимым подбором числителя/знаменателя и legacy-usage-fallback;metadata: {...} | nullделаетrow(null)в тесте типобезопасным; серверный.spec.tsсоответствует jest-конвенции каталога.)Test coverage
Покрыто. Вся новая/изменённая логика дельты сопровождается тестами: бейдж (пустой ввод, оба значения из свежей строки, legacy-fallback, числитель/знаменатель с разных строк, не-затенение свежим нулём, пропуск null-metadata, без клампа); MCP-кнопка (idle/ok-с-tools/ok-без-tools/failed + ветки тултипа);
parsePositiveInt(целое/дробь/0/отрицательное/пустое/нечисловое/undefined/null/number). Измененияai-chat-window.tsx,ai-mcp-servers.tsx,ai-settings.service.ts— делегация в покрытые хелперы; CI-правкаdevelop.yml(> /tmp/server.log 2>&1+ dump on failure) логики не несёт.