feat: dashboard UX enhancements, Inngest config UI, storage bucket management, and server improvements#68
Merged
weroperking merged 1 commit intomainfrom Apr 29, 2026
Conversation
…nagement, CLI browser auto-open, and server-side Inngest settings resolution - MetricsPage: added latency distribution chart, P95 latency card, loading/error states for timeseries and latency data - StoragePage: added bucket creation dialog with mutation, updated API path and response types - StorageBucketPage: updated to new API format (Key/Size/LastModified), added human-readable size formatting - InngestDashboardPage: added in-app Inngest connection configuration (API key, env ID, base URL) via instance settings patch - OverviewPage: added empty state for recent activity - TeamPage: fixed role assignment for global scope (__global__ sentinel value) - ProjectLayout: fixed tab matching to prefer longest-matching path - CommandPalette: minor cleanup - CLI login: auto-opens browser for device authorization flow - Server inngest routes: made getInngestBaseUrl/isSelfHosted async, reading from DB instance_settings - Server instance routes: added inngest_api_key, inngest_env_id, inngest_base_url to patch schema - bun.lock: dependency updates - Utility scripts: check-admin.ts, check-hash.ts, reset-password.ts
WalkthroughPR introduces dashboard UI enhancements (command palette commands, metrics visualizations, storage bucket creation, connection configuration), backend API modifications (Inngest async resolution, additional settings validation), CLI browser auto-open functionality, and new database utility scripts for admin operations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/dashboard/src/pages/OverviewPage.tsx (1)
186-218:⚠️ Potential issue | 🟠 MajorRender loading explicitly before empty state, and replace inline empty markup with the shared EmptyState component.
At Line 187,
(auditData?.logs ?? []).length === 0treatsundefined(initial fetch) as empty, so Line 189 can show a false “No recent activity yet.” state before data arrives. Gate on loading first, then evaluate emptiness, and use the sharedEmptyStatecomponent instead of inline markup.Proposed fix
- const { data: auditData } = useQuery({ + const { data: auditData, isPending: isAuditPending } = useQuery({ queryKey: QK.audit({ limit: "8" }), queryFn: () => api.get<{ logs: any[] }>("/admin/audit?limit=8"), refetchInterval: 30_000, }); ... - {(auditData?.logs ?? []).length === 0 ? ( - <div className="text-center py-6 text-xs" style={{ color: "var(--color-text-muted)" }}> - No recent activity yet. - </div> - ) : ( - (auditData?.logs ?? []).map((log: any) => ( + {isAuditPending ? ( + <div className="space-y-2"> + <div className="h-4 rounded bg-[var(--color-surface-elevated)] animate-pulse" /> + <div className="h-4 rounded bg-[var(--color-surface-elevated)] animate-pulse" /> + </div> + ) : (auditData?.logs?.length ?? 0) === 0 ? ( + <EmptyState title="No recent activity yet." /> + ) : ( + (auditData?.logs ?? []).map((log: any) => ( ... )) )}As per coding guidelines, "Every list view needs an EmptyState component. No blank pages." and "Loading states use skeleton components, not spinners."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/OverviewPage.tsx` around lines 186 - 218, The current check treats undefined auditData as empty and shows "No recent activity yet." too early; change the render logic in OverviewPage to first check for a loading state (e.g., auditData?.logs === undefined or a dedicated auditLoading flag from the query) and render the skeleton/loading UI, then if logs exist render the list, and if logs length === 0 render the shared EmptyState component instead of the inline div; keep existing usages like auditData, auditData.logs and formatRelative but replace the inline empty markup with EmptyState and insert the skeleton while loading.apps/dashboard/src/pages/TeamPage.tsx (1)
475-485:⚠️ Potential issue | 🟠 MajorMigrate assign-role form to React Hook Form + Zod resolver.
Lines 476–86 extract and cast unvalidated FormData instead of using React Hook Form with schema validation. This violates the explicit requirement: "All forms use React Hook Form + Zod resolver. Never use uncontrolled inputs or useState for form state in data-entry forms."
The same pattern exists in the invite form at lines 426–434. Both require migration to RHF + Zod to enforce type safety and validation at the form boundary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/TeamPage.tsx` around lines 475 - 485, The assign-role form currently builds unvalidated FormData and casts values directly; replace it with React Hook Form + Zod resolver by creating a Zod schema (e.g., AssignRoleSchema) and initializing useForm({ resolver: zodResolver(AssignRoleSchema) }), then change the form to use handleSubmit to call assignRoleMutation.mutate with typed values (map project_id === "__global__" to undefined inside the submit handler) instead of FormData; apply the same pattern to the invite form (create InviteSchema, useForm with zodResolver, and call the existing invite mutation via handleSubmit), and ensure you reset the form or show errors based on mutation results.packages/server/src/routes/admin/instance.ts (1)
54-60:⚠️ Potential issue | 🟠 MajorAudit log write must be fire-and-forget; also leaks
inngest_api_keyto audit_log.Two issues:
- Per coding guidelines, audit log writes must NOT be awaited. Use fire-and-forget with
.catch(() => {}).afterData: dataincludesinngest_api_keyin plaintext. The audit_log table has no update/delete routes, so this credential persists forever. Redact sensitive fields before logging.Proposed fix
+ // Redact sensitive fields from audit log + const { inngest_api_key, ...safeData } = data; + const redactedData = inngest_api_key ? { ...safeData, inngest_api_key: "[REDACTED]" } : safeData; + - await writeAuditLog({ + writeAuditLog({ actorId: admin.id, actorEmail: admin.email, action: "settings.update", - afterData: data, + afterData: redactedData, ipAddress: getClientIp(c.req.raw.headers), - }); + }).catch(() => {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/routes/admin/instance.ts` around lines 54 - 60, The current awaited call to writeAuditLog leaks sensitive data and blocks flow; make it fire-and-forget and redact sensitive fields first: create a shallow copy of the payload (the object currently passed as data) with the sensitive key "inngest_api_key" removed or replaced (e.g., mask or delete) and then call writeAuditLog({ actorId: admin.id, actorEmail: admin.email, action: "settings.update", afterData: redactedData, ipAddress: getClientIp(c.req.raw.headers) }) without awaiting it, appending .catch(() => {}) to swallow errors; locate the call to writeAuditLog in this function and change to use the redactedData variable and non-awaited call pattern.apps/dashboard/src/pages/settings/InngestDashboardPage.tsx (1)
82-86:⚠️ Potential issue | 🟡 MinorInline query keys violate dashboard guidelines.
Lines 83, 90, 97, and 117 use inline string arrays (
["inngest-status"],["inngest-functions"],["inngest-runs", ...]). Per guidelines, query keys must come fromsrc/lib/query-keys.tsfor reliable invalidation.Add these keys to the QK factory and use them here.
Example addition to query-keys.ts
// In src/lib/query-keys.ts export const QK = { // ...existing keys inngest: { status: () => ["inngest", "status"] as const, functions: () => ["inngest", "functions"] as const, runs: (functionId: string, status?: string) => ["inngest", "runs", functionId, status] as const, }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/pages/settings/InngestDashboardPage.tsx` around lines 82 - 86, Add new query-key factories to the QK object (e.g. QK.inngest.status = () => ["inngest","status"] as const, QK.inngest.functions = () => ["inngest","functions"] as const, and QK.inngest.runs = (functionId, status?) => ["inngest","runs", functionId, status] as const) and then replace the inline query keys in InngestDashboardPage (the useQuery calls that currently pass ["inngest-status"], ["inngest-functions"], and ["inngest-runs", ...]) to use the new factory methods (e.g. useQuery({ queryKey: QK.inngest.status(), queryFn: inngestApi.getStatus, ... }) and similarly QK.inngest.functions() and QK.inngest.runs(functionId, status)). Ensure you import QK where used and preserve existing refetchInterval/params.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/CommandPalette.tsx`:
- Around line 55-57: projectTabCommands currently returns duplicate CommandItem
entries (same href `/projects/${projectId}`) which causes unstable identity in
the Projects list; update the code that builds per-project commands
(projectTabCommands and any other arrays producing per-project items) to
deduplicate by href before rendering — e.g., produce a unique list of
CommandItem where each href is unique or ensure items use a stable unique id
instead of duplicated href; specifically modify projectTabCommands and the
Projects list builder (the component that uses href as item identity) to filter
out duplicates (by href) so each CommandItem.href is unique when passed to the
renderer.
In `@apps/dashboard/src/pages/MetricsPage.tsx`:
- Around line 254-258: The XAxis tickFormatter currently always renders buckets
as hour-only, which fails for multi-day periods; change the tickFormatter on
XAxis (dataKey="bucket") to be period-aware: read the chart's selected period
variable (e.g., selectedPeriod / timeRange / period prop used by MetricsPage)
and call a small helper like formatBucket(bucket, period) that returns hour-only
for sub-day ranges, returns "MMM d" or "MMM d, HH:MM" for multi-day ranges, and
returns full date for 30d; implement formatBucket as a pure function (or method)
and swap the inline tickFormatter to delegate to it so labels adapt to the
current period and avoid repeated identical hour labels.
- Around line 228-349: The Request Trends and Latency Distribution branches
currently only handle loading/error states and render blank charts when the API
returns an empty dataset; update the render logic around the timeseries block
(check timeseriesLoading, timeseriesError and the data variable ts) and the
latency block (check latencyLoading, latencyError and the data variable
latencyBars) to add explicit EmptyState renders when ts is empty or latencyBars
is empty (e.g., ts?.length === 0 and latencyBars?.length === 0), keeping the
existing loading/error branches intact and showing a brief explanatory message
and action (like "No data for selected period") via your app's EmptyState
component or similar.
- Around line 93-99: The latencyBars array built in MetricsPage.tsx is missing
the P90 percentile; update the latencyBars construction (the latencyBars
variable using l.p50, l.p95, l.p99, l.avg) to include a P90 bar (use l.p90) with
an appropriate color (e.g., "var(--color-warning)" or a distinct palette) placed
in the percentile sequence (e.g., P50, P90, P95, P99, Avg) so the chart dataset
includes the required P90 metric.
In `@apps/dashboard/src/pages/settings/InngestDashboardPage.tsx`:
- Around line 70-75: The config UI in InngestDashboardPage currently manages
fields with useState (showConfig, configForm, setConfigForm); replace this with
React Hook Form by creating a Zod schema (e.g., ConfigSchema) for
inngest_api_key, inngest_env_id, and inngest_base_url, then call useForm({
resolver: zodResolver(ConfigSchema), defaultValues: { ... }}) and wire the form
inputs via register and handleSubmit instead of setConfigForm; update submit and
reset logic to use formState and reset from useForm, and remove the useState
configForm usage and any direct setConfigForm updates. Ensure to import zod and
`@hookform/resolvers/zod` and keep showConfig for toggling visibility only.
- Around line 122-135: The onSuccess handler of saveConfigMutation currently
calls refetchStatus() directly; replace that with queryClient.invalidateQueries
for cache correctness by invalidating QK.inngest.status() and also invalidate
QK.inngest.functions() because connection changes can affect functions; update
the saveConfigMutation onSuccess block (the handler referenced by
saveConfigMutation and refetchStatus) to call queryClient.invalidateQueries({
queryKey: QK.inngest.status() }) and queryClient.invalidateQueries({ queryKey:
QK.inngest.functions() }), then keep the existing toast.success,
setShowConfig(false), and remove the direct refetchStatus() call.
- Around line 233-238: The current saveConfigMutation.mutate call uses `field ||
undefined`, which turns empty strings into undefined and prevents clearing
stored values; update the three fields passed to saveConfigMutation
(inngest_api_key, inngest_env_id, inngest_base_url) to use nullish coalescing so
empty strings are preserved (e.g. replace `configForm.inngest_base_url ||
undefined` with `configForm.inngest_base_url ?? undefined`) so the server
receives "" when a user clears a field.
In `@apps/dashboard/src/pages/StorageBucketPage.tsx`:
- Around line 34-39: The local formatSize helper returns only up to MB and
mislabels large values; replace its implementation with the shared formatter by
calling formatBytes(bytes) instead of manual calculation, and import formatBytes
from the shared utils module; also find the other instance referenced (the
helper used at the other occurrence) and swap its usage to formatBytes as well
so all sizes use the centralized correct formatter.
- Around line 25-28: Replace the inline query key array and add a guard so the
query does not run when bucketName is missing: use the QK (query-keys) factory
from src/lib/query-keys.ts to build the key (e.g., QK.storageObjects(bucketName)
or the corresponding factory method) instead of ["storageObjects", bucketName],
and pass an enabled flag (enabled: Boolean(bucketName)) to the useQuery that
prevents calling api.get(`/admin/storage/buckets/${bucketName}/objects`) when
bucketName is undefined; keep queryFn using the bucketName variable but ensure
it only runs when enabled.
In `@apps/dashboard/src/pages/StoragePage.tsx`:
- Around line 27-29: StoragePage currently uses local state (bucketName,
setBucketName) and submits untrimmed input; migrate the create-bucket dialog to
React Hook Form with a Zod resolver so validation/normalization happen
client-side. Remove bucketName/setBucketName from the form, define a Zod schema
for the bucket name that trims and enforces non-empty/format rules (e.g.,
z.string().trim().min(1) or a .transform(trim)), wire useForm({ resolver:
zodResolver(schema) }) in the StoragePage component, replace the
uncontrolled/input-with-useState with register("bucketName") on the input inside
the dialog, and change the submit path to use handleSubmit(onSubmit) (replace
the current submit handler that posts bucketName directly and the trim-gate
check) so the payload sent to the create-bucket API uses the normalized value
from form.getValues/onSubmit; keep dialog open state (open, setOpen) as-is.
In `@check-admin.ts`:
- Around line 3-5: The check-admin.ts file currently hardcodes DB credentials in
the Client instantiation; replace this with an environment-driven approach by
reading process.env.DATABASE_URL (or a shared factory) instead of the literal
connection string, e.g., move creation into a shared function like createClient
that throws if DATABASE_URL is missing and returns new Client({ connectionString
}), then update check-admin.ts to call that factory; alternatively remove these
scripts and add them to .gitignore if they are not meant to be committed.
In `@check-hash.ts`:
- Around line 3-5: The file currently constructs a new Client with a hardcoded
connectionString containing plaintext credentials; remove the embedded secrets
and read the DB connection parameters from environment variables instead (e.g.,
replace the literal passed to Client(...) / connectionString with
process.env.DATABASE_URL or assembled values from process.env.DB_USER, DB_PASS,
DB_HOST, DB_NAME), optionally load a .env in local/dev only (using dotenv) and
ensure any one-off debug scripts with secrets are not committed (add to
.gitignore or delete); also ensure any rotated/invalidated credentials are
updated only via env/config stores rather than in code.
In `@packages/cli/src/commands/login.ts`:
- Line 71: The verification URL is built by string-concatenation which breaks if
verificationUri already has query parameters; update the code in
packages/cli/src/commands/login.ts where fullVerificationUri is constructed (and
the similar construction at the other occurrence) to create a URL object from
verificationUri and append the code via url.searchParams.set('code', userCode)
(or append) so existing query params are preserved and the code is properly
encoded, then use url.toString() for the final string.
- Around line 79-91: The try block currently awaits Bun.spawn(...) but doesn't
check the spawned process exit, so the success message in login.ts is printed
even on failure; change the three Bun.spawn calls to capture the returned
Subprocess (e.g., const proc = Bun.spawn([...])) and then await proc.exited (and
check proc.exitCode or the resolved value) before logging success; on non-zero
exit or thrown error fall back to the existing "Waiting for browser
authorization" message. Ensure you reference Bun.spawn, fullVerificationUri, and
use subprocess.exited/exitCode to determine success.
In `@packages/server/src/routes/admin/inngest.ts`:
- Around line 6-17: The code directly reads process.env in getInngestBaseUrl and
elsewhere; replace those with validateEnv() usage: import validateEnv from
src/lib/env.ts and change process.env.INNGEST_BASE_URL to
validateEnv().INNGEST_BASE_URL inside getInngestBaseUrl (preserving the DB
fallback logic), and replace the other direct process.env.INNGEST_API_KEY usage
to validateEnv().INNGEST_API_KEY; also update the Zod schema in env.ts to add
INNGEST_API_KEY as an optional string, ensure validateEnv() returns it, and
import/use that returned value in this module instead of any direct process.env
access.
In `@packages/server/src/routes/admin/instance.ts`:
- Around line 34-36: The schema allows empty strings for the inngest_api_key
field so blank values get stored; update the validation for inngest_api_key to
explicitly allow either a non-empty string or the empty literal (mirroring
inngest_base_url) by changing its validator to require .min(1) on the string and
combine with .optional().or(z.literal("")) so empty string is only accepted
intentionally; locate the schema where inngest_api_key is defined and apply this
change.
In `@reset-password.ts`:
- Around line 4-6: The Client is initialized with a hardcoded connection string
(the new Client(...) assigned to client) which leaks credentials; remove the
literal and load the DB connection string from a secure environment variable
(e.g., process.env.DATABASE_URL or a dedicated secret like
process.env.NEON_DATABASE_URL) when constructing Client in reset-password.ts,
and ensure no plaintext fallback is committed (add a runtime check that throws
or logs a clear error if the env var is missing) so credentials are not stored
in source.
- Around line 19-20: Remove the console.log that prints the plaintext
newPassword (do not emit newPassword to stdout); either delete the line logging
newPassword or replace it with a non-sensitive message and, if needed, only log
the hash variable (or a masked/shortened representation) from the same block
where newPassword and hash are handled so you never persist plaintext passwords
in logs — optionally gate any debug logging behind an explicit debug
flag/environment check.
- Around line 10-11: The script reset-password.ts currently hardcodes
newPassword and calls bcrypt.hash directly; instead accept the password via a
parameter or environment variable (e.g., process.env.NEW_ADMIN_PASSWORD or a CLI
arg) and import and call the canonical hashPassword function from
packages/server/src/lib/auth.ts (use that instead of bcrypt.hash) so hashing
strategy stays centralized and no secret is committed to source control; update
references in reset-password.ts to use the imported hashPassword and
validate/throw if no password provided.
---
Outside diff comments:
In `@apps/dashboard/src/pages/OverviewPage.tsx`:
- Around line 186-218: The current check treats undefined auditData as empty and
shows "No recent activity yet." too early; change the render logic in
OverviewPage to first check for a loading state (e.g., auditData?.logs ===
undefined or a dedicated auditLoading flag from the query) and render the
skeleton/loading UI, then if logs exist render the list, and if logs length ===
0 render the shared EmptyState component instead of the inline div; keep
existing usages like auditData, auditData.logs and formatRelative but replace
the inline empty markup with EmptyState and insert the skeleton while loading.
In `@apps/dashboard/src/pages/settings/InngestDashboardPage.tsx`:
- Around line 82-86: Add new query-key factories to the QK object (e.g.
QK.inngest.status = () => ["inngest","status"] as const, QK.inngest.functions =
() => ["inngest","functions"] as const, and QK.inngest.runs = (functionId,
status?) => ["inngest","runs", functionId, status] as const) and then replace
the inline query keys in InngestDashboardPage (the useQuery calls that currently
pass ["inngest-status"], ["inngest-functions"], and ["inngest-runs", ...]) to
use the new factory methods (e.g. useQuery({ queryKey: QK.inngest.status(),
queryFn: inngestApi.getStatus, ... }) and similarly QK.inngest.functions() and
QK.inngest.runs(functionId, status)). Ensure you import QK where used and
preserve existing refetchInterval/params.
In `@apps/dashboard/src/pages/TeamPage.tsx`:
- Around line 475-485: The assign-role form currently builds unvalidated
FormData and casts values directly; replace it with React Hook Form + Zod
resolver by creating a Zod schema (e.g., AssignRoleSchema) and initializing
useForm({ resolver: zodResolver(AssignRoleSchema) }), then change the form to
use handleSubmit to call assignRoleMutation.mutate with typed values (map
project_id === "__global__" to undefined inside the submit handler) instead of
FormData; apply the same pattern to the invite form (create InviteSchema,
useForm with zodResolver, and call the existing invite mutation via
handleSubmit), and ensure you reset the form or show errors based on mutation
results.
In `@packages/server/src/routes/admin/instance.ts`:
- Around line 54-60: The current awaited call to writeAuditLog leaks sensitive
data and blocks flow; make it fire-and-forget and redact sensitive fields first:
create a shallow copy of the payload (the object currently passed as data) with
the sensitive key "inngest_api_key" removed or replaced (e.g., mask or delete)
and then call writeAuditLog({ actorId: admin.id, actorEmail: admin.email,
action: "settings.update", afterData: redactedData, ipAddress:
getClientIp(c.req.raw.headers) }) without awaiting it, appending .catch(() =>
{}) to swallow errors; locate the call to writeAuditLog in this function and
change to use the redactedData variable and non-awaited call pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3a11a89-5e4a-4add-be3d-cd103cf5d0f0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock,!**/bun.lock,!**/*.lock
📒 Files selected for processing (14)
apps/dashboard/src/components/CommandPalette.tsxapps/dashboard/src/pages/MetricsPage.tsxapps/dashboard/src/pages/OverviewPage.tsxapps/dashboard/src/pages/StorageBucketPage.tsxapps/dashboard/src/pages/StoragePage.tsxapps/dashboard/src/pages/TeamPage.tsxapps/dashboard/src/pages/projects/ProjectLayout.tsxapps/dashboard/src/pages/settings/InngestDashboardPage.tsxcheck-admin.tscheck-hash.tspackages/cli/src/commands/login.tspackages/server/src/routes/admin/inngest.tspackages/server/src/routes/admin/instance.tsreset-password.ts
Comment on lines
+55
to
+57
| const projectTabCommands = (projectId: string): CommandItem[] => [ | ||
| { label: "Overview", href: `/projects/${projectId}`, icon: FolderOpen }, | ||
| { label: "Observability", href: `/projects/${projectId}/observability`, icon: Activity }, |
There was a problem hiding this comment.
Deduplicate per-project commands before rendering.
Line 90 and Line 56 both produce the same route (/projects/${projectId}), then Line 167 uses href as the item identity. This introduces duplicate command identities in the Projects list and unstable item behavior.
Proposed fix
const allCommands = useMemo(() => {
const commands = [...staticCommands];
if (projectsData?.projects) {
for (const project of projectsData.projects) {
commands.push({
label: project.name,
href: `/projects/${project.id}`,
icon: FolderOpen,
keywords: `project ${project.name}`,
});
for (const tab of projectTabCommands(project.id)) {
commands.push({
label: `${project.name} > ${tab.label}`,
href: tab.href,
icon: tab.icon,
keywords: `project ${project.name} ${tab.label}`,
});
}
}
}
- return commands;
+ const seen = new Set<string>();
+ return commands.filter((cmd) => {
+ if (seen.has(cmd.href)) return false;
+ seen.add(cmd.href);
+ return true;
+ });
}, [projectsData]);Also applies to: 90-103, 163-168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/CommandPalette.tsx` around lines 55 - 57,
projectTabCommands currently returns duplicate CommandItem entries (same href
`/projects/${projectId}`) which causes unstable identity in the Projects list;
update the code that builds per-project commands (projectTabCommands and any
other arrays producing per-project items) to deduplicate by href before
rendering — e.g., produce a unique list of CommandItem where each href is unique
or ensure items use a stable unique id instead of duplicated href; specifically
modify projectTabCommands and the Projects list builder (the component that uses
href as item identity) to filter out duplicates (by href) so each
CommandItem.href is unique when passed to the renderer.
Comment on lines
+93
to
+99
| const latencyBars = l | ||
| ? [ | ||
| { label: "P50", value: l.p50, color: "var(--color-success)" }, | ||
| { label: "P95", value: l.p95, color: "var(--color-warning)" }, | ||
| { label: "P99", value: l.p99, color: "var(--color-danger)" }, | ||
| { label: "Avg", value: l.avg, color: "var(--color-brand)" }, | ||
| ] |
There was a problem hiding this comment.
Add P90 to latency distribution bars (currently omitted).
Line 93-Line 99 builds bars as P50/P95/P99/Avg, but the feature scope specifies percentile distribution including P90. This is a functional mismatch in the new chart dataset.
Proposed fix
const latencyBars = l
? [
{ label: "P50", value: l.p50, color: "var(--color-success)" },
+ { label: "P90", value: l.p90, color: "var(--color-warning-muted)" },
{ label: "P95", value: l.p95, color: "var(--color-warning)" },
{ label: "P99", value: l.p99, color: "var(--color-danger)" },
- { label: "Avg", value: l.avg, color: "var(--color-brand)" },
]
: [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/pages/MetricsPage.tsx` around lines 93 - 99, The
latencyBars array built in MetricsPage.tsx is missing the P90 percentile; update
the latencyBars construction (the latencyBars variable using l.p50, l.p95,
l.p99, l.avg) to include a P90 bar (use l.p90) with an appropriate color (e.g.,
"var(--color-warning)" or a distinct palette) placed in the percentile sequence
(e.g., P50, P90, P95, P99, Avg) so the chart dataset includes the required P90
metric.
Comment on lines
+228
to
+349
| {/* Charts row */} | ||
| <div className="grid grid-cols-1 lg:grid-cols-2 gap-6"> | ||
| {/* Request Trends */} | ||
| {timeseriesLoading ? ( | ||
| <Skeleton className="h-64 rounded-xl" /> | ||
| ) : timeseriesError ? ( | ||
| <div | ||
| className="rounded-xl p-5" | ||
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | ||
| > | ||
| <div style={{ color: "var(--color-danger)" }}>Failed to load request trends</div> | ||
| </div> | ||
| ) : ( | ||
| <div | ||
| className="rounded-xl p-5" | ||
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | ||
| > | ||
| <h2 | ||
| className="text-sm font-medium mb-4" | ||
| style={{ color: "var(--color-text-primary)" }} | ||
| > | ||
| Request Trends — {period} | ||
| </h2> | ||
| <div className="h-64"> | ||
| <ResponsiveContainer width="100%" height="100%"> | ||
| <AreaChart data={ts} margin={{ top: 10, right: 10, left: 0, bottom: 0 }}> | ||
| <XAxis | ||
| dataKey="bucket" | ||
| tickFormatter={(v) => | ||
| new Date(v).toLocaleTimeString([], { hour: "2-digit" }) | ||
| } | ||
| stroke="var(--color-text-muted)" | ||
| fontSize={11} | ||
| /> | ||
| <YAxis stroke="var(--color-text-muted)" fontSize={11} /> | ||
| <Tooltip | ||
| contentStyle={{ | ||
| background: "var(--color-surface)", | ||
| border: "1px solid var(--color-border)", | ||
| borderRadius: "6px", | ||
| }} | ||
| /> | ||
| <Area | ||
| type="monotone" | ||
| dataKey="total" | ||
| stroke="var(--color-brand)" | ||
| fill="var(--color-brand-muted)" | ||
| strokeWidth={2} | ||
| name="Total Requests" | ||
| /> | ||
| <Area | ||
| type="monotone" | ||
| dataKey="errors" | ||
| stroke="var(--color-danger)" | ||
| fill="var(--color-danger-muted)" | ||
| strokeWidth={2} | ||
| name="Errors" | ||
| /> | ||
| </AreaChart> | ||
| </ResponsiveContainer> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Latency Distribution */} | ||
| {latencyLoading ? ( | ||
| <Skeleton className="h-64 rounded-xl" /> | ||
| ) : latencyError ? ( | ||
| <div | ||
| className="rounded-xl p-5" | ||
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | ||
| > | ||
| <div style={{ color: "var(--color-danger)" }}>Failed to load latency data</div> | ||
| </div> | ||
| ) : ( | ||
| <div | ||
| className="rounded-xl p-5" | ||
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | ||
| > | ||
| <h2 | ||
| className="text-sm font-medium mb-4" | ||
| style={{ color: "var(--color-text-primary)" }} | ||
| > | ||
| Latency Distribution — {period} | ||
| </h2> | ||
| <div className="h-64"> | ||
| <ResponsiveContainer width="100%" height="100%"> | ||
| <BarChart data={latencyBars} margin={{ top: 10, right: 10, left: 0, bottom: 0 }}> | ||
| <XAxis | ||
| dataKey="label" | ||
| stroke="var(--color-text-muted)" | ||
| fontSize={12} | ||
| /> | ||
| <YAxis | ||
| stroke="var(--color-text-muted)" | ||
| fontSize={11} | ||
| label={{ | ||
| value: "ms", | ||
| angle: -90, | ||
| position: "insideLeft", | ||
| style: { fill: "var(--color-text-muted)", fontSize: 11 }, | ||
| }} | ||
| /> | ||
| <Tooltip | ||
| contentStyle={{ | ||
| background: "var(--color-surface)", | ||
| border: "1px solid var(--color-border)", | ||
| borderRadius: "6px", | ||
| }} | ||
| formatter={(value: number) => [`${value}ms`, "Latency"]} | ||
| /> | ||
| <Bar dataKey="value" radius={[6, 6, 0, 0]}> | ||
| {latencyBars.map((entry, index) => ( | ||
| <Cell key={`cell-${index}`} fill={entry.color} /> | ||
| ))} | ||
| </Bar> | ||
| </BarChart> | ||
| </ResponsiveContainer> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Add explicit empty states for successful-but-empty chart datasets.
In Line 228-Line 349, only loading/error branches exist. When the API succeeds with no points, both chart cards render blank plot areas with no user guidance.
Proposed fix
- ) : (
+ ) : ts.length === 0 ? (
+ <div
+ className="rounded-xl p-5"
+ style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }}
+ >
+ <div style={{ color: "var(--color-text-muted)" }}>No request trend data for {period}</div>
+ </div>
+ ) : (
...
- ) : (
+ ) : latencyBars.length === 0 ? (
+ <div
+ className="rounded-xl p-5"
+ style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }}
+ >
+ <div style={{ color: "var(--color-text-muted)" }}>No latency data for {period}</div>
+ </div>
+ ) : (As per coding guidelines, "Every list view needs an EmptyState component. No blank pages."
📝 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
| {/* Charts row */} | |
| <div className="grid grid-cols-1 lg:grid-cols-2 gap-6"> | |
| {/* Request Trends */} | |
| {timeseriesLoading ? ( | |
| <Skeleton className="h-64 rounded-xl" /> | |
| ) : timeseriesError ? ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <div style={{ color: "var(--color-danger)" }}>Failed to load request trends</div> | |
| </div> | |
| ) : ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <h2 | |
| className="text-sm font-medium mb-4" | |
| style={{ color: "var(--color-text-primary)" }} | |
| > | |
| Request Trends — {period} | |
| </h2> | |
| <div className="h-64"> | |
| <ResponsiveContainer width="100%" height="100%"> | |
| <AreaChart data={ts} margin={{ top: 10, right: 10, left: 0, bottom: 0 }}> | |
| <XAxis | |
| dataKey="bucket" | |
| tickFormatter={(v) => | |
| new Date(v).toLocaleTimeString([], { hour: "2-digit" }) | |
| } | |
| stroke="var(--color-text-muted)" | |
| fontSize={11} | |
| /> | |
| <YAxis stroke="var(--color-text-muted)" fontSize={11} /> | |
| <Tooltip | |
| contentStyle={{ | |
| background: "var(--color-surface)", | |
| border: "1px solid var(--color-border)", | |
| borderRadius: "6px", | |
| }} | |
| /> | |
| <Area | |
| type="monotone" | |
| dataKey="total" | |
| stroke="var(--color-brand)" | |
| fill="var(--color-brand-muted)" | |
| strokeWidth={2} | |
| name="Total Requests" | |
| /> | |
| <Area | |
| type="monotone" | |
| dataKey="errors" | |
| stroke="var(--color-danger)" | |
| fill="var(--color-danger-muted)" | |
| strokeWidth={2} | |
| name="Errors" | |
| /> | |
| </AreaChart> | |
| </ResponsiveContainer> | |
| </div> | |
| </div> | |
| )} | |
| {/* Latency Distribution */} | |
| {latencyLoading ? ( | |
| <Skeleton className="h-64 rounded-xl" /> | |
| ) : latencyError ? ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <div style={{ color: "var(--color-danger)" }}>Failed to load latency data</div> | |
| </div> | |
| ) : ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <h2 | |
| className="text-sm font-medium mb-4" | |
| style={{ color: "var(--color-text-primary)" }} | |
| > | |
| Latency Distribution — {period} | |
| </h2> | |
| <div className="h-64"> | |
| <ResponsiveContainer width="100%" height="100%"> | |
| <BarChart data={latencyBars} margin={{ top: 10, right: 10, left: 0, bottom: 0 }}> | |
| <XAxis | |
| dataKey="label" | |
| stroke="var(--color-text-muted)" | |
| fontSize={12} | |
| /> | |
| <YAxis | |
| stroke="var(--color-text-muted)" | |
| fontSize={11} | |
| label={{ | |
| value: "ms", | |
| angle: -90, | |
| position: "insideLeft", | |
| style: { fill: "var(--color-text-muted)", fontSize: 11 }, | |
| }} | |
| /> | |
| <Tooltip | |
| contentStyle={{ | |
| background: "var(--color-surface)", | |
| border: "1px solid var(--color-border)", | |
| borderRadius: "6px", | |
| }} | |
| formatter={(value: number) => [`${value}ms`, "Latency"]} | |
| /> | |
| <Bar dataKey="value" radius={[6, 6, 0, 0]}> | |
| {latencyBars.map((entry, index) => ( | |
| <Cell key={`cell-${index}`} fill={entry.color} /> | |
| ))} | |
| </Bar> | |
| </BarChart> | |
| </ResponsiveContainer> | |
| </div> | |
| </div> | |
| )} | |
| </div> | |
| {/* Charts row */} | |
| <div className="grid grid-cols-1 lg:grid-cols-2 gap-6"> | |
| {/* Request Trends */} | |
| {timeseriesLoading ? ( | |
| <Skeleton className="h-64 rounded-xl" /> | |
| ) : timeseriesError ? ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <div style={{ color: "var(--color-danger)" }}>Failed to load request trends</div> | |
| </div> | |
| ) : ts.length === 0 ? ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <div style={{ color: "var(--color-text-muted)" }}>No request trend data for {period}</div> | |
| </div> | |
| ) : ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <h2 | |
| className="text-sm font-medium mb-4" | |
| style={{ color: "var(--color-text-primary)" }} | |
| > | |
| Request Trends — {period} | |
| </h2> | |
| <div className="h-64"> | |
| <ResponsiveContainer width="100%" height="100%"> | |
| <AreaChart data={ts} margin={{ top: 10, right: 10, left: 0, bottom: 0 }}> | |
| <XAxis | |
| dataKey="bucket" | |
| tickFormatter={(v) => | |
| new Date(v).toLocaleTimeString([], { hour: "2-digit" }) | |
| } | |
| stroke="var(--color-text-muted)" | |
| fontSize={11} | |
| /> | |
| <YAxis stroke="var(--color-text-muted)" fontSize={11} /> | |
| <Tooltip | |
| contentStyle={{ | |
| background: "var(--color-surface)", | |
| border: "1px solid var(--color-border)", | |
| borderRadius: "6px", | |
| }} | |
| /> | |
| <Area | |
| type="monotone" | |
| dataKey="total" | |
| stroke="var(--color-brand)" | |
| fill="var(--color-brand-muted)" | |
| strokeWidth={2} | |
| name="Total Requests" | |
| /> | |
| <Area | |
| type="monotone" | |
| dataKey="errors" | |
| stroke="var(--color-danger)" | |
| fill="var(--color-danger-muted)" | |
| strokeWidth={2} | |
| name="Errors" | |
| /> | |
| </AreaChart> | |
| </ResponsiveContainer> | |
| </div> | |
| </div> | |
| )} | |
| {/* Latency Distribution */} | |
| {latencyLoading ? ( | |
| <Skeleton className="h-64 rounded-xl" /> | |
| ) : latencyError ? ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <div style={{ color: "var(--color-danger)" }}>Failed to load latency data</div> | |
| </div> | |
| ) : latencyBars.length === 0 ? ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <div style={{ color: "var(--color-text-muted)" }}>No latency data for {period}</div> | |
| </div> | |
| ) : ( | |
| <div | |
| className="rounded-xl p-5" | |
| style={{ background: "var(--color-surface)", border: "1px solid var(--color-border)" }} | |
| > | |
| <h2 | |
| className="text-sm font-medium mb-4" | |
| style={{ color: "var(--color-text-primary)" }} | |
| > | |
| Latency Distribution — {period} | |
| </h2> | |
| <div className="h-64"> | |
| <ResponsiveContainer width="100%" height="100%"> | |
| <BarChart data={latencyBars} margin={{ top: 10, right: 10, left: 0, bottom: 0 }}> | |
| <XAxis | |
| dataKey="label" | |
| stroke="var(--color-text-muted)" | |
| fontSize={12} | |
| /> | |
| <YAxis | |
| stroke="var(--color-text-muted)" | |
| fontSize={11} | |
| label={{ | |
| value: "ms", | |
| angle: -90, | |
| position: "insideLeft", | |
| style: { fill: "var(--color-text-muted)", fontSize: 11 }, | |
| }} | |
| /> | |
| <Tooltip | |
| contentStyle={{ | |
| background: "var(--color-surface)", | |
| border: "1px solid var(--color-border)", | |
| borderRadius: "6px", | |
| }} | |
| formatter={(value: number) => [`${value}ms`, "Latency"]} | |
| /> | |
| <Bar dataKey="value" radius={[6, 6, 0, 0]}> | |
| {latencyBars.map((entry, index) => ( | |
| <Cell key={`cell-${index}`} fill={entry.color} /> | |
| ))} | |
| </Bar> | |
| </BarChart> | |
| </ResponsiveContainer> | |
| </div> | |
| </div> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/pages/MetricsPage.tsx` around lines 228 - 349, The Request
Trends and Latency Distribution branches currently only handle loading/error
states and render blank charts when the API returns an empty dataset; update the
render logic around the timeseries block (check timeseriesLoading,
timeseriesError and the data variable ts) and the latency block (check
latencyLoading, latencyError and the data variable latencyBars) to add explicit
EmptyState renders when ts is empty or latencyBars is empty (e.g., ts?.length
=== 0 and latencyBars?.length === 0), keeping the existing loading/error
branches intact and showing a brief explanatory message and action (like "No
data for selected period") via your app's EmptyState component or similar.
Comment on lines
+254
to
+258
| <XAxis | ||
| dataKey="bucket" | ||
| tickFormatter={(v) => | ||
| new Date(v).toLocaleTimeString([], { hour: "2-digit" }) | ||
| } |
There was a problem hiding this comment.
Use period-aware x-axis formatting; hour-only labels break 7d/30d readability.
Line 256-Line 258 always formats buckets as hour-only. For multi-day periods this can produce repeated labels (e.g., daily buckets all showing the same hour), making the chart hard to interpret.
Proposed fix
+ const formatBucketLabel = (value: string) => {
+ const d = new Date(value);
+ if (period === "24h") return d.toLocaleTimeString([], { hour: "2-digit", minute: "2-digit" });
+ return d.toLocaleDateString([], { month: "short", day: "numeric" });
+ };
...
<XAxis
dataKey="bucket"
- tickFormatter={(v) =>
- new Date(v).toLocaleTimeString([], { hour: "2-digit" })
- }
+ tickFormatter={formatBucketLabel}
stroke="var(--color-text-muted)"
fontSize={11}
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/pages/MetricsPage.tsx` around lines 254 - 258, The XAxis
tickFormatter currently always renders buckets as hour-only, which fails for
multi-day periods; change the tickFormatter on XAxis (dataKey="bucket") to be
period-aware: read the chart's selected period variable (e.g., selectedPeriod /
timeRange / period prop used by MetricsPage) and call a small helper like
formatBucket(bucket, period) that returns hour-only for sub-day ranges, returns
"MMM d" or "MMM d, HH:MM" for multi-day ranges, and returns full date for 30d;
implement formatBucket as a pure function (or method) and swap the inline
tickFormatter to delegate to it so labels adapt to the current period and avoid
repeated identical hour labels.
Comment on lines
+70
to
+75
| const [showConfig, setShowConfig] = useState(false); | ||
| const [configForm, setConfigForm] = useState({ | ||
| inngest_api_key: "", | ||
| inngest_env_id: "", | ||
| inngest_base_url: "", | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Form state uses useState instead of React Hook Form.
Per dashboard guidelines, data-entry forms must use React Hook Form + Zod resolver. This config form uses useState for configForm, which is non-compliant. Consider refactoring to useForm with a Zod schema for validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/pages/settings/InngestDashboardPage.tsx` around lines 70 -
75, The config UI in InngestDashboardPage currently manages fields with useState
(showConfig, configForm, setConfigForm); replace this with React Hook Form by
creating a Zod schema (e.g., ConfigSchema) for inngest_api_key, inngest_env_id,
and inngest_base_url, then call useForm({ resolver: zodResolver(ConfigSchema),
defaultValues: { ... }}) and wire the form inputs via register and handleSubmit
instead of setConfigForm; update submit and reset logic to use formState and
reset from useForm, and remove the useState configForm usage and any direct
setConfigForm updates. Ensure to import zod and `@hookform/resolvers/zod` and keep
showConfig for toggling visibility only.
Comment on lines
+6
to
17
| const getInngestBaseUrl = async (): Promise<string> => { | ||
| const envUrl = process.env.INNGEST_BASE_URL; | ||
| if (envUrl) return envUrl; | ||
|
|
||
| const pool = getPool(); | ||
| const { rows } = await pool.query( | ||
| "SELECT value FROM betterbase_meta.instance_settings WHERE key = 'inngest_base_url'", | ||
| ); | ||
| const storedValue = rows[0]?.value; | ||
| const url = typeof storedValue === "string" ? storedValue : (storedValue?.value ?? null); | ||
| return url || "https://api.inngest.com"; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if INNGEST_BASE_URL or INNGEST_API_KEY are already defined in env.ts
rg -n "INNGEST" packages/server/src/lib/env.tsRepository: weroperking/Betterbase
Length of output: 850
🏁 Script executed:
cat -n packages/server/src/routes/admin/inngest.ts | head -80Repository: weroperking/Betterbase
Length of output: 3168
Replace direct process.env access with validateEnv().
Lines 7 and 29 violate the server guidelines by accessing process.env directly. All env access must go through validateEnv() from src/lib/env.ts.
- Line 7: Replace
process.env.INNGEST_BASE_URLwithvalidateEnv().INNGEST_BASE_URL(already defined in env schema) - Line 29:
process.env.INNGEST_API_KEYis not in the env schema and must be added
Update the Zod schema in env.ts to include INNGEST_API_KEY as an optional string, extract it in validateEnv(), and import it into this module.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/inngest.ts` around lines 6 - 17, The code
directly reads process.env in getInngestBaseUrl and elsewhere; replace those
with validateEnv() usage: import validateEnv from src/lib/env.ts and change
process.env.INNGEST_BASE_URL to validateEnv().INNGEST_BASE_URL inside
getInngestBaseUrl (preserving the DB fallback logic), and replace the other
direct process.env.INNGEST_API_KEY usage to validateEnv().INNGEST_API_KEY; also
update the Zod schema in env.ts to add INNGEST_API_KEY as an optional string,
ensure validateEnv() returns it, and import/use that returned value in this
module instead of any direct process.env access.
Comment on lines
+34
to
+36
| inngest_api_key: z.string().optional(), | ||
| inngest_env_id: z.string().optional(), | ||
| inngest_base_url: z.string().url().optional().or(z.literal("")), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
No minimum length validation on inngest_api_key.
Empty strings will pass validation and be stored. If the intent is to allow clearing the key, consider using .min(1).optional().or(z.literal("")) to be explicit, similar to how inngest_base_url handles clearing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/routes/admin/instance.ts` around lines 34 - 36, The
schema allows empty strings for the inngest_api_key field so blank values get
stored; update the validation for inngest_api_key to explicitly allow either a
non-empty string or the empty literal (mirroring inngest_base_url) by changing
its validator to require .min(1) on the string and combine with
.optional().or(z.literal("")) so empty string is only accepted intentionally;
locate the schema where inngest_api_key is defined and apply this change.
Comment on lines
+4
to
+6
| const client = new Client({ | ||
| connectionString: 'postgresql://neondb_owner:npg_NDrg3StRE4jY@ep-still-thunder-an4tpncc-pooler.c-6.us-east-1.aws.neon.tech/neondb?sslmode=require&channel_binding=require' | ||
| }) |
There was a problem hiding this comment.
Same hardcoded credentials leak as check-hash.ts.
See previous comment. Connection string with plaintext credentials should not be committed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@reset-password.ts` around lines 4 - 6, The Client is initialized with a
hardcoded connection string (the new Client(...) assigned to client) which leaks
credentials; remove the literal and load the DB connection string from a secure
environment variable (e.g., process.env.DATABASE_URL or a dedicated secret like
process.env.NEON_DATABASE_URL) when constructing Client in reset-password.ts,
and ensure no plaintext fallback is committed (add a runtime check that throws
or logs a clear error if the env var is missing) so credentials are not stored
in source.
Comment on lines
+10
to
+11
| const newPassword = 'AdminPass123!' | ||
| const hash = await bcrypt.hash(newPassword, 12) |
There was a problem hiding this comment.
Hardcoded password and direct bcrypt usage bypasses application patterns.
- Hardcoded password
AdminPass123!will be committed to source control. - Direct
bcrypt.hashcall duplicates logic frompackages/server/src/lib/auth.ts:hashPassword(). If the hashing strategy changes (rounds, algorithm), this script will diverge.
If this must exist, parameterize the password and import the canonical function:
-const newPassword = 'AdminPass123!'
-const hash = await bcrypt.hash(newPassword, 12)
+import { hashPassword } from './packages/server/src/lib/auth'
+
+const newPassword = process.argv[2]
+if (!newPassword || newPassword.length < 8) {
+ console.error('Usage: npx tsx reset-password.ts <password>')
+ process.exit(1)
+}
+const hash = await hashPassword(newPassword)📝 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 newPassword = 'AdminPass123!' | |
| const hash = await bcrypt.hash(newPassword, 12) | |
| import { hashPassword } from './packages/server/src/lib/auth' | |
| const newPassword = process.argv[2] | |
| if (!newPassword || newPassword.length < 8) { | |
| console.error('Usage: npx tsx reset-password.ts <password>') | |
| process.exit(1) | |
| } | |
| const hash = await hashPassword(newPassword) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@reset-password.ts` around lines 10 - 11, The script reset-password.ts
currently hardcodes newPassword and calls bcrypt.hash directly; instead accept
the password via a parameter or environment variable (e.g.,
process.env.NEW_ADMIN_PASSWORD or a CLI arg) and import and call the canonical
hashPassword function from packages/server/src/lib/auth.ts (use that instead of
bcrypt.hash) so hashing strategy stays centralized and no secret is committed to
source control; update references in reset-password.ts to use the imported
hashPassword and validate/throw if no password provided.
Comment on lines
+19
to
+20
| console.log('Password updated to:', newPassword) | ||
| console.log('Hash:', hash) |
There was a problem hiding this comment.
Logging plaintext password is a security risk.
If stdout is persisted (CI logs, terminal history), the plaintext password is exposed. Remove this output or limit to the hash only if debugging is required.
-console.log('Password updated to:', newPassword)
-console.log('Hash:', hash)
+console.log('Password updated successfully')📝 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
| console.log('Password updated to:', newPassword) | |
| console.log('Hash:', hash) | |
| console.log('Password updated successfully') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@reset-password.ts` around lines 19 - 20, Remove the console.log that prints
the plaintext newPassword (do not emit newPassword to stdout); either delete the
line logging newPassword or replace it with a non-sensitive message and, if
needed, only log the hash variable (or a masked/shortened representation) from
the same block where newPassword and hash are handled so you never persist
plaintext passwords in logs — optionally gate any debug logging behind an
explicit debug flag/environment check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Key/Size/LastModifiedAPI format, added human-readable size formattingPATCH /admin/instance__global__sentinel value for project_id/projects/123/settingsmatches "Settings" not "Overview")xdg-open/open/start)getInngestBaseUrl/isSelfHostedasync, readinginngest_base_urlfrominstance_settingsDB tableinngest_api_key,inngest_env_id,inngest_base_urlto PATCH schemacheck-admin.ts,check-hash.ts,reset-password.tsbun.lockupdatesSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
UI Improvements