feat: all secondary dashboard views — vault, logs, tokens, analytics, graph, patterns, sessions, system#21
Merged
Merged
Conversation
Create api_tokens table for client token authentication with bcrypt hash storage, prefix-based lookup index, and usage tracking columns.
GORM-based store with Create, List, FindByPrefix, Revoke, IncrementStats, IncrementErrorCount, GetByID, and BatchIncrementStats methods. APIToken model added to models.go.
Handlers for POST /api/auth/login (master token validation + HMAC cookie), POST /api/auth/logout (cookie clear), GET /api/auth/me (status check), GET/POST /api/auth/tokens (list/create), DELETE /api/auth/tokens/:id (revoke). All handlers include swaggo annotations.
TokenAuth now supports three auth methods: master token (admin), client API tokens (eng_* prefix with bcrypt verification and scope enforcement), and HMAC-SHA256 signed session cookies (dashboard). Read-only tokens are restricted to GET + whitelisted POST endpoints. HMAC cookie key derived deterministically from master token via SHA-256.
Add public routes (login, logout) and authenticated routes (me, tokens CRUD) to setupRoutes(). Create TokenStore in initializeAsync() and wire it into TokenAuth middleware after DB initialization.
Background goroutine reads token IDs from a buffered channel and flushes accumulated request counts to the database every 5 seconds, reducing per-request UPDATE overhead for client token authentication.
New handlers in handlers_vault.go:
- GET /api/vault/credentials — list credentials (no values)
- GET /api/vault/credentials/{name} — get with decrypted value
- POST /api/vault/credentials — store credential
- DELETE /api/vault/credentials/{name} — delete credential
- GET /api/vault/status — vault encryption status
All handlers delegate to existing crypto.Vault and ObservationStore.
Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_tags.go:
- POST /api/observations/{id}/tags — add/remove/set tags
- GET /api/observations/by-tag/{tag} — list observations by tag
Logic mirrors MCP tag_observation and get_observations_by_tag handlers.
Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_sessions_rest.go: - GET /api/sessions-index — list indexed sessions - GET /api/sessions-index/search — full-text search transcripts Uses /api/sessions-index path to avoid conflict with existing /api/sessions live session management routes. Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_maintenance.go: - POST /api/maintenance/consolidation — trigger consolidation cycle - POST /api/maintenance/run — trigger full maintenance - GET /api/maintenance/stats — maintenance statistics Delegates to existing consolidationScheduler and maintenanceService. Includes swaggo annotations for OpenAPI docs.
New handler in handlers_analytics.go: - GET /api/analytics/trends — temporal trends (obs per period) Supports daily/weekly/hourly grouping with project filter. Logic mirrors MCP get_temporal_trends handler. Includes swaggo annotations for OpenAPI docs.
Wire all new REST handlers into the requireReady + auth route group:
- /api/vault/* (5 endpoints)
- /api/observations/{id}/tags, /api/observations/by-tag/{tag}
- /api/sessions-index, /api/sessions-index/search
- /api/maintenance/* (3 endpoints)
- /api/analytics/trends
Add composables for observation CRUD (useObservation), generic pagination (usePagination), and search state (useSearch). Add API functions for observation detail, update, archive, feedback, and search endpoints. Add SearchResult types and rejected[] field to Observation type.
Replace placeholder with paginated observation list using server-side pagination (20 items/page). Add project dropdown, type filter pills, and concept filter. Each row is clickable to navigate to detail view.
Replace placeholder with full observation detail view including: - Read-only display of all fields (narrative, facts, concepts, files, metadata) - Rejected alternatives section for decision observations - Inline edit mode with ObservationEditor component (explicit save/discard) - beforeunload warning for unsaved changes - Archive with confirmation dialog - Feedback buttons (thumbs up/down)
Add SearchBar with "/" hotkey focus and compact/expanded modes. Add SearchResults displaying relevance percentage, type badges, concept tags, and rejected alternatives for decisions. Implement SearchView with project filter, decision mode toggle, and URL query parameter support for header search integration.
Replace inline search form with reusable SearchBar component in compact mode. Adds "/" hotkey focus and consistent styling across header and dedicated search page.
- VaultView: credential list, reveal with 30s countdown, copy, delete - TokensView: token list with create modal and revoke confirmation - useVault/useTokens composables for state management - Extended api.ts with all new API functions for phases 6-9
- LogsView: SSE log stream with level filters, search, pause/resume - useLogs composable: SSE connection, 1000-entry buffer, level toggle - TagEditor component: add/remove concept tags on observations - ObservationsView: bulk selection with select-all, archive, delete - Batch action toolbar with confirmation dialog
- Search performance stats cards (total, latency, cache hits, errors) - Latency breakdown with CSS bar charts (vector vs filter) - Retrieval stats, search misses table, recent queries list - Search type distribution visualization
- GraphView: vis-network graph with type-colored nodes, click navigation - PatternsView: pattern list with type badges, expandable insight panels, deprecate and delete actions with confirmation - SessionsView: session list with project/date filters, transcript search - SystemView: component health cards, vector metrics, graph stats, maintenance controls (consolidation, maintenance), update checker - usePatterns/useSessions composables for state management
|
Caution Review failedPull request was closed or merged during review WalkthroughДобавлены серверные и клиентские возможности аутентификации и управления API-токенами: миграция и модель для Changes
Диаграмма последовательностиsequenceDiagram
participant Client
participant Browser
participant AuthHandler as AuthHandler/Middleware
participant TokenStore
participant Database
Client->>Browser: Открывает SPA
Browser->>AuthHandler: GET /api/auth/me (cookie/без токена)
AuthHandler-->>Browser: 401 / redirect на /login
Client->>Browser: POST /api/auth/login {token}
Browser->>AuthHandler: POST /api/auth/login
AuthHandler->>AuthHandler: const-time сравнение master token
AuthHandler-->>Browser: Set-Cookie (HMAC-signed session)
Browser->>AuthHandler: Запрос защищённого ресурса (Authorization: Bearer eng_...)
AuthHandler->>TokenStore: FindByPrefix(prefix)
TokenStore->>Database: SELECT ... WHERE token_prefix=? AND revoked=false
Database-->>TokenStore: APIToken rows
TokenStore->>AuthHandler: Верный токен / scope
AuthHandler->>AuthHandler: Асинхронно push tokenID -> statsCh
AuthHandler-->>Browser: 200 + данные
Estimated code review effort🎯 5 (Критический) | ⏱️ ~120 минут Possibly related PRs
Стихотворение
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the dashboard's functionality by introducing a suite of new views for managing various aspects of the system, from API tokens and vault credentials to analytics and system health. It also lays the groundwork for a more robust and interactive user interface with enhanced authentication mechanisms and reusable components, ensuring a more complete and user-friendly experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (17)
internal/db/gorm/models.go-370-370 (1)
370-370:⚠️ Potential issue | 🟡 MinorИсправьте синтаксис значения по умолчанию для
Scopeв GORM.Значение
read-writeсодержит дефис и должно быть заключено в одинарные кавычки в GORM теге. Текущий синтаксисdefault:read-writeбез кавычек не соответствует определению в миграции базы данных, где используетсяDEFAULT 'read-write'.Предлагаемое исправление
- Scope string `gorm:"type:text;not null;default:read-write"` + Scope string `gorm:"type:text;not null;default:'read-write'"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/db/gorm/models.go` at line 370, Поле структуры Scope использует некорректный GORM default без кавычек; замените метку gorm для поля Scope (Scope string) так, чтобы значение по умолчанию было в одинарных кавычках (например default:'read-write'), чтобы соответствовать миграции и корректному синтаксису GORM.ui/src/components/layout/Pagination.vue-14-15 (1)
14-15:⚠️ Potential issue | 🟡 MinorПотенциальное деление на ноль при
limit <= 0.Если
limitравен 0 или отрицательному значению, вычисленияtotalPagesиcurrentPageприведут кInfinityилиNaN. Рекомендуется добавить защитную проверку.🛡️ Предлагаемое исправление
-const totalPages = computed(() => Math.max(1, Math.ceil(props.total / props.limit))) -const currentPage = computed(() => Math.floor(props.offset / props.limit) + 1) +const safeLimit = computed(() => Math.max(1, props.limit)) +const totalPages = computed(() => Math.max(1, Math.ceil(props.total / safeLimit.value))) +const currentPage = computed(() => Math.floor(props.offset / safeLimit.value) + 1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/Pagination.vue` around lines 14 - 15, Guard against division-by-zero by validating props.limit before computing pages; update the computed properties totalPages and currentPage to use a safeLimit (e.g., if props.limit <= 0 fall back to 1) or return sensible defaults (totalPages = 1 and currentPage = 1) when limit is non-positive, ensuring calculations use safeLimit for Math.ceil(props.total / safeLimit) and Math.floor(props.offset / safeLimit) + 1 and preserving reactive computed usage.ui/src/views/GraphView.vue-208-208 (1)
208-208:⚠️ Potential issue | 🟡 MinorНереактивная проверка DOM в шаблоне.
Условие
!graphContainer?.querySelector('canvas')не является реактивным — Vue не отслеживает изменения DOM. После рендеринга графа условие не пересчитается автоматически, и статистика может отображаться некорректно.🔧 Предлагаемое исправление с использованием реактивного флага
В секции
<script setup>:const observationIdInput = ref('') const projects = ref<string[]>([]) +const graphRendered = ref(false) let network: Network | null = nullВ функции
renderGraph:network.on('click', (params) => { if (params.nodes.length > 0) { const nodeId = params.nodes[0] router.push({ name: 'observation-detail', params: { id: nodeId } }) } }) + graphRendered.value = true }В шаблоне:
- <div v-if="graphStats && !graphContainer?.querySelector('canvas')" class="grid grid-cols-2 md:grid-cols-4 gap-4 mb-4"> + <div v-if="graphStats && !graphRendered" class="grid grid-cols-2 md:grid-cols-4 gap-4 mb-4">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/GraphView.vue` at line 208, The template uses a non-reactive DOM check (!graphContainer?.querySelector('canvas')) so Vue won't update the condition after the graph is rendered; introduce a reactive ref (e.g., hasCanvas or canvasRendered) in the <script setup>, initialize it based on graphContainer presence, update it to true at the end of the renderGraph function (and set false before re-render if needed), and replace the template condition to use the reactive ref alongside graphStats (e.g., v-if="graphStats && !hasCanvas") so the UI updates correctly when the canvas is added or removed; reference graphContainer, renderGraph and graphStats to locate where to add and update the reactive flag.ui/src/views/AnalyticsView.vue-31-47 (1)
31-47:⚠️ Potential issue | 🟡 MinorВнешний блок
catchнедостижим — ошибки перехватываются раньше.Поскольку каждый вызов fetch в
Promise.allимеет собственный.catch(), возвращающий fallback-значение (nullили[]), внешнийcatchна строке 42-44 никогда не получит ошибку от этих запросов. Переменнаяerrorвсегда останетсяnull, и пользователь не увидит сообщение об ошибке, даже если все запросы завершатся неудачей.🛠️ Предлагаемое исправление
Вариант 1: Убрать внешний
try/catchи устанавливатьerrorпри проверке результатов:async function loadAll() { abortController?.abort() abortController = new AbortController() loading.value = true error.value = null - try { - const [analyticsData, recent, misses, retrieval] = await Promise.all([ - fetchSearchAnalytics(abortController.signal).catch(() => null), - fetchRecentSearches(20, abortController.signal).catch(() => []), - fetchSearchMisses(abortController.signal).catch(() => []), - fetchRetrievalStats(undefined, abortController.signal).catch(() => null), - ]) - analytics.value = analyticsData - recentQueries.value = recent || [] - searchMisses.value = misses || [] - retrievalStats.value = retrieval - } catch (err) { - if (err instanceof Error && err.name === 'AbortError') return - error.value = err instanceof Error ? err.message : 'Failed to load analytics' - } finally { - loading.value = false - } + const [analyticsData, recent, misses, retrieval] = await Promise.all([ + fetchSearchAnalytics(abortController.signal).catch((e) => + e.name === 'AbortError' ? null : { error: e }), + fetchRecentSearches(20, abortController.signal).catch((e) => + e.name === 'AbortError' ? [] : { error: e }), + fetchSearchMisses(abortController.signal).catch((e) => + e.name === 'AbortError' ? [] : { error: e }), + fetchRetrievalStats(undefined, abortController.signal).catch((e) => + e.name === 'AbortError' ? null : { error: e }), + ]) + + // Check if primary data failed + if (analyticsData && 'error' in analyticsData) { + error.value = 'Failed to load analytics' + loading.value = false + return + } + + analytics.value = analyticsData + recentQueries.value = Array.isArray(recent) ? recent : [] + searchMisses.value = Array.isArray(misses) ? misses : [] + retrievalStats.value = retrieval && !('error' in retrieval) ? retrieval : null + loading.value = false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/AnalyticsView.vue` around lines 31 - 47, The per-promise .catch() handlers swallow errors so the outer try/catch around Promise.all never runs; remove the individual .catch() calls on fetchSearchAnalytics, fetchRecentSearches, fetchSearchMisses, and fetchRetrievalStats so Promise.all can reject and the outer catch can set error.value (and then use nullish/coalesce when assigning analytics.value, recentQueries.value, searchMisses.value, retrievalStats.value as needed); alternatively, if you want per-call fallbacks plus overall error detection use Promise.allSettled and set error.value when all results are rejected—update the code paths around analytics.value, recentQueries.value, searchMisses.value, and retrievalStats.value accordingly.ui/src/views/TokensView.vue-188-189 (1)
188-189:⚠️ Potential issue | 🟡 MinorРиск случайного закрытия модального окна с созданным токеном.
Клик по backdrop (строка 189) вызывает
closeCreateModal, даже когда отображается только что созданный токен. Пользователь может случайно закрыть окно, не скопировав токен, который больше не будет показан.🛡️ Предлагаемое исправление
- <div class="absolute inset-0 bg-black/60 backdrop-blur-sm" `@click`="closeCreateModal" /> + <div + class="absolute inset-0 bg-black/60 backdrop-blur-sm" + `@click`="!createdToken && closeCreateModal()" + />Или добавьте подтверждение перед закрытием, когда токен отображается.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/TokensView.vue` around lines 188 - 189, Modify the backdrop click behavior so clicking the overlay does not immediately call closeCreateModal when the newly created token is being shown: add a boolean flag/prop (e.g., createdTokenVisible or isShowingNewToken) tied to the created-token state and replace the direct `@click`="closeCreateModal" with a handler (e.g., onBackdropClick) that checks that flag and either prevents closing (or opens a confirmation prompt) when createdTokenVisible is true, otherwise calls closeCreateModal; update the TokensView.vue methods to implement onBackdropClick and ensure showCreateModal and the created-token display logic toggle the flag appropriately.ui/src/router/index.ts-78-93 (1)
78-93:⚠️ Potential issue | 🟡 MinorПотенциальная проблема: начальное состояние аутентификации не проверено.
Навигационный guard проверяет
loading.value, но при первой загрузке приложенияloadingиauthenticatedмогут бытьfalseдо вызоваcheckAuth(). Это может вызвать мгновенное перенаправление на/loginдаже для аутентифицированных пользователей, покаApp.vueне завершит проверку.Рекомендуется гарантировать, что
checkAuth()вызывается до первого разрешения маршрута.🛡️ Предлагаемое исправление
+let authChecked = false + router.beforeEach(async (to) => { const { authenticated, loading, checkAuth } = useAuth() - if (loading.value) { + if (loading.value || !authChecked) { await checkAuth() + authChecked = true } if (!to.meta.public && !authenticated.value) { return { name: 'login' } } if (to.name === 'login' && authenticated.value) { return { name: 'home' } } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/router/index.ts` around lines 78 - 93, The guard in router.beforeEach can return too early because loading.value and authenticated.value may both be false initially; ensure checkAuth() is invoked and awaited before deciding redirects by calling await checkAuth() whenever authentication hasn't been initialized (e.g., when both loading.value and authenticated.value are false or when a separate "initialized" flag is absent) so that the subsequent checks against authenticated.value use the real state; update the guard around useAuth(), checkAuth, loading and authenticated to guarantee checkAuth() runs and completes before returning a route or redirect.ui/src/components/layout/AppHeader.vue-22-32 (1)
22-32:⚠️ Potential issue | 🟡 MinorОтсутствует обработка ошибки от POST-запроса на рестарт.
Функция
restartWorkerне проверяет статус ответа от/api/update/restart. Если запрос вернёт ошибку (4xx/5xx), код всё равно будет ожидать готовности воркера.🔧 Предлагаемое исправление
const restartWorker = async () => { isRestarting.value = true try { - await fetch('/api/update/restart', { method: 'POST' }) + const response = await fetch('/api/update/restart', { method: 'POST' }) + if (!response.ok) { + throw new Error(`Restart request failed: ${response.status}`) + } await waitForWorker() globalThis.location.reload() } catch (error) { console.error('Failed to restart:', error) isRestarting.value = false } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/AppHeader.vue` around lines 22 - 32, The restartWorker function must check the POST response from fetch('/api/update/restart') and handle non-OK status before awaiting waitForWorker; update restartWorker to inspect the Response (e.g., response.ok or response.status), throw or reject with the error body/message when the response is not successful, and in the catch ensure isRestarting.value is set to false and the error is logged; ensure references to restartWorker and waitForWorker are used so the fix is applied in the correct function.ui/src/components/search/SearchBar.vue-17-17 (1)
17-17:⚠️ Potential issue | 🟡 Minor
localValueне синхронизируется при измененииmodelValueродителем.Если родительский компонент изменит
modelValue(например, сброситsearchQueryвAppHeader.vueпосле навигации),localValueне обновится, так как отсутствуетwatch.🔧 Предлагаемое исправление
-import { ref, onMounted, onUnmounted } from 'vue' +import { ref, watch, onMounted, onUnmounted } from 'vue' const localValue = ref(props.modelValue || '') +watch(() => props.modelValue, (newVal) => { + if (newVal !== undefined && newVal !== localValue.value) { + localValue.value = newVal + } +})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/search/SearchBar.vue` at line 17, localValue is initialized from props.modelValue but never updates when the parent changes it; add a watcher that observes props.modelValue (e.g. watch(() => props.modelValue, ...)) and updates localValue accordingly so the internal ref stays in sync with the parent; ensure you reference the existing localValue and props.modelValue in the watcher and keep existing emit/update logic (e.g. update:modelValue) intact so two-way binding remains consistent.internal/worker/handlers_analytics.go-53-58 (1)
53-58:⚠️ Potential issue | 🟡 MinorНекорректные значения
daysмолча игнорируются.Если параметр
daysсодержит невалидное значение (нечисловое, ≤0 или >365), код молча использует значение по умолчанию вместо возврата ошибки 400. Это несогласованно с обработкой параметраperiod, где явно возвращается ошибка.🔧 Предлагаемое исправление
days := 30 if val := r.URL.Query().Get("days"); val != "" { - if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 && parsed <= 365 { - days = parsed + parsed, err := strconv.Atoi(val) + if err != nil { + http.Error(w, "invalid days: must be an integer", http.StatusBadRequest) + return + } + if parsed <= 0 || parsed > 365 { + http.Error(w, "invalid days: must be between 1 and 365", http.StatusBadRequest) + return } + days = parsed }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_analytics.go` around lines 53 - 58, The code silently falls back to default when the days query parameter is invalid; change the parsing block for days (reading via r.URL.Query().Get("days") into variable days and using strconv.Atoi) to mirror the period validation: if the parameter is present but non-numeric, ≤0, or >365, return an HTTP 400 Bad Request with an explanatory error (use http.Error or write status http.StatusBadRequest) instead of silently using the default; keep the default only when the parameter is absent.ui/src/components/layout/AppHeader.vue-34-51 (1)
34-51:⚠️ Potential issue | 🟡 Minor
waitForWorkerне сигнализирует о тайм-ауте, что приводит к преждевременной перезагрузке страницы.Если воркер не становится готовым за 30 попыток (15 секунд), функция завершается без индикации ошибки. В
restartWorkerэто приведёт к вызовуlocation.reload()при неготовом воркере.🔧 Предлагаемое исправление
const waitForWorker = async (maxAttempts = 30, delayMs = 500): Promise<void> => { for (let i = 0; i < maxAttempts; i++) { await new Promise(resolve => setTimeout(resolve, delayMs)) try { const response = await fetch('/api/health', { signal: AbortSignal.timeout(2000), }) if (response.ok) { const data = await response.json() if (data.status === 'ready') { return } } } catch { // Worker not ready yet, continue polling } } + throw new Error('Worker failed to become ready after restart') } const restartWorker = async () => { isRestarting.value = true try { await fetch('/api/update/restart', { method: 'POST' }) await waitForWorker() globalThis.location.reload() } catch (error) { console.error('Failed to restart:', error) isRestarting.value = false } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/AppHeader.vue` around lines 34 - 51, The waitForWorker function currently swallows failures and simply returns after exhausting attempts, so restartWorker cannot tell if the worker never became ready; change waitForWorker to signal failure by throwing an Error (or returning a rejected Promise) when the loop finishes without seeing data.status === 'ready' (refer to waitForWorker), and update restartWorker to catch that rejection (or inspect the boolean) before calling location.reload() so reload only happens on an actual successful ready state; ensure the thrown Error includes a clear message like "worker readiness timed out" to aid logging/handling.ui/src/composables/useObservation.ts-18-36 (1)
18-36:⚠️ Potential issue | 🟡 MinorОтменённый
load()сбрасываетloadingу нового запроса.После
abort()предыдущий вызов всё равно доходит доfinally, поэтому быстрый повторныйload()может скрыть состояние загрузки до завершения актуального запроса. Здесь нужен тот же guard по локальномуcontroller/request id.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/useObservation.ts` around lines 18 - 36, The load function currently aborts a previous request using the shared abortController, but the previous request's finally block can still run and overwrite the new request's state; to fix this, create a local const localController = new AbortController() (or capture the signal) inside load, assign abortController = localController, and then in the try/catch/finally only update observation.value, error.value and loading.value if abortController === localController (or the localController was not aborted) so aborted/old requests do not clear the latest loading state; apply this guard in the catch (when checking for AbortError) and in finally so only the active request manipulates loading.value and error.value.ui/src/composables/usePatterns.ts-16-30 (1)
16-30:⚠️ Potential issue | 🟡 MinorЗащитите
loadingот завершения отменённого запроса.После
abort()предыдущийloadPatterns()всё равно доходит доfinallyи может выставитьloading = falseуже во время нового запроса. При быстром повторном обновлении индикатор загрузки станет недостоверным; здесь лучше привязать сброс состояния к локальномуcontroller, а не к общему полю.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/usePatterns.ts` around lines 16 - 30, loadPatterns currently aborts a shared abortController and then always clears loading in its finally block, which lets an earlier aborted call set loading=false during a newer request; capture the current controller into a local variable (e.g., const controller = abortController) before the await and use that local controller to call fetchPatterns(controller.signal) and to decide in the finally block whether to reset loading — only set loading.value = false (and/or clear error.value) if controller === abortController (meaning this call is still the latest); update references to abortController, loadPatterns, loading, and fetchPatterns accordingly.internal/worker/handlers_vault.go-40-42 (1)
40-42:⚠️ Potential issue | 🟡 MinorОтсутствует валидация параметра
project.В отличие от
handleGetObservationsByTag, здесь параметрprojectне проверяется черезValidateProjectName, что может привести к path traversal или некорректным значениям.🛡️ Предлагаемое исправление
project := r.URL.Query().Get("project") + if err := ValidateProjectName(project); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } creds, err := s.observationStore.ListCredentials(r.Context(), project)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_vault.go` around lines 40 - 42, Validate the incoming project query param before calling s.observationStore.ListCredentials: call ValidateProjectName(project) (same validation used in handleGetObservationsByTag), and if it returns an error respond with a 400 and an explanatory message instead of proceeding; only call s.observationStore.ListCredentials(r.Context(), project) when the project name passes validation to prevent path traversal/invalid values.internal/worker/handlers_vault.go-98-98 (1)
98-98:⚠️ Potential issue | 🟡 MinorОтсутствует валидация параметра
project.Аналогично
handleListCredentials, параметрprojectне проверяется черезValidateProjectName.🛡️ Предлагаемое исправление
project := r.URL.Query().Get("project") + if err := ValidateProjectName(project); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_vault.go` at line 98, The handler that reads project := r.URL.Query().Get("project") is missing validation; update the handler function (the one that mirrors handleListCredentials in internal/worker/handlers_vault.go) to call ValidateProjectName(project) after extracting the query param, and if validation fails respond with a 400 error (using http.Error or the existing error helper) and stop processing; ensure you reuse the same validation flow/response style used in handleListCredentials so invalid project names are rejected consistently.internal/worker/handlers_vault.go-253-268 (1)
253-268:⚠️ Potential issue | 🟡 MinorОтсутствует валидация формата
project.Параметр
projectне проверяется черезValidateProjectName, как и в других обработчиках vault.🛡️ Предлагаемое исправление
project := r.URL.Query().Get("project") + if err := ValidateProjectName(project); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } scope := r.URL.Query().Get("scope")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_vault.go` around lines 253 - 268, Добавьте валидацию параметра project через существующую функцию ValidateProjectName: после получения project := r.URL.Query().Get("project") и перед проверкой scope == "project" && project == "" вызовите ValidateProjectName(project) и при ошибке верните http.StatusBadRequest с понятным сообщением (аналогично другим обработчикам vault). Это обеспечит одинаковую проверку формата project в handlers_vault.go и предотвратит некорректные имена проектов.internal/worker/handlers_vault.go-181-184 (1)
181-184:⚠️ Potential issue | 🟡 MinorОтсутствует валидация формата
project.Параметр
req.Projectпроверяется только на пустоту, но не валидируется черезValidateProjectNameна корректность формата (path traversal, допустимые символы).🛡️ Предлагаемое исправление
if req.Scope == "project" && req.Project == "" { http.Error(w, "project is required for project-scoped credentials", http.StatusBadRequest) return } + if err := ValidateProjectName(req.Project); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_vault.go` around lines 181 - 184, В блоке проверки scope в обработчике (где проверяются req.Scope и req.Project) добавьте валидацию формата проекта: при req.Scope == "project" после проверки на пустоту вызовите ValidateProjectName(req.Project) и, если он возвращает ошибку, ответьте http.Error(w, "<validation message>", http.StatusBadRequest); тем самым заменить/расширить текущую простую пустую проверку и использовать ValidateProjectName для защиты от path traversal и недопустимых символов.internal/worker/handlers_tags.go-133-161 (1)
133-161:⚠️ Potential issue | 🟡 MinorПараметр
offsetзадокументирован, но не реализован.В godoc (строка 133) указан параметр
offsetдля пагинации, однако в коде обработчика он не парсится и не передаётся вSearchParams.🔧 Предлагаемое исправление
limit := 50 if val := r.URL.Query().Get("limit"); val != "" { if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 && parsed <= 200 { limit = parsed } } + + offset := 0 + if val := r.URL.Query().Get("offset"); val != "" { + if parsed, err := strconv.Atoi(val); err == nil && parsed >= 0 { + offset = parsed + } + } if s.searchMgr == nil { http.Error(w, "search manager not available", http.StatusServiceUnavailable) return } searchParams := search.SearchParams{ Query: tag, Type: "observations", Project: project, Limit: limit, + Offset: offset, Concepts: tag, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_tags.go` around lines 133 - 161, The handler handleGetObservationsByTag documents an "offset" query param but never parses or uses it; update this function to read r.URL.Query().Get("offset"), parse it to an int (e.g., strconv.Atoi), validate it is non-negative (and apply any project-specific upper bound if needed, similar to limit), and pass the parsed offset into the SearchParams (or equivalent struct) used when calling the observation store search method so pagination actually works; ensure invalid offsets return http.StatusBadRequest with a clear error message.
🧹 Nitpick comments (22)
ui/src/components/layout/ConfirmDialog.vue (1)
20-48: Рекомендуется улучшить доступность (a11y).Модальное окно не имеет атрибутов
role="dialog"иaria-modal="true", а также отсутствует управление фокусом (focus trap) и закрытие по клавише Escape. Это может затруднить использование компонента для пользователей с ограниченными возможностями.♿ Предлагаемые улучшения доступности
<!-- Dialog --> - <div class="relative glass border border-white/10 rounded-xl p-6 max-w-sm w-full shadow-2xl"> + <div + role="dialog" + aria-modal="true" + :aria-labelledby="'confirm-title'" + class="relative glass border border-white/10 rounded-xl p-6 max-w-sm w-full shadow-2xl" + `@keydown.escape`="emit('cancel')" + > - <h3 class="text-lg font-semibold text-white mb-2">{{ title }}</h3> + <h3 id="confirm-title" class="text-lg font-semibold text-white mb-2">{{ title }}</h3>Также рекомендуется добавить
v-focusдирективу илиrefсfocus()на кнопку отмены при открытии диалога.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/ConfirmDialog.vue` around lines 20 - 48, The ConfirmDialog.vue markup lacks accessibility features: add role="dialog" and aria-modal="true" to the dialog container (the element rendering title/message), implement focus management by adding a ref (e.g., cancelButtonRef) to the Cancel button (the element that calls emit('cancel')) and set focus to it when show becomes true, and implement a focus trap inside the dialog so Tab/Shift+Tab cycle within the dialog while open. Also add a keydown handler on the dialog (or window while show is true) to listen for Escape and call emit('cancel'), and ensure backdrop click remains wired to emit('cancel').ui/src/components/observation/ObservationEditor.vue (1)
3-4: Неиспользуемые импорты.
TYPE_CONFIGиOBSERVATION_TYPESимпортированы, но не используются в компоненте.♻️ Предлагаемое исправление
-import type { Observation, ObservationType, ObservationScope } from '@/types' -import { TYPE_CONFIG, OBSERVATION_TYPES } from '@/types/observation' +import type { Observation, ObservationScope } from '@/types'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/observation/ObservationEditor.vue` around lines 3 - 4, Удалите неиспользуемые импорты TYPE_CONFIG и OBSERVATION_TYPES из строки импорта в ObservationEditor.vue (там же импортируются Observation, ObservationType и ObservationScope) — откройте импорт, уберите упоминания TYPE_CONFIG и OBSERVATION_TYPES или замените весь импорт на только используемые символы, чтобы убрать предупреждение об неиспользуемых переменных.internal/worker/token_stats.go (2)
70-72: Использованиеcontext.Background()вместо переданного контекста.При вызове
store.BatchIncrementStatsиспользуетсяcontext.Background(), что означает, что операция продолжит выполняться даже после отмены основного контекста (shutdown). Для финального flush это может быть желательно, но для регулярных flush по таймеру может привести к зависанию при остановке сервиса.♻️ Предлагаемое исправление
Если требуется гарантированный flush при shutdown, можно использовать отдельный контекст с таймаутом:
-func (s *Service) flushTokenStats(counts map[string]int) { +func (s *Service) flushTokenStats(ctx context.Context, counts map[string]int) { if len(counts) == 0 { return } s.initMu.RLock() store := s.tokenStore s.initMu.RUnlock() if store == nil { return } - if err := store.BatchIncrementStats(context.Background(), counts); err != nil { + flushCtx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + if err := store.BatchIncrementStats(flushCtx, counts); err != nil { log.Warn().Err(err).Int("tokens", len(counts)).Msg("auth: failed to flush token stats") } }И обновить вызовы в
startTokenStatsFlusher:case <-ctx.Done(): - s.flushTokenStats(pending) + s.flushTokenStats(context.Background(), pending) // Final flush with background ctx return case <-ticker.C: - s.flushTokenStats(pending) + s.flushTokenStats(ctx, pending)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/token_stats.go` around lines 70 - 72, The call to store.BatchIncrementStats currently uses context.Background(), causing flushes to ignore cancellation; change it to use the passed-in ctx (replace context.Background() with the function's ctx) so regular timer-driven flushes respect shutdown; for the special "final flush on shutdown" path in startTokenStatsFlusher, create a short timeout context (context.WithTimeout) and use that when invoking store.BatchIncrementStats to guarantee a bounded final flush. Ensure you update the call sites that reference counts and store.BatchIncrementStats accordingly.
45-50: Оптимизация очистки map.Удаление элементов в цикле менее эффективно, чем создание нового map. При высокой нагрузке это может создавать дополнительную работу для GC.
♻️ Предлагаемое исправление
case <-ticker.C: s.flushTokenStats(pending) - // Reset the map - for k := range pending { - delete(pending, k) - } + pending = make(map[string]int)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/token_stats.go` around lines 45 - 50, Вместо очистки map в цикле, которая вызывает лишнюю работу GC, переинициализируй переменную pending новым пустым map-ом после вызова s.flushTokenStats(pending); то есть в блоке, где обрабатывается case <-ticker.C, вызывай s.flushTokenStats(pending) как сейчас, затем присвой pending = make(<тип pending>) (используя тот же тип ключа/значения, что у переменной pending) чтобы быстро создать пустую map; это касается куска кода с s.flushTokenStats(pending) и переменной pending.ui/src/views/AnalyticsView.vue (1)
101-105: Потенциальная ошибка при доступе к свойствамanalytics.Если
fetchSearchAnalyticsвернёт объект без некоторых свойств (например,avg_latency_msбудетundefined), вызовtoFixed(1)приведёт к runtime-ошибке. Рекомендуется добавить проверку или использовать optional chaining с fallback.♻️ Пример защитного подхода
- <span class="text-2xl font-bold text-white font-mono">{{ analytics.avg_latency_ms.toFixed(1) }}<span class="text-sm text-slate-500">ms</span></span> + <span class="text-2xl font-bold text-white font-mono">{{ (analytics.avg_latency_ms ?? 0).toFixed(1) }}<span class="text-sm text-slate-500">ms</span></span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/AnalyticsView.vue` around lines 101 - 105, The template directly calls analytics.avg_latency_ms.toFixed(1) which will throw if avg_latency_ms is undefined; update the component (AnalyticsView.vue) to guard this access by using optional chaining with a fallback or a computed property—e.g., compute a safeAvgLatency in the component (derived from analytics and fetchSearchAnalytics results) that returns a number or null, and then render safeAvgLatency?.toFixed(1) ?? '-' (or (safeAvgLatency ?? 0).toFixed(1)) in the template; ensure you reference analytics.avg_latency_ms and the fetchSearchAnalytics result when populating the computed value so the template never calls toFixed on undefined.ui/src/views/HomeView.vue (1)
2-7: Дублирующийся импортuseSSE.
useSSEимпортируется дважды: на строке 2 (из@/composables) и на строке 7 (снова из@/composables). Это избыточно.♻️ Предлагаемое исправление
-import { useStats, useTimeline, useHealth } from '@/composables' +import { useStats, useTimeline, useHealth, useSSE } from '@/composables' import StatsCards from '@/components/StatsCards.vue' import Sidebar from '@/components/Sidebar.vue' import FilterTabs from '@/components/FilterTabs.vue' import Timeline from '@/components/Timeline.vue' -import { useSSE } from '@/composables'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/HomeView.vue` around lines 2 - 7, The file HomeView.vue imports useSSE twice (from '@/composables') — remove the duplicate import so useSSE is only imported once; locate the duplicate import lines referencing useSSE in the import block (symbols: useSSE, useStats, useTimeline, useHealth, StatsCards, Sidebar, FilterTabs, Timeline) and delete the redundant useSSE import or merge it into the existing import statement to avoid the duplicated symbol.ui/src/components/layout/AppSidebar.vue (2)
49-52: Обработка ошибок при logout.Если
logout()выбросит исключение (например, сетевая ошибка), пользователь не будет перенаправлен на страницу входа. Возможно, стоит перенаправлять в любом случае.♻️ Предлагаемое исправление
async function handleLogout() { - await logout() - router.push({ name: 'login' }) + try { + await logout() + } finally { + router.push({ name: 'login' }) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/AppSidebar.vue` around lines 49 - 52, The handleLogout function currently awaits logout() and only navigates on success; wrap the logout() call in try/catch (or use try/finally) so any exception is caught and the user is still redirected by calling router.push({ name: 'login' }) in the finally (or in both success and catch paths); also log the error (e.g., console.error or app logger) inside the catch and optionally show a user-friendly notification, referencing the handleLogout function and the logout and router.push calls.
42-47: Потенциально некорректное сопоставление активного маршрута.Использование
startsWithможет привести к ложным срабатываниям. Например, если добавится маршрут/tokens-admin, он будет подсвечиваться при нахождении на/tokens. Рекомендуется использовать точное совпадение или проверку по сегментам пути.♻️ Предлагаемое исправление
function isActive(item: NavItem): boolean { if (item.path === '/') { return route.path === '/' } - return route.path.startsWith(item.path) + return route.path === item.path || route.path.startsWith(item.path + '/') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/AppSidebar.vue` around lines 42 - 47, The isActive function is using startsWith which can falsely match routes (e.g., item.path '/tokens' matching '/tokens-admin'); update isActive (the function that takes NavItem and reads route.path) to only treat an item as active when the paths are exactly equal or when the route path continues with a path-segment boundary (for example check route.path === item.path OR route.path.startsWith(item.path + '/') ), keeping the special-case for '/' unchanged; implement this change inside isActive so submenu and edge routes no longer produce false positives.ui/src/composables/usePagination.ts (1)
58-61: Отсутствует валидация вsetOffset.Функция
setOffsetне проверяет валидность значенияnewOffset. Отрицательные значения или значения, превышающиеtotal, могут привести к некорректному состоянию пагинации.♻️ Предлагаемое исправление
function setOffset(newOffset: number) { + if (newOffset < 0) { + newOffset = 0 + } offset.value = newOffset fetchPage() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/usePagination.ts` around lines 58 - 61, The setOffset function currently assigns offset.value directly and calls fetchPage() without validation; update setOffset to validate and clamp newOffset to a safe range before applying it (e.g., ensure newOffset is at least 0 and at most Math.max(0, total - 1) or the computed max offset based on pageSize/total if pageSize is available), then set offset.value to the clamped value and call fetchPage(); reference the setOffset function, the offset reactive ref, total (and pageSize or totalPages if present), and fetchPage to locate and implement the check.ui/src/composables/useTokens.ts (1)
29-39: Рассмотрите передачу AbortSignal вcreateToken.В отличие от
loadTokens, функцияcreateне используетAbortControllerдля отмены запроса. При размонтировании компонента во время создания токена запрос продолжится.♻️ Предлагаемое улучшение
async function create(name: string, scope: string): Promise<CreateTokenResponse> { error.value = null try { - const result = await createToken({ name, scope }) + const result = await createToken({ name, scope }, abortController?.signal) await loadTokens() return result } catch (err) { + if (err instanceof Error && err.name === 'AbortError') throw err error.value = err instanceof Error ? err.message : 'Failed to create token' throw err } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/useTokens.ts` around lines 29 - 39, The create function should support cancellation like loadTokens by accepting/creating an AbortController and passing its signal into createToken; update the create function (and any callers of createToken) to call createToken({ name, scope, signal }) and ensure the controller is aborted on component unmount or when appropriate (mirror the existing AbortController handling used in loadTokens) so in-flight create requests are cancelled when the component is destroyed.ui/src/components/observation/TagEditor.vue (1)
71-86: Рекомендация: кэшировать конфигурацию концепта для устранения повторных вызовов.
getConceptConfig(concept)вызывается 4 раза для каждого концепта в цикле. Это незначительное снижение производительности, но можно оптимизировать.♻️ Предлагаемое улучшение
- <span - v-for="concept in concepts" - :key="concept" - class="inline-flex items-center gap-1 group" - > - <Badge - :icon="getConceptConfig(concept).icon" - :color-class="getConceptConfig(concept).colorClass" - :bg-class="getConceptConfig(concept).bgClass" - :border-class="getConceptConfig(concept).borderClass" - > + <span + v-for="concept in concepts" + :key="concept" + class="inline-flex items-center gap-1 group" + > + <Badge + v-bind="(() => { + const cfg = getConceptConfig(concept) + return { icon: cfg.icon, colorClass: cfg.colorClass, bgClass: cfg.bgClass, borderClass: cfg.borderClass } + })()" + >Или создайте computed property с маппингом концептов на их конфигурации.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/observation/TagEditor.vue` around lines 71 - 86, getConceptConfig(concept) is being called repeatedly inside the Badge rendering; cache the result once per loop iteration (or create a computed mapping) and reuse it to avoid 4 redundant calls per concept. For example, compute a local const/config object from getConceptConfig(concept) before rendering the Badge (or add a computed property mapping concepts to configs) and then reference that single object for icon, colorClass, bgClass, and borderClass; keep the existing removeTag, updating and concept bindings unchanged.ui/src/views/SearchView.vue (1)
10-10: Неиспользуемая функцияclearиз деструктуризации.
clearизвлекается изuseSearch(), но нигде не используется в компоненте. Рассмотрите удаление для чистоты кода или добавление кнопки очистки поиска.♻️ Предлагаемое исправление
-const { query, project, results, totalCount, loading, error, decisionMode, intent, search, clear } = useSearch() +const { query, project, results, totalCount, loading, error, decisionMode, intent, search } = useSearch()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/SearchView.vue` at line 10, В деструктуризации const { query, project, results, totalCount, loading, error, decisionMode, intent, search, clear } = useSearch() свойство clear не используется — удалите clear из списка, чтобы убрать неиспользуемую переменную, или если предусматривалась функциональность очистки, добавьте соответствующую кнопку/обработчик, который вызывает clear() (найдите useSearch и компонент SearchView.vue, обновите шаблон и/или методы, чтобы либо убрать clear из деструктуризации, либо подключить clear() к UI).ui/src/views/SessionsView.vue (1)
24-31: Несогласованное поведение поиска и фильтрации.
setProjectавтоматически вызываетloadSessions(), аhandleSearchтребует нажатия Enter. При изменении дат также вызываетсяloadSessions(). Рассмотрите возможность унификации поведения: либо поиск также запускается автоматически с debounce, либо все фильтры требуют явного подтверждения.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/SessionsView.vue` around lines 24 - 31, Несогласованное поведение: setProject вызывает loadSessions() сразу, а handleSearch() вызывает только search() по Enter—унифицируй поведение; либо изменяй handleSearch чтобы вызывал loadSessions() автоматически с debounce (например 300–500ms) вместо требовать Enter, либо убери автоматический вызов loadSessions() из setProject и вызови loadSessions() только по явному подтверждению пользователем; обнови также обработчики изменения дат, чтобы они следовали той же логике; изменения должны затронуть функции handleSearch, setProject, loadSessions и реактивные фильтры (filterProject и обработчики дат).ui/src/views/VaultView.vue (1)
72-81: Рассмотрите отмену асинхронных операций при размонтировании компонента.Если компонент размонтируется во время выполнения
handleDelete, вызовremoveCredentialпродолжится и попытается обновить состояние размонтированного компонента. Это не критично, но может вызвать предупреждения в консоли.♻️ Возможное улучшение
+const isMounted = ref(true) + +onUnmounted(() => { + isMounted.value = false + stopTick() +}) -onUnmounted(() => stopTick()) async function handleDelete() { if (!deleteTarget.value) return showDeleteConfirm.value = false try { await removeCredential(deleteTarget.value) } catch { // Error handled by composable } - deleteTarget.value = null + if (isMounted.value) { + deleteTarget.value = null + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/VaultView.vue` around lines 72 - 81, handleDelete can continue running after the component unmounts and then call removeCredential and mutate refs (deleteTarget, showDeleteConfirm) on an unmounted component; to fix, make the async operation cancellable or guard post-await state updates: create an AbortController or a mounted flag in the setup() and pass a cancellation token to removeCredential (or check the mounted flag) and only update showDeleteConfirm.value and deleteTarget.value if not cancelled/unmounted; ensure removeCredential accepts and respects an AbortSignal or that you catch AbortError and skip state updates in handleDelete and in the composable.internal/worker/handlers_analytics.go (2)
60-61: Магическое числоdays*50требует документации или пересмотра.Множитель
50для лимита выборки не обоснован. Если за день создаётся больше 50 наблюдений, данные будут неполными. Рекомендуется либо добавить комментарий с обоснованием, либо использовать более надёжный подход (например, фильтрацию по дате на уровне запроса).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_analytics.go` around lines 60 - 61, The magic multiplier days*50 used when calling s.observationStore.GetRecentObservations should be removed or documented and replaced with a robust date-based query; update the call site in handlers_analytics.go (where obs, err := s.observationStore.GetRecentObservations(r.Context(), project, days*50) is invoked) to compute an explicit cutoff time (e.g. cutoff := time.Now().Add(-time.Duration(days)*24*time.Hour)) and either call a store method that accepts a time range (e.g. GetObservationsSince or extend GetRecentObservations to accept a since timestamp) or pass a clearly named limit variable with a comment explaining any non-obvious choice, ensuring observations are filtered by created_at >= cutoff rather than relying on an arbitrary multiplier.
126-135: Рассмотрите использованиеslices.SortFuncиз стандартной библиотеки Go.Ручная реализация частичной сортировки работает, но менее читабельна. Начиная с Go 1.21+ доступен
slices.SortFunc, который делает код понятнее.♻️ Предлагаемый рефакторинг
+import "slices" + // Sort and get top concepts - type conceptEntry struct { - name string - count int - } - var topConcepts []conceptEntry - for name, count := range conceptCounts { - topConcepts = append(topConcepts, conceptEntry{name, count}) - } - for i := 0; i < len(topConcepts) && i < 10; i++ { - for j := i + 1; j < len(topConcepts); j++ { - if topConcepts[j].count > topConcepts[i].count { - topConcepts[i], topConcepts[j] = topConcepts[j], topConcepts[i] - } - } - } - if len(topConcepts) > 10 { - topConcepts = topConcepts[:10] - } + type conceptEntry struct { + name string + count int + } + topConcepts := make([]conceptEntry, 0, len(conceptCounts)) + for name, count := range conceptCounts { + topConcepts = append(topConcepts, conceptEntry{name, count}) + } + slices.SortFunc(topConcepts, func(a, b conceptEntry) int { + return b.count - a.count // descending + }) + if len(topConcepts) > 10 { + topConcepts = topConcepts[:10] + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_analytics.go` around lines 126 - 135, Replace the manual nested-loop partial sort over topConcepts with the standard library slices.SortFunc: import "slices" and call slices.SortFunc(topConcepts, func(a, b YourType) bool { return a.count > b.count }) (use the actual element type of topConcepts) to sort by count descending, then keep the existing truncation topConcepts = topConcepts[:min(len(topConcepts), 10)] (or the current if-check). Update imports accordingly and ensure the comparator uses the correct field name `count`.ui/src/views/ObservationDetailView.vue (1)
2-2: Неиспользуемый импортonUnmounted.
onUnmountedимпортируется, но не используется в компоненте. Вероятно, это осталось после рефакторинга, так как для очистки listener'а используетсяonBeforeUnmount.🧹 Предлагаемое исправление
-import { ref, computed, onMounted, onUnmounted, onBeforeUnmount } from 'vue' +import { ref, computed, onMounted, onBeforeUnmount } from 'vue'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/ObservationDetailView.vue` at line 2, The import list contains an unused symbol onUnmounted; remove onUnmounted from the import statement (leave ref, computed, onMounted, onBeforeUnmount) in the ObservationDetailView.vue import line so only actually used lifecycle hooks (onMounted, onBeforeUnmount) are imported.ui/src/composables/useVault.ts (1)
76-81: Оптимистичное обновлениеcredential_countможет привести к отрицательному значению.Если
credential_countравен 0 (например, из-за рассинхронизации), декремент приведёт к -1. Хотя маловероятно, рекомендуется добавить защиту.🛡️ Предлагаемое исправление
if (vaultStatus.value) { vaultStatus.value = { ...vaultStatus.value, - credential_count: vaultStatus.value.credential_count - 1, + credential_count: Math.max(0, vaultStatus.value.credential_count - 1), } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/useVault.ts` around lines 76 - 81, The optimistic decrement of vaultStatus.value.credential_count can produce a negative number; update the block in useVault.ts that touches vaultStatus (vaultStatus.value) to clamp or floor the new credential_count at 0 instead of blindly subtracting 1 (e.g., compute newCount = Math.max(0, vaultStatus.value.credential_count - 1) and assign that back into vaultStatus.value), ensuring the guard runs only when vaultStatus.value is present and preserving other fields via the existing spread.ui/src/views/ObservationsView.vue (1)
95-107: Клиентская фильтрация на странице может приводить к пустым страницам.Если сервер возвращает 20 записей, но все они отфильтровываются по
currentTypeилиcurrentConcept, пользователь увидит пустую страницу, хотя данные есть на других страницах. Рекомендуется применять фильтры на сервере или отображать предупреждение.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/ObservationsView.vue` around lines 95 - 107, filteredObservations is doing client-side filtering on the paginated observations (observations.value) using currentType.value and currentConcept.value which can produce empty pages even though matching items exist on other pages; update the code so filters are applied server-side by including currentType and currentConcept in the API request that loads observations (or call the existing fetch/load function whenever currentType.value or currentConcept.value changes) and remove or simplify client-side filtering, and additionally handle the case where a page returns no items by resetting/adjusting the current page (or fetching page 1) or showing a user-visible warning; reference filteredObservations, observations.value, currentType.value, currentConcept.value and the fetch/load method you use to load paginated observations when implementing this change.internal/worker/handlers_auth.go (3)
95-102: Рассмотрите добавление флагаSecureдля cookie в production.Cookie устанавливается с
HttpOnlyиSameSite=Strict, что хорошо. Однако флагSecureотсутствует, что позволяет передавать cookie по HTTP. Для production-окружения рекомендуется добавитьSecure: trueили сделать его условным на основе конфигурации.🔒 Предлагаемое исправление
http.SetCookie(w, &http.Cookie{ Name: sessionCookieName, Value: cookieValue, Path: "/", MaxAge: sessionMaxAge, HttpOnly: true, SameSite: http.SameSiteStrictMode, + Secure: r.TLS != nil, // или использовать конфигурацию })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_auth.go` around lines 95 - 102, The cookie set in the http.SetCookie call is missing the Secure flag; update the cookie construction (the http.SetCookie call that uses sessionCookieName, cookieValue and sessionMaxAge) to include Secure: true in production (or conditionally set Secure based on your runtime configuration/environment), e.g., read the app environment flag/config and set Secure accordingly so the session cookie is only sent over HTTPS in production.
358-370: Рассмотрите использованиеstrings.Containsдля читаемости.Ручная реализация поиска подстроки менее читаема, чем стандартная функция
strings.Contains.♻️ Предлагаемое упрощение
+import "strings" + // containsDuplicateKey checks error message for duplicate key indicators. func containsDuplicateKey(msg string) bool { - for _, s := range []string{"duplicate key", "23505", "UNIQUE constraint"} { - if len(msg) >= len(s) { - for i := 0; i <= len(msg)-len(s); i++ { - if msg[i:i+len(s)] == s { - return true - } - } - } + for _, pattern := range []string{"duplicate key", "23505", "UNIQUE constraint"} { + if strings.Contains(msg, pattern) { + return true + } } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_auth.go` around lines 358 - 370, The manual substring search in containsDuplicateKey is verbose and should be replaced with the standard strings.Contains for readability and correctness; update containsDuplicateKey to iterate over the slice of indicators using strings.Contains(msg, s) and add the import "strings" if not already present so the function returns true when any indicator is found and false otherwise.
318-322: Все ошибки возвращают 404, что может скрывать реальные проблемы.При ошибке
tokenStore.Revokeвсегда возвращается 404, даже если ошибка связана с недоступностью БД или другими внутренними проблемами. Это может затруднить диагностику.♻️ Предлагаемое исправление с разграничением ошибок
if err := tokenStore.Revoke(r.Context(), id); err != nil { log.Error().Err(err).Str("token_id", id).Msg("auth: failed to revoke token") - http.Error(w, "not found", http.StatusNotFound) + // Можно проверить тип ошибки для более точного статуса + if errors.Is(err, gorm.ErrRecordNotFound) { + http.Error(w, "not found", http.StatusNotFound) + } else { + http.Error(w, "internal error", http.StatusInternalServerError) + } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_auth.go` around lines 318 - 322, The Revoke error handling always returns 404 which hides real failures; update the block around tokenStore.Revoke(id) to distinguish "not found" from other errors by checking against the token store's sentinel not-found error (e.g., tokenStore.ErrNotFound or errors.Is(err, sql.ErrNoRows)) and only return http.StatusNotFound for that case, otherwise log the error with Err(err).Str("token_id", id) and return http.StatusInternalServerError; keep the existing log call but ensure messages accurately reflect the error type and do not map all failures to 404.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4e1546c-56c0-4d55-befd-486e1d0b3af1
📒 Files selected for processing (53)
internal/db/gorm/migrations.gointernal/db/gorm/models.gointernal/db/gorm/token_store.gointernal/worker/handlers_analytics.gointernal/worker/handlers_auth.gointernal/worker/handlers_maintenance.gointernal/worker/handlers_sessions_rest.gointernal/worker/handlers_tags.gointernal/worker/handlers_vault.gointernal/worker/middleware.gointernal/worker/service.gointernal/worker/token_stats.goui/package.jsonui/src/App.vueui/src/components/layout/AppHeader.vueui/src/components/layout/AppSidebar.vueui/src/components/layout/ConfirmDialog.vueui/src/components/layout/EmptyState.vueui/src/components/layout/Pagination.vueui/src/components/observation/ObservationEditor.vueui/src/components/observation/TagEditor.vueui/src/components/search/SearchBar.vueui/src/components/search/SearchResults.vueui/src/composables/index.tsui/src/composables/useAuth.tsui/src/composables/useLogs.tsui/src/composables/useObservation.tsui/src/composables/usePagination.tsui/src/composables/usePatterns.tsui/src/composables/useSearch.tsui/src/composables/useSessions.tsui/src/composables/useTokens.tsui/src/composables/useVault.tsui/src/main.tsui/src/router/index.tsui/src/types/index.tsui/src/types/observation.tsui/src/types/search.tsui/src/utils/api.tsui/src/views/AnalyticsView.vueui/src/views/GraphView.vueui/src/views/HomeView.vueui/src/views/LoginView.vueui/src/views/LogsView.vueui/src/views/ObservationDetailView.vueui/src/views/ObservationsView.vueui/src/views/PatternsView.vueui/src/views/SearchView.vueui/src/views/SessionsView.vueui/src/views/SystemView.vueui/src/views/TokensView.vueui/src/views/VaultView.vueui/tailwind.config.js
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant set of new features, primarily adding backend APIs and corresponding UI views for a comprehensive dashboard. The new functionality includes API token management, a secure vault, log streaming, analytics, and system maintenance endpoints. The authentication system has been robustly expanded to support master tokens, client API tokens, and session cookies.
My review identified one critical bug in the frontend API client for tagging observations, which would prevent the feature from working correctly. I also found a couple of medium-severity issues in the backend related to code efficiency and robustness. Specifically, I've suggested improvements for an inefficient sorting algorithm and a brittle method of parsing database error messages.
Overall, this is a well-architected and substantial contribution. The backend code is generally solid, and the UI refactoring to a multi-page application is a major improvement. After addressing the identified issues, this pull request will be a great addition.
Owner
Author
|
@codex review |
Owner
Author
🤖 PR Review MCP State (auto-managed, do not edit){
"version": 2,
"parentChildren": {},
"resolvedNitpicks": {
"coderabbit-nitpick-0303c523-95": {
"resolvedAt": "2026-03-19T05:34:35.517Z",
"resolvedBy": "agent"
},
"coderabbit-nitpick-71f12b56-358": {
"resolvedAt": "2026-03-19T05:34:40.954Z",
"resolvedBy": "agent"
},
"coderabbit-nitpick-cff407bf-318": {
"resolvedAt": "2026-03-19T05:34:45.870Z",
"resolvedBy": "agent"
}
},
"updatedAt": "2026-03-19T05:34:46.274Z"
} |
Backend fixes: - handlers_auth.go: add Secure cookie flag (r.TLS != nil), discriminate Revoke errors (404 vs 500), simplify containsDuplicateKey with strings.Contains - handlers_analytics.go: use slices.SortFunc for top concepts, add heuristic comment - handlers_maintenance.go: use context.Background() for async RunNow call - handlers_sessions_rest.go: cap sessions limit at 200, sanitize 500 error messages - middleware.go: fix FindByPrefix prefix-collision — iterate all candidates with bcrypt - token_store.go: FindByPrefix returns []APIToken to handle collisions - token_stats.go: pass ctx to flushTokenStats, use context.WithTimeout for shutdown flush Frontend fixes: - api.ts: unwrap tokens response, rename revokeToken param id, fix updateObservationTags payload - useAuth.ts: add credentials: include to all fetch calls, catch network errors in login - useTokens.ts: revoke by id, createToken passes signal - useLogs.ts: track reconnectTimer, disconnect() cancels pending reconnects - usePagination.ts: clamp setOffset to [0, maxOffset] - useVault.ts: guard credential_count decrement with Math.max(0, ...) - ObservationEditor.vue: fix array mutation in computed/handleSave with spread sort - ObservationEditor.vue: remove unused TYPE_CONFIG/OBSERVATION_TYPES imports - ObservationDetailView.vue: remove unused onUnmounted import - SearchResults.vue: replace div with RouterLink for keyboard accessibility - TagEditor.vue: pre-map conceptConfigs computed, avoid repeated getConceptConfig calls - ConfirmDialog.vue: add role=dialog, aria-modal, aria-labelledby, Escape key handler - AppSidebar.vue: fix isActive for nested routes, guard handleLogout with try/finally - ObservationsView.vue: remove Delete button (no backend endpoint), fix batch action type - TokensView.vue: use token.id for key/revoke, display token_prefix and last_used_at - AnalyticsView.vue: guard all .toFixed() calls with ?? 0 - HomeView.vue: merge duplicate useSSE import - SearchView.vue: remove unused clear from useSearch destructuring - SessionsView.vue: setProject calls search() for consistency - VaultView.vue: guard post-await state with isMounted flag - SystemView.vue: replace Promise.all with Promise.allSettled, surface real errors
This was referenced Mar 19, 2026
This was referenced Apr 12, 2026
This was referenced Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
All remaining dashboard views (Phases 6-9). Depends on: PR #20, #19, #18, #17
Test plan
Summary by CodeRabbit
Новые возможности
Обновление пользовательского интерфейса