feat(desktop): add project icon support with custom protocol#1377
feat(desktop): add project icon support with custom protocol#1377saddlepaddle wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe changes implement a complete project icon management system that enables users to upload, discover, and display custom icons for projects. This includes database schema extension, backend TRPC procedures for icon operations, file storage utilities using a custom protocol handler, favicon auto-discovery functionality, and UI components for uploading and displaying project icons. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as ProjectSettings
participant TRPC as setProjectIcon<br/>(TRPC)
participant Icons as project-icons<br/>utilities
participant FS as File System
participant DB as Database
User->>UI: Select & upload image file
UI->>UI: Read as data URL
UI->>TRPC: mutate({ id, icon: dataUrl })
TRPC->>Icons: saveProjectIconFromDataUrl()
Icons->>Icons: Decode base64, validate size
Icons->>FS: Write icon file
FS-->>Icons: File saved
Icons->>DB: Update project.iconUrl
DB-->>Icons: Updated
Icons-->>TRPC: Return protocol URL
TRPC-->>UI: Success, invalidate cache
UI->>User: Show updated icon
sequenceDiagram
participant User as User
participant Sidebar as ProjectThumbnail
participant Protocol as superset-icon<br/>handler
participant FS as File System
participant Display as Browser render
User->>Sidebar: View project list
Sidebar->>Sidebar: Receive iconUrl prop
Sidebar->>Protocol: Fetch superset-icon://projects/{id}
Protocol->>FS: getProjectIconPath(projectId)
FS-->>Protocol: File path
Protocol->>FS: Read icon file
FS-->>Protocol: Icon bytes
Protocol-->>Sidebar: Icon data
Sidebar->>Display: Render <img>
Display-->>User: Show project icon
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts`:
- Around line 48-58: The current selection of iconPath relies on fast-glob
results but fg does not guarantee pattern-priority ordering, so change the logic
after the fg(...) call to pick the highest-priority match based on
FAVICON_PATTERNS: build a lookup of matches, then iterate FAVICON_PATTERNS in
order and for each pattern check which matched paths correspond to it (e.g.,
using minimatch or by re-testing the pattern against each match) and choose the
first hit as iconPath; update the code referencing matches[0] to use this
prioritized selection while preserving ignore/cwd/absolute settings from the
original fg call.
In `@apps/desktop/src/main/lib/project-icons.ts`:
- Around line 91-95: The data URL regex in project-icons.ts (the
dataUrl.match(...) call) incorrectly uses \w+ so it rejects MIME types like
image/svg+xml; update that regex to accept characters used in MIME subtype names
(e.g., plus, dot, hyphen and alphanumerics) or use a more permissive token for
the subtype (for example match [a-zA-Z0-9.+-]+ or [^;]+) so image/svg+xml is
accepted and SVG uploads no longer throw "Invalid data URL format".
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx`:
- Around line 46-47: The iconError state isn't reset when iconUrl changes so a
previously failed icon blocks rendering new icons; add a useEffect that watches
the iconUrl prop and calls setIconError(false) whenever iconUrl changes (also
ensure useEffect is imported from react) so the <img> in ProjectThumbnail can
attempt to load the new icon; reference the iconError state setter setIconError
and the iconUrl prop in your effect.
🧹 Nitpick comments (5)
apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts (1)
73-78: RedundantBuffer.from()—readFilealready returns aBuffer.
readFile(iconPath)returns aBufferin Node.js. Wrapping it inBuffer.from(buffer)creates an unnecessary copy.Proposed fix
if (ext === "ico") { const buffer = await readFile(iconPath); return await saveProjectIconFromBuffer({ projectId, - buffer: Buffer.from(buffer), + buffer, ext: "ico", }); }apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
1100-1145: Invalid data URL input surfaces asINTERNAL_SERVER_ERRORinstead ofBAD_REQUEST.If
input.iconis a non-null string that isn't a valid data URL,saveProjectIconFromDataUrlthrows"Invalid data URL format". Without a try-catch, this propagates as an opaqueINTERNAL_SERVER_ERRORto the client. Wrapping the save call would give a more descriptive error:Proposed fix
// Save icon from data URL - const iconUrl = await saveProjectIconFromDataUrl({ - projectId: input.id, - dataUrl: input.icon, - }); + let iconUrl: string; + try { + iconUrl = await saveProjectIconFromDataUrl({ + projectId: input.id, + dataUrl: input.icon, + }); + } catch (error) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: error instanceof Error ? error.message : "Failed to save icon", + }); + }apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
104-120: No client-side file size validation before uploading.The file is read entirely into memory as a data URL before the mutation sends it to the main process, which then rejects files over 512KB. Adding a quick size check in
handleFileChangewould avoid unnecessary work and give the user immediate feedback.Proposed fix
const handleFileChange = useCallback( (e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0]; if (!file) return; + const MAX_ICON_SIZE_KB = 512; + if (file.size > MAX_ICON_SIZE_KB * 1024) { + console.warn(`[project-settings] Icon too large: ${Math.round(file.size / 1024)}KB`); + e.target.value = ""; + return; + } + const reader = new FileReader(); reader.onload = () => { const dataUrl = reader.result as string; setProjectIcon.mutate({ id: projectId, icon: dataUrl }); }; reader.readAsDataURL(file); // Reset input so the same file can be re-selected e.target.value = ""; }, [projectId, setProjectIcon], );Consider also showing a toast to the user when the file is too large.
apps/desktop/src/main/lib/project-icons.ts (2)
59-74:saveProjectIconFromFileskips the file size check that other save methods enforce.Both
saveProjectIconFromDataUrlandsaveProjectIconFromBuffervalidate againstMAX_ICON_SIZE, butsaveProjectIconFromFilecopies the file without any size check. While the current caller (favicon-discovery.ts) has its own upstream size check (256KB), this public function could be called from new code without that safeguard.Proposed fix
export async function saveProjectIconFromFile({ projectId, sourcePath, }: { projectId: string; sourcePath: string; }): Promise<string> { ensureProjectIconsDir(); removeExistingIcon(projectId); + const { size } = await import("node:fs/promises").then(m => m.stat(sourcePath)); + if (size > MAX_ICON_SIZE) { + throw new Error( + `Icon file too large (${Math.round(size / 1024)}KB). Maximum is ${MAX_ICON_SIZE / 1024}KB.`, + ); + } + const ext = extname(sourcePath) || ".png"; const destPath = join(PROJECT_ICONS_DIR, `${projectId}${ext}`); await copyFile(sourcePath, destPath); return getProjectIconProtocolUrl(projectId); }Or more simply, since
statis already imported at the top of the file (fromnode:fs/promises):+import { copyFile, stat, writeFile } from "node:fs/promises"; ... export async function saveProjectIconFromFile({ projectId, sourcePath, }: { projectId: string; sourcePath: string; }): Promise<string> { ensureProjectIconsDir(); + + const fileStat = await stat(sourcePath); + if (fileStat.size > MAX_ICON_SIZE) { + throw new Error( + `Icon file too large (${Math.round(fileStat.size / 1024)}KB). Maximum is ${MAX_ICON_SIZE / 1024}KB.`, + ); + } + removeExistingIcon(projectId); const ext = extname(sourcePath) || ".png";
25-35: Edge case: files without extensions cause incorrect matching.On line 30,
f.substring(0, f.lastIndexOf("."))— if a file somehow has no extension,lastIndexOf(".")returns-1, andsubstring(0, -1)returns all characters except the last. For example, a file namedabc123would produceabc12as the name. This is unlikely since all save functions add extensions, but a defensive check would be safer.Proposed fix
const match = files.find((f) => { - const name = f.substring(0, f.lastIndexOf(".")); + const dotIdx = f.lastIndexOf("."); + const name = dotIdx !== -1 ? f.substring(0, dotIdx) : f; return name === projectId; });
| const matches = await fg(FAVICON_PATTERNS, { | ||
| cwd: repoPath, | ||
| absolute: true, | ||
| ignore: ["**/node_modules/**", "**/.git/**", "**/dist/**", "**/build/**"], | ||
| onlyFiles: true, | ||
| }); | ||
|
|
||
| if (matches.length === 0) return null; | ||
|
|
||
| // Use the first match (ordered by FAVICON_PATTERNS priority) | ||
| const iconPath = matches[0]; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
fast-glob pattern order results documentation
💡 Result:
In fast-glob (Node.js):
- Result ordering is not guaranteed: matched paths are returned in arbitrary order, so you must sort the returned array yourself if you need deterministic ordering. [1][2]
- Pattern list order is not respected: when you pass multiple patterns, all negative patterns (
!…) are applied first, and only then the positive patterns—regardless of the order you wrote them in. If you need “pattern A results first, then pattern B”, you must split into multiple calls and combine/sort manually. [3]
Sources:
[1] fast-glob README (GitHub) — “results are returned in arbitrary order”
[2] fast-glob package page (npm) — same statement
[3] fast-glob README (GitHub) — “does not respect the order of patterns… negative… then positive… use sorting or split calls”
fast-glob does not preserve input pattern order in results.
The comment on line 57 states results are "ordered by FAVICON_PATTERNS priority," but fast-glob returns matches in arbitrary filesystem traversal order. This means icons may be selected in unintended priority (e.g., public/logo.svg before favicon.ico), contradicting the intention of the FAVICON_PATTERNS array.
Sort matches by pattern priority after globbing:
Proposed fix
const matches = await fg(FAVICON_PATTERNS, {
cwd: repoPath,
absolute: true,
ignore: ["**/node_modules/**", "**/.git/**", "**/dist/**", "**/build/**"],
onlyFiles: true,
});
if (matches.length === 0) return null;
- // Use the first match (ordered by FAVICON_PATTERNS priority)
- const iconPath = matches[0];
+ // Sort matches by FAVICON_PATTERNS priority (fast-glob doesn't preserve pattern order)
+ const relativize = (abs: string) => abs.replace(`${repoPath}/`, "").replace(`${repoPath}\\`, "");
+ matches.sort((a, b) => {
+ const aIdx = FAVICON_PATTERNS.indexOf(relativize(a));
+ const bIdx = FAVICON_PATTERNS.indexOf(relativize(b));
+ return (aIdx === -1 ? Infinity : aIdx) - (bIdx === -1 ? Infinity : bIdx);
+ });
+ const iconPath = matches[0];🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts` around
lines 48 - 58, The current selection of iconPath relies on fast-glob results but
fg does not guarantee pattern-priority ordering, so change the logic after the
fg(...) call to pick the highest-priority match based on FAVICON_PATTERNS: build
a lookup of matches, then iterate FAVICON_PATTERNS in order and for each pattern
check which matched paths correspond to it (e.g., using minimatch or by
re-testing the pattern against each match) and choose the first hit as iconPath;
update the code referencing matches[0] to use this prioritized selection while
preserving ignore/cwd/absolute settings from the original fg call.
| // Parse data URL: data:image/png;base64,<data> | ||
| const match = dataUrl.match(/^data:image\/(\w+);base64,(.+)$/); | ||
| if (!match) { | ||
| throw new Error("Invalid data URL format"); | ||
| } |
There was a problem hiding this comment.
Data URL regex fails for image/svg+xml — SVG uploads will always be rejected.
The regex ^data:image\/(\w+);base64,(.+)$ uses \w+ which matches [a-zA-Z0-9_] only. SVG data URLs have MIME type image/svg+xml, and the + character is not matched by \w. This means any SVG file uploaded via the settings UI will throw "Invalid data URL format".
The file input accepts image/svg+xml, so users can select SVGs but they'll silently fail.
Proposed fix
- const match = dataUrl.match(/^data:image\/(\w+);base64,(.+)$/);
+ const match = dataUrl.match(/^data:image\/([\w+.-]+);base64,(.+)$/s);
if (!match) {
throw new Error("Invalid data URL format");
}
- const ext = match[1] === "jpeg" ? "jpg" : match[1];
+ // Normalize MIME subtype to file extension
+ const mimeSubtype = match[1];
+ let ext: string;
+ if (mimeSubtype === "jpeg") {
+ ext = "jpg";
+ } else if (mimeSubtype === "svg+xml") {
+ ext = "svg";
+ } else if (mimeSubtype === "x-icon" || mimeSubtype === "vnd.microsoft.icon") {
+ ext = "ico";
+ } else {
+ ext = mimeSubtype;
+ }🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/project-icons.ts` around lines 91 - 95, The data
URL regex in project-icons.ts (the dataUrl.match(...) call) incorrectly uses \w+
so it rejects MIME types like image/svg+xml; update that regex to accept
characters used in MIME subtype names (e.g., plus, dot, hyphen and
alphanumerics) or use a more permissive token for the subtype (for example match
[a-zA-Z0-9.+-]+ or [^;]+) so image/svg+xml is accepted and SVG uploads no longer
throw "Invalid data URL format".
| const [imageError, setImageError] = useState(false); | ||
| const [iconError, setIconError] = useState(false); |
There was a problem hiding this comment.
iconError is never reset when iconUrl changes — stale error state will prevent replaced icons from rendering.
If an icon fails to load, iconError is set to true. When the user later replaces the icon (new iconUrl prop), the component won't re-render the <img> because iconError remains true. The fallback stays stuck until the component remounts.
Reset iconError when iconUrl changes:
Proposed fix
const [imageError, setImageError] = useState(false);
const [iconError, setIconError] = useState(false);
+
+// Reset error state when icon URL changes
+useEffect(() => {
+ setIconError(false);
+}, [iconUrl]);(Requires adding useEffect to the import from react.)
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx`
around lines 46 - 47, The iconError state isn't reset when iconUrl changes so a
previously failed icon blocks rendering new icons; add a useEffect that watches
the iconUrl prop and calls setIconError(false) whenever iconUrl changes (also
ensure useEffect is imported from react) so the <img> in ProjectThumbnail can
attempt to load the new icon; reference the iconError state setter setIconError
and the iconUrl prop in your effect.
1 participant
Summary
~/.superset/project-icons/{projectId}.{ext}and serve them via a customsuperset-icon://Electron protocol — making<img src={project.iconUrl}>work transparentlyfavicon.ico,logo.png, etc.)iconUrlthrough the sidebar component chain (WorkspaceSidebar→ProjectSection→ProjectHeader→ProjectThumbnail)Changes
Backend (main process):
project-icons.ts— new utility for icon disk storage (save from file/data URL/buffer, delete, lookup)favicon-discovery.ts— auto-discovers favicons in project repos usingfast-globwith sensible ignore patternsmain/index.ts— registerssuperset-icon://protocol on both default andpersist:supersetsessionsprojects.ts— addstriggerFaviconDiscoveryandsetProjectIcontRPC mutationsSchema:
icon_urltext column to projects table (migration 0020)Frontend:
ProjectThumbnail— renders icon with priority:iconUrl→ GitHub avatar → first letter fallbackProjectSettings— adds "Project Icon" section with upload/replace/remove controlsWorkspaceSidebar→ProjectSection→ProjectHeaderTest plan
favicon.icoin its root → triggers auto-discovery, icon appears in sidebarbun run typecheckpasses (verified)bun run testpasses (verified — 1205 pass, 0 fail)Summary by CodeRabbit