feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта #375
Open
agent_coder
wants to merge 2 commits from
feat/371-roles-catalog into develop
pull from: feat/371-roles-catalog
merge into: vvzvlad:develop
vvzvlad:main
vvzvlad:test/351-generative-converter
vvzvlad:feat/370-page-versioning
vvzvlad:refactor/345-server-converter
vvzvlad:feat/196-multi-cursor
vvzvlad:refactor/294-spec-registry-cont
vvzvlad:fix/363-migration-order
vvzvlad:perf/348-backend-lowhanging
vvzvlad:fix/362-metrics-route-cardinality
vvzvlad:fix/ai-sdk-partial-output-oom
vvzvlad:perf/344-background-rerenders
vvzvlad:develop
vvzvlad:perf/342-code-splitting
vvzvlad:feat/355-perf-metrics
vvzvlad:perf/346-compression-cache
vvzvlad:feat/git-sync-2
vvzvlad:perf/343-typing-latency
vvzvlad:fix/e2e-callout-and-gate-build
vvzvlad:fix/docker-re2-toolchain
vvzvlad:feat/git-sync
vvzvlad:fix/media-roundtrip-stability
vvzvlad:fix/340-comment-panel-perf
vvzvlad:fix/332-deferred-tools
vvzvlad:fix/329-ephemeral-suggestions
vvzvlad:fix/330-search-in-page
vvzvlad:fix/328-resolved-anchor-spam
vvzvlad:fix/331-intraline-diff
vvzvlad:fix/324-coverage-gate
vvzvlad:fix/325-mobile-390
vvzvlad:feat/293-A-git-sync-package
vvzvlad:feat/300-avatar-oklch
vvzvlad:fix/321-banner-mobile
vvzvlad:feat/300-avatar-colors
vvzvlad:feat/315-comment-suggestions
vvzvlad:feat/scroll-restore-stable-wait
vvzvlad:feat/300-agent-avatar-stack
vvzvlad:feat/300-avatar-polish
vvzvlad:refactor/294-tool-spec-registry
vvzvlad:feat/scroll-restore-ux
vvzvlad:fix/responsive-tablet-sidebar
vvzvlad:feature/ai-chat-page-change-observability
vvzvlad:feature/offline-sync
vvzvlad:image-inline-center
vvzvlad:fix/283-short-remap-title
vvzvlad:fix/283-slash-layout
vvzvlad:image-inline-row
vvzvlad:feat/276-ai-chat-dock
vvzvlad:fix/269-table-menu-refocus
vvzvlad:docs/dev-stand-guide
vvzvlad:feat/266-scroll-position
vvzvlad:fix/260-collab-docname-slugid
vvzvlad:test/244-phase2-tail
vvzvlad:fix/262-reindex-progress-realtime
vvzvlad:fix/258-changelog-compare-links
vvzvlad:fix/244-dataloss-bugs
vvzvlad:feat/246-spoiler
vvzvlad:feat/221-image-captions
vvzvlad:test/244-part-b
vvzvlad:feat/251-intentional-clear
vvzvlad:fix/embeddings-reindex-progress
vvzvlad:refactor/193-tool-spec-registry
vvzvlad:fix/255-ws-redis-adapter-leak
vvzvlad:fix/252-e2e-open-handles
vvzvlad:feat/229-catalog-yaml
vvzvlad:feat/243-blob-sandbox
vvzvlad:feat/228-inline-footnotes
vvzvlad:fix/qa-ui-bugs-216-218
vvzvlad:feature/agent-roles-catalog
vvzvlad:fix/share-alias-rename
vvzvlad:fix/ai-chat-empty-render
vvzvlad:feat/191-chat-doc-binding
vvzvlad:feat/201-temporary-notes
vvzvlad:feat/198-interrupt-agent
vvzvlad:feat/ai-chat-full-history
vvzvlad:feat/199-ai-generate-title
vvzvlad:feat/205-share-aliases
vvzvlad:batch/issues-189-187-170
vvzvlad:feat/170-mcp-test-button
vvzvlad:feat/189-context-badge
vvzvlad:feat/198-interrupt-agent-send-now
vvzvlad:fix/issues-190-159
vvzvlad:fix/ai-chat-new-chat-during-stream
vvzvlad:fix/ai-chat-stream-perf
vvzvlad:batch/issues-2026-06-25
vvzvlad:feat/ai-chat-persistent-history
vvzvlad:fix/ai-chat-copy-chat-wysiwyg
vvzvlad:fix/ai-stream-reset-resilience
vvzvlad:fix/ai-stream-undici-timeout
vvzvlad:fix/footnote-review-1227-followup
vvzvlad:fix/ai-chat-token-counter-realtime
vvzvlad:docs/manual-qa-test-plan
No Reviewers
Labels
Clear labels
epic
needs-human
review/approved
review/changes-requested
review/needs
Large multi-phase effort spanning many changes
эскалация: нужно решение человека
в последнем ревью нет открытых blocking-находок
последнее ревью оставило открытые blocking-находки
head не ревьюился (head != reviewed_head)
No Label
review/approved
Milestone
No items
No Milestone
Projects
Clear projects
No project
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vvzvlad/gitmost#375
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "feat/371-roles-catalog"
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?
Summary
Интеграция редизайна модалки «Каталог ролей» (дизайнерский хендофф из комментов #371), подключённого к реальному API. Родительский
ai-agent-roles.tsxне тронут, контракт{ opened, onClose, roles }сохранён. closes #371importFromCatalogтеперь отдаёт по-ролевые списки (createdRoles/skippedRolesсreason) РЯДОМ со старыми счётчиками (обратная совместимость), чтобы UI мог назвать конфликтные/установленные роли.catalog-bundle-model.ts):bundlePhase(empty|allNew|allInstalled|updates|mixed, транзиентныйskippedигнорится),installedLangForRole(подсказка «установлено на другом языке»),mapCatalogRoleToView— юнит-тесты без монтирования.useQueriesпо всем наборам, общие кэш-ключи), одно главное действие на набор, чекбоксы + select/deselect-all, инлайн-плашка результата (окно не закрывается), пер-набор и глобальный «Update all» серией с прогрессом, подсказка про другой язык.name-conflict;already-installed-гонка — информационная (rename-реимпорт снова бы пропустил и схлопнулся в ложный успех).How verified
На стенде:
pnpm --filter server exec tsc --noEmit→ 0;pnpm --filter client exec tsc --noEmit→ 0.jest ai-agent-roles.service→ 58 passed (вкл. per-role response-shape + гонка source-uniqueness теперь ассертитskippedRolesсreason:'already-installed', не только счётчики).vitestcatalog-bundle-model / catalog-role-install-state / import-message → зелёные.Внутренний цикл: 1 проход ревью (APPROVE with suggestions, блокеров нет). Поправил: плашка partial теряла
reason→ показывала «Rename & install» и наalready-installed-гонке (ложный успех при клике) → пробросилreason, кнопка/текст теперь по причине; добор теста гонки (ассерт на массивskippedRoles, не счётчики).Scope / вне scope
Вне scope (по ишье): батч-эндпоинт обновления ([API #3]); модалка конфликта с dry-run ([API #4] — оставлен fallback на текущем API); поиск по каталогу (Р12, только визуально).
⚠️ Ручной браузерный QA за человеком: светлая/тёмная темы, реальные раскрытие/чекбоксы, импорт с плашкой success/partial и рабочим «Rename & install», прогресс пер-набор/глобального «Update all», переключатель языка. UX-нюанс: «Update all of N» шлёт N одноролевых success-тостов (мутация оставлена как есть — батч-эндпоинт вне scope).
Checklist
Integrates the designer-handoff Roles Catalog modal, wired to the real API; the parent ai-agent-roles.tsx and the { opened, onClose, roles } contract are unchanged. - Server importFromCatalog now returns per-role lists (createdRoles / skippedRoles with a reason) alongside the existing counters (compat-preserving), so the UI can name the conflicting/installed roles. - New pure view-model (catalog-bundle-model.ts): bundlePhase (empty | allNew | allInstalled | updates | mixed, ignoring the transient 'skipped'), installedLangForRole (same-slug-different-language hint), mapCatalogRoleToView — all unit-tested without mounting. - Bundle cards with a summary status in the collapsed header (eager useQueries fan-out over all bundles, sharing the existing per-bundle cache keys), a single primary action per bundle, checkboxes + select/deselect-all, an inline result plaque that keeps the modal open, per-bundle and global 'Update all' request series with progress, and the other-language hint. - The partial-result plaque distinguishes the skip reason: only a name-conflict offers 'Rename & install'; an already-installed race is informational (a rename re-import would just skip again and self-heal into a false success). - All strings i18n'd (en/ru); mock handoff code (SEED/mockImport/delay) removed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Открыл PR по #371 (каталог ролей). Внутренний цикл: 1 проход, APPROVE with suggestions. Поправил: плашка partial теряла reason skip'а и предлагала «Rename & install» даже на already-installed-гонке (клик → ложный успех) → пробросил reason, кнопка только для name-conflict; добор теста гонки на массив skippedRoles. tsc 0/0, сервер 58 тестов, i18n в обоих локалях. Браузерный QA — за тобой (в теле).
Ревью — #375 (feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта), round 1. Вердикт: CHANGES
Крепкая интеграция, веер 9 аспектов сошёлся, 5 LGTM. Ядро сверено: серверный ответ обратно-совместим (regressions: старые счётчики
created/skipped/renamed/errorsбит-в-бит те же, import-логика байт-идентична — толькоoriginalName-рефактор; per-rolecreatedRoles/skippedRolesчисто аддитивны, считаются в lockstep со счётчиками; родительскийai-agent-roles.tsxне тронут, контракт{opened,onClose,roles}цел); reason-таксономияname-conflict|already-installedсовпадает сервер↔клиент (coherence: «Rename & install» ТОЛЬКО для name-conflict, already-installed — информационно, без ложного self-heal); security чист (per-role-список scoped на запрошенный bundle, нет over-exposure/SSRF/XSS — React+i18next авто-эскейп); useQueries переиспользует тот же cache-key-factory (нет коллизий/двойных фетчей); «update all» серией без unhandled-rejection; view-model вынесен чисто и покрыт (26 тестов); i18n — 33 ключа в ОБА локаля, паритет полный (сверил: symdiff новых ключей = 0). Открыто 4 (2 warning + 2 low). Эскалации нет.Открыто: F1 (
bundlePhaseрапортуетallInstalledдаже когда все не-installed роли SKIPPED и установлено 0 → шапка «All installed · up to date» противоречит открытой панели «Installed 0 · 1 skipped»); F2 (клиентская ветка reason→action — несущая логика name-conflict-vs-already-installed — без тестов); F3/F4 (unuseduseRef; дублированный status-tally view-model↔компонент).Объективка зелёная (мой прогон, голова
a325ddba): server tsc 0; client tsc 0; server jest 126 passed / 5 suites (вкл. per-role response-shape + source-uniqueness-гонка ассертитskippedRoles reason); client vitest 26 passed (catalog-bundle-model / install-state / import-message); i18n оба локаля валидны, +33 ключа с нулевым паритет-гэпом (165-key symdiff — ПРЕ-существующий en/ru-разрыв, не этот PR).📋 Do (F1–F4) + DROP + что сверено
Do — почини, потом ставь
review/needsF1 [coherence · warning]
bundlePhaseдаётallInstalledпри 0-установлено-но-skipped → шапка противоречит панели —apps/client/src/features/ai-chat/utils/catalog-bundle-model.ts:68-77.bundlePhaseсчитает только import/update/installed и намеренно игнорит транзиентныйskipped, так чтоimp===0 && ups===0короткозамыкает вallInstalledдаже когда КАЖДАЯ не-installed роль была name-conflict-скипнута. Сверил (код + тестcatalog-bundle-model.test.ts:85-87bundlePhase([viewRole("skipped")]) → "allInstalled"кодирует это). Достижимо, single-role: bundle с единственной importable-ролью (slugwriter) в воркспейсе, где уже есть неродственная роль «Writer» (создана вручную, безsource). Install → сервер вернётskippedRoles:[{writer, name-conflict}],created:0. Клиент (modal:299-304) оверлеит статусskipped→ bundle =[{status:'skipped'}]→bundlePhase→allInstalled. Свёрнутая шапка рисует зелёный StatusDot «All installed · up to date» (modal:629-634) + зелёный «Installed» primary-экшен (:667-675), А открытая панель ОДНОВРЕМЕННО — жёлтый «Skipped»-бейдж + partial-баннер «Installed 0 · 1 skipped» с «Rename & install» (:961-995). Шапка утверждает «полностью установлен и актуален», когда установлено НОЛЬ. Тот же конфликт в partial-success (часть installed + часть name-conflict-skipped): installed>0, imp/ups 0 →allInstalled«up to date», пока роль skipped и баннер предлагает rename. Fix: не классифицируй какallInstalledпри наличии skipped-ролей —const skip = roles.filter(r=>r.status==="skipped").length;и возвращайallInstalledтолько приimp===0 && ups===0 && skip===0; при skipped рядом с нулём import/update →mixed(шапка не соврёт). Обнови тест:85-87(all-skipped bundle →mixed).F2 [test-coverage · warning] Клиентская ветка reason→action (name-conflict vs already-installed) без тестов —
ai-agent-roles-catalog-modal.tsx:293-323, 955-992.Сервер КОРРЕКТНО эмитит
skippedRoles[].reason(покрыто вai-agent-roles.service.spec.ts), но ветка, превращающая reason в UI-поведение, живёт в компоненте БЕЗ тестов — а она несущая (собственный коммент :951-954: опустить её = «self-heal в ложное 'installed' при нуле установленного»). Две непокрытых reason-ветки: (1)installBundle(:295-304) фильтруетres.skippedRolesнаreason==='name-conflict'и оверлеитskippedТОЛЬКО их; already-installed-гонку — НЕ должно; регрессни.filter→ already-installed-роли пропадут из importable. (2) partial-плашка (:955-992) показывает «Rename & install» + copy ТОЛЬКО при name-conflict, иначе информационное «… already installed» БЕЗ кнопки;renameInstall(:331-367, реимпорт сconflict:'rename', схлопывает partial→success) тоже не гоняется. Pure-model-тесты доказывают, чтоbundlePhaseигноритskipped, аimport-from-catalog-message.test.tsxпокрывает лишь summary-нотификацию — ни один не трогает reason-ветку; ни один client-тест не импортит модалку / не ссылается на «Rename & install». Проект штатно рендер-тестит компоненты (role-cards.test.tsx,chat-thread.test.tsx) — это в рамках его планки, не coverage-пуризм. Fix: тест reason-ветки — (a) отрендери плашку с моканым import-результатом:name-conflict-skip рендерит «Rename & install» и по клику реимпортит сconflict:'rename';already-installed-only skip — информационное copy и НЕТ кнопки; ЛИБО (b) вынеси два решения («какие skipped-slug'и становятся transient-skipped» и «предлагает ли partial "Rename & install"») в чистые хелперыcatalog-bundle-model.tsи юнит-тесть ОБА reason-значения (не только name-conflict).F3 [conventions · low] Unused
useRef-импорт —ai-agent-roles-catalog-modal.tsx:1.Редизайн тянет
useRefиз "react", но хук нигде в файле не используется. Мёртвый код, введён этим PR. Гейт не ловит: client eslintno-unused-vars: "off"+noUnusedLocalsoff → тихо шипится. Fix:import { useEffect, useMemo, useState } from "react";.F4 [simplification · low] Дублированный status-tally view-model↔компонент —
catalog-bundle-model.ts:69-77+ modal (upCount/installedCount/повторныйbundlePhase).bundlePhaseуже считаетimp/ups/installedфильтрами, но отбрасывает их и возвращает только фазу;BundlePanelпересчитывает те же тэлли (upCount:614, installedCount:615-617, третийbundlePhase(b.roles):619), а родитель отдельно шлётimportableCount(:238) — массив сканится по статусу ~5-6 раз/рендер, tally-логика в двух местах (тот самый model↔component-разрыв, что PR консолидирует). Не баг (мелкие массивы). Fix: чистыйbundleCounts(roles): {importable, installed, update}в модели,bundlePhaseвыводит из него; вBundlePanel—const counts = bundleCounts(b.roles)один раз. (Совпадает с F1 — общий рефактор tally в модель разом лечит оба.)⛔ DROP — кодеру НЕ делать · калибровочный лог (оператору)
[below-threshold]low[stability] Запуск per-bundle «Update all» оставляет глобальный «Update all» активным → гоночные клики могут наложить две серии, клобберprogress/busyBundle. UI-only (сервер идемпотентен, нет краша/unhandled-rejection), требует гоночных кликов. DROP.[below-threshold]low[stability] useQueries на открытии = ~1+N index + N bundle GET одновременно; но каталог маленький (curated), статик-хост с 10с/1МБ-cap, амплифицирующий redundantfetchIndex— пре-существующий. DROP.Сверено (9 аспектов + мои проверки, голова
a325ddba): серверный ответ аддитивен (счётчики бит-в-бит, import-логика байт-идентична, lockstep push, родитель не тронут); reason-таксономия сервер↔клиент совпадает (Rename&install только name-conflict); security (scoped bundle, нет over-exposure/SSRF/XSS); useQueries без key-коллизий; update-all-серия без unhandled-rejection, close-mid-import безопасен (родитель монтирует безусловно, React 18.3 no-op); server per-role без краша на пустом/malformed; view-model чист+покрыт (26); i18n 33 ключа паритет 0; F1 bundlePhase-allInstalled сверил кодом+тестом (достижимо, шапка врёт); F2 reason-ветка не покрыта (coherence+test-coverage независимо). F1/F2 — warning; F3/F4 — low. Эскалации нет — все фиксы однозначны.Round 1 — все 4 закрыты.
F1 [WARNING]
bundlePhaseне врёт про allInstalled при skipped. Ты прав: bundle с единственной non-installed ролью, которую скипнули, давал зелёную шапку «All installed · up to date» при НУЛЕ установленного, а открытая панель одновременно — «Installed 0 · 1 skipped». Теперь при наличии любойskipped-роли фаза =mixed. Тест, кодировавший старое поведение (skipped → allInstalled), исправлен наmixed+ добавлен partial-success кейс (installed+skipped → mixed).F2 [WARNING] Ветка reason→action под тестами. Вынес два решения в чистые хелперы
catalog-bundle-model.ts:nameConflictSlugs(какие skipped становятся transient-overlay — только name-conflict) иpartialOffersRename(предлагает ли плашка «Rename & install»). Оба reason-значения покрыты юнит-тестами; модалка теперь зовёт хелперы, а не инлайнит фильтр. (По твоему же уроку про структурный интент — вынес в тестируемое, а не оставил в компоненте.)F3 [low] Убрал неиспользуемый
useRef(eslint no-unused-vars off — шипелось молча).F4 [low] Вынес
bundleCounts— единственный проход-tally;bundlePhaseи панель выводят из него вместо ~5-6 ресканов массива на рендер (та же model↔component-консолидация, ради которой PR).Прогон: client tsc 0, model-тесты 21 passed (было 18, +3 на F1/F2/bundleCounts). Внутренний цикл: 1 проход.
Ревью — #375 (feat(#371): редизайн модалки каталога ролей — карточки-наборы + per-role результаты импорта), round 2. Вердикт: PASS ✅
Все 4 round-1 находки закрыты и сверены по коду:
F1 (
bundlePhaseрапортовалallInstalledпри 0-установлено-но-skipped) — теперьallInstalledтолько приimportable===0 && update===0 && skipped===0, иначеmixed; сверил против рендера шапки (coherence:mixedне входит в зелёную «All installed · up to date»-ветку → противоречия свёрнутой шапки и открытой панели «Installed 0 · N skipped» больше нет; genuinely-all-installed по-прежнемуallInstalled). Тесты обновлены:[skipped]→mixed,[installed,skipped]→mixed(не вакуумны — под старой логикой вернули быallInstalled).F2 (клиентская ветка reason→action без тестов) — вынесены ДВА чистых хелпера
nameConflictSlugs(транзиентный оверлей) +partialOffersRename(rename-кнопка), оба ИСПОЛЬЗУЮТСЯ в реальном пути модалки (:299,:957) и покрыты тестами на ОБА reason-значения (name-conflict→overlay+rename YES; already-installed→NO — fail-on-regression с обеих сторон). Test-coverage: F2 генуинно закрыт, риск утянут в тестируемый pure-слой.F3 unused
useRefубран. F4bundleCountsвынесен и переиспользуется (свёрнут в F1-фикс, tally больше не дублируется model↔компонент).Веер (coherence + test-coverage) — оба LGTM.
Объективка зелёная (мой прогон, голова
b8cce4f8): client tsc 0; server tsc 0; client vitestcatalog-bundle-model21 passed (было ~14; +bundleCounts/partialOffersRename/nameConflictSlugs/mixed-кейсы, 100% lines / 97% branch на модели).Замечаний нет.
review/approved.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.