Feat: Extension overhaul and new features - #2
Merged
Merged
Conversation
…d background/content scripts - Added icons for the extension in various sizes (128px, 96px, 48px, 32px, 16px). - Introduced a new SVG logo for the extension. - Implemented a ThemeProvider to manage light/dark themes and system preferences. - Created a reusable Button component with variant and size options. - Set up background and content scripts for the extension to log messages on load. - Developed a basic popup UI with a loading message. - Configured HTML entry point for the popup. - Established main entry point for the popup with React rendering. - Added utility functions for class name management. - Included Tailwind CSS and custom styles for the extension. - Configured TypeScript settings and WXT for the project. - Defined extension manifest with permissions and commands.
- Implemented URL management API functions in `src/api/urls.ts` for listing, updating, and deleting URLs. - Created authentication hooks in `src/hooks/use-auth.ts` for login, registration, and user session management. - Added hooks for managing API keys in `src/hooks/use-keys.ts`. - Introduced settings management hooks in `src/hooks/use-settings.ts`. - Developed URL shortening functionality in `src/hooks/use-shorten.ts`. - Added statistics retrieval hooks in `src/hooks/use-stats.ts`. - Implemented theme management in `src/hooks/use-theme.ts`. - Created URL management hooks in `src/hooks/use-urls.ts`. - Defined constants and error handling in `src/lib/constants.ts` and `src/lib/errors.ts`. - Established messaging system in `src/lib/messaging.ts` for communication with background scripts. - Implemented migration logic for settings and history in `src/lib/migration.ts`. - Set up query client configuration in `src/lib/query-client.ts`. - Created storage management utilities in `src/lib/storage.ts`. - Added URL utility functions in `src/lib/url-utils.ts`. - Defined API and settings schemas in `src/schemas/api.ts` and `src/schemas/settings.ts`. - Created Zustand stores for authentication and settings management in `src/stores/auth.ts` and `src/stores/settings.ts`. - Implemented UI state management in `src/stores/ui.ts`.
- Implemented AuthSection component for user authentication with login and API key options. - Added OfflineBanner component to notify users when offline. - Created ThemeProvider to manage theme settings based on user preferences. - Introduced various UI components including Avatar, Badge, Card, Input, Label, Separator, Tooltip, and Tabs for consistent styling. - Developed HistoryList and ShortenForm components for URL management and shortening functionality. - Updated App component to integrate new features and manage application state. - Added necessary dependencies in package.json for new components and functionality.
…ttings, and URLs tabs
feat: add Chart component with tooltip and legend functionalities feat: implement HighlightedBarChart component with interactive features fix: improve side panel layout and spacing for better UI fix: enhance useShortenMutation to handle QR code generation and history storage fix: update API schemas to allow nullable fields for optional properties style: adjust chart color variables for improved visibility chore: configure development server port in wxt.config
fix: update AnalyticsTab to always show account analytics and URL analytics refactor: remove AuthSection from DashboardTab and adjust layout feat: implement Dialog component for consistent modal UI feat: create DropdownMenu component for enhanced dropdown functionality chore: remove unused HighlightedBarChart component feat: add Kbd and KbdGroup components for keyboard shortcut display fix: adjust Slider component thumb key handling fix: enhance ShortenForm with autoFocus and reset behavior chore: update App components to include UserMenu and improve layout chore: update SidePanel to include UserMenu and adjust tab content display style: improve notification skeleton animation for better UX
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Zingzy, your pull request is larger than the review limit of 150000 diff characters
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the legacy Chrome extension with a WXT-based TypeScript React codebase: removes prior manifest/background/content/popup assets and adds typed API clients, Zod schemas, Zustand stores, React UI primitives/pages (popup/sidepanel), background/content entrypoints, storage/migration, hooks, utilities, CI, and build tooling. Changes
Sequence Diagram(s)sequenceDiagram
participant BG as BackgroundWorker
participant Storage as WXTStorage
participant API as spoo.me API
participant Tab as BrowserTab
participant Notify as NotificationEngine
BG->>Storage: read settings & auth
BG->>Tab: get active tab / normalized URL
BG->>API: POST /shorten (client.request with auth)
API-->>BG: 200 + shortened URL
BG->>Storage: prepend history item (short_url, alias, timestamp, qrUrl?)
BG->>Storage: drain queued items (if applicable)
BG->>Tab: attempt page script injection to show toast
alt injection succeeds
BG->>Notify: injected toast shows QR/copy (scripting)
else
BG->>Notify: create browser notification fallback
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR overhauls the browser extension into a WXT + React (MV3) codebase, introducing a new popup + side panel UI, structured API clients, Zustand stores, and a migration path from the prior v1 extension storage format.
Changes:
- Replaces the legacy manifest/background/content/popup implementation with a WXT-based MV3 architecture and React entrypoints (popup + sidepanel).
- Adds typed API clients + Zod schemas, persistent storage abstractions, and Zustand stores for auth/settings/UI state.
- Implements storage migration (v1 → v2) and introduces richer UI features (history, URL management, settings, analytics scaffolding).
Reviewed changes
Copilot reviewed 80 out of 91 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| wxt.config.ts | Adds WXT configuration and MV3 manifest definition (permissions, commands, omnibox, WAR). |
| tsconfig.json | Sets TS config extending WXT defaults and adds @/* path aliasing. |
| src/styles/app.css | Introduces Tailwind + shadcn styling, theme tokens, and base layer styles. |
| src/stores/ui.ts | Adds Zustand UI store for online/offline state. |
| src/stores/settings.ts | Adds Zustand settings store backed by extension storage. |
| src/stores/auth.ts | Adds Zustand auth store with persisted/session auth state. |
| src/schemas/storage.ts | Adds Zod schemas for parsing legacy (v1) storage formats. |
| src/schemas/settings.ts | Adds Zod schema/types for v2 settings + history items. |
| src/schemas/api.ts | Adds Zod schemas for API responses (auth/urls/stats/keys/errors). |
| src/lib/utils.ts | Adds cn() helper for className composition. |
| src/lib/url-utils.ts | Adds URL validation/normalization utilities and shortcode extraction. |
| src/lib/storage.ts | Defines typed WXT storage items (auth, settings, history, migration, offline queue). |
| src/lib/query-client.ts | Centralizes React Query client defaults/retry behavior. |
| src/lib/notification.ts | Implements injected in-page toast notifications via scripting.executeScript. |
| src/lib/migration.ts | Implements v1 → v2 storage migration for settings/history. |
| src/lib/messaging.ts | Adds typed runtime messaging definitions/util. |
| src/lib/errors.ts | Adds API/network error types + offline helper. |
| src/lib/constants.ts | Adds endpoint constants, default settings, and token timing constants. |
| src/hooks/use-urls.ts | Adds URL management hooks (list/update/delete) via React Query. |
| src/hooks/use-theme.ts | Adds theme resolution/apply helper hook. |
| src/hooks/use-stats.ts | Adds stats hooks (account + per-shortcode). |
| src/hooks/use-shorten.ts | Adds shorten mutation with history persistence + QR URL generation. |
| src/hooks/use-settings.ts | Adds convenience wrapper around settings store mutations. |
| src/hooks/use-keys.ts | Adds API key management hooks (list/create/delete). |
| src/hooks/use-auth.ts | Adds auth hooks (login/register/logout/me + form helpers). |
| src/entrypoints/sidepanel/main.tsx | Side panel React bootstrap. |
| src/entrypoints/sidepanel/index.html | Side panel HTML shell. |
| src/entrypoints/sidepanel/App.tsx | Side panel app composition (tabs, providers, init, online/offline listeners). |
| src/entrypoints/popup/main.tsx | Popup React bootstrap. |
| src/entrypoints/popup/index.html | Popup HTML shell. |
| src/entrypoints/popup/App.tsx | Popup UI (shorten/history, theme toggle, open side panel). |
| src/entrypoints/content/index.ts | Adds content script placeholder (reserved for OAuth flow completion). |
| src/entrypoints/background/index.ts | Adds MV3 service worker: context menu, commands, omnibox, migration, token refresh, offline queue. |
| src/components/url/ShortenForm.tsx | Adds shorten form UI with copy-to-clipboard and error display. |
| src/components/url/HistoryList.tsx | Adds history list UI backed by storage watch. |
| src/components/ui/tooltip.tsx | Adds shadcn-style tooltip wrappers. |
| src/components/ui/tabs.tsx | Adds shadcn-style tabs wrappers/variants. |
| src/components/ui/switch.tsx | Adds shadcn-style switch wrapper. |
| src/components/ui/sonner.tsx | Adds Sonner toaster wrapper component. |
| src/components/ui/slider.tsx | Adds shadcn-style slider wrapper. |
| src/components/ui/separator.tsx | Adds shadcn-style separator wrapper. |
| src/components/ui/select.tsx | Adds shadcn-style select wrapper components. |
| src/components/ui/label.tsx | Adds shadcn-style label wrapper. |
| src/components/ui/kbd.tsx | Adds shadcn-style keyboard key components. |
| src/components/ui/input.tsx | Adds shadcn-style input wrapper. |
| src/components/ui/dropdown-menu.tsx | Adds shadcn-style dropdown menu wrappers. |
| src/components/ui/dialog.tsx | Adds shadcn-style dialog wrappers. |
| src/components/ui/chart.tsx | Adds shadcn-style chart wrappers/helpers (Recharts). |
| src/components/ui/card.tsx | Adds shadcn-style card components. |
| src/components/ui/button.tsx | Adds shadcn-style button variants. |
| src/components/ui/badge.tsx | Adds shadcn-style badge variants. |
| src/components/ui/avatar.tsx | Adds shadcn-style avatar components. |
| src/components/ui/alert-dialog.tsx | Adds shadcn-style alert dialog wrappers. |
| src/components/sidepanel/UrlsTab.tsx | Adds URL management UI (search/sort/pagination/actions). |
| src/components/sidepanel/SettingsTab.tsx | Adds settings UI for theme/behavior/notifications/QR defaults. |
| src/components/sidepanel/DashboardTab.tsx | Adds dashboard tab composing shorten + recent history. |
| src/components/sidepanel/AccountTab.tsx | Adds account/API key management UI for authenticated users. |
| src/components/shared/ThemeProvider.tsx | Applies theme class to document root based on settings. |
| src/components/shared/OfflineBanner.tsx | Adds offline banner driven by UI store state. |
| src/components/auth/UserMenu.tsx | Adds authenticated user dropdown + logout/dashboard links. |
| src/components/auth/AuthSection.tsx | Adds sign-in dialog with email + API key modes. |
| src/api/urls.ts | Adds URL management API client functions. |
| src/api/types.ts | Adds typed API request/response models. |
| src/api/stats.ts | Adds stats client with v1→v0 fallback conversion. |
| src/api/shorten.ts | Adds shorten API client function. |
| src/api/qr.ts | Adds QR URL builder helpers. |
| src/api/keys.ts | Adds API key client functions. |
| src/api/export.ts | Adds stats export client returning Blob. |
| src/api/client.ts | Adds core request wrapper (auth injection, refresh, errors, schema parsing). |
| src/api/auth.ts | Adds auth API client functions. |
| public/icon/96.png | Adds updated extension icon asset. |
| public/icon/48.png | Adds updated extension icon asset. |
| public/icon/32.png | Adds updated extension icon asset. |
| public/icon/16.png | Adds updated extension icon asset. |
| popup/popup.js | Removes legacy popup implementation. |
| popup/popup.html | Removes legacy popup HTML. |
| popup/popup.css | Removes legacy popup CSS. |
| package.json | Introduces WXT/React/Tailwind/shadcn dependencies and Bun-based scripts. |
| manifest.json | Removes legacy static MV3 manifest (replaced by WXT config). |
| content.js | Removes legacy content script implementation. |
| components.json | Adds shadcn/ui generator configuration. |
| biome.json | Adds Biome formatting/lint configuration. |
| background.js | Removes legacy background service worker implementation. |
| .gitignore | Adds WXT/bundler/editor ignores. |
| .github/workflows/ci.yml | Adds CI workflow for lint/typecheck/build and bundle size enforcement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
src/components/ui/kbd.tsx-16-23 (1)
16-23:⚠️ Potential issue | 🟡 MinorType mismatch: props typed as
<div>but renders<kbd>.
KbdGroupacceptsReact.ComponentProps<"div">but renders a<kbd>element. This inconsistency could cause unexpected behavior if div-specific attributes are passed.🔧 Proposed fix
-function KbdGroup({ className, ...props }: React.ComponentProps<"div">) { +function KbdGroup({ className, ...props }: React.ComponentProps<"kbd">) { return ( <kbd data-slot="kbd-group" className={cn("inline-flex items-center gap-1", className)} {...props} /> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/kbd.tsx` around lines 16 - 23, The KbdGroup component is typed as React.ComponentProps<"div"> but renders a <kbd> element; change the props type to the correct intrinsic element (e.g., React.ComponentProps<"kbd"> or JSX.IntrinsicElements["kbd"]) so attributes match the rendered element, update the function signature for KbdGroup to use that type (preserving className and rest props) and ensure any callers still work with the new kbd prop types.src/hooks/use-stats.ts-13-14 (1)
13-14:⚠️ Potential issue | 🟡 MinorAdd guard to prevent queries with empty or whitespace-only
shortCode.The
useUrlStatshook is currently unused in the codebase, but once deployed, gateenabledwith a trimmed value check to avoid unnecessary API requests whenshortCodeis empty.Suggested change
export function useUrlStats(shortCode: string, enabled = true) { - return useStats({ short_code: shortCode }, enabled); + return useStats({ short_code: shortCode }, enabled && shortCode.trim().length > 0); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-stats.ts` around lines 13 - 14, Guard the hook against empty/whitespace short codes by combining the existing enabled flag with a trimmed-value check before calling useStats: in useUrlStats, compute enabled = enabled && shortCode.trim().length > 0 (or equivalent) and pass that into useStats({ short_code: shortCode }, enabled) so no request is made when shortCode is empty or only whitespace; update the function useUrlStats accordingly, referencing useUrlStats and the call to useStats.src/entrypoints/popup/App.tsx-23-23 (1)
23-23:⚠️ Potential issue | 🟡 Minor
isDarkis not reactive to theme changes.Reading
document.documentElement.classList.contains("dark")directly gives a snapshot value at render time. If the theme changes viaupdateSettings, the Sun/Moon icon won't update until another re-render is triggered. Consider deriving this from the settings store or using a state that syncs with the DOM.🔧 Suggested fix
+import { useSettingsStore } from "@/stores/settings"; + function PopupContent() { const { mode } = useAuthStore(); - const { updateSettings } = useSettingsStore(); - const isDark = document.documentElement.classList.contains("dark"); + const { settings, updateSettings } = useSettingsStore(); + const isDark = settings.theme === "dark";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/popup/App.tsx` at line 23, isDark is computed once from document.documentElement.classList and won't update when theme changes via updateSettings, so replace the static check with a reactive source: either read theme from the existing settings store (e.g., useSettings or settings.theme) and derive isDark from that, or keep a local state (isDark) and sync it via useEffect that reads document.documentElement.classList.contains("dark") and registers a MutationObserver to update state on class changes; apply this to the Sun/Moon icon render so it reacts to updateSettings changes and clean up the observer in the effect.src/entrypoints/sidepanel/App.tsx-96-104 (1)
96-104:⚠️ Potential issue | 🟡 MinorInitialize online status on mount.
Only subscribing to events misses the initial state. Set
setOnline(navigator.onLine)in the effect before registering listeners.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/sidepanel/App.tsx` around lines 96 - 104, The effect in App.tsx currently registers online/offline listeners but never sets the initial online state; inside the useEffect that calls initAuth() and initSettings(), call setOnline(navigator.onLine) before adding listeners so the component reflects the current connectivity on mount; keep the existing handleOnline/handleOffline functions and window.addEventListener/cleanup logic intact.src/entrypoints/sidepanel/App.tsx-42-73 (1)
42-73:⚠️ Potential issue | 🟡 MinorPrevent invalid active tab when auth state changes.
When signed-out while
urlsis selected, the selected tab can reference removed content. Control tab state and reset todashboardwhenurlsbecomes unavailable.Suggested fix
-import { useEffect } from "react"; +import { useEffect, useState } from "react"; function SidePanelContent() { const { mode } = useAuthStore(); const isAuthenticated = mode !== "anonymous"; + const [activeTab, setActiveTab] = useState("dashboard"); + + useEffect(() => { + if (!isAuthenticated && activeTab === "urls") { + setActiveTab("dashboard"); + } + }, [isAuthenticated, activeTab]); - <Tabs defaultValue="dashboard" className="px-4 pt-3 pb-6"> + <Tabs value={activeTab} onValueChange={setActiveTab} className="px-4 pt-3 pb-6">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/sidepanel/App.tsx` around lines 42 - 73, App currently uses uncontrolled Tabs (defaultValue) so when isAuthenticated flips to false while "urls" is active the UI still references removed content; make Tabs controlled by adding local state (e.g., activeTab) in the App component, pass that state into Tabs via value={activeTab} and update it on tab changes, and add a useEffect that watches isAuthenticated and if isAuthenticated becomes false and activeTab === "urls" sets activeTab to "dashboard" so the active tab is reset; touch the Tabs, TabsTrigger, TabsContent usage and maintain existing values ("dashboard","urls","analytics","settings") and preserve DashboardTab and UrlsTab rendering logic.src/entrypoints/sidepanel/App.tsx-21-23 (1)
21-23:⚠️ Potential issue | 🟡 MinorGate UI on auth hydration to avoid anonymous flash.
modeis consumed before auth initialization completes, so authenticated users can briefly see anonymous-only UI. UseisLoadingto defer rendering auth-gated sections.Suggested fix
function SidePanelContent() { - const { mode } = useAuthStore(); + const { mode, isLoading } = useAuthStore(); + if (isLoading) { + return <div className="min-h-screen bg-background" />; + } const isAuthenticated = mode !== "anonymous";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/sidepanel/App.tsx` around lines 21 - 23, The UI is reading mode from useAuthStore before auth hydration, causing a flash of anonymous UI; update the component to also read isLoading (from useAuthStore) and gate auth-dependent rendering until hydration completes — e.g., derive isAuthenticated only when isLoading is false or simply return early (null/placeholder) while isLoading is true, then compute isAuthenticated = mode !== "anonymous" after hydration; update any places that currently use mode/isAuthenticated to respect this loading gate (useAuthStore, mode, isLoading, isAuthenticated).src/hooks/use-auth.ts-21-30 (1)
21-30:⚠️ Potential issue | 🟡 MinorMissing query invalidation in
useRegister.
useLogininvalidates queries on success (line 16), butuseRegisterdoesn't. After successful registration, the user is authenticated, so cached queries should likely be invalidated for consistency.🔧 Proposed fix
export function useRegister() { const { setJwtAuth } = useAuthStore(); + const queryClient = useQueryClient(); return useMutation({ mutationFn: authApi.register, onSuccess: async (data) => { await setJwtAuth(data.access_token, "", data.user); + queryClient.invalidateQueries(); }, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-auth.ts` around lines 21 - 30, useRegister currently sets auth via setJwtAuth but does not invalidate cached queries like useLogin does; update useRegister to get the query client (e.g., via useQueryClient) and call queryClient.invalidateQueries() (or invalidate specific keys) inside the onSuccess after awaiting setJwtAuth so cached data is refreshed; reference the useRegister function, its onSuccess handler, setJwtAuth, and mirror the invalidation logic used in useLogin.src/api/client.ts-154-155 (1)
154-155:⚠️ Potential issue | 🟡 MinorPotential failure on empty or non-JSON responses.
res.json()is called unconditionally for successful responses. Some endpoints might return204 No Contentor non-JSON responses, causing a parse error.🔧 Proposed fix
- const json = await res.json(); - return schema ? schema.parse(json) : (json as T); + // Handle empty responses + const text = await res.text(); + if (!text) { + return (schema ? schema.parse(undefined) : undefined) as T; + } + const json = JSON.parse(text); + return schema ? schema.parse(json) : (json as T);Or check status code:
+ if (res.status === 204) { + return undefined as T; + } const json = await res.json(); return schema ? schema.parse(json) : (json as T);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/client.ts` around lines 154 - 155, Handle empty or non-JSON responses before calling res.json(): check res.status === 204 (or res.headers.get('content-type') for 'application/json') and return undefined or an appropriate empty value instead of calling res.json(); otherwise call res.json() and then return schema ? schema.parse(json) : (json as T). Update the logic around the existing res.json() call and the schema.parse branch so you only parse JSON when the response actually contains JSON.src/schemas/settings.ts-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorChange the import path from
"zod/v4"to"zod".Zod v4 (installed as v4.3.6) should be imported as
import { z } from "zod", not from the non-existent subpath"zod/v4". Update line 1 to:import { z } from "zod";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/settings.ts` at line 1, Update the import for Zod in src/schemas/settings.ts: replace the incorrect subpath import "zod/v4" with the package root "zod" in the top-level import statement (the line that currently reads an import of z from "zod/v4"), i.e., modify the import that defines the symbol z so it imports from "zod".src/components/ui/chart.tsx-218-222 (1)
218-222:⚠️ Potential issue | 🟡 MinorZero values will not be displayed due to truthy check.
The condition
{item.value && (...)}will not render whenitem.valueis0, which is a valid data point in charts. Consider using an explicit null/undefined check instead.Proposed fix
- {item.value && ( + {item.value !== undefined && item.value !== null && ( <span className="font-mono font-medium text-foreground tabular-nums"> {item.value.toLocaleString()} </span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/chart.tsx` around lines 218 - 222, The render condition currently uses a truthy check on item.value which hides valid zero values; update the conditional around the span that displays item.value (the JSX that checks item.value) to explicitly test for null/undefined (e.g., item.value !== null && item.value !== undefined or item.value != null) so 0 is rendered while still skipping absent values.
🧹 Nitpick comments (20)
src/components/ui/tabs.tsx (1)
7-20: Minor: Redundantdata-orientationattribute.Radix's
TabsPrimitive.Rootalready setsdata-orientationbased on theorientationprop. Explicitly setting it is harmless but redundant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/tabs.tsx` around lines 7 - 20, The Tabs component redundantly sets data-orientation on TabsPrimitive.Root even though Radix's TabsPrimitive.Root already emits that attribute from the orientation prop; remove the explicit data-orientation={orientation} prop from the Tabs function so that TabsPrimitive.Root relies on the orientation passed via ...props/prop forwarding (refer to the Tabs function and TabsPrimitive.Root identifiers and the orientation prop) and keep the rest of the attributes (className, data-slot, {...props}) unchanged.src/lib/url-utils.ts (1)
45-54: Consider handlingwww.spoo.mesubdomain.
extractShortCodeonly matchesspoo.meexactly. URLs likehttps://www.spoo.me/abc123would returnnull. If the service supports the www subdomain, consider:🔧 Proposed fix
export function extractShortCode(shortUrl: string): string | null { try { const url = new URL(shortUrl); - if (url.hostname !== "spoo.me") return null; + if (url.hostname !== "spoo.me" && url.hostname !== "www.spoo.me") return null; const code = url.pathname.slice(1); return code || null; } catch { return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/url-utils.ts` around lines 45 - 54, extractShortCode currently rejects URLs with the www subdomain; update the hostname check in extractShortCode to accept both "spoo.me" and "www.spoo.me" (e.g., normalize url.hostname or check url.hostname === "spoo.me" || url.hostname === "www.spoo.me") so URLs like "https://www.spoo.me/abc123" return the code as before, leaving the rest of the try/catch and pathname-slicing logic intact.src/components/ui/slider.tsx (1)
16-19: Default fallback creates a 2-thumb range slider.When neither
valuenordefaultValueis provided,_valuesfalls back to[min, max], rendering two thumbs. If single-value sliders are common, consider defaulting to[min]instead, or document this behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/slider.tsx` around lines 16 - 19, The _values computed inside the React.useMemo currently falls back to [min, max] when neither value nor defaultValue is provided, producing a 2-thumb slider; change the fallback to [min] to default to a single-thumb slider (update the useMemo expression that computes _values to return [min] instead of [min, max]), and then verify and adjust any downstream logic that assumes two values (rendering, thumb mapping, and types) in the same component (look for references to _values, value/defaultValue props, and any thumb-rendering code) so the single-value case is handled correctly.src/api/types.ts (1)
107-115: Normalize timestamp fields before exporting these shared app models.
UrlResponse.created_atandUpdateUrlResponse.updated_atare numbers, whileUrlListItem.created_atandlast_clickare strings. That forces every consumer to branch on transport encoding instead of domain meaning. Consider keeping raw DTOs separate and mapping them to one timestamp shape in the API layer.Also applies to: 134-145, 147-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/types.ts` around lines 107 - 115, The models expose inconsistent timestamp types — normalize all exported API model timestamp fields to a single shape (e.g., ISO string or epoch millis) instead of mixing number and string; update the type definitions (UrlResponse.created_at, UpdateUrlResponse.updated_at, UrlListItem.created_at, UrlListItem.last_click) to that chosen type and implement a mapping/conversion step in the API layer that takes raw DTOs and converts their timestamp fields to that unified representation (keep raw internal DTO shapes separate if needed and perform conversion inside the functions that produce UrlResponse, UpdateUrlResponse, and UrlListItem).src/components/ui/alert-dialog.tsx (1)
72-79:sm:flex-rowis dead wheneversize="sm"is active.
group-data-[size=sm]/alert-dialog-content:gridoverrides the baseflex, so this branch never switches back to a flex row for that size. Either make the grid variant breakpoint-scoped or drop thesm:flex-rowutilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/alert-dialog.tsx` around lines 72 - 79, AlertDialogFooter's class list contains a conflicting grid utility "group-data-[size=sm]/alert-dialog-content:grid" that permanently overrides the base flex layout and makes "sm:flex-row" ineffective; fix by scoping the grid utility to the desired breakpoint (for example prefix the group-data token with the breakpoint, e.g. "sm:group-data-[size=sm]/alert-dialog-content:grid") so the component uses flex by default and switches to grid only at the breakpoint, or alternatively remove the "sm:flex-row" utility—update the class string in AlertDialogFooter where className is composed to apply the chosen fix.src/entrypoints/content/index.ts (1)
1-7: Placeholder content script with empty implementation.The
main()function is reserved for future OAuth flow completion but currently does nothing. Consider adding a// TODO:comment or tracking this in an issue if OAuth support is a planned feature.Would you like me to open an issue to track the OAuth flow implementation?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/content/index.ts` around lines 1 - 7, The content script's main() in defineContentScript is empty and should explicitly indicate it's intentionally unimplemented; update the main() implementation to include a TODO comment referencing the planned OAuth flow (e.g., "// TODO: implement OAuth flow completion — tracked in ISSUE-<n>" or a URL to an issue) or create that tracker issue and reference it in the comment so future contributors know it's intentional and where to find the work item; locate the defineContentScript export and the main() function in src/entrypoints/content/index.ts to add the comment..github/workflows/ci.yml (1)
17-20: Pin Bun version for deterministic CI runs.Using
bun-version: latestintroduces non-determinism across CI runs. Replace with a specific version (e.g.,bun-version: "1.0.0") or create a.bun-versionfile in the repository root.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 17 - 20, The workflow currently uses oven-sh/setup-bun@v2 with bun-version: latest which causes non-deterministic CI; update the CI step that references "uses: oven-sh/setup-bun@v2" to set a fixed bun-version string (e.g., "1.0.0" or your tested release) instead of "latest", or alternatively add a .bun-version file at the repository root to pin the Bun runtime; ensure the change updates the bun-version key in the workflow or adds the .bun-version file so CI runs are deterministic.src/api/export.ts (2)
34-36: Include status code in error message.
res.statusTextcan be empty in HTTP/2 responses. Include the status code for better debugging:💡 Improved error message
if (!res.ok) { - throw new Error(`Export failed: ${res.statusText}`); + throw new Error(`Export failed: ${res.status} ${res.statusText}`.trim()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/export.ts` around lines 34 - 36, The error thrown when the response is not ok only uses res.statusText which can be empty; update the throw in the if (!res.ok) block (the code referencing res) to include the numeric status code as well (e.g., include res.status and optionally res.statusText or a fallback) so the error message contains both the HTTP status code and text for clearer debugging.
20-28: Extract auth header logic into a shared utility to eliminate duplication.The auth header construction (lines 20-28) duplicates the
getAuthHeader()logic inclient.ts:23-37. WhileexportStatsrequires directfetch()for binary blob responses (unlike therequest()client which parses JSON), the credential retrieval and Bearer token formatting can be extracted into a reusable function. This reduces maintenance burden if auth logic changes in the future.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/export.ts` around lines 20 - 28, The auth header construction in exportStats duplicates the logic already implemented in getAuthHeader() (client.ts lines 23-37); extract the credential retrieval and Bearer formatting into a shared utility (e.g., export a getAuthHeader function that uses authModeStorage, apiKeyStorage, accessTokenStorage and returns a headers object or Authorization value), update client.ts to import and use that shared getAuthHeader, and replace the duplicated block inside exportStats with a call to the shared utility before performing the direct fetch so both code paths use the same auth logic.src/components/sidepanel/AccountTab.tsx (1)
224-234: Minor: Delete button disables all rows during any deletion.
deleteKey.isPendingdisables all delete buttons when any key is being deleted. For a better UX, track whichkeyIdis being deleted and disable only that row's button.💡 Suggested approach
Track the pending key ID in local state:
const [deletingKeyId, setDeletingKeyId] = useState<string | null>(null); // In the delete handler: onClick={() => { setDeletingKeyId(key.id); deleteKey.mutate( { keyId: key.id, revoke: true }, { onSettled: () => setDeletingKeyId(null) } ); }} disabled={deletingKeyId === key.id}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidepanel/AccountTab.tsx` around lines 224 - 234, The delete button currently uses deleteKey.isPending which disables every row while any deletion is in progress; change this to track the specific key being deleted by adding local state (e.g., deletingKeyId via useState<string | null>) and update the onClick for the Button that calls deleteKey.mutate to set deletingKeyId to key.id before mutating and clear it in onSettled; then replace disabled={deleteKey.isPending} with disabled={deletingKeyId === key.id} so only the row whose key.id is being deleted is disabled (refer to deleteKey, key.id, and the Button in AccountTab).src/components/url/ShortenForm.tsx (1)
20-25: Potential state update after unmount.The
setTimeouton line 24 may fire after the component unmounts, causing a React warning. Consider using a ref to track mounted state or move this logic into auseEffectwith cleanup.♻️ Suggested fix using a ref
+import { useRef } from "react"; export function ShortenForm() { const [url, setUrl] = useState(""); const [copied, setCopied] = useState(false); + const timeoutRef = useRef<ReturnType<typeof setTimeout>>(); const shorten = useShortenMutation(); + // Cleanup on unmount + useEffect(() => { + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + }; + }, []); + const handleCopy = async () => { if (!shorten.data) return; await navigator.clipboard.writeText(shorten.data.short_url); setCopied(true); - setTimeout(() => setCopied(false), 2000); + timeoutRef.current = setTimeout(() => setCopied(false), 2000); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/url/ShortenForm.tsx` around lines 20 - 25, The setTimeout in handleCopy can call setCopied after the component unmounts; modify handleCopy to store the timeout id (from setTimeout) in a ref (e.g., copyTimeoutRef) and set copied state immediately, then in a useEffect return cleanup that clears the timeout (clearTimeout(copyTimeoutRef.current)) and/or uses an isMounted ref to avoid calling setCopied if unmounted; update references to shorten.data and setCopied in handleCopy to use this safe pattern so no state update occurs after unmount.src/hooks/use-theme.ts (1)
30-35: Consolidate theme logic betweenuseThemehook andThemeProvidercomponent.Both apply the same resolved theme to
document.documentElementand independently manage matchMedia listeners for system theme changes. IfThemeProviderwraps the app anduseThemeis used in descendant components, both effects will run simultaneously, causing redundant DOM manipulation and duplicate listeners.Since
useThemeprovides state andThemeProvideris a wrapper, consider either:
- Making
ThemeProviderthe only source of truth and consuming theme state from store/context- Or removing
ThemeProviderifuseThemecovers all use cases🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-theme.ts` around lines 30 - 35, The useTheme hook and ThemeProvider both manipulate document.documentElement and register matchMedia listeners, causing duplicate DOM changes and listeners; pick one source of truth (prefer making ThemeProvider the single source) and update useTheme to consume ThemeProvider state: remove the DOM classList manipulation and any matchMedia listener logic from useTheme and instead have useTheme read theme state and setter from the ThemeProvider context/store (or conversely remove ThemeProvider entirely if you want useTheme to remain independent); specifically modify the useTheme hook to drop its useEffect that references document.documentElement and the matchMedia registration, and ensure ThemeProvider exposes resolved theme and a setter (or store) that useTheme reads from so only ThemeProvider updates document.documentElement and manages matchMedia listeners.src/components/url/HistoryList.tsx (1)
20-24: Consider handling clipboard errors gracefully.
navigator.clipboard.writeTextcan reject (e.g., if permissions are denied or the document isn't focused). An unhandled rejection here would leave the user without feedback.🛡️ Suggested improvement
const handleCopy = async () => { - await navigator.clipboard.writeText(item.shortUrl); - setCopied(true); - setTimeout(() => setCopied(false), 2000); + try { + await navigator.clipboard.writeText(item.shortUrl); + setCopied(true); + setTimeout(() => setCopied(false), 2000); + } catch { + // Silently fail or show toast notification + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/url/HistoryList.tsx` around lines 20 - 24, The handleCopy function currently calls navigator.clipboard.writeText(item.shortUrl) without handling rejections; update handleCopy to catch errors from writeText (use try/catch or .catch) and handle failures by setting a failure state or showing user feedback (e.g., setCopied(false) and set a new error/toast state) while still setting setCopied(true) only on success; reference the handleCopy function, item.shortUrl, setCopied (and add a setCopyError or toast handler) so users get visible feedback when clipboard access is denied.src/hooks/use-shorten.ts (1)
11-52: Consider wrappingonSuccesslogic in try-catch.If
settingsStorage.getValue(),historyStorage.getValue(), orhistoryStorage.setValue()throws, the error will propagate as an unhandled promise rejection. Since history persistence is a side effect, you may want to catch and log errors silently rather than disrupting the success flow.🛡️ Suggested improvement
onSuccess: async (data) => { + try { // Save to persistent history const settings = await settingsStorage.getValue(); // ... existing logic ... await historyStorage.setValue(updated); + } catch (err) { + console.error("Failed to persist history:", err); + } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-shorten.ts` around lines 11 - 52, Wrap the entire onSuccess handler body in a try-catch so failures in settingsStorage.getValue(), historyStorage.getValue(), historyStorage.setValue(), or QR generation (gradientQrUrl/classicQrUrl) are swallowed and logged instead of bubbling as unhandled promise rejections; inside catch log the error (e.g., processLogger.error or console.error) with context like "persisting history failed" and do not rethrow so the success flow continues, while leaving the existing logic that builds updated (using HISTORY_MAX_ITEMS) unchanged.src/entrypoints/popup/App.tsx (1)
31-38: Handle edge case when no active tab exists.If
browser.tabs.queryreturns an empty array (e.g., no active tab in the current window),tabwill beundefinedand the side panel won't open silently. This is acceptable behavior, but you might want to provide user feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/popup/App.tsx` around lines 31 - 38, In openSidePanel, handle the case when browser.tabs.query returns an empty array by checking if tab is undefined before using tab.windowId; if undefined, provide user feedback (for example call alert(...) or browser.notifications.create(...) or update the UI) and return early, otherwise proceed to call (browser as any).sidePanel.open({ windowId: tab.windowId }) and window.close(); update the function openSidePanel to include this guard and feedback so the silent no-op is avoided.src/components/sidepanel/UrlsTab.tsx (1)
148-151: Validate localStorage value before type assertion.The saved sort value is cast directly without validation. If localStorage contains an unexpected value (e.g., from a previous version), this could cause subtle bugs.
🛡️ Suggested improvement
const [sortBy, setSortByState] = useState<"created_at" | "total_clicks" | "last_click">(() => { const saved = localStorage.getItem("spoo-urls-sort"); - return (saved as "created_at" | "total_clicks" | "last_click") || "created_at"; + const validOptions = ["created_at", "total_clicks", "last_click"] as const; + return validOptions.includes(saved as any) ? (saved as typeof sortBy) : "created_at"; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidepanel/UrlsTab.tsx` around lines 148 - 151, The initialization for sort state (useState in UrlsTab, variables sortBy and setSortByState) unsafely casts localStorage.getItem("spoo-urls-sort") to the union type; change it to validate the saved string against the allowed values ("created_at", "total_clicks", "last_click") (e.g., check with an array.includes or a small type-guard) and only cast/return it when valid, otherwise return the default "created_at"; update the initialization function to perform this validation before setting the initial state.src/components/sidepanel/AnalyticsTab.tsx (1)
93-97: URL normalization may be too narrow.The regex
^https?:\/\/spoo\.me\/only strips the spoo.me domain prefix. Users might paste URLs withwww.spoo.meor trailing paths/query params. Consider a more robust extraction:♻️ More robust short code extraction
const handleSearch = (e: React.FormEvent) => { e.preventDefault(); - const code = shortCode.trim().replace(/^https?:\/\/spoo\.me\//, ""); + const code = shortCode + .trim() + .replace(/^https?:\/\/(www\.)?spoo\.me\//, "") + .split(/[?#]/)[0]; // Remove query params and fragments if (code) setActiveCode(code); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidepanel/AnalyticsTab.tsx` around lines 93 - 97, The current handleSearch uses a narrow regex to strip only "https://spoo.me/" which misses variants like "http://", "www.spoo.me", and URLs with extra path/query segments; update handleSearch to robustly extract the short code from shortCode before calling setActiveCode: inside handleSearch, try to parse shortCode with the URL constructor and, on success, take the first non-empty path segment (or a specific query param if your app uses one); on failure (invalid URL) fall back to stripping an optional protocol and optional "www." via a regex like /^(?:https?:\/\/)?(?:www\.)?spoo\.me\/?/ and then split by / or ? and take the first segment; ensure you still trim and only call setActiveCode(code) when a non-empty code is obtained.src/schemas/api.ts (1)
66-102: Inconsistentcreated_attype between schemas.
urlResponseSchemadefinescreated_atasz.number()(line 71), whileurlListItemSchemadefines it asz.string().nullable()(line 94). If this reflects actual API behavior, consider adding a comment. If unintentional, align the types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/schemas/api.ts` around lines 66 - 102, The created_at field is inconsistent between urlResponseSchema (created_at: z.number()) and urlListItemSchema (created_at: z.string().nullable()); pick the correct API type and update the schema to match: either change urlResponseSchema.created_at to z.string().nullable() or change urlListItemSchema.created_at to z.number() (and adjust nullability accordingly), and add a short comment above the schemas if the API intentionally returns different types to document the difference; ensure to update any dependent code/tests that rely on created_at shape (references: urlResponseSchema, urlListItemSchema).src/api/stats.ts (1)
60-71: Consider adding runtime validation for v0 API response.The response is typed as
V0StatsResponsebut not validated at runtime. While this is a legacy API unlikely to change, adding validation would catch unexpected response shapes early.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/stats.ts` around lines 60 - 71, After fetching the legacy API response (the code that does const res = await fetch(url, { ... }) and currently does return res.json()), validate the parsed JSON against the V0StatsResponse shape at runtime before returning; parse the body (const data = await res.json()), run a validation (either a lightweight manual shape check or use a schema validator like zod/io-ts) that asserts required fields/types from V0StatsResponse, and if validation fails throw a descriptive Error (e.g., "Invalid V0StatsResponse: <details>"). Ensure you reference V0StatsResponse and validate the object returned from res.json() in the same function before returning the data.src/entrypoints/background/index.ts (1)
125-138: Queue items are lost on failure after coming back online.The queue is cleared before processing (line 129), so if
processUrlfails for any items, they're permanently lost. Consider re-queuing failed items or processing with a retry mechanism.Proposed improvement to preserve failed items
async function processOfflineQueue(): Promise<void> { const queue = await shortenQueueStorage.getValue(); if (queue.length === 0) return; await shortenQueueStorage.setValue([]); + const failed: typeof queue = []; for (const item of queue) { try { await processUrl(item.url); - } catch { - // Items lost if still failing + } catch { + failed.push(item); } } + + if (failed.length > 0) { + const current = await shortenQueueStorage.getValue(); + await shortenQueueStorage.setValue([...current, ...failed]); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/background/index.ts` around lines 125 - 138, processOfflineQueue currently clears shortenQueueStorage immediately which permanently drops items when processUrl fails; change the logic in processOfflineQueue to preserve failures by processing the read queue and only removing successfully-processed items from storage: read queue via shortenQueueStorage.getValue(), iterate over items calling processUrl, collect failed items into a retry array, and after the loop call shortenQueueStorage.setValue(failedItems) (or implement exponential retry/backoff for failures) so failed entries are re-queued instead of lost; reference functions/vars: processOfflineQueue, shortenQueueStorage.getValue/setValue, and processUrl.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fedba1a7-9475-4800-a0e1-f70d70604ce8
⛔ Files ignored due to path filters (9)
bun.lockis excluded by!**/*.lockpublic/icon/128.pngis excluded by!**/*.pngpublic/icon/16.pngis excluded by!**/*.pngpublic/icon/32.pngis excluded by!**/*.pngpublic/icon/48.pngis excluded by!**/*.pngpublic/icon/96.pngis excluded by!**/*.pngpublic/icon/favicon.pngis excluded by!**/*.pngpublic/icon/logo-black.pngis excluded by!**/*.pngpublic/icon/logo-white.pngis excluded by!**/*.png
📒 Files selected for processing (82)
.github/workflows/ci.yml.gitignorebackground.jsbiome.jsoncomponents.jsoncontent.jsmanifest.jsonpackage.jsonpopup/popup.csspopup/popup.htmlpopup/popup.jssrc/api/auth.tssrc/api/client.tssrc/api/export.tssrc/api/keys.tssrc/api/qr.tssrc/api/shorten.tssrc/api/stats.tssrc/api/types.tssrc/api/urls.tssrc/components/auth/AuthSection.tsxsrc/components/auth/UserMenu.tsxsrc/components/shared/OfflineBanner.tsxsrc/components/shared/ThemeProvider.tsxsrc/components/sidepanel/AccountTab.tsxsrc/components/sidepanel/AnalyticsTab.tsxsrc/components/sidepanel/DashboardTab.tsxsrc/components/sidepanel/SettingsTab.tsxsrc/components/sidepanel/UrlsTab.tsxsrc/components/ui/alert-dialog.tsxsrc/components/ui/avatar.tsxsrc/components/ui/badge.tsxsrc/components/ui/button.tsxsrc/components/ui/card.tsxsrc/components/ui/chart.tsxsrc/components/ui/dialog.tsxsrc/components/ui/dropdown-menu.tsxsrc/components/ui/input.tsxsrc/components/ui/kbd.tsxsrc/components/ui/label.tsxsrc/components/ui/select.tsxsrc/components/ui/separator.tsxsrc/components/ui/slider.tsxsrc/components/ui/sonner.tsxsrc/components/ui/switch.tsxsrc/components/ui/tabs.tsxsrc/components/ui/tooltip.tsxsrc/components/url/HistoryList.tsxsrc/components/url/ShortenForm.tsxsrc/entrypoints/background/index.tssrc/entrypoints/content/index.tssrc/entrypoints/popup/App.tsxsrc/entrypoints/popup/index.htmlsrc/entrypoints/popup/main.tsxsrc/entrypoints/sidepanel/App.tsxsrc/entrypoints/sidepanel/index.htmlsrc/entrypoints/sidepanel/main.tsxsrc/hooks/use-auth.tssrc/hooks/use-keys.tssrc/hooks/use-settings.tssrc/hooks/use-shorten.tssrc/hooks/use-stats.tssrc/hooks/use-theme.tssrc/hooks/use-urls.tssrc/lib/constants.tssrc/lib/errors.tssrc/lib/messaging.tssrc/lib/migration.tssrc/lib/notification.tssrc/lib/query-client.tssrc/lib/storage.tssrc/lib/url-utils.tssrc/lib/utils.tssrc/schemas/api.tssrc/schemas/settings.tssrc/schemas/storage.tssrc/stores/auth.tssrc/stores/settings.tssrc/stores/ui.tssrc/styles/app.csstsconfig.jsonwxt.config.ts
💤 Files with no reviewable changes (6)
- popup/popup.html
- manifest.json
- popup/popup.js
- popup/popup.css
- content.js
- background.js
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
src/stores/auth.ts (2)
54-72:⚠️ Potential issue | 🟠 MajorClear persisted state from the other auth mode.
Switching to JWT leaves any old API key behind, and switching to API-key mode leaves the old user profile behind. That stale data can be rehydrated later and desynchronize the UI from the real auth mode.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/auth.ts` around lines 54 - 72, When switching auth modes you must clear persisted data for the other mode to avoid stale rehydration: update setJwtAuth to also clear apiKeyStorage (call apiKeyStorage.setValue(null)) as part of the Promise.all, and update setApiKeyAuth to also clear userProfileStorage (call userProfileStorage.setValue(null)) alongside the existing accessTokenStorage/refreshTokenStorage clears; keep updating authModeStorage and the in-memory state via set({ mode: ..., user: ... }).
37-42:⚠️ Potential issue | 🟠 MajorMake initialization fail-safe so loading always resolves.
If either storage read rejects,
isLoadingstaystrueindefinitely. Wrap the read intry/catch/finallyand clear loading in thefinallypath.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/auth.ts` around lines 37 - 42, The initialize function currently awaits Promise.all([...]) so if authModeStorage.getValue() or userProfileStorage.getValue() rejects, isLoading remains true; wrap the Promise.all call in a try/catch/finally inside initialize (use the existing authModeStorage.getValue and userProfileStorage.getValue calls in the try), in catch optionally log the error and set sensible defaults for mode/user, and ensure set({ ..., isLoading: false }) is always called in the finally block so loading is cleared regardless of failures.src/components/auth/AuthSection.tsx (1)
94-126:⚠️ Potential issue | 🟠 MajorGuard duplicate submits and surface API-key auth failures.
There is no pending state here, so repeated clicks can race, and any rejection from
setApiKeyAuthescapes the submit handler with no user feedback. Add localisSubmittingstate, disable the button while pending, and wrap the await intry/catch/finally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/auth/AuthSection.tsx` around lines 94 - 126, The submit handler (handleSubmit) doesn't guard against duplicate submissions nor surface setApiKeyAuth failures; add a local isSubmitting state (e.g., const [isSubmitting, setIsSubmitting] = useState(false)), check and return early if isSubmitting at the start of handleSubmit, set setIsSubmitting(true) before calling await setApiKeyAuth(trimmed) and wrap that call in try/catch/finally so any thrown error sets setError(...) and prevents onSuccess from running on failure, and in finally call setIsSubmitting(false); also update the Button disabled prop to disabled={!key.trim() || isSubmitting} so the button is disabled while submission is pending.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 108: The license badge image lacks alt text; update the <img> element
inside the anchor (the license badge) to include a descriptive alt attribute
(e.g., alt="Apache 2.0 license badge") so the badge is accessible and passes
MD045; locate the <img> tag in the README license badge link and add the alt
attribute value.
In `@src/components/auth/AuthSection.tsx`:
- Around line 66-70: In handleWebLogin, await browser.tabs.create instead of
fire-and-forget and wrap it in try/catch so you only call onSuccess() after the
tab is successfully opened; if browser.tabs.create throws or returns an
unexpected result, keep the dialog open and remove the saved CSRF state by
calling deviceAuthStateStorage.clearValue()/setValue(null) (or equivalent) to
avoid leaving stale state; use the existing symbols handleWebLogin,
deviceAuthStateStorage.setValue, browser.tabs.create, and onSuccess to implement
this flow and ensure state is cleared when the user aborts or the tab creation
fails.
In `@src/entrypoints/content/index.ts`:
- Around line 21-24: Currently you clear state before awaiting the background
response; change the flow so you await browser.runtime.sendMessage({ type:
"device-auth-code", code }) and inspect the returned response (check for
response.success === true and absence of response.error) before calling
deviceAuthStateStorage.setValue(null); if the response contains an error do not
clear the state and propagate or handle the error instead so the user can retry;
update the logic in the content script around the browser.runtime.sendMessage
call and the deviceAuthStateStorage.setValue invocation accordingly.
- Around line 3-5: The matches array passed to defineContentScript currently
includes "http://127.0.0.1/*" and "http://localhost/*" unconditionally; move
those localhost entries behind a dev-only guard so they are excluded from
production builds (use import.meta.env.DEV) — update the matches value in the
export default defineContentScript block to conditionally include the localhost
patterns (or alternatively build the matches list inside the main function and
only push localhost entries when import.meta.env.DEV is true) while leaving
runAt and the rest of the script unchanged.
In `@src/lib/messaging.ts`:
- Around line 9-27: The union type ExtensionMessage advertises an
AuthChangedMessage variant (type "auth-changed") but the background message
handler does not handle that case; either remove AuthChangedMessage from the
ExtensionMessage union or add a handling branch in the background message
listener to process messages with type "auth-changed" (e.g., detect
AuthChangedMessage, update auth state or forward the event, and return the
appropriate response/void). Locate the AuthChangedMessage / ExtensionMessage
definitions and either delete the AuthChangedMessage interface from the union or
implement the "auth-changed" case in the background listener’s message dispatch
so the contract matches runtime behavior.
In `@src/stores/auth.ts`:
- Around line 37-42: The initialize function currently reads authModeStorage and
userProfileStorage and may set mode: "jwt" while user is null; update initialize
to also read session-scoped accessTokenStorage (from src/lib/storage.ts)
together with authModeStorage and userProfileStorage, then if the persisted mode
is "jwt" but either user is null or accessToken is missing/empty, treat it as
anonymous (set mode = "anonymous") or trigger the session rehydrate path before
calling set; ensure you reference initialize, authModeStorage,
userProfileStorage, accessTokenStorage and the set call when making this change.
In `@wxt.config.ts`:
- Around line 33-37: The web_accessible_resources entry is overly broad and
exposes icon/* to <all_urls>; remove the web_accessible_resources block entirely
or restrict it to only the specific paths and origins that truly need runtime
access (e.g., list exact icon files instead of "icon/*" or limit matches to
extension pages), and verify any runtime access uses browser.runtime.getURL()
(no WER needed) — update the web_accessible_resources, resources, and matches
fields accordingly to eliminate the unnecessary fingerprinting surface.
---
Duplicate comments:
In `@src/components/auth/AuthSection.tsx`:
- Around line 94-126: The submit handler (handleSubmit) doesn't guard against
duplicate submissions nor surface setApiKeyAuth failures; add a local
isSubmitting state (e.g., const [isSubmitting, setIsSubmitting] =
useState(false)), check and return early if isSubmitting at the start of
handleSubmit, set setIsSubmitting(true) before calling await
setApiKeyAuth(trimmed) and wrap that call in try/catch/finally so any thrown
error sets setError(...) and prevents onSuccess from running on failure, and in
finally call setIsSubmitting(false); also update the Button disabled prop to
disabled={!key.trim() || isSubmitting} so the button is disabled while
submission is pending.
In `@src/stores/auth.ts`:
- Around line 54-72: When switching auth modes you must clear persisted data for
the other mode to avoid stale rehydration: update setJwtAuth to also clear
apiKeyStorage (call apiKeyStorage.setValue(null)) as part of the Promise.all,
and update setApiKeyAuth to also clear userProfileStorage (call
userProfileStorage.setValue(null)) alongside the existing
accessTokenStorage/refreshTokenStorage clears; keep updating authModeStorage and
the in-memory state via set({ mode: ..., user: ... }).
- Around line 37-42: The initialize function currently awaits Promise.all([...])
so if authModeStorage.getValue() or userProfileStorage.getValue() rejects,
isLoading remains true; wrap the Promise.all call in a try/catch/finally inside
initialize (use the existing authModeStorage.getValue and
userProfileStorage.getValue calls in the try), in catch optionally log the error
and set sensible defaults for mode/user, and ensure set({ ..., isLoading: false
}) is always called in the finally block so loading is cleared regardless of
failures.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec98b487-64bb-4c95-95de-cda2b7c466f7
📒 Files selected for processing (15)
.env.example.gitignoreREADME.mdSECURITY.mdsrc/components/auth/AuthSection.tsxsrc/entrypoints/background/index.tssrc/entrypoints/content/index.tssrc/env.d.tssrc/hooks/use-auth.tssrc/lib/constants.tssrc/lib/messaging.tssrc/lib/storage.tssrc/schemas/api.tssrc/stores/auth.tswxt.config.ts
✅ Files skipped from review due to trivial changes (5)
- .env.example
- SECURITY.md
- .gitignore
- src/lib/constants.ts
- src/schemas/api.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/use-auth.ts
- src/entrypoints/background/index.ts
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/auth.ts`:
- Around line 99-106: The refresh network call to AUTH_ENDPOINTS.refresh (inside
the try block performing fetch) lacks a timeout/abort; wrap the fetch in an
AbortController, pass controller.signal to fetch, start a setTimeout (e.g. 5s or
configurable) that calls controller.abort(), and clear that timer when the fetch
completes or errors to avoid leaks; update the error handling around the await
fetch to treat an AbortError as a timeout/failure for JWT restoration.
In `@src/components/auth/AuthSection.tsx`:
- Around line 66-85: The handleWebLogin flow can be run multiple times from
rapid clicks and overwrite deviceAuthStateStorage; add a local "submitting"
guard (useRef or useState) checked at the start of handleWebLogin to return
early if already submitting, set it true before generating state and writing
deviceAuthStateStorage, and ensure it's reset to false on error (catch) or
finally if onSuccess doesn't immediately unmount; this prevents duplicate writes
to deviceAuthStateStorage and opening multiple tabs. Use the existing
handleWebLogin function and the Button's onClick to reference the guard.
In `@src/components/sidepanel/AnalyticsTab.tsx`:
- Around line 356-363: The chart data is being sliced before sorting which can
drop the largest buckets; change the pipeline in AnalyticsTab.tsx (the chartData
computation) to sort the raw data by the numeric count (using countKey)
descending first, then take the top N (slice(0,8)), then map to {name:
trunc(...), value: Number(...)}, and finally filter out zero values; apply the
same change to the other identical block referenced around lines 453-466 so both
"Top URLs" / capped pie charts are sorted before truncation and truncation
applies only to display names.
- Around line 101-110: The input and icon-only submit button lack accessible
names; update the Input and Button used in the form (the controlled value
shortCode and onSubmit handler handleSearch) to provide explicit labels—add
either a visible <label> tied to the Input's id or an aria-label/aria-labelledby
on the Input (e.g. "Short code or URL") and add an aria-label on the Button
(e.g. "Lookup analytics" or "Search by short code") so screen readers can
identify both controls while leaving the existing shortCode state, onChange, and
disabled logic intact.
- Around line 93-96: handleSearch currently strips the origin prefix with a
regex but leaves query strings and trailing slashes (so
"https://spoo.me/abc123?utm=x" becomes "abc123?utm=x"); update handleSearch to
robustly extract only the pathname segment: if shortCode looks like a URL
(starts with http/https) parse it with URL (or fallback) and take url.pathname,
then trim leading/trailing slashes; otherwise treat shortCode as the raw code
and trim slashes/spaces; finally call setActiveCode with that cleaned code.
Target the handleSearch function and its use of shortCode/setActiveCode to
implement this pathname extraction and cleanup.
- Around line 555-560: The fmtDate function mis-parses date-only strings because
new Date("YYYY-MM-DD") becomes UTC midnight and then gets shifted by
toLocaleDateString; detect date-only inputs (e.g. /^\d{4}-\d{2}-\d{2}$/) in
fmtDate and parse them using UTC construction (extract year, month, day and call
Date.UTC or equivalent) so the resulting Date represents that calendar day in
UTC before calling toLocaleDateString; keep the existing fallback for other
formats and the current NaN check.
In `@src/components/sidepanel/SettingsTab.tsx`:
- Around line 115-133: ToggleRow's onChange is incorrectly typed as () => void
but Switch's onCheckedChange supplies a boolean; update ToggleRow's prop type to
onChange: (checked: boolean) => void and pass the checked value through (i.e.,
Switch onCheckedChange={onChange}) so callers receive the boolean directly; then
update all ToggleRow call sites (where state toggles were inverting booleans) to
accept the checked parameter and set state accordingly (e.g., setX(checked)).
In `@src/stores/auth.ts`:
- Around line 95-124: The auth mode is being written in parallel with
tokens/profile causing watchers to observe mode changes before dependent storage
writes finish; update setJwtAuth, setApiKeyAuth, and clearAuth to write/remove
accessTokenStorage, refreshTokenStorage, apiKeyStorage, and userProfileStorage
first (await their Promise.all), then call
authModeStorage.setValue("jwt"/"apikey"/"anonymous") and only after that call
set({ mode: ..., user: ... }); ensure authModeStorage.setValue is not included
in the first Promise.all so the mode publish happens strictly after other
storage updates complete.
- Around line 56-93: The initialize function registers a watcher via
authModeStorage.watch but never captures or returns the unsubscribe cleanup,
causing listener leaks on repeated initialize() calls; modify initialize (in
src/stores/auth.ts) to store the return value of authModeStorage.watch(...) (the
unsubscribe function) and return that function from initialize so callers can
call it on unmount, ensuring you still call userProfileStorage.getValue() and
use useAuthStore.getState() inside the watcher as before.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14b4225e-764a-48af-a645-5d44a26f6457
📒 Files selected for processing (12)
src/api/auth.tssrc/components/auth/AuthSection.tsxsrc/components/shared/ThemeProvider.tsxsrc/components/sidepanel/AnalyticsTab.tsxsrc/components/sidepanel/SettingsTab.tsxsrc/components/url/ShortenForm.tsxsrc/entrypoints/background/index.tssrc/entrypoints/content/index.tssrc/lib/messaging.tssrc/lib/migration.tssrc/stores/auth.tswxt.config.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- wxt.config.ts
- src/lib/messaging.ts
- src/components/url/ShortenForm.tsx
- src/entrypoints/background/index.ts
…improve UI components across the application
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/lib/notification.ts (1)
85-324:⚠️ Potential issue | 🟠 MajorStop interpolating
shortUrlandqrUrlinto the HTML template.Line 318 and Line 324 still splice untrusted URLs straight into
shadow.innerHTML. That leaves the toast open to markup injection, and DOM-building alone is not enough here because a non-http(s)scheme would still be dangerous once assigned tohref/srcor passed towindow.open. Build these nodes with DOM APIs (textContent,setAttribute) and whitelist allowed protocols before wiring them up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/notification.ts` around lines 85 - 324, The template currently interpolates shortUrl and qrUrl directly into shadow.innerHTML (the anchor with class "url" and img.qr), which allows injection; instead stop string-splicing these values and construct the toast DOM via DOM APIs: createElement for the anchor (.url) and img (.qr), set anchor.textContent (not innerHTML), and use setAttribute('href', ...) / setAttribute('src', ...) only after validating the URL. Validate by constructing a URL object (new URL(value, document.baseURI)) and whitelist protocol === 'http:' || protocol === 'https:'; if validation fails, omit href/src or render a safe fallback text. Apply the same pattern to qrUrl (set crossorigin via setAttribute only after validation) and keep the rest of the markup/static styles in shadow.innerHTML, wiring in the created nodes into the .url-row and .qr-container instead of interpolating shortUrl/qrUrl.
🧹 Nitpick comments (2)
src/entrypoints/background/index.ts (2)
139-163: Consider adding a timeout to the device token exchange fetch.The
fetchcall at line 140 has no timeout. While this is a user-initiated flow where the user can close the tab to abort, a hung request could leave the auth callback in an indeterminate state. Adding a timeout would improve reliability.♻️ Add timeout to device token exchange
async function exchangeDeviceCode(code: string): Promise<void> { const res = await fetch(AUTH_ENDPOINTS.deviceToken, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ code }), + signal: AbortSignal.timeout(15_000), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/background/index.ts` around lines 139 - 163, The fetch in exchangeDeviceCode has no timeout; update exchangeDeviceCode to use an AbortController and a setTimeout that calls controller.abort() after a reasonable timeout (e.g., 10s), pass controller.signal into fetch, clear the timeout when fetch completes or errored, and convert an AbortError into a clear error (e.g., "Token exchange timed out") before throwing; reference the function exchangeDeviceCode and the fetch call to locate where to add the AbortController, timeout handle, and signal usage.
112-125: Offline queue items lost on retry failure.When processing the offline queue, the queue is cleared immediately (line 116) before iterating. If
processUrlfails for any item, it's silently lost. Consider re-queuing failed items or using a more resilient pattern.♻️ Optional: Re-queue failed items
async function processOfflineQueue(): Promise<void> { const queue = await shortenQueueStorage.getValue(); if (queue.length === 0) return; await shortenQueueStorage.setValue([]); + const failed: typeof queue = []; for (const item of queue) { try { await processUrl(item.url); } catch { - // Items lost if still failing + failed.push(item); } } + + if (failed.length > 0) { + const current = await shortenQueueStorage.getValue(); + await shortenQueueStorage.setValue([...current, ...failed]); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/entrypoints/background/index.ts` around lines 112 - 125, processOfflineQueue currently clears shortenQueueStorage immediately then processes items so any processUrl failures are silently lost; change it to retrieve queue via shortenQueueStorage.getValue(), iterate while collecting items that fail (catch errors from processUrl in the loop inside processOfflineQueue), and after the loop write the remaining failed items back with shortenQueueStorage.setValue(failedItems) (or only clear the storage after every item is successfully processed), and add logging in the catch to surface failures; use the existing symbols processOfflineQueue, shortenQueueStorage.getValue, shortenQueueStorage.setValue, and processUrl to locate and implement this re-queue-on-failure behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/sidepanel/AnalyticsTab.tsx`:
- Line 508: The tooltip is using a hard-coded nameKey ("value") so
ChartTooltipContent resolves every slice label to config.value; update the
ChartTooltip usage to pass the datum/category key (e.g., the slice key like
"browser" / "os" / "referrer") or remove the nameKey prop so ChartTooltipContent
derives the label from the hovered datum. Locate ChartTooltip and
ChartTooltipContent in AnalyticsTab.tsx (the line with ChartTooltip
content={<ChartTooltipContent nameKey="value" .../>}) and replace the
nameKey="value" with the actual slice key used by the pie data (or omit nameKey
entirely) so each tooltip shows the correct category name.
In `@src/components/ui/chart.tsx`:
- Around line 218-222: The tooltip row currently uses a truthy check on
item.value (the JSX fragment with item.value.toLocaleString()) which hides valid
zero values; change the conditional to explicitly check for null/undefined
(e.g., item.value !== null && item.value !== undefined or item.value != null) so
0 renders while still excluding null/undefined values in the component that
renders the tooltip row containing item.value and its toLocaleString() call.
In `@src/lib/notification.ts`:
- Around line 367-378: The QR image handlers may miss the load/error events if
the image is already cached; after attaching the existing event listeners on
qrImg (the element queried as ".qr"), immediately check qrImg.complete and use
qrImg.naturalWidth to determine success: if complete and naturalWidth > 0,
remove qrSkeleton and show qrImg (set display), otherwise if complete and
naturalWidth === 0 remove qrSkeleton (or handle as error). Update the logic
around qrImg, qrSkeleton and qrContainer in the notification module so the
skeleton is removed correctly for cached/fast responses.
---
Duplicate comments:
In `@src/lib/notification.ts`:
- Around line 85-324: The template currently interpolates shortUrl and qrUrl
directly into shadow.innerHTML (the anchor with class "url" and img.qr), which
allows injection; instead stop string-splicing these values and construct the
toast DOM via DOM APIs: createElement for the anchor (.url) and img (.qr), set
anchor.textContent (not innerHTML), and use setAttribute('href', ...) /
setAttribute('src', ...) only after validating the URL. Validate by constructing
a URL object (new URL(value, document.baseURI)) and whitelist protocol ===
'http:' || protocol === 'https:'; if validation fails, omit href/src or render a
safe fallback text. Apply the same pattern to qrUrl (set crossorigin via
setAttribute only after validation) and keep the rest of the markup/static
styles in shadow.innerHTML, wiring in the created nodes into the .url-row and
.qr-container instead of interpolating shortUrl/qrUrl.
---
Nitpick comments:
In `@src/entrypoints/background/index.ts`:
- Around line 139-163: The fetch in exchangeDeviceCode has no timeout; update
exchangeDeviceCode to use an AbortController and a setTimeout that calls
controller.abort() after a reasonable timeout (e.g., 10s), pass
controller.signal into fetch, clear the timeout when fetch completes or errored,
and convert an AbortError into a clear error (e.g., "Token exchange timed out")
before throwing; reference the function exchangeDeviceCode and the fetch call to
locate where to add the AbortController, timeout handle, and signal usage.
- Around line 112-125: processOfflineQueue currently clears shortenQueueStorage
immediately then processes items so any processUrl failures are silently lost;
change it to retrieve queue via shortenQueueStorage.getValue(), iterate while
collecting items that fail (catch errors from processUrl in the loop inside
processOfflineQueue), and after the loop write the remaining failed items back
with shortenQueueStorage.setValue(failedItems) (or only clear the storage after
every item is successfully processed), and add logging in the catch to surface
failures; use the existing symbols processOfflineQueue,
shortenQueueStorage.getValue, shortenQueueStorage.setValue, and processUrl to
locate and implement this re-queue-on-failure behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f274b85b-d3f3-44fa-a3be-e84d8f856e6d
⛔ Files ignored due to path filters (4)
public/icon/128.pngis excluded by!**/*.pngpublic/icon/favicon.pngis excluded by!**/*.pngpublic/icon/logo-black.pngis excluded by!**/*.pngpublic/icon/logo-white.pngis excluded by!**/*.png
📒 Files selected for processing (19)
.github/workflows/ci.ymlsrc/api/auth.tssrc/components/auth/AuthSection.tsxsrc/components/shared/ThemeProvider.tsxsrc/components/sidepanel/AnalyticsTab.tsxsrc/components/sidepanel/SettingsTab.tsxsrc/components/sidepanel/UrlsTab.tsxsrc/components/ui/badge.tsxsrc/components/ui/chart.tsxsrc/components/url/HistoryList.tsxsrc/entrypoints/background/index.tssrc/entrypoints/popup/App.tsxsrc/entrypoints/sidepanel/App.tsxsrc/hooks/use-shorten.tssrc/lib/constants.tssrc/lib/format-date.tssrc/lib/messaging.tssrc/lib/notification.tssrc/stores/auth.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/messaging.ts
- src/components/url/HistoryList.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/shared/ThemeProvider.tsx
- .github/workflows/ci.yml
- src/entrypoints/popup/App.tsx
- src/hooks/use-shorten.ts
- src/components/ui/badge.tsx
- src/components/sidepanel/UrlsTab.tsx
…ast notification for cached responses
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/sidepanel/AnalyticsTab.tsx`:
- Around line 454-460: chartColors defines 5 colors but the component renders up
to 6 slices so the last slice reuses the first color; either add a sixth color
to chartColors or change the slice cap logic to use chartColors.length instead
of a hardcoded 6 — update the chartColors array (add "var(--chart-6)") or
replace the hardcoded cap (the code that limits buckets/slices) with
chartColors.length so the palette and cap stay in sync (also update the same
pattern referenced around the other slice rendering block).
- Around line 365-372: chartData currently overwrites the raw category with a
truncated string causing duplicate React keys and merged categories; change the
chart data shape produced by chartData (using nameKey and countKey) to preserve
the original value (e.g., include originalName or id: String(d[nameKey] ??
"Unknown")) and add a separate displayLabel or truncatedLabel using trunc(...,
16) for rendering; ensure any React key or Cell key uses the preserved original
(originalName/id) and only use the truncated label in the tick renderer and
displayed text (also apply the same change to the other chartData usages
referenced around the lines noted).
- Around line 30-35: The component AnalyticsTab currently treats mode ===
"anonymous" as unauthenticated during auth hydration; update it to use
useAuthStore().isLoading so we don't render the anonymous surface while loading:
read both mode and isLoading from useAuthStore, and change the logic so that
while isLoading you return a neutral placeholder (e.g., null or a loading
skeleton) and only compute isAuthenticated as mode !== "anonymous" once
isLoading is false before returning UrlAnalytics; reference AnalyticsTab,
useAuthStore, mode, isLoading, and UrlAnalytics in your change.
In `@src/components/ui/chart.tsx`:
- Around line 35-57: ChartContainer is consuming the caller's id for an internal
chart-scoping token (chartId) and never forwarding that id to the DOM; change
the API to introduce a dedicated prop (e.g., chartScope or internalChartId) for
the internal data-chart generation while forwarding the original id prop to the
wrapper div so caller-supplied ids still appear on the DOM; update the internal
generation to use the new chartScope prop (or fallback to React.useId()) to
build chartId and keep ChartContext.Provider usage and the div's data-chart
attribute unchanged, and apply the same fix to the other occurrence of id usage
in this file (the similar block at the later ChartContainer/ResponsiveContainer
usage).
In `@src/lib/notification.ts`:
- Around line 13-19: Validate and normalize shortUrl and qrUrl inside
showToastNotification before calling executeScript: parse each with new URL(...)
and ensure protocol is "http:" or "https:" (treat qrUrl === null as allowed),
and if validation fails replace the value with a safe fallback (empty string or
null) so you never pass a non-http/https scheme into href, img.src or
window.open; update the code paths that call executeScript (the same validation
applies to the second usage around line 37) to use the sanitized values only.
- Around line 247-260: The QR overlay is currently a non-focusable div with only
a click handler, so make it keyboard/AT accessible by replacing the .qr-overlay
div (and other similar divs at the other occurrences) with a semantic <button>
(or add role="button" tabindex="0" if you must keep a div), give it an
aria-label, and wire its keydown handler to invoke the same download/open action
on Enter/Space as the existing click handler; also add a visible :focus-visible
CSS rule for .qr-overlay (or .qr-overlay:focus-visible) to show an outline.
Ensure pointer-events remain correct and that the existing click handler on the
.qr-container/.qr-overlay is reused (or delegated) so behavior is identical for
mouse, keyboard, and assistive tech.
- Around line 45-50: The notification creation is currently awaited and can
reject, which will surface errors to the caller even though the main work
already succeeded; modify the code around the browser.notifications.create(...)
call in src/lib/notification.ts to treat it as best-effort by wrapping it in a
try/catch, log the caught error locally (e.g., console.error or the module
logger) and do not rethrow so failures are swallowed; optionally return/ignore
the result of create, but ensure the promise rejection is handled locally to
avoid bubbling to the caller.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a0463da-f3ef-450a-885a-66eb66de4a4e
⛔ Files ignored due to path filters (5)
public/icon/128.pngis excluded by!**/*.pngpublic/icon/16.pngis excluded by!**/*.pngpublic/icon/32.pngis excluded by!**/*.pngpublic/icon/48.pngis excluded by!**/*.pngpublic/icon/96.pngis excluded by!**/*.png
📒 Files selected for processing (4)
src/components/sidepanel/AnalyticsTab.tsxsrc/components/ui/chart.tsxsrc/lib/notification.tstsconfig.json
✅ Files skipped from review due to trivial changes (1)
- tsconfig.json
…in authentication flow
This was referenced Aug 14, 2026
10 tasks
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
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 by CodeRabbit
New Features
Settings