fix(desktop): accept uppercase icon URL schemes (#5470)

This commit is contained in:
tt-a1i 2026-06-21 01:52:01 +08:00 committed by GitHub
parent fc15ae6cb7
commit cf3e57fa50
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 177 additions and 31 deletions

View file

@ -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<string, { dataUrl: string; sourceUrl: string }>()
* @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<string | undefined>(() => {
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<string, string>
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<string, string>
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
}

View file

@ -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
// ============================================================================

View file

@ -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

View file

@ -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)

View file

@ -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'), '<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)
})
})

View file

@ -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

View file

@ -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)
})
})

View file

@ -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);
}
/**