Feat : seo, add blog, admin manager - #64
Conversation
The GitHub and Spotify rows in the auto-transition mode used single-line flex without wrapping, causing content overflow on small screens. Added flex-wrap, truncation for long text, and repositioned navigation arrows to be absolutely placed on mobile. https://claude.ai/code/session_019VM1w9ahExw5WzTnVgfPyK
…mote to v6 - Add `export const dynamic = 'force-dynamic'` to admin layout to fix cookies() dynamic server usage error on /admin/projects - Update next-mdx-remote from 5.0.0 to 6.0.0 (security update) - Add comprehensive admin section review document covering UI, a11y, mobile responsiveness, and functionality https://claude.ai/code/session_019VM1w9ahExw5WzTnVgfPyK
…, viewing blog statistics, user metrics, and contact/comment data, and add `oxlint` and `prettier` development dependencies.
…experience, projects, SEO, and analytics.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideThis PR enhances admin/dev tooling, SEO, and analytics while tightening security and UX: it centralizes admin detection, adds dev-access gating and dynamic route introspection, improves project admin and legal/blog UIs, refines Spotify OAuth token handling, strengthens GitHub token usage, and updates sitemap/robots/PostHog configuration for better SEO and privacy. Sequence diagram for updated Spotify dev OAuth flowsequenceDiagram
actor DevUser
participant Browser
participant DevSpotifyPage as DevSpotifyPage_/dev/spotify
participant AuthUrlAPI as API_spotify_auth_url
participant Spotify as Spotify_Accounts_API
participant CallbackAPI as API_spotify_callback
participant DevTokenAPI as API_spotify_dev_token
participant DevAccess as DevToolsAccess
DevUser->>DevSpotifyPage: Open /dev/spotify
DevSpotifyPage->>AuthUrlAPI: GET /api/spotify/auth-url
AuthUrlAPI->>DevAccess: requireDevToolsAccess
DevAccess-->>AuthUrlAPI: allow or 401
alt unauthorized
AuthUrlAPI-->>DevSpotifyPage: 401 Unauthorized
DevSpotifyPage-->>DevUser: Show access error
else authorized
AuthUrlAPI->>Spotify: Build authorization_url
AuthUrlAPI-->>DevSpotifyPage: { authUrl }
DevSpotifyPage->>DevUser: Render "Sign in with Spotify" link
DevUser->>Spotify: Authorize application
Spotify-->>CallbackAPI: GET /api/spotify/callback?code=...
CallbackAPI->>DevAccess: requireDevToolsAccess
DevAccess-->>CallbackAPI: allow or 401
alt unauthorized
CallbackAPI-->>Spotify: 401 Unauthorized
else authorized
CallbackAPI->>Spotify: POST /api/token code_exchange
Spotify-->>CallbackAPI: { access_token, refresh_token }
CallbackAPI->>CallbackAPI: Validate tokens
alt missing refresh_token
CallbackAPI-->>Browser: Redirect /dev/spotify?error=missing_refresh_token
else tokens_ok
CallbackAPI->>Browser: Set cookies spotify_dev_refresh_token, spotify_dev_access_token
CallbackAPI-->>Browser: Redirect /dev/spotify?success=true
end
end
Browser->>DevSpotifyPage: Load /dev/spotify?success=true
DevSpotifyPage->>DevTokenAPI: GET /api/spotify/dev-token
DevTokenAPI->>DevAccess: requireDevToolsAccess
DevAccess-->>DevTokenAPI: allow or 401
alt unauthorized
DevTokenAPI-->>DevSpotifyPage: 401 Unauthorized
DevSpotifyPage-->>DevUser: Show access error
else authorized
DevTokenAPI->>DevTokenAPI: Read cookies spotify_dev_* tokens
DevTokenAPI-->>DevSpotifyPage: { refresh_token, access_token }
DevTokenAPI->>DevTokenAPI: Delete spotify_dev_* cookies
DevSpotifyPage->>DevSpotifyPage: Store tokens in local state
DevSpotifyPage->>Browser: Replace URL with /dev/spotify
DevSpotifyPage-->>DevUser: Show tokens and helper UI
end
end
Sequence diagram for dev routes discovery and displaysequenceDiagram
actor DevUser
participant DevWidget as DevWidget
participant RoutesSection as RoutesSection_Component
participant RoutesAPI as API_dev_routes
participant DevAccess as DevToolsAccess
participant FSScanner as RouteScanner_glob
DevUser->>DevWidget: Open dev tools overlay
DevWidget->>RoutesSection: Render with pathname
activate RoutesSection
RoutesSection->>RoutesSection: useEffect on mount
RoutesSection->>RoutesAPI: GET /api/dev/routes
activate RoutesAPI
RoutesAPI->>DevAccess: requireDevToolsAccess
DevAccess-->>RoutesAPI: allow or 401
alt unauthorized
RoutesAPI-->>RoutesSection: 401 Unauthorized
RoutesSection->>RoutesSection: setError(true), setLoading(false)
RoutesSection-->>DevUser: Show "Failed to load routes"
else authorized
RoutesAPI->>FSScanner: glob('**/page.{tsx,js,jsx}') under src/app
FSScanner-->>RoutesAPI: list of page_files
RoutesAPI->>RoutesAPI: Map files to route_paths
RoutesAPI->>RoutesAPI: Strip route_groups and api folders
RoutesAPI->>RoutesAPI: Mark isDynamic for paths with [param]
RoutesAPI->>RoutesAPI: Deduplicate and sort
RoutesAPI-->>RoutesSection: { routes: RouteItem[] }
deactivate RoutesAPI
RoutesSection->>RoutesSection: setRoutes(routes)
RoutesSection->>RoutesSection: Categorize by core, blog, dev, legal, other
RoutesSection->>RoutesSection: setLoading(false)
RoutesSection-->>DevUser: Render categorized route list
DevUser->>RoutesSection: Click static route link
RoutesSection-->>DevUser: Next.js navigation to route.path
DevUser->>RoutesSection: Hover dynamic route
RoutesSection-->>DevUser: Show disabled row with Zap icon
end
deactivate RoutesSection
Class diagram for consolidated admin and dev access controlclassDiagram
class IsAdminUtil {
+isAdmin() Promise~boolean~
-getAdminEmails() string[]
-isAdminEmail(email string) boolean
}
class AuthGuardLib {
+isAdmin() Promise~boolean~
+requireAdmin() Promise~true~
}
class AuthActions {
+checkAdminStatus() Promise~boolean~
}
class DevAccessLib {
+canAccessDevTools() Promise~boolean~
+requireDevToolsAccess() Promise~NextResponse_or_null~
}
class AdminLayout {
+dynamic string
+AdminLayout(children ReactNode) Promise~JSXElement~
}
class DevLayout {
+dynamic string
+DevLayout(children ReactNode) Promise~JSXElement~
}
class SpotifyAuthUrlAPI {
+GET() Promise~NextResponse~
}
class SpotifyCallbackAPI {
+GET(request NextRequest) Promise~NextResponse~
}
class SpotifyTokenAPI {
+POST(request Request) Promise~NextResponse~
}
class SpotifyRefreshAPI {
+POST(request NextRequest) Promise~NextResponse~
}
class SpotifyDevTokenAPI {
+GET() Promise~NextResponse~
}
class DevRoutesAPI {
+dynamic string
+GET() Promise~NextResponse~
-formatRouteLabel(routePath string) string
}
%% Relationships
AuthGuardLib --> IsAdminUtil : uses isAdmin
AuthActions --> IsAdminUtil : uses isAdmin
DevAccessLib --> IsAdminUtil : uses isAdmin
AdminLayout --> AuthActions : uses checkAdminStatus
DevLayout --> DevAccessLib : uses canAccessDevTools
SpotifyAuthUrlAPI --> DevAccessLib : requireDevToolsAccess
SpotifyCallbackAPI --> DevAccessLib : requireDevToolsAccess
SpotifyTokenAPI --> DevAccessLib : requireDevToolsAccess
SpotifyRefreshAPI --> DevAccessLib : requireDevToolsAccess
SpotifyDevTokenAPI --> DevAccessLib : requireDevToolsAccess
DevRoutesAPI --> DevAccessLib : requireDevToolsAccess
DevRoutesAPI --> DevRoutesAPI : uses formatRouteLabel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis PR consolidates GitHub authentication to server-only, implements dev-access controls across Spotify API routes, refactors blog routing from "categories" to "topics", enhances accessibility with ARIA attributes and semantic roles, updates Spotify callback handling with secure cookies, and adds dev utilities for route enumeration and admin functions. Changes
Sequence DiagramsequenceDiagram
participant Client as Client (Dev Tools)
participant Frontend as /dev/spotify Page
participant AuthAPI as /api/spotify/auth-url
participant SpotifyAPI as Spotify OAuth
participant Callback as /api/spotify/callback
participant DevToken as /api/spotify/dev-token
participant CookieStore as Secure Cookies
Client->>Frontend: Navigate to /dev/spotify
Frontend->>AuthAPI: GET /api/spotify/auth-url
AuthAPI->>AuthAPI: Check dev access
AuthAPI->>Frontend: Return Spotify auth URL
Frontend->>SpotifyAPI: Redirect to auth endpoint
SpotifyAPI->>Callback: Callback with code
Callback->>Callback: Verify dev access, exchange code for tokens
Callback->>CookieStore: Store refresh_token & access_token (httpOnly)
Callback->>Frontend: Redirect to /dev/spotify
Frontend->>DevToken: GET /api/spotify/dev-token
DevToken->>DevToken: Check dev access, read cookies
DevToken->>Frontend: Return refresh_token & access_token
DevToken->>CookieStore: Delete cookies
Frontend->>Frontend: Display tokens for copying
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (2 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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
RoutesSection.tsx, you're usingReact.KeyboardEventin thehandleJumpsignature without importingReactor theKeyboardEventtype; addimport type React from 'react'or useimport type { KeyboardEvent } from 'react'withKeyboardEvent<HTMLInputElement>to avoid type errors. - The new
/api/dev/routesendpoint runs agloboversrc/appon every request withdynamic = 'force-dynamic'; consider gating this to development only or adding some in-memory caching/throttling to avoid unnecessary filesystem scans in non-dev environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `RoutesSection.tsx`, you're using `React.KeyboardEvent` in the `handleJump` signature without importing `React` or the `KeyboardEvent` type; add `import type React from 'react'` or use `import type { KeyboardEvent } from 'react'` with `KeyboardEvent<HTMLInputElement>` to avoid type errors.
- The new `/api/dev/routes` endpoint runs a `glob` over `src/app` on every request with `dynamic = 'force-dynamic'`; consider gating this to development only or adding some in-memory caching/throttling to avoid unnecessary filesystem scans in non-dev environments.
## Individual Comments
### Comment 1
<location> `tools/dev-menu/components/sections/RoutesSection.tsx:135-144` </location>
<code_context>
+ <span className="font-mono text-primary/80 truncate max-w-[150px]">{pathname}</span>
</div>
+
+ {loading && routes.length === 0 ? (
+ <div className="flex items-center justify-center py-4 text-muted-foreground">
+ <Loader2 className="w-4 h-4 animate-spin" />
+ </div>
+ ) : error ? (
+ <div className="px-2 py-2 text-[10px] text-red-400 text-center">
+ Failed to load routes
+ </div>
+ ) : (
+ <div className="space-y-2 max-h-[300px] overflow-y-auto scrollbar-hide mt-1 pb-4">
+ {(Object.entries(categorized) as [keyof typeof CATEGORY_CONFIG, RouteItem[]][]).map(([key, items]) => {
</code_context>
<issue_to_address>
**suggestion:** Avoid hiding already-loaded routes when a subsequent refresh fails
With the current conditions, any error replaces the list with “Failed to load routes”, even after a prior successful load. This hides still-valid routes on a failed refresh. You could instead only show the full-page error when there are no routes yet (e.g., `if (loading && !routes.length)`, `else if (error && !routes.length)`, else always render the list and optionally show a smaller inline error).
```suggestion
{loading && routes.length === 0 ? (
<div className="flex items-center justify-center py-4 text-muted-foreground">
<Loader2 className="w-4 h-4 animate-spin" />
</div>
) : error && routes.length === 0 ? (
<div className="px-2 py-2 text-[10px] text-red-400 text-center">
Failed to load routes
</div>
) : (
<>
{error && routes.length > 0 && (
<div className="px-2 py-1 text-[10px] text-red-400 text-center">
Failed to refresh routes
</div>
)}
<div className="space-y-2 max-h-[300px] overflow-y-auto scrollbar-hide mt-1 pb-4">
```
</issue_to_address>
### Comment 2
<location> `tools/dev-menu/components/sections/RoutesSection.tsx:96-103` </location>
<code_context>
}
}
+ // Categorize routes
+ const categorized = {
+ core: [] as RouteItem[],
+ blog: [] as RouteItem[],
</code_context>
<issue_to_address>
**suggestion:** Derive route categories from CATEGORY_CONFIG to avoid key drift
`categorized` and `CATEGORY_CONFIG` both define the same category keys and you later assert `Object.entries(categorized)` as `[keyof typeof CATEGORY_CONFIG, RouteItem[]][]`. This will silently break if someone changes categories in only one place. Consider deriving `categorized` from `CATEGORY_CONFIG` (e.g. `Object.fromEntries(Object.keys(CATEGORY_CONFIG).map(key => [key, [] as RouteItem[]]))`) so the keys stay in sync without a manual type assertion.
```suggestion
// Categorize routes derived from CATEGORY_CONFIG to keep keys in sync
const categorized = Object.fromEntries(
(Object.keys(CATEGORY_CONFIG) as (keyof typeof CATEGORY_CONFIG)[]).map(key => [
key,
[] as RouteItem[]
])
) as Record<keyof typeof CATEGORY_CONFIG, RouteItem[]>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| {loading && routes.length === 0 ? ( | ||
| <div className="flex items-center justify-center py-4 text-muted-foreground"> | ||
| <Loader2 className="w-4 h-4 animate-spin" /> | ||
| </div> | ||
| ) : error ? ( | ||
| <div className="px-2 py-2 text-[10px] text-red-400 text-center"> | ||
| Failed to load routes | ||
| </div> | ||
| ) : ( | ||
| <div className="space-y-2 max-h-[300px] overflow-y-auto scrollbar-hide mt-1 pb-4"> |
There was a problem hiding this comment.
suggestion: Avoid hiding already-loaded routes when a subsequent refresh fails
With the current conditions, any error replaces the list with “Failed to load routes”, even after a prior successful load. This hides still-valid routes on a failed refresh. You could instead only show the full-page error when there are no routes yet (e.g., if (loading && !routes.length), else if (error && !routes.length), else always render the list and optionally show a smaller inline error).
| {loading && routes.length === 0 ? ( | |
| <div className="flex items-center justify-center py-4 text-muted-foreground"> | |
| <Loader2 className="w-4 h-4 animate-spin" /> | |
| </div> | |
| ) : error ? ( | |
| <div className="px-2 py-2 text-[10px] text-red-400 text-center"> | |
| Failed to load routes | |
| </div> | |
| ) : ( | |
| <div className="space-y-2 max-h-[300px] overflow-y-auto scrollbar-hide mt-1 pb-4"> | |
| {loading && routes.length === 0 ? ( | |
| <div className="flex items-center justify-center py-4 text-muted-foreground"> | |
| <Loader2 className="w-4 h-4 animate-spin" /> | |
| </div> | |
| ) : error && routes.length === 0 ? ( | |
| <div className="px-2 py-2 text-[10px] text-red-400 text-center"> | |
| Failed to load routes | |
| </div> | |
| ) : ( | |
| <> | |
| {error && routes.length > 0 && ( | |
| <div className="px-2 py-1 text-[10px] text-red-400 text-center"> | |
| Failed to refresh routes | |
| </div> | |
| )} | |
| <div className="space-y-2 max-h-[300px] overflow-y-auto scrollbar-hide mt-1 pb-4"> |
| // Categorize routes | ||
| const categorized = { | ||
| core: [] as RouteItem[], | ||
| blog: [] as RouteItem[], | ||
| dev: [] as RouteItem[], | ||
| legal: [] as RouteItem[], | ||
| other: [] as RouteItem[] | ||
| } |
There was a problem hiding this comment.
suggestion: Derive route categories from CATEGORY_CONFIG to avoid key drift
categorized and CATEGORY_CONFIG both define the same category keys and you later assert Object.entries(categorized) as [keyof typeof CATEGORY_CONFIG, RouteItem[]][]. This will silently break if someone changes categories in only one place. Consider deriving categorized from CATEGORY_CONFIG (e.g. Object.fromEntries(Object.keys(CATEGORY_CONFIG).map(key => [key, [] as RouteItem[]]))) so the keys stay in sync without a manual type assertion.
| // Categorize routes | |
| const categorized = { | |
| core: [] as RouteItem[], | |
| blog: [] as RouteItem[], | |
| dev: [] as RouteItem[], | |
| legal: [] as RouteItem[], | |
| other: [] as RouteItem[] | |
| } | |
| // Categorize routes derived from CATEGORY_CONFIG to keep keys in sync | |
| const categorized = Object.fromEntries( | |
| (Object.keys(CATEGORY_CONFIG) as (keyof typeof CATEGORY_CONFIG)[]).map(key => [ | |
| key, | |
| [] as RouteItem[] | |
| ]) | |
| ) as Record<keyof typeof CATEGORY_CONFIG, RouteItem[]> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/app/api/activity/combined/route.ts (1)
1-2:⚠️ Potential issue | 🟠 MajorReplace
unstable_cachewith theuse cachedirective in Next.js 16.The
unstable_cacheAPI is still available in Next.js 16 but has been superseded by theuse cachedirective. Migrate to the recommended Cache Components API instead of continuing to use the legacyunstable_cacheapproach.src/app/api/spotify/callback/route.ts (2)
18-27:⚠️ Potential issue | 🟡 MinorInconsistent error redirect destinations.
Error cases on Lines 20 and 26 redirect to
/?error=...(the site root), while success (Line 81) and the missing-refresh-token error (Line 77) redirect to/dev/spotify. Since this is a dev-tools flow, all redirects should go to/dev/spotifyfor a consistent user experience.Proposed fix
if (error) { return NextResponse.redirect( - new URL('/?error=' + error, request.url) + new URL('/dev/spotify?error=' + error, request.url) ) } if (!code) { return NextResponse.redirect( - new URL('/?error=no_code', request.url) + new URL('/dev/spotify?error=no_code', request.url) ) }
63-68:⚠️ Potential issue | 🟡 MinorSame inconsistency for the token exchange failure redirect.
This error redirect also goes to root instead of
/dev/spotify.Proposed fix
return NextResponse.redirect( new URL( - `/?error=token_exchange_failed&details=${errorData.error_description || errorData.error}`, + `/dev/spotify?error=token_exchange_failed&details=${encodeURIComponent(errorData.error_description || errorData.error)}`, request.url ) )Note: the
detailsvalue from Spotify should also be URI-encoded to avoid malformed URLs.src/components/landing/activity/activity-feed.tsx (1)
588-701:⚠️ Potential issue | 🔴 CriticalMissing closing
</div>— JSX parse error.The
<div className="relative">opened at Line 588 is never closed. There are three opening<div>tags (Lines 588, 589, 657) but only two closing</div>tags (Lines 700, 701). This will cause a build failure.Add a
</div>after Line 701 to close therelativewrapper before the Spotify row.Proposed fix
</div> </div> + </div> {/* SPOTIFY ROW */}This closes the
<div className="relative">from Line 588 before the Spotify row starts.Static analysis (Biome) also flagged this: "Expected corresponding JSX closing tag for 'div'."
src/core/metadata/categories.ts (1)
3-19:⚠️ Potential issue | 🟡 MinorTitle, description, and keywords still reference "categories" while the canonical points to
/blog/topics.The canonical URL was updated to
/blog/topics, but the metadata text (title: "Categories", description: "categories", keywords: "categories", "blog categories") still uses the old terminology. This creates an SEO inconsistency — search engines will see a page at/blog/topicswhose metadata describes "categories."Consider updating the copy to align with "topics."
Proposed fix
export const categoriesMetadata = createBaseMetadata({ - title: 'Categories - Blog Organization', + title: 'Topics - Blog Organization', description: - 'Browse blog posts by categories including Engineering, Design, React, CSS, TypeScript, and more. Find content that matches your interests.', + 'Browse blog posts by topics including Engineering, Design, React, CSS, TypeScript, and more. Find content that matches your interests.', keywords: [ - 'categories', - 'blog categories', + 'topics', + 'blog topics', 'engineering', 'design', 'react',
🤖 Fix all issues with AI agents
In
`@src/app/`(marketing)/blog/posts/drafts/stop-using-arrow-fnc-and-HUGE-REVEAL.md:
- Around line 3-4: The publishedAt and updatedAt date strings in the draft use
'11-02-2026' which is not ISO (YYYY-MM-DD); update both publishedAt and
updatedAt to ISO 8601 format (e.g., '2026-02-11' or '2026-11-02' depending on
intended month/day) so the formatDate() logic and Date constructor parse them
correctly; locate the keys publishedAt and updatedAt in the post frontmatter and
replace the values with the correct YYYY-MM-DD strings.
In `@src/app/api/dev/routes/route.ts`:
- Around line 27-41: The route depends on scanning source files (appDir, glob ->
files) which may not exist in production, so update the handler to avoid
returning misleading empty results: either gate the endpoint to development-only
by checking process.env.NODE_ENV === 'development' (or similar) before running
the glob/cwd logic and return a 403/empty-with-message if not dev, or if you
must allow access in non-dev environments keep the existing canAccessDevTools()
check but detect files.length === 0 after the glob call and return a clear
explanatory response (e.g., "no routes found — source files not present in
production build") instead of silently returning an empty array; reference the
variables/functions appDir, files, glob, process.cwd(), and canAccessDevTools()
when making the change.
In `@src/components/projects/admin/project-editor.tsx`:
- Around line 53-54: The outer modal container (the large <div> used as the
overlay in the ProjectEditor component) must be made accessible: add
role="dialog", aria-modal="true", and aria-label="Edit project" to that outer
container element, attach an onKeyDown handler (e.g., handleOverlayKeyDown) that
closes the modal when Escape is pressed, and implement a focus trap around the
modal content (use focus-trap-react or a small custom trap that moves focus into
the first focusable element on open and restores focus on close); update the
component to call the existing close method (or prop) from the Escape handler
and ensure focus is managed on mount/unmount.
In `@src/components/projects/admin/project-list.tsx`:
- Around line 44-53: The project row is only selectable via mouse—update the
element rendered in projects.map (the div with role="row", key={project.id},
onClick={() => onSelect(project.id)} and aria-selected using selectedId) to be
keyboard-focusable and activate on Enter/Space: add tabIndex={0} and an
onKeyDown handler that calls onSelect(project.id) when the user presses Enter or
Space (handle Space with preventDefault to avoid scrolling); keep the existing
onClick and aria-selected to preserve behavior and semantics.
- Around line 44-57: The ARIA issue is that the intermediate <div
className="grid..."> inside the ProjectList row breaks the row→cell contract;
update the JSX in the ProjectList component so that the grid layout is applied
directly on the row container (the div with role="row", key={project.id},
onClick={() => onSelect(project.id)}, className=...) or alternatively mark the
intermediate grid wrapper with role="presentation" so the role="cell" elements
(the spans rendering project.idx and other cells) become direct children of the
row for screen readers; adjust the className usage accordingly (remove the extra
closing wrapper div if merging classes into the row).
In `@src/utils/is-admin.ts`:
- Around line 5-17: Remove the hardcoded FALLBACK_ADMIN_EMAILS and update
getAdminEmails to be fail-closed: parse env.ADMIN_EMAIL (env.ADMIN_EMAIL || '')
into a trimmed, lower-cased array and return that array directly (which may be
empty) instead of falling back to any built-in emails; ensure any code
referencing getAdminEmails or FALLBACK_ADMIN_EMAILS is updated to stop relying
on the fallback and to handle an empty admin list safely.
🧹 Nitpick comments (19)
src/components/projects/components/project-preview.tsx (1)
24-24: Dead code: expression always evaluates tonullorfalse.
preview.type === 'iframe' && !isLoading && nullcan never render anything — the final operand isnull. This is a leftover from removing the live badge and should be deleted.🧹 Remove dead code
- {preview.type === 'iframe' && !isLoading && null}src/components/seo/web-vitals-reporter.tsx (1)
32-34:web-vitalsis still bundled in the production client bundle despite being unused.The runtime guard skips execution in production, but the top-level
import { onCLS, onFCP, onLCP, onTTFB } from 'web-vitals'ensures the library is included in the client JS regardless. Since this component returnsnullin production, consider either:
- Conditionally rendering the component from the parent (so it's never mounted in prod), or
- Dynamically importing
web-vitalsinside theuseEffect:♻️ Option 2: dynamic import to eliminate the production bundle cost
-import { onCLS, onFCP, onLCP, onTTFB } from 'web-vitals' export function WebVitalsReporter() { useEffect(() => { if (process.env.NODE_ENV === 'production') { return } const logMetric = (metric: Metric) => { /* ... */ } - onLCP(logMetric) - onFCP(logMetric) - onCLS(logMetric) - onTTFB(logMetric) + import('web-vitals').then(({ onLCP, onFCP, onCLS, onTTFB }) => { + onLCP(logMetric) + onFCP(logMetric) + onCLS(logMetric) + onTTFB(logMetric) + }) }, [])src/app/api/github/repo/route.ts (1)
37-40: Minor inconsistency: token is not trimmed here.Both
src/server/services/github.ts(line 64) andsrc/app/api/activity/combined/route.ts(line 18) call.trim()on the token before use. This file uses it raw. For consistency and to guard against accidental whitespace in env vars, consider trimming here too.Proposed fix
const token = process.env.GITHUB_TOKEN if (token) { - headers['Authorization'] = `token ${token}` + headers['Authorization'] = `token ${token.trim()}` }src/components/providers/posthog-provider.tsx (1)
27-45: Guard is good, but uninitialized client is still passed toPHProvider.When
NEXT_PUBLIC_POSTHOG_KEYis absent,posthog.init()is skipped but the module-levelposthoginstance is still handed to<PHProvider client={posthog}>.PostHogPageViewwill then call.capture()on an uninitialized client. In practice posthog-js silently drops these calls, so this won't crash — but if you want a belt-and-suspenders approach you could track initialization state and skip renderingPostHogPageViewentirely..gitignore (1)
43-44: Trailing whitespace on line 44.Minor cleanup — line 44 has trailing spaces.
tools/dev-menu/components/DevWidget.tsx (1)
130-136: Inconsistent color tokens with sibling buttons.The Home link uses semantic Tailwind classes (
text-muted-foreground,hover:text-foreground) while the adjacent Settings and Close buttons use hardcoded HSL values (text-[hsl(0,0%,55%)],hover:text-[hsl(0,0%,85%)]). These may not resolve to the same colors, producing a visual mismatch in the header row.Consider aligning to one approach — either all semantic tokens or all hardcoded HSL.
Proposed fix
<Link href="/" - className="text-muted-foreground hover:text-foreground transition-colors p-1" + className="text-[hsl(0,0%,55%)] hover:text-[hsl(0,0%,85%)] transition-colors p-1" title="Go Home" >src/components/projects/admin/projects-admin.tsx (1)
26-37:window.location.reload()discards client state unnecessarily.After
createProjectsucceeds, you setselectedId(line 33) but immediately reload the page (line 34), which discards that state. Consider appending the new project toprojectsstate and removing the reload to keep the experience seamless — similar to howonUpdateandonDeletealready manage state locally.Proposed fix
async function handleCreate() { startTransition(async () => { const result = await createProject({ title: 'New Project', desc: 'Project description' }) if (result.success && result.data) { + setProjects(prev => [result.data!, ...prev]) setSelectedId(result.data.id) - window.location.reload() } }) }src/components/layout/breadcrumbs.tsx (1)
72-78: Consideraria-labelfor the icon-only Home link.
titleprovides a tooltip butaria-labelis more reliably announced by screen readers for icon-only links. You could add both for maximum compatibility.Proposed fix
<Link href={buildHref('/', params)} - className="hover:text-foreground transition-colors flex items-center" - title="Home" + className="hover:text-foreground transition-colors flex items-center" + title="Home" + aria-label="Home" >src/utils/is-admin.ts (1)
19-23:getAdminEmails()is re-evaluated on every call.Each call to
isAdminEmailre-reads and re-parsesenv.ADMIN_EMAIL. For a server utility called on every request, consider caching the result at module level (if env values are stable at runtime).src/lib/auth-guard.ts (1)
1-5: Consider whether this thin wrapper is still needed.
isAdmin()here is a direct pass-through toisAdminUser()from@/utils/is-admin. If no additional logic is planned, consumers could import directly from@/utils/is-admin, reducing one layer of indirection. TherequireAdmin()helper on the other hand adds value (throw-on-unauthorized pattern).ADMIN_SECTION_REVIEW.md (1)
303-305: Documentation accurately reflects current state but note the consolidation opportunity.All three files are marked "(active)", which is correct. However, as the document itself notes in section E,
auth-guard.tsandactions/auth.tsare now thin wrappers overutils/is-admin.ts. Consider updating this section to clarify that consolidation is partially done (single source of truth achieved) even though multiple entry points remain.src/app/api/dev/routes/route.ts (1)
10-21:formatRouteLabelonly capitalizes the first character.For multi-word segments like
work-experience, this produces"Work experience"rather than"Work Experience". If title case is desired:Proposed fix
return lastSegment .replace(/-/g, ' ') - .replace(/^\w/, c => c.toUpperCase()) + .replace(/\b\w/g, c => c.toUpperCase())tools/dev-menu/components/sections/RoutesSection.tsx (3)
11-15: DuplicateRouteItemtype definition.This type is already defined in
tools/dev-menu/utils/generate-routes.ts. Consider importing it to stay DRY.
81-83:fetchRoutesmissing fromuseEffectdependency array.The ESLint
react-hooks/exhaustive-depsrule will flag this. Since you only want to fetch on mount and via the refresh button, either move the fetch call inline into the effect or wrapfetchRoutesinuseCallback.Proposed fix — inline the fetch
useEffect(() => { - fetchRoutes() + const load = async () => { + try { + setLoading(true) + const res = await fetch('/api/dev/routes') + if (!res.ok) throw new Error('Failed to fetch') + const data: RouteResponse = await res.json() + setRoutes(data.routes) + setError(false) + } catch (e) { + console.error(e) + setError(true) + } finally { + setLoading(false) + } + } + load() }, [])Keep the current
fetchRoutesfor the refresh button, or extract a stable reference withuseCallback.
85-94:window.location.hrefcauses a full-page reload — consideruseRouter.For a Next.js app,
router.push()fromnext/navigationwould give client-side navigation without losing state. This is a dev tool so impact is low, but it would feel snappier.src/components/landing/activity/activity-feed.tsx (2)
587-589: Wrapping GitHub row in<div className="relative">introduces an extra nesting level — ensure this is intentional.The
relativewrapper appears to exist solely to position the navigation arrows absolutely on small screens (Line 657:absolute right-0 top-0). If the flex-wrap container itself were maderelative, you could eliminate the extra div and the nesting complexity that led to the missing close tag.Simplified structure
{/* GITHUB ROW */} - <div className="relative"> - <div className="flex flex-wrap items-center gap-x-2 gap-y-1.5 pr-20 sm:pr-0"> + <div className="relative flex flex-wrap items-center gap-x-2 gap-y-1.5 pr-20 sm:pr-0">This merges
relativeinto the flex wrapper, eliminating one nesting level and the missing close tag bug.
724-724: Spotify linkmax-w-[200px]may truncate track names aggressively on medium viewports.The
sm:max-w-nonebreakpoint is missing here (unlike the artist span at Line 748 which hasmax-w-[150px] sm:max-w-none). On small-to-medium screens, 200px may cut off track names unnecessarily.src/app/(marketing)/dev/spotify/page.tsx (2)
58-61: Emptycatch {}discards error details useful for debugging.Both here and on Line 84, the bare
catch {}swallows the original error. For a dev-only tool page, logging the actual error would help troubleshoot OAuth issues.Proposed fix
- } catch { - if (!isActive) return - setError('Failed to load generated tokens') + } catch (err) { + if (!isActive) return + console.error('Token load failed:', err) + setError('Failed to load generated tokens') }
91-95:navigator.clipboard.writeTextcan reject — consider wrapping in try/catch.If clipboard access is denied (e.g., non-secure context, permissions policy), this will throw an unhandled promise rejection.
Proposed fix
const handleCopy = async (text: string, type: string) => { - await navigator.clipboard.writeText(text) - setCopied(type) - setTimeout(() => setCopied(null), 2000) + try { + await navigator.clipboard.writeText(text) + setCopied(type) + setTimeout(() => setCopied(null), 2000) + } catch { + setError('Failed to copy to clipboard') + } }
| publishedAt: '11-02-2026' | ||
| updatedAt: '11-02-2026' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all blog post files and their date formats
rg -n "publishedAt:|updatedAt:" --type=md -A0 | head -40Repository: remcostoeten/remcostoeten.nl
Length of output: 1624
🏁 Script executed:
# Also check if there are any published posts (not drafts) to see the format pattern
fd "\.md$" src/app/\(marketing\)/blog/posts --type f | grep -v drafts | xargs grep -H "publishedAt:" | head -20Repository: remcostoeten/remcostoeten.nl
Length of output: 824
🏁 Script executed:
# Search for date parsing or formatting logic in the codebase
rg "publishedAt|updatedAt|date.*format" --type=ts --type=tsx --type=js --type=jsx -i | grep -i "parse\|format\|date" | head -20Repository: remcostoeten/remcostoeten.nl
Length of output: 99
🏁 Script executed:
# Search for date parsing/formatting in TypeScript/JavaScript files
rg "publishedAt|parseDate|formatDate|Date\(" --type=ts --type=js -B2 -A2 | grep -v "node_modules" | head -50Repository: remcostoeten/remcostoeten.nl
Length of output: 2411
🏁 Script executed:
# Check if there's any date handling in the blog component or metadata processing
fd -e ts -e js src/app/\(marketing\)/blog | xargs grep -l "Date\|parse\|format" 2>/dev/null | head -10Repository: remcostoeten/remcostoeten.nl
Length of output: 473
🏁 Script executed:
# Read the formatDate function in full
sed -n '1,300p' src/utils/utils.ts | grep -A 30 "export function formatDate"Repository: remcostoeten/remcostoeten.nl
Length of output: 864
🏁 Script executed:
# Check if there are any tests or examples showing how dates should be formatted
fd -e test -e spec src | head -5
rg "formatDate\|publishedAt.*=" --type=ts --type=js -B1 -A1 | head -30Repository: remcostoeten/remcostoeten.nl
Length of output: 54
Change date format to YYYY-MM-DD to match other blog posts and enable proper parsing.
The date '11-02-2026' doesn't follow the ISO 8601 format (YYYY-MM-DD) used consistently throughout other blog posts (e.g., '2025-12-31', '2024-12-30'). The formatDate() function in src/utils/utils.ts relies on JavaScript's Date constructor, which expects ISO 8601 format and will misparsé or fail on '11-02-2026T00:00:00'. Change to '2026-02-11' (or '2026-11-02' if intended for November).
🤖 Prompt for AI Agents
In
`@src/app/`(marketing)/blog/posts/drafts/stop-using-arrow-fnc-and-HUGE-REVEAL.md
around lines 3 - 4, The publishedAt and updatedAt date strings in the draft use
'11-02-2026' which is not ISO (YYYY-MM-DD); update both publishedAt and
updatedAt to ISO 8601 format (e.g., '2026-02-11' or '2026-11-02' depending on
intended month/day) so the formatDate() logic and Date constructor parse them
correctly; locate the keys publishedAt and updatedAt in the post frontmatter and
replace the values with the correct YYYY-MM-DD strings.
| try { | ||
| const cwd = process.cwd() | ||
| const appDir = path.join(cwd, 'src/app') | ||
|
|
||
| // On Windows, glob returns forward slashes, but we should be careful with path joins | ||
| // We use fast-glob indirectly via glob, which supports forward slashes on Windows | ||
| const files = await glob('**/page.{tsx,js,jsx}', { | ||
| cwd: appDir, | ||
| ignore: [ | ||
| '**/api/**', // skip API routes | ||
| '**/_*/**', // skip private folders | ||
| '**/.*/**', // skip dotfiles/folders | ||
| ], | ||
| nodir: true | ||
| }) |
There was a problem hiding this comment.
Route scanning relies on source files that may not exist in production deployments.
On platforms like Vercel, the deployed artifact typically contains compiled output, not the original src/app directory structure. In production, glob would likely find no matching files, returning an empty routes array silently. Since canAccessDevTools() allows admin access in production, this endpoint could be invoked there and return misleading empty results.
Consider either:
- Restricting this route to development only (not just dev-tools access), or
- Adding a clear note in the response when no routes are found.
🤖 Prompt for AI Agents
In `@src/app/api/dev/routes/route.ts` around lines 27 - 41, The route depends on
scanning source files (appDir, glob -> files) which may not exist in production,
so update the handler to avoid returning misleading empty results: either gate
the endpoint to development-only by checking process.env.NODE_ENV ===
'development' (or similar) before running the glob/cwd logic and return a
403/empty-with-message if not dev, or if you must allow access in non-dev
environments keep the existing canAccessDevTools() check but detect files.length
=== 0 after the glob call and return a clear explanatory response (e.g., "no
routes found — source files not present in production build") instead of
silently returning an empty array; reference the variables/functions appDir,
files, glob, process.cwd(), and canAccessDevTools() when making the change.
| <div className="fixed inset-0 z-50 w-full h-full bg-black/80 lg:bg-transparent lg:static lg:z-auto lg:h-fit lg:w-auto lg:sticky lg:top-6 lg:border lg:border-zinc-800 backdrop-blur-sm lg:backdrop-blur-none flex flex-col justify-end lg:block"> | ||
| <div className="bg-zinc-950 w-full h-[90vh] lg:h-auto rounded-t-xl lg:rounded-none border-t border-zinc-800 lg:border-none flex flex-col"> |
There was a problem hiding this comment.
Modal overlay lacks role="dialog", escape-key handling, and focus trapping.
On mobile, this renders as a full-screen modal overlay, but it's a plain <div>. Screen readers won't announce it as a dialog, keyboard users can tab behind the backdrop, and pressing Escape doesn't close it.
At minimum, add role="dialog", aria-modal="true", and aria-label="Edit project" to the outer container, plus an onKeyDown handler for Escape. A focus trap (e.g., via focus-trap-react or a custom solution) would complete the accessibility story.
Minimal proposed fix for the outer container
- <div className="fixed inset-0 z-50 w-full h-full bg-black/80 lg:bg-transparent lg:static lg:z-auto lg:h-fit lg:w-auto lg:sticky lg:top-6 lg:border lg:border-zinc-800 backdrop-blur-sm lg:backdrop-blur-none flex flex-col justify-end lg:block">
+ <div
+ role="dialog"
+ aria-modal="true"
+ aria-label="Edit project"
+ onKeyDown={e => { if (e.key === 'Escape') onClose() }}
+ className="fixed inset-0 z-50 w-full h-full bg-black/80 lg:bg-transparent lg:static lg:z-auto lg:h-fit lg:w-auto lg:sticky lg:top-6 lg:border lg:border-zinc-800 backdrop-blur-sm lg:backdrop-blur-none flex flex-col justify-end lg:block"
+ >📝 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.
| <div className="fixed inset-0 z-50 w-full h-full bg-black/80 lg:bg-transparent lg:static lg:z-auto lg:h-fit lg:w-auto lg:sticky lg:top-6 lg:border lg:border-zinc-800 backdrop-blur-sm lg:backdrop-blur-none flex flex-col justify-end lg:block"> | |
| <div className="bg-zinc-950 w-full h-[90vh] lg:h-auto rounded-t-xl lg:rounded-none border-t border-zinc-800 lg:border-none flex flex-col"> | |
| <div | |
| role="dialog" | |
| aria-modal="true" | |
| aria-label="Edit project" | |
| onKeyDown={e => { if (e.key === 'Escape') onClose() }} | |
| className="fixed inset-0 z-50 w-full h-full bg-black/80 lg:bg-transparent lg:static lg:z-auto lg:h-fit lg:w-auto lg:sticky lg:top-6 lg:border lg:border-zinc-800 backdrop-blur-sm lg:backdrop-blur-none flex flex-col justify-end lg:block" | |
| > | |
| <div className="bg-zinc-950 w-full h-[90vh] lg:h-auto rounded-t-xl lg:rounded-none border-t border-zinc-800 lg:border-none flex flex-col"> |
🤖 Prompt for AI Agents
In `@src/components/projects/admin/project-editor.tsx` around lines 53 - 54, The
outer modal container (the large <div> used as the overlay in the ProjectEditor
component) must be made accessible: add role="dialog", aria-modal="true", and
aria-label="Edit project" to that outer container element, attach an onKeyDown
handler (e.g., handleOverlayKeyDown) that closes the modal when Escape is
pressed, and implement a focus trap around the modal content (use
focus-trap-react or a small custom trap that moves focus into the first
focusable element on open and restores focus on close); update the component to
call the existing close method (or prop) from the Escape handler and ensure
focus is managed on mount/unmount.
| {projects.map(project => ( | ||
| <div | ||
| key={project.id} | ||
| role="row" | ||
| aria-selected={selectedId === project.id} | ||
| onClick={() => onSelect(project.id)} | ||
| className={`px-4 py-3 cursor-pointer transition-colors ${ | ||
| selectedId === project.id | ||
| ? 'bg-zinc-800/50' | ||
| : 'hover:bg-zinc-900/50' | ||
| }`} | ||
| className={`px-4 py-3 cursor-pointer transition-colors ${selectedId === project.id | ||
| ? 'bg-zinc-800/50' | ||
| : 'hover:bg-zinc-900/50' | ||
| }`} |
There was a problem hiding this comment.
Row selection is mouse-only — inaccessible via keyboard.
The row uses onClick for selection but has no tabIndex, onKeyDown, or interactive role. Keyboard users cannot select a project. Consider adding tabIndex={0} and an onKeyDown handler that triggers onSelect on Enter/Space.
Proposed fix
<div
key={project.id}
role="row"
+ tabIndex={0}
aria-selected={selectedId === project.id}
onClick={() => onSelect(project.id)}
+ onKeyDown={e => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault()
+ onSelect(project.id)
+ }
+ }}📝 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.
| {projects.map(project => ( | |
| <div | |
| key={project.id} | |
| role="row" | |
| aria-selected={selectedId === project.id} | |
| onClick={() => onSelect(project.id)} | |
| className={`px-4 py-3 cursor-pointer transition-colors ${ | |
| selectedId === project.id | |
| ? 'bg-zinc-800/50' | |
| : 'hover:bg-zinc-900/50' | |
| }`} | |
| className={`px-4 py-3 cursor-pointer transition-colors ${selectedId === project.id | |
| ? 'bg-zinc-800/50' | |
| : 'hover:bg-zinc-900/50' | |
| }`} | |
| {projects.map(project => ( | |
| <div | |
| key={project.id} | |
| role="row" | |
| tabIndex={0} | |
| aria-selected={selectedId === project.id} | |
| onClick={() => onSelect(project.id)} | |
| onKeyDown={e => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault() | |
| onSelect(project.id) | |
| } | |
| }} | |
| className={`px-4 py-3 cursor-pointer transition-colors ${selectedId === project.id | |
| ? 'bg-zinc-800/50' | |
| : 'hover:bg-zinc-900/50' | |
| }`} |
🤖 Prompt for AI Agents
In `@src/components/projects/admin/project-list.tsx` around lines 44 - 53, The
project row is only selectable via mouse—update the element rendered in
projects.map (the div with role="row", key={project.id}, onClick={() =>
onSelect(project.id)} and aria-selected using selectedId) to be
keyboard-focusable and activate on Enter/Space: add tabIndex={0} and an
onKeyDown handler that calls onSelect(project.id) when the user presses Enter or
Space (handle Space with preventDefault to avoid scrolling); keep the existing
onClick and aria-selected to preserve behavior and semantics.
| {projects.map(project => ( | ||
| <div | ||
| key={project.id} | ||
| role="row" | ||
| aria-selected={selectedId === project.id} | ||
| onClick={() => onSelect(project.id)} | ||
| className={`px-4 py-3 cursor-pointer transition-colors ${ | ||
| selectedId === project.id | ||
| ? 'bg-zinc-800/50' | ||
| : 'hover:bg-zinc-900/50' | ||
| }`} | ||
| className={`px-4 py-3 cursor-pointer transition-colors ${selectedId === project.id | ||
| ? 'bg-zinc-800/50' | ||
| : 'hover:bg-zinc-900/50' | ||
| }`} | ||
| > | ||
| <div className="grid grid-cols-[auto,1fr,auto,auto] gap-4 items-center"> | ||
| <span className="w-8 text-xs text-zinc-600 font-mono"> | ||
| <span role="cell" className="w-8 text-xs text-zinc-600 font-mono"> | ||
| {project.idx} |
There was a problem hiding this comment.
Intermediate <div> breaks the ARIA table row→cell contract.
The role="cell" elements (Lines 56, 60, 76, 84) are wrapped inside a <div className="grid ..."> (Line 55) that sits between role="row" and the cells. Per the ARIA spec, role="cell" must be a direct child of role="row" (or owned via aria-owns). Screen readers will not correctly associate these cells with the row.
Move the grid styling onto the row element itself, or add role="presentation" to the intermediate div.
Proposed fix — merge grid into the row div
<div
key={project.id}
role="row"
aria-selected={selectedId === project.id}
onClick={() => onSelect(project.id)}
- className={`px-4 py-3 cursor-pointer transition-colors ${selectedId === project.id
+ className={`px-4 py-3 cursor-pointer transition-colors grid grid-cols-[auto,1fr,auto,auto] gap-4 items-center ${selectedId === project.id
? 'bg-zinc-800/50'
: 'hover:bg-zinc-900/50'
}`}
>
- <div className="grid grid-cols-[auto,1fr,auto,auto] gap-4 items-center">
<span role="cell" className="w-8 text-xs text-zinc-600 font-mono">(and remove the closing </div> for the grid wrapper)
📝 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.
| {projects.map(project => ( | |
| <div | |
| key={project.id} | |
| role="row" | |
| aria-selected={selectedId === project.id} | |
| onClick={() => onSelect(project.id)} | |
| className={`px-4 py-3 cursor-pointer transition-colors ${ | |
| selectedId === project.id | |
| ? 'bg-zinc-800/50' | |
| : 'hover:bg-zinc-900/50' | |
| }`} | |
| className={`px-4 py-3 cursor-pointer transition-colors ${selectedId === project.id | |
| ? 'bg-zinc-800/50' | |
| : 'hover:bg-zinc-900/50' | |
| }`} | |
| > | |
| <div className="grid grid-cols-[auto,1fr,auto,auto] gap-4 items-center"> | |
| <span className="w-8 text-xs text-zinc-600 font-mono"> | |
| <span role="cell" className="w-8 text-xs text-zinc-600 font-mono"> | |
| {project.idx} | |
| {projects.map(project => ( | |
| <div | |
| key={project.id} | |
| role="row" | |
| aria-selected={selectedId === project.id} | |
| onClick={() => onSelect(project.id)} | |
| className={`px-4 py-3 cursor-pointer transition-colors grid grid-cols-[auto,1fr,auto,auto] gap-4 items-center ${selectedId === project.id | |
| ? 'bg-zinc-800/50' | |
| : 'hover:bg-zinc-900/50' | |
| }`} | |
| > | |
| <span role="cell" className="w-8 text-xs text-zinc-600 font-mono"> | |
| {project.idx} |
🤖 Prompt for AI Agents
In `@src/components/projects/admin/project-list.tsx` around lines 44 - 57, The
ARIA issue is that the intermediate <div className="grid..."> inside the
ProjectList row breaks the row→cell contract; update the JSX in the ProjectList
component so that the grid layout is applied directly on the row container (the
div with role="row", key={project.id}, onClick={() => onSelect(project.id)},
className=...) or alternatively mark the intermediate grid wrapper with
role="presentation" so the role="cell" elements (the spans rendering project.idx
and other cells) become direct children of the row for screen readers; adjust
the className usage accordingly (remove the extra closing wrapper div if merging
classes into the row).
| const FALLBACK_ADMIN_EMAILS = [ | ||
| 'remcostoeten@gmail.com', | ||
| 'remcostoeten@hotmail.com' | ||
| ] | ||
|
|
||
| function getAdminEmails() { | ||
| const configured = (env.ADMIN_EMAIL || '') | ||
| .split(',') | ||
| .map(email => email.trim().toLowerCase()) | ||
| .filter(Boolean) | ||
|
|
||
| return configured.length > 0 ? configured : FALLBACK_ADMIN_EMAILS | ||
| } |
There was a problem hiding this comment.
Hardcoded fallback admin emails are a security risk.
If ADMIN_EMAIL is unset or empty in production, any account matching these hardcoded emails automatically gets admin access. This should be fail-closed: if the environment variable is not configured, no one should be admin by default.
Proposed fix — remove fallback, require explicit config
-const FALLBACK_ADMIN_EMAILS = [
- 'remcostoeten@gmail.com',
- 'remcostoeten@hotmail.com'
-]
-
function getAdminEmails() {
const configured = (env.ADMIN_EMAIL || '')
.split(',')
.map(email => email.trim().toLowerCase())
.filter(Boolean)
- return configured.length > 0 ? configured : FALLBACK_ADMIN_EMAILS
+ return configured
}📝 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.
| const FALLBACK_ADMIN_EMAILS = [ | |
| 'remcostoeten@gmail.com', | |
| 'remcostoeten@hotmail.com' | |
| ] | |
| function getAdminEmails() { | |
| const configured = (env.ADMIN_EMAIL || '') | |
| .split(',') | |
| .map(email => email.trim().toLowerCase()) | |
| .filter(Boolean) | |
| return configured.length > 0 ? configured : FALLBACK_ADMIN_EMAILS | |
| } | |
| function getAdminEmails() { | |
| const configured = (env.ADMIN_EMAIL || '') | |
| .split(',') | |
| .map(email => email.trim().toLowerCase()) | |
| .filter(Boolean) | |
| return configured | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/is-admin.ts` around lines 5 - 17, Remove the hardcoded
FALLBACK_ADMIN_EMAILS and update getAdminEmails to be fail-closed: parse
env.ADMIN_EMAIL (env.ADMIN_EMAIL || '') into a trimmed, lower-cased array and
return that array directly (which may be empty) instead of falling back to any
built-in emails; ensure any code referencing getAdminEmails or
FALLBACK_ADMIN_EMAILS is updated to stop relying on the fallback and to handle
an empty admin list safely.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export async function GET(request: NextRequest) { | ||
| const denied = await requireDevToolsAccess() | ||
| if (denied) return denied |
There was a problem hiding this comment.
Auth check in OAuth callback may break the redirect flow
requireDevToolsAccess() calls isAdmin(), which requires a valid user session via auth.api.getSession(). However, this route is Spotify's OAuth redirect target — the user's browser is redirected here by Spotify after authorization. If the session cookie isn't present or has expired during the redirect, this endpoint will return a 401 JSON response instead of completing the OAuth flow, leaving the user stranded with a raw JSON error.
Consider whether the auth guard is appropriate here, or if this route should instead validate a state/nonce parameter set before the OAuth flow began.
Additional Comments (1)
Removing the authorization token from this client-side hook means all GitHub API calls from Consider routing these calls through a server-side API endpoint (like the existing |
2 participants
Summary by Sourcery
Tighten admin/dev access control, improve SEO and analytics integration, and refine admin UI/UX and accessibility across projects, blog, and dev tools.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Chores:
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Confidence Score: 3/5
src/hooks/use-github.ts(unauthenticated GitHub API calls will hit rate limits),src/app/api/spotify/callback/route.ts(auth guard may break OAuth redirect flow),.lighthouse-desktop.json/.lighthouse-mobile.json(large generated files should not be committed)Important Files Changed
/api/,/admin/,/dev/paths and addedhostdirective./blog/tags/to/blog/topics/, added legal/utility routes, removed stale/blog/categoriesentry.&& null) instead of cleaning up the conditional entirely.isActiveflag andhistory.replaceState. Fixed redirect URI docs.Flowchart
flowchart TD subgraph Auth["Admin & Dev Access Control"] A["isAdmin() - src/utils/is-admin.ts"] -->|delegates to| B["auth.api.getSession()"] B --> C{Session exists?} C -->|No| D[Return false] C -->|Yes| E{Email match OR role=admin?} E -->|Yes| F[Return true] E -->|No| D G["canAccessDevTools() - src/lib/dev-access.ts"] --> H{NODE_ENV=development?} H -->|Yes| I[Allow access] H -->|No| A end subgraph Callers["Consumers"] J["checkAdminStatus() - actions/auth.ts"] --> A K["isAdmin() - lib/auth-guard.ts"] --> A L["requireDevToolsAccess()"] --> G end subgraph DevRoutes["Dev-Protected Routes"] L --> M["/api/spotify/*"] L --> N["/api/dev/routes"] L --> O["dev/layout.tsx guard"] end subgraph SpotifyOAuth["Spotify OAuth Flow (Hardened)"] P["Client: /dev/spotify"] -->|1. GET /api/spotify/auth-url| Q["Generate Spotify Auth URL"] Q -->|2. Redirect to Spotify| R["Spotify Authorization"] R -->|3. Callback to /api/spotify/callback| S["Exchange code for tokens"] S -->|4. Set httpOnly cookies| T["Redirect to /dev/spotify?success=true"] T -->|5. GET /api/spotify/dev-token| U["Read & delete cookies"] U -->|6. Return tokens to client| P endLast reviewed commit: ee91690