[dashboards] Add Open Brain dashboard#57
[dashboards] Add Open Brain dashboard#57headcrest wants to merge 1 commit intoNateBJones-Projects:mainfrom
Conversation
matthallett1
left a comment
There was a problem hiding this comment.
Code Review — PR #57: Open Brain Dashboard
Great first dashboard contribution, @headcrest! This is well-structured SvelteKit with good patterns. A few items to address before merge:
✅ What's good
- Clean architecture: Server-side auth via
hooks.server.ts, MCP calls proxied through/api/mcpso keys stay server-side — exactly right - Proper SSR auth guard:
+layout.server.tsredirects unauthenticated users to/signin - Follows contribution template:
metadata.json,README.md, credential tracker, troubleshooting — all present .env.examplewith both private + public fallback: Nice progressive approach for MCP credentials- Dark theme with custom design tokens: Looks intentional and polished
- Svelte 5 runes: Modern
$state(),$derived(),$props()— not legacy stores - Custom favicons: Light/dark mode variants, SVG brain icon — nice touch
🟡 Should fix before merge
-
package.jsonname is"web"— should be"open-brain-dashboard"to avoid confusion if someone runsnpm installin a monorepo context -
MCP key in query string (
?key=${mcpKey}) — this leaks the key into server logs, Vercel function logs, and any proxy. Should useAuthorization: Bearer ${mcpKey}header instead. Check whether your MCP function accepts this (the standard OB1 edge functions do viaAuthorizationheader check). -
robots.txtallows all crawling — this is a personal dashboard with auth. Should be:User-agent: * Disallow: / -
package-lock.jsonis 3,350 lines — consider adding it to.gitignorefor the contribution (users will regenerate it withnpm install). This inflates the diff and makes review harder. Or at minimum, thenamefield inside it should match the package.json name. -
app.d.tsstill referencesPUBLIC_MCP_URL/PUBLIC_MCP_KEYinImportMetaEnv— but the preferred path uses private env vars (MCP_URL/MCP_KEY). The type declaration should reflect the preferred approach, or note both. -
Indentation inconsistency in
+server.ts(MCP proxy) — line 56 has misalignedreturn json(:if (!mcpUrl || !mcpKey) { return json( // ← should be indented
💡 Nice to have (not blocking)
- No signup flow — the README says "sign in with an existing user" but doesn't explain how to create one. A one-liner about creating a user in Supabase Dashboard would help.
crypto.randomUUID()for thought IDs — these are ephemeral client-side IDs since the MCP response doesn't return the DB UUID. Fine for display, but worth a comment explaining why.- Response parsing is fragile —
parseSearchResultsandparseListResultsparse formatted text output from MCP tools. If the MCP tool output format changes, these break silently. Consider a comment noting this coupling. - No error toast/notification — errors only go to
console.error. A simple visible error banner would improve UX. svelte.config.jshardcodesnodejs22.x— Vercel may not support this runtime yet on all plans. Considernodejs20.xfor broader compatibility.
Verdict
Approve with changes — fix items 1-6 above, then this is ready to merge. This is exactly the kind of contribution the dashboards directory was waiting for. 🎉
|
Hey @headcrest — we're excited to get this merged. Rather than send it back and forth, we're going to make the 6 fixes from the review directly on your branch and test locally. Will push the changes back once verified. Thanks for the great contribution! |
Addresses 6 items from code review: 1. Package name "web" → "open-brain-dashboard" 2. MCP key moved from query string to Authorization header 3. robots.txt disallows crawling (personal dashboard) 4. package-lock.json removed from git + gitignored 5. app.d.ts: MCP env vars optional, comment explains preferred vs fallback 6. Indentation fix in MCP proxy error return svelte-check: 0 errors, 0 warnings vite build: clean production build Original dashboard by @headcrest. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses 6 items from code review: 1. Package name "web" → "open-brain-dashboard" 2. MCP key moved from query string to Authorization header 3. robots.txt disallows crawling (personal dashboard) 4. package-lock.json removed from git + gitignored 5. app.d.ts: MCP env vars optional, comment explains preferred vs fallback 6. Indentation fix in MCP proxy error return svelte-check: 0 errors, 0 warnings vite build: clean production build Original dashboard by @headcrest. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses 6 items from code review: 1. Package name "web" → "open-brain-dashboard" 2. MCP key moved from query string to Authorization header 3. robots.txt disallows crawling (personal dashboard) 4. package-lock.json removed from git + gitignored 5. app.d.ts: MCP env vars optional, comment explains preferred vs fallback 6. Indentation fix in MCP proxy error return svelte-check: 0 errors, 0 warnings vite build: clean production build Original dashboard by @headcrest. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Combined into PR #83 along with the fixes from #66. Thanks for the dashboard @headcrest — the foundation is solid. |
What this adds
dashboards/open-brain-dashboard/.metadata.jsonand full setup documentation inREADME.md.What it requires
Validation
.env.local.