From cf3e57fa504b42fac684626db93612d2e268917a Mon Sep 17 00:00:00 2001 From: tt-a1i <53142663+tt-a1i@users.noreply.github.com> Date: Sun, 21 Jun 2026 01:52:01 +0800 Subject: [PATCH] fix(desktop): accept uppercase icon URL schemes (#5470) --- .../src/renderer/hooks/useWorkspaceIcon.ts | 50 +++++------ .../renderer/lib/__tests__/icon-cache.test.ts | 40 +++++++++ .../electron/src/renderer/lib/icon-cache.ts | 8 +- .../src/renderer/pages/SourceInfoPage.tsx | 3 +- .../__tests__/workspace-icon-url.test.ts | 82 +++++++++++++++++++ .../packages/shared/src/config/storage.ts | 4 +- .../utils/__tests__/icon-constants.test.ts | 19 +++++ .../shared/src/utils/icon-constants.ts | 2 +- 8 files changed, 177 insertions(+), 31 deletions(-) create mode 100644 packages/desktop/packages/shared/src/config/__tests__/workspace-icon-url.test.ts create mode 100644 packages/desktop/packages/shared/src/utils/__tests__/icon-constants.test.ts diff --git a/packages/desktop/apps/electron/src/renderer/hooks/useWorkspaceIcon.ts b/packages/desktop/apps/electron/src/renderer/hooks/useWorkspaceIcon.ts index bfed1a55c3..dea697f93a 100644 --- a/packages/desktop/apps/electron/src/renderer/hooks/useWorkspaceIcon.ts +++ b/packages/desktop/apps/electron/src/renderer/hooks/useWorkspaceIcon.ts @@ -8,8 +8,9 @@ * Used by settings pages that display workspace icons. */ -import { useState, useEffect, useRef } from 'react' +import { useState, useEffect } from 'react' import type { Workspace } from '../../shared/types' +import { isIconUrl } from '@craft-agent/shared/utils/icon-constants' // Module-level cache to avoid redundant fetches across component instances // Key: workspaceId, Value: { dataUrl, sourceUrl } @@ -26,83 +27,86 @@ const iconCache = new Map() * @returns Data URL or remote URL for the icon, or undefined */ export function useWorkspaceIcon(workspace: Workspace | undefined): string | undefined { + const workspaceId = workspace?.id + const workspaceIconUrl = workspace?.iconUrl + const [iconUrl, setIconUrl] = useState(() => { - if (!workspace?.iconUrl) return undefined + if (!workspaceId || !workspaceIconUrl) return undefined // Remote URLs can be used directly - if (workspace.iconUrl.startsWith('http://') || workspace.iconUrl.startsWith('https://')) { - return workspace.iconUrl + if (isIconUrl(workspaceIconUrl)) { + return workspaceIconUrl } // Check cache for file:// URLs - const cached = iconCache.get(workspace.id) - if (cached && cached.sourceUrl === workspace.iconUrl) { + const cached = iconCache.get(workspaceId) + if (cached && cached.sourceUrl === workspaceIconUrl) { return cached.dataUrl } return undefined }) - // Track the workspace to detect changes - const workspaceRef = useRef(workspace) - useEffect(() => { - if (!workspace?.iconUrl) { + if (!workspaceId || !workspaceIconUrl) { setIconUrl(undefined) return } // Remote URLs - use directly - if (workspace.iconUrl.startsWith('http://') || workspace.iconUrl.startsWith('https://')) { - setIconUrl(workspace.iconUrl) + if (isIconUrl(workspaceIconUrl)) { + setIconUrl(workspaceIconUrl) return } // Not a file:// URL - skip - if (!workspace.iconUrl.startsWith('file://')) { + if (!workspaceIconUrl.startsWith('file://')) { setIconUrl(undefined) return } // Check if already cached with same source URL - const cached = iconCache.get(workspace.id) - if (cached && cached.sourceUrl === workspace.iconUrl) { + const cached = iconCache.get(workspaceId) + if (cached && cached.sourceUrl === workspaceIconUrl) { setIconUrl(cached.dataUrl) return } // Extract icon filename from file:// URL // e.g., "file:///path/to/icon.png?t=123" -> "icon.png" - const urlWithoutQuery = workspace.iconUrl.split('?')[0] + const urlWithoutQuery = workspaceIconUrl.split('?')[0] const iconFilename = urlWithoutQuery.split('/').pop() if (!iconFilename) { setIconUrl(undefined) return } + const id = workspaceId + const sourceUrl = workspaceIconUrl + const filename = iconFilename // Fetch via IPC and convert to data URL let cancelled = false async function fetchIcon() { try { - const result = await window.electronAPI.readWorkspaceImage(workspace!.id, iconFilename!) + const result = await window.electronAPI.readWorkspaceImage(id, filename) if (cancelled) return if (result) { // readWorkspaceImage returns raw SVG for .svg files, data URL for others let dataUrl = result - if (iconFilename!.endsWith('.svg')) { + if (filename.endsWith('.svg')) { dataUrl = `data:image/svg+xml;base64,${btoa(result)}` } // Cache the result - iconCache.set(workspace!.id, { dataUrl, sourceUrl: workspace!.iconUrl! }) + iconCache.set(id, { dataUrl, sourceUrl }) setIconUrl(dataUrl) } else { setIconUrl(undefined) } } catch (error) { - console.error(`Failed to load icon for workspace ${workspace!.id}:`, error) + console.error(`Failed to load icon for workspace ${id}:`, error) if (!cancelled) { setIconUrl(undefined) } @@ -114,7 +118,7 @@ export function useWorkspaceIcon(workspace: Workspace | undefined): string | und return () => { cancelled = true } - }, [workspace?.id, workspace?.iconUrl]) + }, [workspaceId, workspaceIconUrl]) return iconUrl } @@ -133,7 +137,7 @@ export function useWorkspaceIcons(workspaces: Workspace[]): Map if (!ws.iconUrl) continue // Remote URLs - if (ws.iconUrl.startsWith('http://') || ws.iconUrl.startsWith('https://')) { + if (isIconUrl(ws.iconUrl)) { map.set(ws.id, ws.iconUrl) continue } @@ -157,7 +161,7 @@ export function useWorkspaceIcons(workspaces: Workspace[]): Map if (!workspace.iconUrl) continue // Remote URLs - use directly - if (workspace.iconUrl.startsWith('http://') || workspace.iconUrl.startsWith('https://')) { + if (isIconUrl(workspace.iconUrl)) { newMap.set(workspace.id, workspace.iconUrl) continue } diff --git a/packages/desktop/apps/electron/src/renderer/lib/__tests__/icon-cache.test.ts b/packages/desktop/apps/electron/src/renderer/lib/__tests__/icon-cache.test.ts index 21a1ea996d..f66a8994be 100644 --- a/packages/desktop/apps/electron/src/renderer/lib/__tests__/icon-cache.test.ts +++ b/packages/desktop/apps/electron/src/renderer/lib/__tests__/icon-cache.test.ts @@ -102,6 +102,46 @@ describe('icon-cache null handling', () => { }) }) +describe('remote icon URLs', () => { + it('returns source icon URLs with uppercase schemes directly', async () => { + const { clearIconCaches, loadSourceIcon } = await import('../icon-cache') + clearIconCaches() + + const icon = 'HTTPS://cdn.example.com/source.svg' + + await expect( + loadSourceIcon({ + workspaceId: 'workspace-id', + config: { + slug: 'source', + name: 'Source', + type: 'api', + icon, + }, + }), + ).resolves.toBe(icon) + expect(mockReadWorkspaceImage).not.toHaveBeenCalled() + }) + + it('returns skill icon URLs with uppercase schemes directly', async () => { + const { clearIconCaches, loadSkillIcon } = await import('../icon-cache') + clearIconCaches() + + const icon = 'HTTP://cdn.example.com/skill.svg' + + await expect( + loadSkillIcon( + { + slug: 'skill', + metadata: { icon }, + }, + 'workspace-id', + ), + ).resolves.toBe(icon) + expect(mockReadWorkspaceImage).not.toHaveBeenCalled() + }) +}) + // ============================================================================ // Pure Function Tests for Null Guards // ============================================================================ diff --git a/packages/desktop/apps/electron/src/renderer/lib/icon-cache.ts b/packages/desktop/apps/electron/src/renderer/lib/icon-cache.ts index c21bf2375e..b1211be3c8 100644 --- a/packages/desktop/apps/electron/src/renderer/lib/icon-cache.ts +++ b/packages/desktop/apps/electron/src/renderer/lib/icon-cache.ts @@ -20,7 +20,7 @@ */ import { useState, useEffect, useMemo } from 'react' -import { isEmoji } from '@craft-agent/shared/utils/icon-constants' +import { isEmoji, isIconUrl } from '@craft-agent/shared/utils/icon-constants' import type { ResolvedEntityIcon } from '@craft-agent/shared/icons' // ============================================================================ @@ -201,7 +201,7 @@ export async function loadSourceIcon( // Priority 3: URL in config.icon - return URL directly // Config URL takes precedence over auto-discovered local files - if (icon && (icon.startsWith('http://') || icon.startsWith('https://'))) { + if (icon && isIconUrl(icon)) { sourceIconCache.set(cacheKey, icon) return icon } @@ -320,7 +320,7 @@ export async function loadSkillIcon( } // Priority 2: URL in metadata - return URL directly - if (iconValue && (iconValue.startsWith('http://') || iconValue.startsWith('https://'))) { + if (iconValue && isIconUrl(iconValue)) { skillIconCache.set(cacheKey, iconValue) return iconValue } @@ -538,7 +538,7 @@ export function useEntityIcon(opts: UseEntityIconOptions): ResolvedEntityIcon { // Guard against non-string values (can happen with malformed config data) if (!iconValue || typeof iconValue !== 'string') return null if (isEmoji(iconValue)) return { type: 'emoji' as const, value: iconValue } - if (iconValue.startsWith('http://') || iconValue.startsWith('https://')) { + if (isIconUrl(iconValue)) { return { type: 'url' as const, value: iconValue } } return null diff --git a/packages/desktop/apps/electron/src/renderer/pages/SourceInfoPage.tsx b/packages/desktop/apps/electron/src/renderer/pages/SourceInfoPage.tsx index ea537de7e6..5c26283c15 100644 --- a/packages/desktop/apps/electron/src/renderer/pages/SourceInfoPage.tsx +++ b/packages/desktop/apps/electron/src/renderer/pages/SourceInfoPage.tsx @@ -29,6 +29,7 @@ import { } from '@/components/info' import type { LoadedSource, McpToolWithPermission } from '../../shared/types' import type { PermissionsConfigFile } from '@craft-agent/shared/agent/modes' +import { isIconUrl } from '@craft-agent/shared/utils/icon-constants' interface SourceInfoPageProps { sourceSlug: string @@ -318,7 +319,7 @@ export default function SourceInfoPage({ sourceSlug, workspaceId, onDelete }: So const handleOpenUrl = useCallback(async () => { if (!source || !sourceUrl) return if (window.electronAPI) { - if (sourceUrl.startsWith('http://') || sourceUrl.startsWith('https://')) { + if (isIconUrl(sourceUrl)) { await window.electronAPI.openUrl(sourceUrl) } else { await window.electronAPI.showInFolder(sourceUrl) diff --git a/packages/desktop/packages/shared/src/config/__tests__/workspace-icon-url.test.ts b/packages/desktop/packages/shared/src/config/__tests__/workspace-icon-url.test.ts new file mode 100644 index 0000000000..d588ed7680 --- /dev/null +++ b/packages/desktop/packages/shared/src/config/__tests__/workspace-icon-url.test.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'bun:test' +import { mkdirSync, writeFileSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' +import { pathToFileURL } from 'url' + +const STORAGE_MODULE_PATH = pathToFileURL(join(import.meta.dir, '..', 'storage.ts')).href + +function setupConfigDir(iconUrl: string) { + const configDir = join(tmpdir(), `qwen-workspace-icon-${crypto.randomUUID()}`) + const workspaceRoot = join(configDir, 'workspace') + mkdirSync(workspaceRoot, { recursive: true }) + writeFileSync(join(workspaceRoot, 'icon.svg'), '', 'utf-8') + writeFileSync( + join(configDir, 'config-defaults.json'), + JSON.stringify({ + version: 'test', + description: 'test defaults', + defaults: { + notificationsEnabled: true, + colorTheme: 'default', + autoCapitalisation: true, + sendMessageKey: 'enter', + spellCheck: false, + keepAwakeWhileRunning: false, + richToolDescriptions: true, + }, + workspaceDefaults: { + permissionMode: 'safe', + cyclablePermissionModes: ['safe', 'allow-all'], + localMcpServers: { enabled: true }, + }, + }), + 'utf-8', + ) + writeFileSync( + join(configDir, 'config.json'), + JSON.stringify({ + workspaces: [ + { + id: 'ws-a', + name: 'A', + slug: 'a', + rootPath: workspaceRoot, + iconUrl, + createdAt: 1, + }, + ], + activeWorkspaceId: 'ws-a', + activeSessionId: null, + }), + 'utf-8', + ) + return configDir +} + +function readWorkspaceIconUrl(configDir: string): string { + const run = Bun.spawnSync([ + process.execPath, + '--eval', + `import { getWorkspaces } from '${STORAGE_MODULE_PATH}'; console.log(getWorkspaces()[0].iconUrl);`, + ], { + env: { ...process.env, CRAFT_CONFIG_DIR: configDir }, + stdout: 'pipe', + stderr: 'pipe', + }) + + if (run.exitCode !== 0) { + throw new Error(`subprocess failed (exit ${run.exitCode})\nstdout:\n${run.stdout.toString()}\nstderr:\n${run.stderr.toString()}`) + } + + return run.stdout.toString().trim() +} + +describe('workspace icon URLs', () => { + it('preserves uppercase remote icon URL schemes instead of falling back to local icons', () => { + const iconUrl = 'HTTPS://cdn.example.com/workspace.svg' + const configDir = setupConfigDir(iconUrl) + + expect(readWorkspaceIconUrl(configDir)).toBe(iconUrl) + }) +}) diff --git a/packages/desktop/packages/shared/src/config/storage.ts b/packages/desktop/packages/shared/src/config/storage.ts index bd8ba80a64..8f76ce22cb 100644 --- a/packages/desktop/packages/shared/src/config/storage.ts +++ b/packages/desktop/packages/shared/src/config/storage.ts @@ -14,7 +14,7 @@ import { createWorkspaceAtPath, isValidWorkspace, } from '../workspaces/storage.ts'; -import { findIconFile } from '../utils/icon.ts'; +import { findIconFile, isIconUrl } from '../utils/icon.ts'; import { extractWorkspaceSlugFromPath } from '../utils/workspace-slug.ts'; import { initializeDocs } from '../docs/index.ts'; import { expandPath, toPortablePath, getBundledAssetsDir } from '../utils/paths.ts'; @@ -869,7 +869,7 @@ export function getWorkspaces(): Workspace[] { // If workspace has a stored iconUrl that's a remote URL, use it // Otherwise check for local icon file let iconUrl = w.iconUrl; - if (!iconUrl || (!iconUrl.startsWith('http://') && !iconUrl.startsWith('https://'))) { + if (!iconUrl || !isIconUrl(iconUrl)) { const localIcon = findWorkspaceIcon(w.rootPath); if (localIcon) { // Convert absolute path to file:// URL for Electron renderer diff --git a/packages/desktop/packages/shared/src/utils/__tests__/icon-constants.test.ts b/packages/desktop/packages/shared/src/utils/__tests__/icon-constants.test.ts new file mode 100644 index 0000000000..9768ff97f9 --- /dev/null +++ b/packages/desktop/packages/shared/src/utils/__tests__/icon-constants.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'bun:test' + +import { isIconUrl } from '../icon-constants.ts' +import { validateIconValue } from '../icon.ts' + +describe('icon URL detection', () => { + it('treats http and https schemes as case-insensitive', () => { + expect(isIconUrl('HTTP://cdn.example.com/icon.svg')).toBe(true) + expect(isIconUrl('HTTPS://cdn.example.com/icon.svg')).toBe(true) + expect(validateIconValue('HTTPS://cdn.example.com/icon.svg')).toBe( + 'HTTPS://cdn.example.com/icon.svg', + ) + }) + + it('rejects non-http icon URLs', () => { + expect(isIconUrl('ftp://cdn.example.com/icon.svg')).toBe(false) + expect(isIconUrl('data:image/svg+xml;base64,abc')).toBe(false) + }) +}) diff --git a/packages/desktop/packages/shared/src/utils/icon-constants.ts b/packages/desktop/packages/shared/src/utils/icon-constants.ts index 00baf2e79f..ad633e7c55 100644 --- a/packages/desktop/packages/shared/src/utils/icon-constants.ts +++ b/packages/desktop/packages/shared/src/utils/icon-constants.ts @@ -42,7 +42,7 @@ export function isEmoji(str: string | undefined): boolean { * Check if a string is a valid icon URL (http or https). */ export function isIconUrl(str: string): boolean { - return str.startsWith('http://') || str.startsWith('https://'); + return /^https?:\/\//i.test(str); } /**