Merged
Conversation
- DB-01: Project schema provisioning function (user, session, account, verification, auth_config, env_vars tables per project) - DB-02: RBAC schema (roles, permissions, role_permissions, admin_roles tables) - DB-03: Audit log schema (immutable audit_log table) - DB-04: API keys schema (long-lived tokens with SHA-256 hashing) - DB-05: Instance settings + SMTP schema (key-value store, SMTP config, notification rules) - DB-06: Webhook delivery + function invocation logs
- DB-07: Fire-and-forget audit logger with IP extraction helper - Adds ContextVariables type declarations for adminUser and project
- DB-08: Instance settings (GET/PATCH /admin/instance, health check) - DB-09: SMTP configuration and test endpoint - DB-10: RBAC routes (roles, permissions, assignments) - DB-11: API keys CRUD with one-time plaintext display - DB-12: CLI sessions management - DB-13: Audit log routes with filtering - DB-14: Enhanced metrics (overview, timeseries, latency, top-endpoints) - DB-01: Updated projects.ts to provision schema on creation
- DB-15: Project router scaffold with validation middleware - DB-16: Per-project user management (CRUD, ban, sessions, export, stats) - DB-17: Per-project auth config (provider settings) - DB-18: Per-project database introspection (tables, columns, status) - DB-19: Per-project realtime stats - DB-20: Per-project environment variables (secret masking) - DB-21: Per-project webhooks (delivery logs, retry, test) - DB-22: Per-project functions (invocation logs, stats)
- DB-23: Notification rules (CRUD for error_rate, storage_pct, auth_failures, response_time_p99 metrics) - DB-24: Updated admin router index with all new routes - Updated admin-middleware to support API key authentication
- DB-25: Added nodemailer and @types/nodemailer to dependencies - Added test, lint, typecheck scripts to package.json - Added bun-types to tsconfig.json for type support
- audit.test.ts: Audit utility functions, IP extraction - instance.test.ts: Instance settings routes - roles.test.ts: RBAC schema and routes - api-keys.test.ts: API key generation and authentication - project-scoped.test.ts: Per-project route logic - routes.test.ts: Route utility functions 71 tests covering all new functionality
- Add backend and frontend specification documents - Update bun.lock from dependency changes
…nd v4, and TypeScript
…og, Select, and more
…, and error boundary
…tions, and API keys
…v, realtime, webhooks, functions
… and real-time log streaming Backend changes: - Add migrations 011-013 for request_logs, webhook_delivery_logs, function_invocation_logs - Create webhook-logger.ts and function-logger.ts for logging utilities - Enhance request logging middleware with project_id, user_agent, ip headers - Add global metrics endpoints: /admin/metrics/overview, /timeseries, /latency, /top-endpoints - Add per-project metrics: /admin/projects/:id/metrics/overview, /timeseries - Add webhook delivery history: /admin/webhooks/:id/deliveries, /stats - Add function invocation tracing: /admin/functions/:id/invocations, /stats - Add real-time log streaming: /admin/logs/stream with polling support
…rics, webhook/function tracing Frontend changes: - Add global ObservabilityPage with stat cards, charts, latency pills, top endpoints, live log stream - Add ProjectObservabilityPage with project-scoped request metrics and log feed - Add WebhookDeliveriesPage with delivery history, stats, and status filtering - Add FunctionInvocationsPage with invocation tracing, cold starts, latency percentiles - Add LiveLogStream component with 3s polling, pause/resume, auto-scroll, ring buffer - Add useGlobalMetrics and useLogStream hooks - Add Observability nav item to sidebar - Register new routes: /observability, /projects/:id/observability, /webhooks/:id/deliveries, /functions/:id/invocations - Add Observability tab to project detail page - Add query keys for new API endpoints
📝 WalkthroughWalkthroughThis PR introduces the complete BetterBase self-hosted admin dashboard and orchestration infrastructure. It adds a full React frontend application (apps/dashboard) with multi-page UI, comprehensive backend admin API routes for project/user/RBAC/audit/metrics management, 13 new database migrations for schema extensions and audit/logging tables, extensive TypeScript type definitions, test suites, and detailed architectural documentation. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes The PR introduces a substantial, interconnected system spanning frontend, backend, database, and documentation layers. It exhibits high logic density (metrics aggregation, audit logging, RBAC enforcement, per-project schema isolation), extensive heterogeneity across 100+ new files covering distinct domains (React components, TypeScript types, SQL migrations, Hono routes, test suites), and multi-pattern changes that require separate reasoning for each major section. Comprehension requires understanding the full orchestration flow from dashboard UI through API routes to database persistence, plus validation that security patterns (API key hashing, admin middleware, audit logging) and observability hooks (logging utilities, metrics endpoints) function correctly. Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
Author
|
Fuck it always forgot check the Lint and typecheck , test |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/server/src/index.ts (1)
29-46:⚠️ Potential issue | 🟠 MajorLog failed requests from a
finallyblock.If any downstream handler throws,
await next()rejects and this insert never runs. That drops 5xx requests frombetterbase_meta.request_logs, and the newerror_messagecolumn stays unused from this code path. Capture the error, insert infinally, then rethrow.Possible fix
app.use("*", async (c, next) => { const start = Date.now(); - await next(); - const duration = Date.now() - start; - - const projectId = c.req.header("X-Project-ID") ?? null; - const userAgent = c.req.header("User-Agent")?.slice(0, 255) ?? null; - const ip = c.req.header("X-Forwarded-For")?.split(",")[0] ?? null; - - // Fire-and-forget log insert (don't await, don't fail requests on log error) - getPool() - .query( - `INSERT INTO betterbase_meta.request_logs - (method, path, status, duration_ms, project_id, user_agent, ip) - VALUES ($1, $2, $3, $4, $5, $6, $7)`, - [c.req.method, new URL(c.req.url).pathname, c.res.status, duration, projectId, userAgent, ip], - ) - .catch(() => {}); // Silently ignore log failures + let errorMessage: string | null = null; + try { + await next(); + } catch (err) { + errorMessage = err instanceof Error ? err.message : String(err); + throw err; + } finally { + const duration = Date.now() - start; + const projectId = c.req.header("X-Project-ID") ?? null; + const userAgent = c.req.header("User-Agent")?.slice(0, 255) ?? null; + const ip = c.req.header("X-Forwarded-For")?.split(",")[0]?.trim() ?? null; + + getPool() + .query( + `INSERT INTO betterbase_meta.request_logs + (method, path, status, duration_ms, project_id, user_agent, ip, error_message) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + [c.req.method, new URL(c.req.url).pathname, c.res.status, duration, projectId, userAgent, ip, errorMessage], + ) + .catch(() => {}); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/index.ts` around lines 29 - 46, The current middleware registered with app.use("*", async (c, next) => { ... }) only performs the DB insert after await next(), so if a downstream handler throws the insert is skipped; modify this middleware to wrap await next() in try/catch/finally: capture any thrown error in the catch (store its message), ensure the getPool().query call that inserts into betterbase_meta.request_logs runs in the finally block and includes the error_message column populated from the caught error (or null when none), and then rethrow the original error after scheduling the fire-and-forget query so requests still fail as before; keep the query invocation fire-and-forget and preserve existing fields (use c.req.method, new URL(c.req.url).pathname, c.res.status, duration, projectId, userAgent, ip) while adding the error_message parameter.packages/server/src/routes/admin/projects.ts (1)
62-73:⚠️ Potential issue | 🔴 CriticalMake project creation transactional.
Line 62 persists the project before Line 70 provisions its schema. If provisioning throws, the row and
admin_key_hashare already committed, but the one-timeadmin_keyis never returned, leaving a half-created project that is hard to recover from. Run the insert + provisioning on the same client inside a single transaction and rollback on any failure.Suggested transaction pattern
- const { rows } = await pool.query( - `INSERT INTO betterbase_meta.projects (id, name, slug, admin_key_hash) - VALUES ($1, $2, $3, $4) - RETURNING id, name, slug, created_at`, - [nanoid(), name, slug, adminKeyHash], - ); - - // Provision project schema - await pool.query("SELECT betterbase_meta.provision_project_schema($1)", [slug]); - - // Return admin key plaintext ONCE — not stored, cannot be recovered - return c.json({ project: rows[0], admin_key: adminKeyPlaintext }, 201); + const client = await pool.connect(); + try { + await client.query("BEGIN"); + const { rows } = await client.query( + `INSERT INTO betterbase_meta.projects (id, name, slug, admin_key_hash) + VALUES ($1, $2, $3, $4) + RETURNING id, name, slug, created_at`, + [nanoid(), name, slug, adminKeyHash], + ); + + await client.query("SELECT betterbase_meta.provision_project_schema($1)", [slug]); + await client.query("COMMIT"); + return c.json({ project: rows[0], admin_key: adminKeyPlaintext }, 201); + } catch (error) { + await client.query("ROLLBACK"); + throw error; + } finally { + client.release(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/routes/admin/projects.ts` around lines 62 - 73, The insert into betterbase_meta.projects and the subsequent call to betterbase_meta.provision_project_schema must run in a single DB transaction so a provisioning failure rolls back the inserted row and admin_key_hash; modify the handler that currently calls pool.query(...) for the INSERT and then pool.query("SELECT betterbase_meta.provision_project_schema($1)", [slug]) to instead acquire a client (pool.connect()), client.query('BEGIN'), perform the INSERT using client.query(...) and then call the provision function using the same client, client.query('COMMIT') on success, and client.query('ROLLBACK') plus rethrow on error, finally releasing the client; return the one-time admin_key_plaintext only after a successful commit.
🟡 Minor comments (27)
docs/core/providers.md-131-132 (1)
131-132:⚠️ Potential issue | 🟡 MinorAlign best-practices guidance with the supported-provider list.
Line 131 recommends SQLite, but SQLite is not included in this page’s supported providers/config examples. Please either add SQLite sections or adjust the recommendation text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/core/providers.md` around lines 131 - 132, The guidance under the "Development" / "Production" list recommends SQLite for development but the page’s supported-provider examples don’t include SQLite; either add a new supported-provider example and configuration snippet for SQLite/Turso (matching the existing format used for PostgreSQL/Neon examples) or change the recommendation text to one of the providers already shown (e.g., replace "SQLite/Turso" with "Turso" or "PostgreSQL/Neon"). Update the "Development" list item and the supported-provider examples so they are consistent, and ensure any referenced config keys or example headings follow the same naming pattern used elsewhere on the page.README.md-297-297 (1)
297-297:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced diagram blocks.
Both blocks are missing a fence language and trigger markdownlint MD040.
Suggested fix
-``` +```text ┌────────────────────────────────────────────────────────────────────────────────┐ ... -``` +```-``` +```text ┌────────────────────────────────────────────────────────────────────────────────┐ ... -``` +```Also applies to: 1075-1075
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 297, The two fenced diagram blocks use plain triple backticks with no language, causing markdownlint MD040; update each diagram fence from ``` to include a language identifier (e.g., ```text) so the top and bottom diagram fences become ```text ... ```text (apply the same change to the second block around the ASCII box at the other occurrence), ensuring both fenced blocks are annotated to satisfy markdownlint.apps/dashboard/.gitignore-15-15 (1)
15-15:⚠️ Potential issue | 🟡 MinorFix log glob typo to prevent accidental log commits.
Line 15 uses
_.log, which won’t match normal log filenames likeapp.log.Suggested fix
-_.log +*.log🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/.gitignore` at line 15, Replace the incorrect gitignore entry `_.log` with a proper glob that matches typical log filenames (e.g., `*.log`) so files like `app.log` are ignored; update the entry in apps/dashboard/.gitignore where the `_.log` pattern appears.BETTERBASE.md-27-27 (1)
27-27:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced code block.
This triggers markdownlint MD040.
Suggested fix
-``` +```text my-project/ ├── betterbase.config.ts # Project configuration (defineConfig) ... -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@BETTERBASE.md` at line 27, The fenced code block in BETTERBASE.md is missing a language tag (triggering markdownlint MD040); update the opening fence from ``` to include an appropriate language tag such as ```text (or ```bash/```none if preferred) so the tree snippet is fenced as ```text, ensuring the closing fence remains ```; this change affects the markdown block that begins with the project tree (the fenced code block containing "my-project/ ├── betterbase.config.ts ...").packages/server/src/lib/audit.ts-73-77 (1)
73-77:⚠️ Potential issue | 🟡 MinorBug: Empty
x-forwarded-forheader returns empty string instead of falling back.When
x-forwarded-foris set to an empty string"", the function returns""instead of falling back tox-real-ipor"unknown". This is because""is not nullish (onlynull/undefinedtrigger??).🐛 Proposed fix
export function getClientIp(headers: Headers): string { - return ( - headers.get("x-forwarded-for")?.split(",")[0]?.trim() ?? headers.get("x-real-ip") ?? "unknown" - ); + const forwarded = headers.get("x-forwarded-for")?.split(",")[0]?.trim(); + if (forwarded) return forwarded; + return headers.get("x-real-ip") || "unknown"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/lib/audit.ts` around lines 73 - 77, getClientIp returns an empty string when x-forwarded-for header is present but empty; update getClientIp (the function using headers.get("x-forwarded-for") and headers.get("x-real-ip")) to treat an empty or all-whitespace x-forwarded-for as absent by trimming and checking length (or using a falsy check) before selecting the first CSV entry, and only then fall back to x-real-ip or "unknown" so empty header values don't short-circuit the fallback.packages/server/test/audit.test.ts-34-41 (1)
34-41:⚠️ Potential issue | 🟡 MinorIncomplete test: Should assert
getClientIpbehavior, not just header value.This test verifies that
headers.get()returns an empty string but doesn't test whatgetClientIp()returns in this case. This would have caught the bug where emptyx-forwarded-forreturns""instead of falling back.💚 Proposed fix to complete the test
it("should handle empty x-forwarded-for", () => { const headers = new Headers({ "x-forwarded-for": "" }); - // Empty string from get() returns empty string, not null - // This tests the logic that empty string should be handled - const value = headers.get("x-forwarded-for"); - expect(value).toBe(""); + // Empty x-forwarded-for should fall back to x-real-ip or "unknown" + expect(getClientIp(headers)).toBe("unknown"); + }); + + it("should fall back to x-real-ip when x-forwarded-for is empty", () => { + const headers = new Headers({ "x-forwarded-for": "", "x-real-ip": "10.0.0.1" }); + expect(getClientIp(headers)).toBe("10.0.0.1"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/audit.test.ts` around lines 34 - 41, Test currently asserts only headers.get() returns "" but must assert getClientIp() behavior when x-forwarded-for is empty; update the test to call the getClientIp function (reference getClientIp) with the Headers instance (the headers variable) and a fallback remote address (e.g., "1.2.3.4" or the server's remote IP) and assert that getClientIp returns the fallback IP (not an empty string). Ensure the test covers both the empty header case and that getClientIp does not return "" but instead falls back to the provided remote address.CODEBASE_MAP.md-98-98 (1)
98-98:⚠️ Potential issue | 🟡 MinorAdd a language to these new fenced blocks.
Both new ASCII-art fences are bare ``` blocks, and markdownlint is already flagging MD040 in this section. Use
textso the docs stay lint-clean.Also applies to: 150-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CODEBASE_MAP.md` at line 98, Change the two bare fenced code blocks in CODEBASE_MAP.md (the new triple-backtick blocks around the ASCII-art at the locations noted) to specify a language of "text" (i.e., replace ``` with ```text) so markdownlint MD040 is satisfied and the ASCII-art blocks remain lint-clean.packages/server/src/routes/admin/metrics-enhanced.ts-126-126 (1)
126-126:⚠️ Potential issue | 🟡 MinorSanitize
limitbefore sending it to Postgres.
parseIntreturnsNaNfor?limit=foo, and negative values surviveMath.min. Both cases reachLIMIT $1and turn a bad query param into a 500.Suggested guard
- const limit = Math.min(Number.parseInt(c.req.query("limit") ?? "10"), 50); + const parsedLimit = Number.parseInt(c.req.query("limit") ?? "10", 10); + const limit = + Number.isFinite(parsedLimit) && parsedLimit > 0 ? Math.min(parsedLimit, 50) : 10;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/routes/admin/metrics-enhanced.ts` at line 126, The limit computed as const limit = Math.min(Number.parseInt(c.req.query("limit") ?? "10"), 50) can become NaN for non-numeric input or remain negative, which leads to invalid SQL when used in LIMIT $1; fix by explicitly parsing and validating the query param: get rawLimit = c.req.query("limit"), parse to integer, if parse yields NaN or raw value is missing fallback to 10, then clamp the value to the range 1..50 before assigning to the variable limit (the same const limit used in the SQL LIMIT $1), ensuring only a valid positive integer is ever passed to Postgres.apps/dashboard/src/components/ui/ConfirmDialog.tsx-37-41 (1)
37-41:⚠️ Potential issue | 🟡 MinorReset
typedstate when dialog opens/closes.The
typedstate persists across dialog open/close cycles. If a user partially types a confirmation value, closes the dialog, then reopens it, the previous input remains. This can lead to confusing UX or unintended confirmations if the value happens to match.🛠️ Proposed fix using useEffect to reset state
-export function ConfirmDialog({ +import { useState, useEffect } from "react"; + +export function ConfirmDialog({ open, onOpenChange, title, description, confirmLabel = "Confirm", confirmValue, variant = "danger", onConfirm, loading, }: ConfirmDialogProps) { const [typed, setTyped] = useState(""); + + useEffect(() => { + if (!open) { + setTyped(""); + } + }, [open]); + const canConfirm = confirmValue ? typed === confirmValue : true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ui/ConfirmDialog.tsx` around lines 37 - 41, The typed state (typed / setTyped) persists across dialog open/close cycles; add a useEffect that listens to the AlertDialog open prop and resets typed to "" whenever open changes (e.g., useEffect(() => setTyped(""), [open])) so confirmValue and canConfirm logic starts fresh on each open; place this effect alongside the existing state and hooks in ConfirmDialog to clear the input whenever the dialog opens or closes.apps/dashboard/src/hooks/useTheme.ts-6-13 (1)
6-13:⚠️ Potential issue | 🟡 MinorValidate the stored theme before trusting it.
The cast on Line 7 accepts any persisted string, so a stale or corrupted
bb_themevalue gets written straight todata-themeuntil the next toggle. Check for"dark"/"light"explicitly and fall back to"dark".Possible fix
export function useTheme() { const [theme, setTheme] = useState<Theme>(() => { - return (localStorage.getItem("bb_theme") as Theme) ?? "dark"; + const storedTheme = localStorage.getItem("bb_theme"); + return storedTheme === "dark" || storedTheme === "light" ? storedTheme : "dark"; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/hooks/useTheme.ts` around lines 6 - 13, The initial state in useTheme reads localStorage blindly and casts to Theme, allowing invalid values to be applied; change the initialization logic in the useTheme hook (the useState initializer) so it reads localStorage.getItem("bb_theme"), checks explicitly whether the value is "dark" or "light", and returns that value only if valid, otherwise return "dark"; ensure the effect that sets document.documentElement.setAttribute("data-theme", theme) and localStorage.setItem("bb_theme", theme) keeps using the validated theme from state.apps/dashboard/src/pages/StorageBucketPage.tsx-63-85 (1)
63-85:⚠️ Potential issue | 🟡 MinorUse the filtered list for the empty state.
Line 64 branches on the unfiltered
objectsarray, but Lines 83-108 render a filtered one. A search with zero matches currently shows a blank table instead of the “No objects” state.Suggested filtering cleanup
const objects = data?.objects ?? []; + const filteredObjects = objects.filter((o: any) => + o.name.toLowerCase().includes(search.toLowerCase()), + ); @@ - {objects.length === 0 ? ( + {filteredObjects.length === 0 ? ( <EmptyState icon={HardDrive} title="No objects" description="Upload files to this bucket." /> ) : ( @@ - {objects - .filter((o: any) => o.name.includes(search)) - .map((obj: any) => ( + {filteredObjects.map((obj: any) => ( <TableRow key={obj.name}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/StorageBucketPage.tsx` around lines 63 - 85, The empty-state check uses the raw objects array but the table renders a filtered list, so when a search yields zero results you get an empty table instead of the EmptyState; in StorageBucketPage compute and use a single filtered list (e.g., const filteredObjects = objects.filter((o) => o.name.includes(search))) and replace both the ternary condition (currently checking objects.length) and the .filter(...) inside the TableBody.map to use filteredObjects so the EmptyState shows when no filtered results exist; update references to the filtered list in the TableBody map (where obj is mapped) and keep EmptyState rendering for filteredObjects.length === 0.apps/dashboard/src/components/auth/SetupGuard.tsx-8-25 (1)
8-25:⚠️ Potential issue | 🟡 MinorMissing cleanup for async effect may cause state update on unmounted component.
If the component unmounts before the fetch completes (e.g., during navigation),
setChecking(false)will still execute, causing a React warning and potential memory leak.🛡️ Proposed fix using AbortController
useEffect(() => { + const controller = new AbortController(); // Try hitting /admin/auth/setup without a token. // If setup is complete, login page is appropriate. // If setup is not done, /admin/auth/setup returns 201, not 410. fetch(`${import.meta.env.VITE_API_URL ?? "http://localhost:3001"}/admin/auth/setup`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ _check: true }), // Will fail validation but we only care about 410 + signal: controller.signal, }) .then((res) => { if (res.status === 410) { // Setup complete — redirect to login navigate("/login", { replace: true }); } setChecking(false); }) - .catch(() => setChecking(false)); + .catch((err) => { + if (err.name !== "AbortError") setChecking(false); + }); + return () => controller.abort(); }, [navigate]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/auth/SetupGuard.tsx` around lines 8 - 25, The useEffect that calls fetch in SetupGuard should be made cancellable to avoid calling setChecking on an unmounted component: create an AbortController inside the effect, pass its signal to fetch, and in the .then/.catch handlers check for aborted state (or catch the AbortError) before calling setChecking(false) or navigate; also return a cleanup function that calls controller.abort() so the pending request is cancelled when the component unmounts — update the effect that contains fetch(...)/setChecking to use the controller and guarded state updates.apps/dashboard/src/pages/projects/ProjectRealtimePage.tsx-17-17 (1)
17-17:⚠️ Potential issue | 🟡 MinorUnused state variable
enabled.The
enabledstate is set on line 58 but never read. The Switch is controlled byconfig.realtime_enabledfrom the query data. Remove this dead code.🧹 Proposed fix
export default function ProjectRealtimePage() { const { projectId } = useParams(); - const [enabled, setEnabled] = useState(false);And on line 58:
onCheckedChange={(checked) => { - setEnabled(checked); updateMutation.mutate(checked); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/projects/ProjectRealtimePage.tsx` at line 17, Remove the unused React state by deleting the useState declaration const [enabled, setEnabled] = useState(false); from ProjectRealtimePage and any references to enabled/setEnabled (they are not read—Switch is controlled by config.realtime_enabled from the query). Keep the Switch controlled by config.realtime_enabled and ensure no other logic depends on enabled or setEnabled before removing them.packages/server/src/routes/admin/cli-sessions.ts-11-17 (1)
11-17:⚠️ Potential issue | 🟡 MinorDELETE endpoint should verify admin ownership before revoking a pending code.
The DELETE operation doesn't verify that the device code belongs to the requesting admin. For consistency with the api-keys deletion pattern, add an ownership check:
DELETE FROM betterbase_meta.device_codes WHERE user_code = $1 AND (admin_user_id = $2 OR admin_user_id IS NULL)Additionally, consider whether the GET endpoint should scope pending codes to the requesting admin, since the api_keys query filters by
admin_user_id(line 23) but the device_codes query does not.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/routes/admin/cli-sessions.ts` around lines 11 - 17, Update the DELETE handler that removes device codes to verify admin ownership by changing its SQL to delete only when user_code = $1 AND (admin_user_id = $2 OR admin_user_id IS NULL) and pass the requesting admin's id as the second parameter (locate the DELETE logic using the DELETE handler / pool.query call for device_codes). Also scope the GET query that selects pending device codes (the earlier pool.query returning { rows: pending }) to the requesting admin to match api_keys behavior (filter by admin_user_id) or explicitly document why it should remain global.apps/dashboard/src/pages/SetupPage.tsx-63-64 (1)
63-64:⚠️ Potential issue | 🟡 MinorBind each label to its input.
These
Labelcomponents are not associated with their fields, so click-to-focus and explicit screen-reader labeling are lost.Suggested fix
- <Label>Email</Label> - <Input {...register("email")} type="email" placeholder="admin@example.com" /> + <Label htmlFor="email">Email</Label> + <Input id="email" {...register("email")} type="email" placeholder="admin@example.com" /> ... - <Label>Password</Label> - <Input {...register("password")} type="password" placeholder="Min. 8 characters" /> + <Label htmlFor="password">Password</Label> + <Input id="password" {...register("password")} type="password" placeholder="Min. 8 characters" />Also applies to: 72-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/SetupPage.tsx` around lines 63 - 64, The Label components are not associated with their inputs, breaking click-to-focus and screen-reader labeling; update each pair (e.g., the Label and Input around register("email") and the other pair at lines 72-73) so the Input receives a unique id (like "email" and "password") and the corresponding Label uses htmlFor that id (ensure the Input component forwards the id prop if it's a custom component). This binds the label to the input and restores accessible focus/aria behavior.docs/core/middleware.md-112-123 (1)
112-123:⚠️ Potential issue | 🟡 MinorImport
requireRLSin both examples.Both snippets call
requireRLS()but omit it from the import list, so they are not copy-pasteable as written.Suggested fix
-import { rlsSession, requestLogger } from '@betterbase/core/middleware'; +import { rlsSession, requestLogger, requireRLS } from '@betterbase/core/middleware'; ... -import { rlsSession } from '@betterbase/core/middleware'; +import { rlsSession, requireRLS } from '@betterbase/core/middleware';Also applies to: 132-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/core/middleware.md` around lines 112 - 123, The examples import rlsSession and requestLogger but call requireRLS() without importing it; update the import line that currently reads "import { rlsSession, requestLogger } from '@betterbase/core/middleware';" to also import requireRLS so the snippet is copy-pasteable—ensure the same fix is applied to the other snippet around lines 132-145 that uses requireRLS as well.docs/core/middleware.md-83-87 (1)
83-87:⚠️ Potential issue | 🟡 MinorAdd a language to the output fence.
This block triggers MD040 and keeps the docs linter noisy.
Suggested fix
-``` +```text GET /api/users 200 45ms POST /api/users 201 123ms GET /api/users/1 404 12ms</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/core/middleware.mdaround lines 83 - 87, The fenced code block in
docs/core/middleware.md is missing a language tag which triggers MD040; update
the block surrounding the sample HTTP logs to use a language tag (e.g., change
totext) so the snippet becomes a labeled text fence and the linter stops
flagging it.</details> </blockquote></details> <details> <summary>apps/dashboard/src/pages/projects/ProjectTabs.tsx-1-9 (1)</summary><blockquote> `1-9`: _⚠️ Potential issue_ | _🟡 Minor_ **Remove unused imports to fix build.** Several imports are unused: `PageHeader`, `PageSkeleton`, `TabsContent`, `api`, `QK`, `useQuery`, and `useState`. These likely contribute to the CI build failure. <details> <summary>Proposed fix</summary> ```diff -import { PageHeader } from "@/components/ui/PageHeader"; -import { PageSkeleton } from "@/components/ui/PageSkeleton"; -import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; -import { api } from "@/lib/api"; -import { QK } from "@/lib/query-keys"; -import { useQuery } from "@tanstack/react-query"; +import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { Clock, Database, FolderOpen, Globe, Key, Users, Webhook, Zap } from "lucide-react"; -import { useState } from "react"; import { Link, useParams } from "react-router"; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/projects/ProjectTabs.tsx` around lines 1 - 9, Remove the unused imports to fix the build: delete PageHeader and PageSkeleton from the first import, remove TabsContent from the tabs import, and drop api, QK, useQuery, and useState imports since they are not referenced in ProjectTabs.tsx; if any of the lucide-react icons are unused, trim that import to only the icons actually used in the file (e.g., keep Clock/Database/etc. only if referenced). Ensure the remaining import statements only include symbols actually used by the component (e.g., Tabs, TabsList, TabsTrigger and the used icons). ``` </details> </blockquote></details> <details> <summary>apps/dashboard/src/pages/projects/ProjectAuthPage.tsx-113-121 (1)</summary><blockquote> `113-121`: _⚠️ Potential issue_ | _🟡 Minor_ **Guard against `NaN` when parsing session duration.** `Number.parseInt` returns `NaN` for empty or non-numeric input. Submitting `NaN` to the API may cause unexpected behavior. <details> <summary>Proposed fix</summary> ```diff onChange={(e) => - setConfig({ ...config, session_days: Number.parseInt(e.target.value) }) + { + const parsed = Number.parseInt(e.target.value, 10); + if (!Number.isNaN(parsed) && parsed > 0) { + setConfig({ ...config, session_days: parsed }); + } + } } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/projects/ProjectAuthPage.tsx` around lines 113 - 121, The onChange handler currently sets session_days with Number.parseInt which can produce NaN for empty/non-numeric input; change the handler in the Input component so you parse the value into a variable (e.g., parsed = Number.parseInt(e.target.value)) and if Number.isNaN(parsed) fall back to the previous/ sensible default (e.g., authConfig.session_days ?? 7) when calling setConfig({ ...config, session_days: ... }); keep the onBlur call to updateMutation.mutate(config) as-is so the saved payload never contains NaN. ``` </details> </blockquote></details> <details> <summary>packages/server/src/routes/admin/audit.ts-9-10 (1)</summary><blockquote> `9-10`: _⚠️ Potential issue_ | _🟡 Minor_ **`parseInt` on invalid input produces `NaN`, which may cause SQL errors.** If `limit` or `offset` query parameters contain non-numeric values, `Number.parseInt` returns `NaN`. Using `NaN` in a PostgreSQL `LIMIT` or `OFFSET` clause will cause a query error. <details> <summary>🛡️ Proposed fix: Add fallback for invalid values</summary> ```diff -const limit = Math.min(Number.parseInt(c.req.query("limit") ?? "50"), 200); -const offset = Number.parseInt(c.req.query("offset") ?? "0"); +const limit = Math.min(Number.parseInt(c.req.query("limit") ?? "50") || 50, 200); +const offset = Number.parseInt(c.req.query("offset") ?? "0") || 0; ``` The `|| 50` and `|| 0` ensure that `NaN` falls back to the default value. </details> </blockquote></details> <details> <summary>apps/dashboard/src/pages/FunctionInvocationsPage.tsx-27-40 (1)</summary><blockquote> `27-40`: _⚠️ Potential issue_ | _🟡 Minor_ **Add guard for missing `functionId` route parameter.** Using the non-null assertion (`functionId!`) will throw at runtime if the route parameter is missing or the component is rendered outside the expected route context. <details> <summary>🛡️ Proposed fix</summary> ```diff export default function FunctionInvocationsPage() { const { functionId } = useParams(); const [statusFilter, setStatusFilter] = useState<StatusFilter>("all"); + if (!functionId) { + return <div className="p-8">Function ID is required</div>; + } + const { data: stats, isLoading: loadingStats } = useQuery({ - queryKey: QK.functionStats(functionId!), + queryKey: QK.functionStats(functionId), queryFn: () => api.get<{ stats: any }>(`/admin/functions/${functionId}/stats`), refetchInterval: 30_000, }); const { data: invocations, isLoading: loadingInvocations } = useQuery({ - queryKey: QK.functionInvocations(functionId!, statusFilter), + queryKey: QK.functionInvocations(functionId, statusFilter), ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/FunctionInvocationsPage.tsx` around lines 27 - 40, The code uses non-null assertions (functionId!) in the two useQuery calls (QK.functionStats and QK.functionInvocations) which can crash if the route param is missing; remove the ! and guard the queries by enabling them only when functionId exists (e.g., add the useQuery option enabled: Boolean(functionId) or return early from the component when functionId is undefined), and construct the queryKey and api.get URL conditionally (or skip the request) so QK.functionStats, QK.functionInvocations, and the api.get calls never run with an undefined functionId. ``` </details> </blockquote></details> <details> <summary>apps/dashboard/src/pages/WebhookDeliveriesPage.tsx-52-52 (1)</summary><blockquote> `52-52`: _⚠️ Potential issue_ | _🟡 Minor_ **Remove dead code: `delivers` property does not exist in the API response.** The type annotation explicitly defines the response as `{ deliveries: any[] }`, and the backend consistently returns `deliveries` (not `delivers`). The first fallback check `deliveries?.delivers` is dead code that always evaluates to undefined. <details> <summary>✏️ Suggested fix</summary> ```diff - const d = deliveries?.delivers ?? deliveries?.deliveries ?? []; + const d = deliveries?.deliveries ?? []; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/WebhookDeliveriesPage.tsx` at line 52, The assignment to d uses a non-existent property deliveries?.delivers which is dead code; change the expression so it only uses the actual API shape (deliveries?.deliveries) with a safe fallback, e.g. replace const d = deliveries?.delivers ?? deliveries?.deliveries ?? []; with a single fallback that references deliveries?.deliveries (or destructure/access where used) so only the real property is relied upon (look for the variable d and the deliveries object in WebhookDeliveriesPage). ``` </details> </blockquote></details> <details> <summary>apps/dashboard/src/pages/settings/NotificationsPage.tsx-119-124 (1)</summary><blockquote> `119-124`: _⚠️ Potential issue_ | _🟡 Minor_ **Close the confirm dialog after a successful delete.** `deleteRule` stays set on success, so the modal remains open against a rule that no longer exists and invites duplicate submissions. <details> <summary>Suggested fix</summary> ```diff const deleteMutation = useMutation({ mutationFn: (id: string) => api.delete(`/admin/notifications/${id}`), onSuccess: () => { toast.success("Notification rule deleted"); + setDeleteRule(null); queryClient.invalidateQueries({ queryKey: QK.notifications() }); }, ``` </details> Also applies to: 301-315 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/settings/NotificationsPage.tsx` around lines 119 - 124, The confirm modal stays open after deleting because the deleteRule state isn't cleared; update the deleteMutation.onSuccess handler (the mutation created as deleteMutation) to also clear/close the confirm dialog by calling the state updater that controls the modal (e.g., setDeleteRule(null) or the modal close setter) immediately after toast.success and before/after queryClient.invalidateQueries, and apply the same change to the other delete handler around lines 301-315 so the dialog closes consistently on success. ``` </details> </blockquote></details> <details> <summary>packages/server/migrations/006_rbac.sql-83-87 (1)</summary><blockquote> `83-87`: _⚠️ Potential issue_ | _🟡 Minor_ **Comment does not match implementation for admin role permissions.** The comment states admin gets "all except settings_edit and audit_export", but the `WHERE` clause only excludes `perm_settings_edit`. If the admin role should also exclude `perm_audit_export`, update the SQL: <details> <summary>🔧 Proposed fix if audit_export should be excluded</summary> ```diff -- Admin: all except settings_edit and audit_export INSERT INTO betterbase_meta.role_permissions (role_id, permission_id) SELECT 'role_admin', id FROM betterbase_meta.permissions - WHERE id NOT IN ('perm_settings_edit') + WHERE id NOT IN ('perm_settings_edit', 'perm_audit_export') ON CONFLICT DO NOTHING; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/server/migrations/006_rbac.sql` around lines 83 - 87, The comment and SQL disagree: the INSERT into betterbase_meta.role_permissions for role_id 'role_admin' currently excludes only 'perm_settings_edit' but the comment says "all except settings_edit and audit_export"; update the WHERE clause in that INSERT (the SELECT 'role_admin', id FROM betterbase_meta.permissions ... WHERE ...) to also exclude 'perm_audit_export' (e.g., add id NOT IN ('perm_settings_edit','perm_audit_export')) so the role_admin row set matches the comment while keeping the existing ON CONFLICT DO NOTHING behavior. ``` </details> </blockquote></details> <details> <summary>apps/dashboard/src/lib/api.ts-26-29 (1)</summary><blockquote> `26-29`: _⚠️ Potential issue_ | _🟡 Minor_ **Add error handling for JSON.parse to prevent runtime exceptions.** If `localStorage` contains malformed JSON (e.g., due to manual tampering or storage corruption), `JSON.parse` will throw and crash the app. <details> <summary>🛡️ Proposed fix with try-catch</summary> ```diff export function getStoredAdmin(): { id: string; email: string } | null { const raw = localStorage.getItem("bb_admin"); - return raw ? JSON.parse(raw) : null; + if (!raw) return null; + try { + return JSON.parse(raw); + } catch { + return null; + } } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/api.ts` around lines 26 - 29, The getStoredAdmin function calls JSON.parse on localStorage data which can throw on malformed JSON; wrap the parse in a try-catch inside getStoredAdmin, catch any error from JSON.parse, optionally remove the invalid "bb_admin" item or log the error, and return null when parsing fails so the app doesn't crash (update the function name getStoredAdmin to include this safe-parse behavior). ``` </details> </blockquote></details> <details> <summary>apps/dashboard/src/lib/api.ts-84-95 (1)</summary><blockquote> `84-95`: _⚠️ Potential issue_ | _🟡 Minor_ **Missing 401 handling in `download` function.** Unlike the main `request` function, `download` doesn't handle 401 responses. If a download request returns 401, the user won't be redirected to login and credentials won't be cleared. <details> <summary>🛡️ Proposed fix to add 401 handling</summary> ```diff download: async (path: string): Promise<Blob> => { const token = getToken(); const res = await fetch(`${API_BASE}${path}`, { method: "POST", headers: { "Content-Type": "application/json", ...(token ? { Authorization: `Bearer ${token}` } : {}), }, }); + if (res.status === 401) { + clearToken(); + window.location.href = "/login"; + throw new ApiError(401, "Unauthorized"); + } if (!res.ok) throw new ApiError(res.status, "Download failed"); return res.blob(); }, ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/api.ts` around lines 84 - 95, The download function lacks 401 handling: update the download function to detect res.status === 401, clear stored credentials (call the same token/credential-clearing used elsewhere, e.g., getToken/clearToken or whichever function is used in request flow), redirect the user to the login page with a next parameter (same behavior as the main request flow), and then throw an ApiError (or return/abort) so callers don't proceed; reference the download function, ApiError, getToken and API_BASE to locate where to add the 401 branch and reuse the same logout/redirect logic as the main request handler. ``` </details> </blockquote></details> <details> <summary>apps/dashboard/src/types.ts-123-127 (1)</summary><blockquote> `123-127`: _⚠️ Potential issue_ | _🟡 Minor_ **LatencyMetric interface is unused and contains incorrect property name.** The `LatencyMetric` interface in `apps/dashboard/src/types.ts` defines `p90`, but this interface is never imported or used anywhere in the codebase. The actual working implementation uses `LatencyMetrics` (note the plural) from `apps/dashboard/src/hooks/useGlobalMetrics.ts`, which correctly defines `p95` to match the backend response. Either remove the unused `LatencyMetric` interface or correct it to `p95` for consistency with the actual API contract. <details> <summary>Proposed fix</summary> ```diff export interface LatencyMetric { p50: number; - p90: number; + p95: number; p99: number; } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/types.ts` around lines 123 - 127, The LatencyMetric interface is unused and mismatches the real type; either delete the unused LatencyMetric declaration or rename and correct it to match the actual implementation: replace LatencyMetric with LatencyMetrics and change the p90 property to p95 so it aligns with the backend and the hook useGlobalMetrics.ts; ensure any imports or references use the corrected LatencyMetrics shape (p50, p95, p99) or remove the dead interface entirely. ``` </details> </blockquote></details> </blockquote></details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `41e8ac43-e53b-4b19-af2e-7637674fce30` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d3afee2ab2fe9bebb588403301ff3603615b91e2 and cdeaeadc7ebdad96b185cd911279561136487da7. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `bun.lock` is excluded by `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (139)</summary> * `BETTERBASE.md` * `BetterBase_Dashboard_Backend_Spec.md` * `BetterBase_Dashboard_Frontend_Spec.md` * `BetterBase_Observability_Spec.docx.md` * `CODEBASE_MAP.md` * `README.md` * `apps/dashboard/.gitignore` * `apps/dashboard/Dockerfile` * `apps/dashboard/index.html` * `apps/dashboard/package.json` * `apps/dashboard/src/components/CommandPalette.tsx` * `apps/dashboard/src/components/ErrorBoundary.tsx` * `apps/dashboard/src/components/LiveLogStream.tsx` * `apps/dashboard/src/components/auth/AuthGuard.tsx` * `apps/dashboard/src/components/auth/SetupGuard.tsx` * `apps/dashboard/src/components/ui/Avatar.tsx` * `apps/dashboard/src/components/ui/ConfirmDialog.tsx` * `apps/dashboard/src/components/ui/EmptyState.tsx` * `apps/dashboard/src/components/ui/PageHeader.tsx` * `apps/dashboard/src/components/ui/PageSkeleton.tsx` * `apps/dashboard/src/components/ui/StatCard.tsx` * `apps/dashboard/src/components/ui/alert-dialog.tsx` * `apps/dashboard/src/components/ui/badge.tsx` * `apps/dashboard/src/components/ui/button.tsx` * `apps/dashboard/src/components/ui/card.tsx` * `apps/dashboard/src/components/ui/collapsible.tsx` * `apps/dashboard/src/components/ui/dialog.tsx` * `apps/dashboard/src/components/ui/dropdown-menu.tsx` * `apps/dashboard/src/components/ui/input.tsx` * `apps/dashboard/src/components/ui/label.tsx` * `apps/dashboard/src/components/ui/popover.tsx` * `apps/dashboard/src/components/ui/progress.tsx` * `apps/dashboard/src/components/ui/scroll-area.tsx` * `apps/dashboard/src/components/ui/select.tsx` * `apps/dashboard/src/components/ui/separator.tsx` * `apps/dashboard/src/components/ui/sheet.tsx` * `apps/dashboard/src/components/ui/skeleton.tsx` * `apps/dashboard/src/components/ui/switch.tsx` * `apps/dashboard/src/components/ui/table.tsx` * `apps/dashboard/src/components/ui/tabs.tsx` * `apps/dashboard/src/components/ui/textarea.tsx` * `apps/dashboard/src/components/ui/tooltip.tsx` * `apps/dashboard/src/hooks/useGlobalMetrics.ts` * `apps/dashboard/src/hooks/useLogStream.ts` * `apps/dashboard/src/hooks/useTheme.ts` * `apps/dashboard/src/index.css` * `apps/dashboard/src/layouts/AppLayout.tsx` * `apps/dashboard/src/lib/api.ts` * `apps/dashboard/src/lib/query-keys.ts` * `apps/dashboard/src/lib/utils.ts` * `apps/dashboard/src/main.tsx` * `apps/dashboard/src/pages/AuditPage.tsx` * `apps/dashboard/src/pages/FunctionInvocationsPage.tsx` * `apps/dashboard/src/pages/LoginPage.tsx` * `apps/dashboard/src/pages/LogsPage.tsx` * `apps/dashboard/src/pages/NotFoundPage.tsx` * `apps/dashboard/src/pages/ObservabilityPage.tsx` * `apps/dashboard/src/pages/OverviewPage.tsx` * `apps/dashboard/src/pages/SetupPage.tsx` * `apps/dashboard/src/pages/StorageBucketPage.tsx` * `apps/dashboard/src/pages/StoragePage.tsx` * `apps/dashboard/src/pages/TeamPage.tsx` * `apps/dashboard/src/pages/WebhookDeliveriesPage.tsx` * `apps/dashboard/src/pages/projects/ProjectAuthPage.tsx` * `apps/dashboard/src/pages/projects/ProjectDatabasePage.tsx` * `apps/dashboard/src/pages/projects/ProjectDetailPage.tsx` * `apps/dashboard/src/pages/projects/ProjectEnvPage.tsx` * `apps/dashboard/src/pages/projects/ProjectFunctionsPage.tsx` * `apps/dashboard/src/pages/projects/ProjectObservabilityPage.tsx` * `apps/dashboard/src/pages/projects/ProjectRealtimePage.tsx` * `apps/dashboard/src/pages/projects/ProjectTabs.tsx` * `apps/dashboard/src/pages/projects/ProjectWebhooksPage.tsx` * `apps/dashboard/src/pages/projects/ProjectsPage.tsx` * `apps/dashboard/src/pages/projects/users/ProjectUserPage.tsx` * `apps/dashboard/src/pages/projects/users/ProjectUsersPage.tsx` * `apps/dashboard/src/pages/settings/ApiKeysPage.tsx` * `apps/dashboard/src/pages/settings/NotificationsPage.tsx` * `apps/dashboard/src/pages/settings/SettingsPage.tsx` * `apps/dashboard/src/pages/settings/SmtpPage.tsx` * `apps/dashboard/src/routes.tsx` * `apps/dashboard/src/types.ts` * `apps/dashboard/src/vite-env.d.ts` * `apps/dashboard/tsconfig.json` * `apps/dashboard/vite.config.ts` * `docs/README.md` * `docs/client/overview.md` * `docs/core/logger.md` * `docs/core/middleware.md` * `docs/core/migration.md` * `docs/core/providers.md` * `docs/core/realtime.md` * `docs/templates/overview.md` * `docs/test-project/overview.md` * `packages/server/migrations/005_project_schema_function.sql` * `packages/server/migrations/006_rbac.sql` * `packages/server/migrations/007_audit_log.sql` * `packages/server/migrations/008_api_keys.sql` * `packages/server/migrations/009_instance_settings.sql` * `packages/server/migrations/010_delivery_invocation_logs.sql` * `packages/server/migrations/011_enhance_request_logs.sql` * `packages/server/migrations/012_webhook_delivery_logs.sql` * `packages/server/migrations/013_function_invocation_logs.sql` * `packages/server/package.json` * `packages/server/src/index.ts` * `packages/server/src/lib/admin-middleware.ts` * `packages/server/src/lib/audit.ts` * `packages/server/src/lib/function-logger.ts` * `packages/server/src/lib/webhook-logger.ts` * `packages/server/src/routes/admin/api-keys.ts` * `packages/server/src/routes/admin/audit.ts` * `packages/server/src/routes/admin/cli-sessions.ts` * `packages/server/src/routes/admin/functions.ts` * `packages/server/src/routes/admin/index.ts` * `packages/server/src/routes/admin/instance.ts` * `packages/server/src/routes/admin/logs.ts` * `packages/server/src/routes/admin/metrics-enhanced.ts` * `packages/server/src/routes/admin/metrics.ts` * `packages/server/src/routes/admin/notifications.ts` * `packages/server/src/routes/admin/project-metrics.ts` * `packages/server/src/routes/admin/project-scoped/auth-config.ts` * `packages/server/src/routes/admin/project-scoped/database.ts` * `packages/server/src/routes/admin/project-scoped/env.ts` * `packages/server/src/routes/admin/project-scoped/functions.ts` * `packages/server/src/routes/admin/project-scoped/index.ts` * `packages/server/src/routes/admin/project-scoped/realtime.ts` * `packages/server/src/routes/admin/project-scoped/users.ts` * `packages/server/src/routes/admin/project-scoped/webhooks.ts` * `packages/server/src/routes/admin/projects.ts` * `packages/server/src/routes/admin/roles.ts` * `packages/server/src/routes/admin/smtp.ts` * `packages/server/src/routes/admin/webhooks.ts` * `packages/server/src/types.d.ts` * `packages/server/test/api-keys.test.ts` * `packages/server/test/audit.test.ts` * `packages/server/test/instance.test.ts` * `packages/server/test/project-scoped.test.ts` * `packages/server/test/roles.test.ts` * `packages/server/test/routes.test.ts` * `packages/server/tsconfig.json` </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * apps/dashboard/Dockerfile </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+25
to
+31
| const CardTitle = React.forwardRef<HTMLParagraphElement, React.HTMLAttributes<HTMLHeadingElement>>( | ||
| ({ className, ...props }, ref) => ( | ||
| <h3 | ||
| ref={ref} | ||
| className={cn("font-semibold text-[var(--color-text-primary)]", className)} | ||
| {...props} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the mismatch exists and check for similar forwardRef element mismatches in UI primitives.
rg -nP 'forwardRef<\s*HTMLParagraphElement\s*,\s*React\.HTMLAttributes<HTMLHeadingElement>>' apps/dashboard/src/components/ui/card.tsx -C2
rg -nP 'forwardRef<\s*HTMLParagraphElement\s*,\s*React\.HTMLAttributes<HTMLHeadingElement>>' apps/dashboard/src/components/ui -g '*.tsx'Repository: weroperking/Betterbase
Length of output: 408
Fix CardTitle ref type mismatch (<h3> is not HTMLParagraphElement).
Line 25 declares React.forwardRef<HTMLParagraphElement, ...> but renders an <h3> element on line 27. The ref type should be HTMLHeadingElement to match the rendered element type and avoid TypeScript build failures.
Type-safe fix
-const CardTitle = React.forwardRef<HTMLParagraphElement, React.HTMLAttributes<HTMLHeadingElement>>(
+const CardTitle = React.forwardRef<HTMLHeadingElement, React.HTMLAttributes<HTMLHeadingElement>>(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const CardTitle = React.forwardRef<HTMLParagraphElement, React.HTMLAttributes<HTMLHeadingElement>>( | |
| ({ className, ...props }, ref) => ( | |
| <h3 | |
| ref={ref} | |
| className={cn("font-semibold text-[var(--color-text-primary)]", className)} | |
| {...props} | |
| /> | |
| const CardTitle = React.forwardRef<HTMLHeadingElement, React.HTMLAttributes<HTMLHeadingElement>>( | |
| ({ className, ...props }, ref) => ( | |
| <h3 | |
| ref={ref} | |
| className={cn("font-semibold text-[var(--color-text-primary)]", className)} | |
| {...props} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ui/card.tsx` around lines 25 - 31, The
CardTitle component uses React.forwardRef with the wrong ref type: update the
generic on React.forwardRef in the CardTitle declaration from
HTMLParagraphElement to HTMLHeadingElement so the forwarded ref matches the
rendered <h3> element; ensure the props generic
(React.HTMLAttributes<HTMLHeadingElement>) remains consistent with the ref
change so TypeScript no longer reports a type mismatch for CardTitle.
Comment on lines
+1
to
+8
| import type { LucideIcon } from "lucide-react"; | ||
|
|
||
| interface EmptyStateProps { | ||
| icon: LucideIcon; | ||
| title: string; | ||
| description: string; | ||
| action?: React.ReactNode; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the missing React namespace import pattern in this file.
rg -n "React\\.ReactNode|import type \\{ ReactNode \\}|import \\* as React" apps/dashboard/src/components/ui/EmptyState.tsx
# Inspect TS config flags related to JSX + UMD global namespace behavior.
fd "tsconfig*.json" -x sh -c 'echo "--- $1"; rg -n "\"jsx\"|\"allowUmdGlobalAccess\"|\"types\"" "$1"' sh {}Repository: weroperking/Betterbase
Length of output: 516
🏁 Script executed:
#!/bin/bash
# Check for other instances of React.ReactNode usage
echo "=== Searching for React.ReactNode pattern in codebase ==="
rg "React\.ReactNode" --type ts --type tsx
echo ""
echo "=== Check actual imports in the EmptyState.tsx file ==="
head -15 apps/dashboard/src/components/ui/EmptyState.tsx
echo ""
echo "=== Check TypeScript strict mode and related settings ==="
rg -A 10 "compilerOptions" apps/dashboard/tsconfig.json | head -20Repository: weroperking/Betterbase
Length of output: 1006
Import ReactNode type explicitly to avoid TypeScript compile errors.
Line 7 uses React.ReactNode without importing the React namespace. With TypeScript's strict: true configuration in apps/dashboard/tsconfig.json, this will fail compilation with Cannot find namespace 'React'. The modern jsx: "react-jsx" setting eliminates the need for React imports in JSX syntax, but not for type references.
💡 Proposed fix
+import type { ReactNode } from "react";
import type { LucideIcon } from "lucide-react";
interface EmptyStateProps {
icon: LucideIcon;
title: string;
description: string;
- action?: React.ReactNode;
+ action?: ReactNode;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import type { LucideIcon } from "lucide-react"; | |
| interface EmptyStateProps { | |
| icon: LucideIcon; | |
| title: string; | |
| description: string; | |
| action?: React.ReactNode; | |
| } | |
| import type { ReactNode } from "react"; | |
| import type { LucideIcon } from "lucide-react"; | |
| interface EmptyStateProps { | |
| icon: LucideIcon; | |
| title: string; | |
| description: string; | |
| action?: ReactNode; | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Build failed: '/home/runner/.bun/bin/bun run build' exited with code 2.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ui/EmptyState.tsx` around lines 1 - 8,
EmptyStateProps references React.ReactNode but the React namespace isn't
imported; update the file to import the type directly by adding an import type {
ReactNode } from "react" and then change the action prop type to ReactNode (or
leave as ReactNode if you keep the name) so EmptyStateProps uses the explicitly
imported ReactNode type (refer to the interface EmptyStateProps and the action
property).
Comment on lines
+1
to
+5
| interface PageHeaderProps { | ||
| title: string; | ||
| description?: string; | ||
| action?: React.ReactNode; | ||
| } |
There was a problem hiding this comment.
Missing React import for React.ReactNode type.
The interface uses React.ReactNode but React is not imported. This will cause a TypeScript compilation error and is likely contributing to the build failure.
🐛 Proposed fix
+import type { ReactNode } from "react";
+
interface PageHeaderProps {
title: string;
description?: string;
- action?: React.ReactNode;
+ action?: ReactNode;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface PageHeaderProps { | |
| title: string; | |
| description?: string; | |
| action?: React.ReactNode; | |
| } | |
| import type { ReactNode } from "react"; | |
| interface PageHeaderProps { | |
| title: string; | |
| description?: string; | |
| action?: ReactNode; | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Build failed: '/home/runner/.bun/bin/bun run build' exited with code 2.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ui/PageHeader.tsx` around lines 1 - 5, The
PageHeaderProps interface references React.ReactNode but React is not imported;
add the missing import to fix the TypeScript error—either import React (import
React from 'react') or import the type directly (import type { ReactNode } from
'react') and update PageHeaderProps to use ReactNode if you choose the type
import; ensure the import appears at the top of the file where PageHeaderProps
is declared.
Comment on lines
+1
to
+3
| import { cn } from "@/lib/utils"; | ||
|
|
||
| function Skeleton({ className, ...props }: React.HTMLAttributes<HTMLDivElement>) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/dashboard/src/components/ui/skeleton.tsxRepository: weroperking/Betterbase
Length of output: 423
🏁 Script executed:
find . -name "tsconfig*.json" -o -name "jsconfig*.json" | head -5 | xargs catRepository: weroperking/Betterbase
Length of output: 1401
🏁 Script executed:
rg "React.HTMLAttributes" apps/dashboard/src --type ts --type tsx -B 2 -A 2Repository: weroperking/Betterbase
Length of output: 93
🏁 Script executed:
rg "React\.HTMLAttributes" apps/dashboard/src -A 2 -B 2Repository: weroperking/Betterbase
Length of output: 9212
🏁 Script executed:
fd "\.tsx?$" apps/dashboard/src/components/ui | head -5 | xargs grep -l "HTMLAttributes\|React\.HTML"Repository: weroperking/Betterbase
Length of output: 50
🏁 Script executed:
cat -n apps/dashboard/src/components/ui/*.tsx | grep -A 2 -B 2 "import.*React\|HTMLAttributes"Repository: weroperking/Betterbase
Length of output: 10134
🏁 Script executed:
head -20 apps/dashboard/src/components/ui/sheet.tsxRepository: weroperking/Betterbase
Length of output: 864
🏁 Script executed:
head -20 apps/dashboard/src/components/ui/dialog.tsxRepository: weroperking/Betterbase
Length of output: 822
Import React or use the HTMLAttributes type directly.
React.HTMLAttributes is referenced without importing the React namespace, which will fail TypeScript compilation in an ESM/react-jsx setup. While modern JSX doesn't require React for syntax, type references still do.
Fix
import { cn } from "@/lib/utils";
+import type { HTMLAttributes } from "react";
-function Skeleton({ className, ...props }: React.HTMLAttributes<HTMLDivElement>) {
+function Skeleton({ className, ...props }: HTMLAttributes<HTMLDivElement>) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { cn } from "@/lib/utils"; | |
| function Skeleton({ className, ...props }: React.HTMLAttributes<HTMLDivElement>) { | |
| import { cn } from "@/lib/utils"; | |
| import type { HTMLAttributes } from "react"; | |
| function Skeleton({ className, ...props }: HTMLAttributes<HTMLDivElement>) { |
🧰 Tools
🪛 GitHub Actions: CI
[error] 1-1: Build failed: '/home/runner/.bun/bin/bun run build' exited with code 2.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ui/skeleton.tsx` around lines 1 - 3, The
component uses React.HTMLAttributes in the Skeleton function signature but never
imports React, causing TypeScript errors; fix by adding a React type import or
switching to the global JSX types: either import React (e.g., add "import React
from 'react'") so React.HTMLAttributes resolves, or replace
React.HTMLAttributes<HTMLDivElement> with the direct type
HTMLAttributes<HTMLDivElement> and import that from "react" (e.g., import {
HTMLAttributes } from 'react'), then update the Skeleton signature accordingly.
| ? `/admin/logs/stream?project_id=${projectId}&limit=100` | ||
| : `/admin/logs/stream?limit=100`; | ||
|
|
||
| const params = nextSinceRef.value ? `&since=${encodeURIComponent(nextSinceRef.value)}` : ""; |
There was a problem hiding this comment.
Critical: Fix incorrect ref property access causing build failure.
nextSinceRef.value should be nextSinceRef.current. React refs use .current to access the stored value, not .value. This TypeScript error is likely causing the pipeline build failure.
🐛 Fix the ref access
- const params = nextSinceRef.value ? `&since=${encodeURIComponent(nextSinceRef.value)}` : "";
+ const params = nextSinceRef.current ? `&since=${encodeURIComponent(nextSinceRef.current)}` : "";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const params = nextSinceRef.value ? `&since=${encodeURIComponent(nextSinceRef.value)}` : ""; | |
| const params = nextSinceRef.current ? `&since=${encodeURIComponent(nextSinceRef.current)}` : ""; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/hooks/useLogStream.ts` at line 48, The build fails because
the React ref is accessed using .value instead of .current; update the params
expression where nextSinceRef is used (the line building the params string with
`nextSinceRef.value`) to use `nextSinceRef.current` and ensure any other usages
of nextSinceRef in useLogStream.ts (or related variables) consistently use
`.current` so TypeScript recognizes the ref access.
Comment on lines
+34
to
+59
| const intervalMap: Record<string, string> = { | ||
| "24h": "1 hour", | ||
| "7d": "1 day", | ||
| "30d": "1 day", | ||
| }; | ||
|
|
||
| const rangeMap: Record<string, string> = { | ||
| "24h": "24 hours", | ||
| "7d": "7 days", | ||
| "30d": "30 days", | ||
| }; | ||
|
|
||
| const interval = intervalMap[period] ?? "1 hour"; | ||
| const range = rangeMap[period] ?? "24 hours"; | ||
|
|
||
| const pool = getPool(); | ||
|
|
||
| const { rows } = await pool.query( | ||
| ` | ||
| SELECT date_trunc('${interval}', created_at) AS bucket, | ||
| COUNT(*)::int AS total, | ||
| COUNT(*) FILTER (WHERE status >= 500)::int AS errors | ||
| FROM betterbase_meta.request_logs | ||
| WHERE created_at > NOW() - INTERVAL '${range}' | ||
| GROUP BY bucket ORDER BY bucket ASC | ||
| `, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the actual file to see the context
head -70 packages/server/src/routes/admin/metrics.ts | tail -40Repository: weroperking/Betterbase
Length of output: 1085
🌐 Web query:
PostgreSQL date_trunc function syntax precision argument
💡 Result:
The syntax of the PostgreSQL date_trunc function is: date_trunc('field', source [, time_zone]) where: - 'field' is a text string specifying the precision to truncate to (e.g., 'year', 'month', 'day', 'hour', 'minute', 'second'). - source is a timestamp, timestamptz, or interval value (date and time are automatically cast). - time_zone is an optional text argument for timestamptz inputs, specifying the time zone for truncation. The precision argument ('field') must be one of the valid values accepted by PostgreSQL for date_trunc, which are the same as the fields for EXTRACT/date_part: microseconds, milliseconds, second, minute, hour, day, week, month, quarter, year, decade, century, millennium (and aliases like microsecond, msec). These set all less significant fields to zero (or 1 for day/month). Examples: date_trunc('hour', timestamp '2001-02-16 20:38:40') -> 2001-02-16 20:00:00 date_trunc('day', timestamptz '2001-02-16 20:38:40+00', 'Australia/Sydney') -> 2001-02-16 13:00:00+00 date_trunc('hour', interval '2 days 3 hours 40 minutes') -> 2 days 03:00:00
Citations:
- 1: https://www.postgresql.org/docs/15/functions-datetime.html
- 2: https://www.postgresql.org/docs/16/functions-datetime.html
- 3: https://www.postgresql.org/docs/current/functions-datetime.html
- 4: https://www.postgresql.org/docs/13/functions-datetime.html
- 5: https://www.postgresql.org/docs/10/functions-datetime.html
Use a valid date_trunc precision here.
PostgreSQL date_trunc requires precision keywords like hour and day, not interval strings like 1 hour / 1 day. The current code will fail with a database error at runtime.
Suggested fix
- const intervalMap: Record<string, string> = {
- "24h": "1 hour",
- "7d": "1 day",
- "30d": "1 day",
+ const bucketMap: Record<string, "hour" | "day"> = {
+ "24h": "hour",
+ "7d": "day",
+ "30d": "day",
};
@@
- const interval = intervalMap[period] ?? "1 hour";
+ const bucket = bucketMap[period] ?? "hour";
@@
- SELECT date_trunc('${interval}', created_at) AS bucket,
+ SELECT date_trunc('${bucket}', created_at) AS bucket,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const intervalMap: Record<string, string> = { | |
| "24h": "1 hour", | |
| "7d": "1 day", | |
| "30d": "1 day", | |
| }; | |
| const rangeMap: Record<string, string> = { | |
| "24h": "24 hours", | |
| "7d": "7 days", | |
| "30d": "30 days", | |
| }; | |
| const interval = intervalMap[period] ?? "1 hour"; | |
| const range = rangeMap[period] ?? "24 hours"; | |
| const pool = getPool(); | |
| const { rows } = await pool.query( | |
| ` | |
| SELECT date_trunc('${interval}', created_at) AS bucket, | |
| COUNT(*)::int AS total, | |
| COUNT(*) FILTER (WHERE status >= 500)::int AS errors | |
| FROM betterbase_meta.request_logs | |
| WHERE created_at > NOW() - INTERVAL '${range}' | |
| GROUP BY bucket ORDER BY bucket ASC | |
| `, | |
| const bucketMap: Record<string, "hour" | "day"> = { | |
| "24h": "hour", | |
| "7d": "day", | |
| "30d": "day", | |
| }; | |
| const rangeMap: Record<string, string> = { | |
| "24h": "24 hours", | |
| "7d": "7 days", | |
| "30d": "30 days", | |
| }; | |
| const bucket = bucketMap[period] ?? "hour"; | |
| const range = rangeMap[period] ?? "24 hours"; | |
| const pool = getPool(); | |
| const { rows } = await pool.query( | |
| ` | |
| SELECT date_trunc('${bucket}', created_at) AS bucket, | |
| COUNT(*)::int AS total, | |
| COUNT(*) FILTER (WHERE status >= 500)::int AS errors | |
| FROM betterbase_meta.request_logs | |
| WHERE created_at > NOW() - INTERVAL '${range}' | |
| GROUP BY bucket ORDER BY bucket ASC | |
| `, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/metrics.ts` around lines 34 - 59, The SQL
uses date_trunc('${interval}', ...) but intervalMap currently contains "1
hour"/"1 day" which are invalid precisions; update the mapping or derive a
separate precision value so date_trunc receives a valid keyword (e.g., 'hour' or
'day') while keeping the range string for the INTERVAL clause (e.g., '24 hours',
'7 days'); change intervalMap values to the precision tokens (or introduce a new
precisionMap) and use that precision variable in date_trunc, leaving range
unchanged for the INTERVAL '${range}' usage in the query.
Comment on lines
+8
to
+10
| function schemaName(project: { slug: string }) { | ||
| return `project_${project.slug}`; | ||
| } |
There was a problem hiding this comment.
Do not splice raw project slugs into SQL identifiers.
project.slug is interpolated into ${s}.auth_config unescaped. Slugs containing - already break these queries, and any upstream validation gap turns this into an SQL-injection surface. Quote or escape the schema identifier before interpolation.
Suggested fix
+function quoteIdentifier(value: string) {
+ return `"${value.replace(/"/g, '""')}"`;
+}
+
function schemaName(project: { slug: string }) {
- return `project_${project.slug}`;
+ return quoteIdentifier(`project_${project.slug}`);
}Also applies to: 18-20, 66-68, 82-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/project-scoped/auth-config.ts` around lines
8 - 10, The code currently splices raw project.slug into SQL identifiers via
schemaName(project) and then interpolates `${s}.auth_config`, which breaks on
hyphens and is an SQL-injection risk; replace schemaName with a safe
identifier-quoting helper (e.g., quoteIdentifier or safeSchemaName) that escapes
any double quotes by doubling them and wraps the slug with double quotes (or use
your DB client's identifier-quoting method if available), then update all usages
(the schemaName function and every place that builds `${s}.auth_config` /
`${s}....`) to use the quoted result so identifiers are always safely escaped.
Comment on lines
+55
to
+67
| const [schemaSize, connInfo] = await Promise.all([ | ||
| pool.query( | ||
| `SELECT pg_size_pretty(sum(pg_total_relation_size(quote_ident($1) || '.' || quote_ident(table_name)))::bigint) AS total_size | ||
| FROM information_schema.tables WHERE table_schema = $1`, | ||
| [s], | ||
| ), | ||
| pool.query(`SELECT count FROM pg_stat_activity WHERE state = 'active'`), | ||
| ]); | ||
|
|
||
| return c.json({ | ||
| schema_size: schemaSize.rows[0]?.total_size ?? "0 bytes", | ||
| active_connections: connInfo.rows.length, | ||
| }); |
There was a problem hiding this comment.
Invalid SQL for active connections count.
SELECT count FROM pg_stat_activity references a non-existent column. Use count(*) aggregate instead, and access the result correctly:
Proposed fix
const [schemaSize, connInfo] = await Promise.all([
pool.query(
`SELECT pg_size_pretty(sum(pg_total_relation_size(quote_ident($1) || '.' || quote_ident(table_name)))::bigint) AS total_size
FROM information_schema.tables WHERE table_schema = $1`,
[s],
),
- pool.query(`SELECT count FROM pg_stat_activity WHERE state = 'active'`),
+ pool.query(`SELECT count(*)::int AS count FROM pg_stat_activity WHERE state = 'active'`),
]);
return c.json({
schema_size: schemaSize.rows[0]?.total_size ?? "0 bytes",
- active_connections: connInfo.rows.length,
+ active_connections: connInfo.rows[0]?.count ?? 0,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [schemaSize, connInfo] = await Promise.all([ | |
| pool.query( | |
| `SELECT pg_size_pretty(sum(pg_total_relation_size(quote_ident($1) || '.' || quote_ident(table_name)))::bigint) AS total_size | |
| FROM information_schema.tables WHERE table_schema = $1`, | |
| [s], | |
| ), | |
| pool.query(`SELECT count FROM pg_stat_activity WHERE state = 'active'`), | |
| ]); | |
| return c.json({ | |
| schema_size: schemaSize.rows[0]?.total_size ?? "0 bytes", | |
| active_connections: connInfo.rows.length, | |
| }); | |
| const [schemaSize, connInfo] = await Promise.all([ | |
| pool.query( | |
| `SELECT pg_size_pretty(sum(pg_total_relation_size(quote_ident($1) || '.' || quote_ident(table_name)))::bigint) AS total_size | |
| FROM information_schema.tables WHERE table_schema = $1`, | |
| [s], | |
| ), | |
| pool.query(`SELECT count(*)::int AS count FROM pg_stat_activity WHERE state = 'active'`), | |
| ]); | |
| return c.json({ | |
| schema_size: schemaSize.rows[0]?.total_size ?? "0 bytes", | |
| active_connections: connInfo.rows[0]?.count ?? 0, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/project-scoped/database.ts` around lines 55
- 67, The SQL used to compute active connections is invalid and then
mis-accessed; update the pool.query call that assigns connInfo to use an
aggregate like `SELECT count(*)::int AS active_count FROM pg_stat_activity WHERE
state = 'active'` (in the same Promise.all where pool.query is called), and then
return the count via `connInfo.rows[0].active_count` instead of using
`connInfo.rows.length`; keep the existing schemaSize logic unchanged.
Comment on lines
+77
to
+99
| projectUserRoutes.get("/:userId", async (c) => { | ||
| const pool = getPool(); | ||
| const project = c.get("project") as { id: string; slug: string }; | ||
| const s = schemaName(project); | ||
|
|
||
| const { rows: users } = await pool.query( | ||
| `SELECT u.*, array_agg(DISTINCT a.provider_id) FILTER (WHERE a.provider_id IS NOT NULL) AS providers | ||
| FROM ${s}."user" u | ||
| LEFT JOIN ${s}.account a ON a.user_id = u.id | ||
| WHERE u.id = $1 | ||
| GROUP BY u.id`, | ||
| [c.req.param("userId")], | ||
| ); | ||
| if (users.length === 0) return c.json({ error: "User not found" }, 404); | ||
|
|
||
| const { rows: sessions } = await pool.query( | ||
| `SELECT id, expires_at, ip_address, user_agent, created_at | ||
| FROM ${s}.session WHERE user_id = $1 ORDER BY created_at DESC LIMIT 20`, | ||
| [c.req.param("userId")], | ||
| ); | ||
|
|
||
| return c.json({ user: users[0], sessions }); | ||
| }); |
There was a problem hiding this comment.
Route ordering bug: /:userId will intercept /stats/overview requests.
The route /:userId (line 77) is defined before /stats/overview (line 192). In Hono, routes match in definition order, so a request to /admin/projects/:id/users/stats/overview will be caught by the /:userId route with userId = "stats", returning a 404 "User not found" error.
🐛 Proposed fix: Move stats route before parameterized route
projectUserRoutes.get("/", async (c) => {
// ... list users
});
+// GET /admin/projects/:id/users/stats/overview — growth + activity charts
+projectUserRoutes.get("/stats/overview", async (c) => {
+ // ... stats implementation
+});
+
// GET /admin/projects/:id/users/:userId
projectUserRoutes.get("/:userId", async (c) => {
// ... get single user
});
-// GET /admin/projects/:id/users/stats/overview — growth + activity charts
-projectUserRoutes.get("/stats/overview", async (c) => {
- // ... stats implementation
-});Also applies to: 191-219
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/project-scoped/users.ts` around lines 77 -
99, The parameterized route projectUserRoutes.get("/:userId") is declared before
the more specific stats routes (e.g., the /stats/overview handlers), causing
requests like /stats/overview to be captured as userId="stats"; fix this by
reordering the route registrations so the specific stats routes are defined
before projectUserRoutes.get("/:userId") (or change the param route to a more
specific path), ensuring handlers like the stats/overview route are registered
and matched prior to the catch‑all userId route.
Comment on lines
+4
to
+14
| interface ContextVariables { | ||
| adminUser: { | ||
| id: string; | ||
| email: string; | ||
| }; | ||
| project: { | ||
| id: string; | ||
| name: string; | ||
| slug: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Hono v4, what is the correct module-augmentation interface for values used by c.set/c.get: ContextVariableMaporContextVariables?
💡 Result:
In Hono v4, the correct module-augmentation interface for values used by c.set/c.get is ContextVariableMap.
Citations:
Use Hono's correct augmentation interface (ContextVariableMap).
Line 4 augments ContextVariables, but Hono v4 requires ContextVariableMap for proper typing of c.get() and c.set() operations. Using the wrong interface will cause silent typing failures and break context variable access.
Suggested fix
declare module "hono" {
- interface ContextVariables {
+ interface ContextVariableMap {
adminUser: {
id: string;
email: string;
};
project: {
id: string;
name: string;
slug: string;
};
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/types.d.ts` around lines 4 - 14, Replace the incorrect
Hono augmentation interface name "ContextVariables" with the correct
"ContextVariableMap" in the types declaration so Hono v4 can correctly type
c.get()/c.set(); locate the interface named ContextVariables (including its
nested adminUser and project shapes) and rename it to ContextVariableMap,
preserving the same property structure and types.
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
2 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.
Sorry so many changes , and I have no time , to write them all here , just check the commits
Summary by CodeRabbit
New Features
Documentation