feat(ai-chat): inline Test button per external MCP server row (#170) #208
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#208
Reference in New Issue
Block a user
Delete Branch "feat/170-mcp-test-button"
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?
Closes #170
Adds a per-row Test button to the external MCP servers list so admins can check a server connection inline, without opening the edit modal.
What changed
AiMcpServerRowcomponent, each owning its ownuseTestAiMcpServerMutationinstance → isolated loading/result state per row (a list-level mutation would make every row jump).Test, loading, greenOK · {n}(tool count), redFailed. Fixedmiwso the row does not jump. Tooltip surfaces the tools list (success) or the sanitized error (failure).url/transport/hasHeaderschange so a stale result does not stick on the keyed-by-id (non-remounting) row.POST /workspace/ai-mcp-servers/testendpoint + mutation — backend/service/query unchanged.FailedandOK · {{count}}(en + ru) and the missingTestkey to ru-RU.Verification
pnpm --filter client exec tsc -b→ cleanpnpm --filter client exec vitest run .../ai-mcp-server-row.test.tsx→ 4 passed🤖 Generated with Claude Code
Add a per-row "Test" button to the external MCP servers list so admins can check a server's connection straight from the list, without opening the edit modal. - Extract each list row into its own AiMcpServerRow component, each owning a dedicated useTestAiMcpServerMutation instance. This isolates loading/result state per row — a single list-level mutation would make every row's spinner and colour jump on any test. - Button reflects the outcome with both colour AND label (a11y / colour-blind safe): idle "Test", loading, green "OK · {n}" (tool count), red "Failed". Fixed miw so the row does not jump as the label changes. A tooltip surfaces the tools list (success) or the sanitized error (failure). - Reset the mutation when url/transport/hasHeaders change so a stale result does not stick on the non-remounting (keyed-by-id) row. - Reuse the existing /workspace/ai-mcp-servers/test endpoint and mutation; backend/service/query unchanged. - i18n: add "Failed" and "OK · {{count}}" (en + ru); add the missing "Test" key to ru-RU. - Add a vitest suite covering idle/success/failure states and per-row isolation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Code review — PR #208: inline-кнопка «Test» в каждой строке внешнего MCP-сервера (#170)
Вердикт: Approve with comments. Логика изолированного per-row теста корректна, server-side ошибки рендерятся правильно, security/regressions чисты. Блокирует мердж только одно: PR добавляет пользовательскую фичу, но не трогает CHANGELOG.md, нарушая зафиксированную в проекте конвенцию Keep a Changelog. Остальное — non-blocking шлифовка плюс пара пробелов в тестах.
Объём: дифф
develop…feat/170-mcp-test-button(merge-base3ddc329b), 5 файлов, +284/−53. Прогнаны параллельные аспектные ревьюеры (security, stability, conventions, documentation, regressions, test-coverage, simplification, architecture) + judge-проход.Must fix before merge
[documentation] Добавить запись в CHANGELOG.md про inline-кнопку Test (#170) —
CHANGELOG.md:11-13([Unreleased] / ### Added)Заголовок файла гласит «All notable changes to this project are documented in this file», и каждая сопоставимая MCP/UI-фича имеет буллет в
### Addedсо ссылкой на issue (#180, #166, #168, #175, #143). Эта PR поставляет новую пользовательскую фичу — per-row кнопку Test для внешних MCP-серверов — но#170нет в CHANGELOG нигде. Конвенция нарушена change-introduced. Fix: добавить буллет под## [Unreleased]→### Added, описывающий per-row кнопку «Test» (idleTest→OK · N/Failed, тултип со списком инструментов или санитизированной ошибкой, изолированное per-row состояние), со ссылкой(#170), в стиле существующих записей.[stability][warning] Показывать индикацию провала, когда сам запрос reject-ится, а не только при
{ok:false}—apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx:34-70UI выводится из
testMutation.data/isPending, аuseTestAiMcpServerMutationнамеренно безonError(подтверждено:ai-mcp-server-query.ts:95-99). Сервер отдаёт{ok:false,error}с HTTP 200 — это рендерится верно. Но реальный reject (истёкшая сессия/401, утрата admin-прав/403 отassertAdmin, 500/обрыв сети) даётdata === undefined,isError === true— строка это не читает: кнопка крутится и молча возвращается к «Test». Это расходится с create/update/delete мутациями в этом же модуле, у которых есть красная нотификацияonError. Fix: читатьtestMutation.isErrorи рендерить красное «Failed» (тултипerror.response?.data?.message/t('Failed to update data')), ЛИБО добавитьonError-нотификацию вuseTestAiMcpServerMutationкак у соседних мутаций.[stability][suggestion] Уточнить комментарий reset-эффекта: смена ЗНАЧЕНИЯ auth-заголовка им не покрывается —
apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx:36-42Комментарий говорит, что результат сбрасывается «after the URL/transport/auth changes», но deps —
[server.url, server.transport, server.hasHeaders].hasHeaders— булев флаг присутствия (значения заголовков write-only и не возвращаются), поэтому ротация значения токена оставляетhasHeaders === true, эффект не срабатывает, и устаревший зелёныйOK · Nвисит против уже не тех кредов. Само поведение приемлемо (клиент не видит value-only смену), но комментарий вводит в заблуждение. Fix: переписать комментарий 36-38 — сбрасывается только при изменении присутствия заголовков (add/remove), не их значений, т.к.hasHeaders— единственный auth-сигнал на клиенте.[documentation][suggestion] Добавить ru-RU перевод для «No tools available» —
apps/client/public/locales/ru-RU/translation.json:1175(использование вai-mcp-server-row.tsx:52)Новый код рендерит
t("No tools available")как тултип при нуле инструментов. Ключ есть в en-US (line 719), но в ru-RU отсутствует (подтверждено) — русские пользователи видят английский фолбэк. Долг предсуществующий (ключ уже использовался формой), эта PR лишь делает его достижимым ещё в одном месте, поэтому non-blocking. Fix: добавить"No tools available": "<перевод>"в ru-RU.[simplification][suggestion] Убрать избыточный
disabled={testMutation.isPending}у кнопки Test —apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx:81-83Mantine Button сам дизейблится при загрузке (
disabled: disabled || loading), аloading={testMutation.isPending}уже стоит с тем же условием — строкаdisabledи её комментарий «Only blocked while in flight» ничего не добавляют. Fix: удалить строкуdisabled(и комментарий); одногоloadingдостаточно.[simplification][suggestion] Упростить guard ветки ошибки до
else if (result)—apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx:63result— union{ok:true;tools} | {ok:false;error}. Послеif (result?.ok)остаётся только error-вариант, так что&& "error" in resultизбыточен — TS уже сужает тип в else. Fix: заменить на} else if (result) {;result.errorпродолжит тайп-чекаться.Test coverage
Новый файл
ai-mcp-server-row.test.tsx(+120) покрывает idle/loading/успех с 3 инструментами/ошибку. Две новых ветки остаются без тестов:apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx:36-42useEffectсtestMutation.reset()— новая нетривиальная логика с явной целью корректности (строка никогда не ремаунтится, key по id), но ни один тест не ре-рендерит строку с изменёнными props. Регрессия, дропнувшая dependency или сам reset, оставит вводящий в заблуждениеOK · Nи зелёный suite. Fix: тест черезrerender— смонтировать строку, прогнать успешный тест доOK · N, ре-рендерить с изменённымserver.url(и отдельноtransport/hasHeaders), проверить откат к idle «Test».No tools available/OK · 0) —apps/client/src/features/workspace/components/settings/components/ai-mcp-server-row.tsx:50-53Условие
result.tools.length > 0 ? join : t('No tools available')и текст success-тултипа не тестируются: успешный тест резолвит 3 инструмента, но контент тултипа не ассертит, а веткаtools.length === 0не вызывается. Fix: тест с{ ok: true, tools: [] }, ассертить лейблOK · 0; опционально проверить join-список тултипа в непустом случае.Architecture & design (forward-looking, non-blocking)
[architecture] Дрейф интерпретации
IAiMcpServerTestResultмежду формой и строкой —ai-mcp-server-row.tsx:44-67иai-mcp-server-form.tsx:246-261PR добавляет второго независимого потребителя discriminated union
IAiMcpServerTestResult. Форма и строка интерпретируют семантику (ok → счётчик инструментов, пусто → «No tools available», error → текст) по-разному, и они УЖЕ разошлись: при пустой строке ошибки форма падает вt("Connection failed")(подтверждено:ai-mcp-server-form.tsx:259-261), а строка оставляет тултип пустым. Не блокирует — обе презентации намеренно разные и каждая внутренне корректна; форма уже интерпретировала union на базе, эта PR лишь добавила второго потребителя. Риск — постепенный дальнейший дрейф при добавлении новых вариантов union.извлечь чистый хелпер** (effort: s), маппящий
IAiMcpServerTestResult → { status; toolCount; toolsText; errorText }рядом с типом, потребляемый и формой, и строкой (каждая со своим layout). Pros: единый источник union→текст, фолбэки пустой ошибки и пустых инструментов не разойдутся, изолированно юнит-тестируется, layout остаётся раздельным. Cons: одна индиректность ради всего двух потребителей; хелпер должен оставаться presentation-agnostic.- CHANGELOG: add an [Unreleased]/Added bullet for the per-row "Test" button (idle Test -> OK · N / Failed, tooltip, isolated per-row state). - ai-mcp-server-row: show a red "Failed" when the request itself rejects (401/403/500/network), reading testMutation.isError — previously only a server-reported {ok:false} was surfaced and a real reject silently reverted to "Test". Tooltip uses error.response?.data?.message or the i18n fallback. - ai-mcp-server-row: clarify the reset-effect comment (hasHeaders is a presence flag, so value-only token rotation is intentionally not reset). - ai-mcp-server-row: drop the redundant disabled={isPending} (Mantine already disables a loading button). - ru-RU: add the "No tools available" translation. - tests: cover the request-reject failure, the empty tool list (OK · 0), and the reset-on-change effect (url / transport / hasHeaders) via rerender. Note: kept the `"error" in result` guard instead of the suggested bare `else if (result)` — optional chaining on `result?.ok` doesn't narrow the discriminated union in the else branch, so the bare form fails tsc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Code review (re-review) — PR #208: inline Test-кнопка для каждого внешнего MCP-сервера (#170)
Вердикт: Approve with comments. Прошлый блокер (отсутствующая запись в CHANGELOG) закрыт; дельта не вносит новых блокеров. SSRF / утечки секретов / регрессий авторизации в изменениях нет — ошибка санируется на сервере, кнопка лишь дёргает уже существующий per-server тест-эндпоинт. Остаётся одна не-блокирующая дыра в покрытии тестов.
Ре-ревью дельты
c28d8cc6..34995ca8(4 файлов, +123/−16). Аспекты: security, stability, conventions, documentation, test-coverage (параллельные ревьюеры + judge).Статус прошлых блокеров
CHANGELOG.mdпод### Addedдобавлена содержательная запись про inline-кнопку Test (idleTest→OK · Nс тултипом списка инструментов /No tools available, либоFailedс санированной ошибкой), с тегом(#170). Не косметика — описывает реальное поведение фичи.isErrorпоказываетtestMutation.error?.["response"]?.data?.messageлибо локализованный фолбэк; серверная ветка ("error" in result) использует уже санированную на сервере строку (комментарий в коде это фиксирует). Новый клиентский код не строит URL и не прокидывает заголовки — только вызываетtestMutation.mutate(server.id).Must fix before merge
Нет.
Non-blocking
Failed—ai-mcp-server-row.test.tsx:114-126. Тест на reject мокаетnew Error("Request failed")(безresponse.data.message), поэтому реальная веткаerror.response.data.messageникогда не исполняется, а фолбэкt("Failed to update data")не ассертится — регрессия в optional-chain или потеря сообщения пройдут зелёными. Fix: добавить кейс, отклоняющий с{ response: { data: { message: "<msg>" } } }, и ассертить<msg>в тултипе; отдельным кейсом — фолбэкFailed to update dataпри отсутствии message.…and a 'No tools available' tooltip—ai-mcp-server-row.test.tsx:127-138. Тест назван как проверяющий тултипNo tools available, но ассертит только лейблOK · 0; новая строкаt("No tools available")и её локаль-ключ (ru-RU/translation.json) не покрыты. Fix: дополнительно ассертить текст тултипаNo tools available(через accessible title/tooltip кнопки).Test coverage
Изменённая логика дельты — три ветки рендера состояния кнопки (
OK · N, серверныйerror, клиентскийisError) и сброс устаревшего результата при смене url/transport/hasHeaders. Покрыто почти полностью: есть тесты наFailed(reject),OK · 0, сброс результата. Непокрыто — деривация текста тултипа в веткахisError(путь сresponse.data.messageи фолбэк) и в ветке пустого списка инструментов (No tools available). Это не-блокирующий пробел: видимый лейбл во всех ветках протестирован, не покрыт только производный текст подсказки.Pull request closed