fix: vault UX, sessions, dropdowns, engram_ token prefix#24
Conversation
Bug 1: Vault reveal/delete errors now show as inline banner above credentials list instead of replacing the entire list with an error page. Separated actionError from loadError in useVault composable. Bug 2: Sessions page showed "no sessions found" due to field name mismatch between Go handler (workstation_id, project_id, exchange_count, last_msg_at) and frontend type (workstation, project, message_count, created_at). Added mapping layer in api.ts for both list and search. Bug 3: Native <select> elements had poor contrast in dark mode. Added global dark theme styles in main.css and fixed FilterTabs selects from bg-white/5 to bg-slate-800.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughPR обновляет префикс токена клиентского API с Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ 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 focuses on enhancing the user experience and consistency across several parts of the application. It refines how vault errors are presented to users, ensures accurate display of session information, and improves the visual consistency of dropdowns in dark mode. Additionally, a significant internal change involves updating the API token prefix, which impacts token handling and authentication mechanisms. 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
|
![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 several valuable improvements across the application, including fixing the vault UX for errors, correcting session field mappings, improving dropdown styling in dark mode, and renaming the token prefix for clarity. The changes are well-implemented. I've added a few suggestions to improve maintainability by replacing hardcoded values with constants and removing redundant CSS classes. These changes will make the code easier to manage in the future.
|
|
||
| // 1b. Check if it's a client token (eng_* prefix) | ||
| if strings.HasPrefix(providedToken, "eng_") && store != nil { | ||
| if strings.HasPrefix(providedToken, "engram_") && store != nil { |
There was a problem hiding this comment.
To improve maintainability and avoid hardcoded values, it's better to use the tokenRawPrefix constant which is defined in handlers_auth.go and available in this package.
| if strings.HasPrefix(providedToken, "engram_") && store != nil { | |
| if strings.HasPrefix(providedToken, tokenRawPrefix) && store != nil { |
| // Extract prefix: chars 7-15 (first 8 hex chars after "engram_") | ||
| if len(rawToken) < 15 { | ||
| http.Error(w, "unauthorized", http.StatusUnauthorized) | ||
| return true | ||
| } | ||
| prefix := rawToken[4:12] | ||
| prefix := rawToken[7:15] |
There was a problem hiding this comment.
Using magic numbers for token parsing logic makes the code harder to read and maintain. It's better to use the tokenRawPrefix and tokenPrefixLen constants to make the logic clearer and easier to update if the prefix or length changes in the future.
| // Extract prefix: chars 7-15 (first 8 hex chars after "engram_") | |
| if len(rawToken) < 15 { | |
| http.Error(w, "unauthorized", http.StatusUnauthorized) | |
| return true | |
| } | |
| prefix := rawToken[4:12] | |
| prefix := rawToken[7:15] | |
| // Extract token prefix for DB lookup. | |
| if len(rawToken) < len(tokenRawPrefix)+tokenPrefixLen { | |
| http.Error(w, "unauthorized", http.StatusUnauthorized) | |
| return true | |
| } | |
| prefix := rawToken[len(tokenRawPrefix) : len(tokenRawPrefix)+tokenPrefixLen] |
| <span class="text-xs text-slate-500 mr-1">Type:</span> | ||
| <select | ||
| class="bg-white/5 border border-white/10 rounded px-2 py-1 text-xs text-slate-300 focus:outline-none focus:border-claude-500" | ||
| class="bg-slate-800 border border-slate-600 rounded px-2 py-1 text-xs text-slate-300 focus:outline-none focus:border-claude-500" |
There was a problem hiding this comment.
The bg-slate-800 and border-slate-600 classes are now redundant because these styles are defined globally for <select> elements in ui/src/assets/main.css. Removing them will make the code cleaner and rely on the single source of truth for styling.
class="border rounded px-2 py-1 text-xs text-slate-300 focus:outline-none focus:border-claude-500"
| <span class="text-xs text-slate-500 mr-1">Concept:</span> | ||
| <select | ||
| class="bg-white/5 border border-white/10 rounded px-2 py-1 text-xs text-slate-300 focus:outline-none focus:border-claude-500" | ||
| class="bg-slate-800 border border-slate-600 rounded px-2 py-1 text-xs text-slate-300 focus:outline-none focus:border-claude-500" |
There was a problem hiding this comment.
Similar to the other select element, the bg-slate-800 and border-slate-600 classes are redundant due to the new global styles in main.css. Removing them simplifies the component's template.
class="border rounded px-2 py-1 text-xs text-slate-300 focus:outline-none focus:border-claude-500"
1 participant
Summary
eng_→engram_(handlers + middleware)Test plan
engram_prefixSummary by CodeRabbit
Примечания к выпуску
Новые функции
Улучшения пользовательского интерфейса
Исправления