feat: add optional web UI dashboard#18
Open
RaghavSood wants to merge 2 commits into
Open
Conversation
|
Warning Review limit reached
More reviews will be available in 49 minutes and 50 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
WalkthroughThe changes introduce a UI feature for the notification and queue management system, including backend API handlers for queue info and vault data retrieval, database methods for vault queries, frontend assets with real-time SSE-based queue updates, and configuration support for enabling the UI. Changes
🚥 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
api/server.go (1)
42-44: Consider adding validation for inspector when UI is enabled.When
uiEnabledis true, theinspectorparameter should be non-nil since UI routes depend on it for queue inspection. Adding a validation check would prevent potential nil pointer dereferences in UI handlers.🛡️ Proposed validation
if wsHandler == nil { return nil, fmt.Errorf("ws handler is nil") } + if uiEnabled && inspector == nil { + return nil, fmt.Errorf("inspector is required when UI is enabled") + } return &Server{Also applies to: 76-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/server.go` around lines 42 - 44, The Server constructor that accepts the parameters (vapidPublicKey string, inspector *asynq.Inspector, uiEnabled bool) must validate that inspector is non-nil when uiEnabled is true to avoid nil pointer derefs in UI routes; update the constructor (the function that returns (*Server, error) and references inspector/uiEnabled) to check if uiEnabled && inspector == nil and return a clear error (e.g., fmt.Errorf("inspector required when uiEnabled is true")) instead of proceeding, and add the same validation wherever the same parameter combination is used (the other constructor/call site noted around the same code) to ensure consistent behavior.ui/src/app.js (3)
5-19: SSE connection lacks reconnect logic.If the SSE connection drops (network issues, server restart), the queue status will stop updating with no visual indication to the user. EventSource automatically reconnects on some errors, but not all.
Consider adding
onerrorandonclosehandlers with exponential backoff reconnection:♻️ Suggested reconnect logic
function connectQueueStream() { var es = new EventSource("/ui/api/queue/stream"); es.onmessage = function (e) { try { var d = JSON.parse(e.data); document.getElementById("q-pending").textContent = d.pending; document.getElementById("q-active").textContent = d.active; document.getElementById("q-completed").textContent = d.completed; document.getElementById("q-retry").textContent = d.retry; document.getElementById("q-archived").textContent = d.archived; document.getElementById("q-failed").textContent = d.failed; } catch (_) {} }; + es.onerror = function () { + es.close(); + setTimeout(connectQueueStream, 3000); + }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/app.js` around lines 5 - 19, connectQueueStream currently opens an EventSource but lacks robust reconnect handling; implement onerror (and onclose if needed) handlers inside connectQueueStream to detect drops, close the current EventSource, and retry connection with exponential backoff (e.g., start 1s, double up to a max) while resetting the backoff when onmessage succeeds; ensure you store the EventSource instance in a variable scoped to connectQueueStream so you can call close() before reconnecting and avoid creating multiple concurrent connections, and also provide a max retry or a visible state update if reconnection repeatedly fails.
24-33: Silent error handling on vault fetch may confuse users.When
loadVaults()fails, the catch block is empty and the UI remains in "Loading..." state indefinitely. Consider showing an error state:♻️ Show error state on vault load failure
.catch(function () { + var tbody = document.getElementById("vault-table-body"); + tbody.innerHTML = '<tr><td colspan="3" class="px-4 py-3 text-red-400">Failed to load vaults</td></tr>'; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/app.js` around lines 24 - 33, The fetch in loadVaults currently swallows errors; update the catch block in loadVaults to handle failures by logging the error (console.error) and updating UI state so the "Loading..." doesn't persist — for example set vaults = [], call renderVaultTable() (and/or populateVaultSelect()) to re-render an empty state, and surface a user-visible error message (e.g., show or set text on an existing error element or add a simple vault load error string) so users see the failure instead of a stuck loader.
187-189:escapeAttris incomplete but safe in current usage.The function escapes
&and"but not',<, or>. Since all data attributes in the template use double quotes, this is currently safe. For defensive completeness, consider escaping additional characters:♻️ More complete attribute escaping
function escapeAttr(s) { - return (s || "").replace(/&/g, "&").replace(/"/g, """); + return (s || "").replace(/&/g, "&").replace(/"/g, """).replace(/'/g, "'").replace(/</g, "<").replace(/>/g, ">"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/app.js` around lines 187 - 189, escapeAttr currently only replaces & and " which is fragile; update the escapeAttr function to defensively escape the other common HTML-meaningful characters (at minimum <, > and ') in addition to & and " so attribute values cannot break out or inject HTML. Locate the escapeAttr function and change its replacement logic to map &[amp], " to ", ' to ' (or '), < to <, and > to > (e.g. a single regex replace using a lookup map) while still handling null/undefined by returning an empty string.api/ui_handler.go (1)
27-36: Template is parsed on every request.
template.ParseFSis called on each request to/ui. While acceptable for a low-traffic dev dashboard, consider parsing once at startup or usingsync.Oncefor better performance if traffic increases.♻️ Cache template at server level
Add a field to the Server struct and parse during initialization:
// In server.go or during NewServer uiTemplate *template.Templatefunc (s *Server) uiIndex(c echo.Context) error { - tmpl, err := template.ParseFS(ui.Templates, "templates/index.html") - if err != nil { - s.logger.Errorf("Failed to parse UI template: %v", err) - return c.String(http.StatusInternalServerError, "template error") - } c.Response().Header().Set("Content-Type", "text/html; charset=utf-8") c.Response().WriteHeader(http.StatusOK) - return tmpl.Execute(c.Response().Writer, nil) + return s.uiTemplate.Execute(c.Response().Writer, nil) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/ui_handler.go` around lines 27 - 36, The ui template is being parsed on every request in uiIndex via template.ParseFS(ui.Templates,...); add a cached template field on Server (e.g., uiTemplate *template.Template) and initialize it once during server construction (NewServer or server initialization) by parsing ui.Templates there (or protect lazy init with sync.Once), then change uiIndex to use s.uiTemplate.Execute(...) and return an error if the cached template is nil or execution fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/ui_handler.go`:
- Around line 15-25: In registerUIRoutes, stop ignoring the error returned by
fs.Sub when creating staticFS: change the call to capture (staticFS, err :=
fs.Sub(ui.Static, ".")). If err != nil, log the error via the server logger
(e.g., s.logger.Errorf or use echo's logger) and return (or fail fast) so you
don't register the static file handler with an invalid staticFS; only call
http.FileServer(http.FS(staticFS)) and register the "/ui/static/*" route when
err == nil.
---
Nitpick comments:
In `@api/server.go`:
- Around line 42-44: The Server constructor that accepts the parameters
(vapidPublicKey string, inspector *asynq.Inspector, uiEnabled bool) must
validate that inspector is non-nil when uiEnabled is true to avoid nil pointer
derefs in UI routes; update the constructor (the function that returns (*Server,
error) and references inspector/uiEnabled) to check if uiEnabled && inspector ==
nil and return a clear error (e.g., fmt.Errorf("inspector required when
uiEnabled is true")) instead of proceeding, and add the same validation wherever
the same parameter combination is used (the other constructor/call site noted
around the same code) to ensure consistent behavior.
In `@api/ui_handler.go`:
- Around line 27-36: The ui template is being parsed on every request in uiIndex
via template.ParseFS(ui.Templates,...); add a cached template field on Server
(e.g., uiTemplate *template.Template) and initialize it once during server
construction (NewServer or server initialization) by parsing ui.Templates there
(or protect lazy init with sync.Once), then change uiIndex to use
s.uiTemplate.Execute(...) and return an error if the cached template is nil or
execution fails.
In `@ui/src/app.js`:
- Around line 5-19: connectQueueStream currently opens an EventSource but lacks
robust reconnect handling; implement onerror (and onclose if needed) handlers
inside connectQueueStream to detect drops, close the current EventSource, and
retry connection with exponential backoff (e.g., start 1s, double up to a max)
while resetting the backoff when onmessage succeeds; ensure you store the
EventSource instance in a variable scoped to connectQueueStream so you can call
close() before reconnecting and avoid creating multiple concurrent connections,
and also provide a max retry or a visible state update if reconnection
repeatedly fails.
- Around line 24-33: The fetch in loadVaults currently swallows errors; update
the catch block in loadVaults to handle failures by logging the error
(console.error) and updating UI state so the "Loading..." doesn't persist — for
example set vaults = [], call renderVaultTable() (and/or populateVaultSelect())
to re-render an empty state, and surface a user-visible error message (e.g.,
show or set text on an existing error element or add a simple vault load error
string) so users see the failure instead of a stuck loader.
- Around line 187-189: escapeAttr currently only replaces & and " which is
fragile; update the escapeAttr function to defensively escape the other common
HTML-meaningful characters (at minimum <, > and ') in addition to & and " so
attribute values cannot break out or inject HTML. Locate the escapeAttr function
and change its replacement logic to map &[amp], " to ", ' to ' (or
'), < to <, and > to > (e.g. a single regex replace using a lookup
map) while still handling null/undefined by returning an empty string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3aba03d-fd3a-4f0d-93f1-51c82f321182
⛔ Files ignored due to path filters (1)
ui/dist/output.cssis excluded by!**/dist/**
📒 Files selected for processing (11)
api/server.goapi/ui_handler.gocmd/server/main.goconfig/config.godeploy/dev/01_notification.yamlstorage/database.goui/embed.goui/src/app.jsui/src/input.cssui/tailwind.config.jsui/templates/index.html
rcoderdev
approved these changes
Apr 5, 2026
|
@RaghavSood Still intended to merge this one? |
Collaborator
Author
|
Hmmm let me clean it up and deploy, need to add better auth handling if we intend to use it outside of dev |
…tions Add an embedded web dashboard (HTML + Tailwind + vanilla JS) gated behind a `ui: true` config flag. Provides real-time queue status via SSE, vault/device browsing, and a notification sending form. Enabled in dev deployment only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Ignored error from fs.Sub would silently serve an invalid filesystem. Now logs + returns early, so the static route is only registered when the embedded FS is valid. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gomesalexandre
force-pushed
the
feat/web-ui-dashboard
branch
from
ea7ad4d to
cb74af5
Compare
Contributor
|
@RaghavSood took this off ur plate - rebased onto current main (branch was stale, cut before MySQL migration + websocket + vault_id widening PRs landed), fixed the ignored fs.Sub error flagged by CodeRabbit (cb74af5), dropped the removed deploy yaml, and added dashboard screenshots + API receipt. ci green, still approved, ready to merge. |
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
5 participants
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
/uifor real-time queue monitoring, vault/device browsing, and sending test notificationsui: trueconfig flag (defaultfalse) - all/uiroutes return 404 when disabled//go:embed- no runtime file path dependency, works in DockerChanges
config/config.go-UI boolfield with env/config supportstorage/database.go-ListAllVaults()andListDevicesByVault()query methodsapi/server.go-inspector+uiEnabledfields, conditional route registrationapi/ui_handler.go- UI routes: index page, queue JSON, vault/device APIs, SSE streamcmd/server/main.go- Createsasynq.Inspectorwhen UI enabled, passes to serverui/- Embedded assets: HTML template (dark Tailwind theme), vanilla JS, pre-built CSSKey design decisions
EventSource) for queue updates - simpler than WebSocket for read-only streams//go:embedfor all assets - no runtime file path dependency, works in Docker/notifyendpoint for sending notificationsTest plan
go build ./...passesUI=true, openhttp://localhost:<port>/ui-until worker starts - expected)UI=false(default), confirm/uireturns 404receipts
dashboard landing page - queue status cards + vault registry (2 vaults registered, 1 and 2 devices each) + send notification form:
https://files.catbox.moe/wmj588.png
full page view - all three sections rendered after vault data loads:
https://files.catbox.moe/va89rp.png
/ui/api/vaultsJSON (token omitted from device API - security design correct):[{"vault_id":"prod-vault-xyz789","device_count":1},{"vault_id":"test-vault-abc123","device_count":2}]/ui/api/vaults/test-vault-abc123/devicesJSON (no token field in response):[{"party_name":"iPhone-14","device_type":"iOS","created_at":"2026-05-31T12:34:03.546+02:00"},{"party_name":"MacBook-Pro","device_type":"iOS","created_at":"2026-05-31T12:34:03.508+02:00"}]UI=falsegating -GET /uireturnsHTTP 404(curl confirmed)static assets -
/ui/static/dist/output.cssHTTP 200,/ui/static/src/app.jsHTTP 200Also rebased onto current main (branch was cut before MySQL migration + websocket fixes + vault_id widening), and fixed the ignored
fs.Suberror flagged by CodeRabbit (cb74af5).🤖 Generated with Claude Code