Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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)
}
Expand All @@ -114,7 +118,7 @@ export function useWorkspaceIcon(workspace: Workspace | undefined): string | und
return () => {
cancelled = true
}
}, [workspace?.id, workspace?.iconUrl])
}, [workspaceId, workspaceIconUrl])

return iconUrl
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ============================================================================
Expand Down
8 changes: 4 additions & 4 deletions packages/desktop/apps/electron/src/renderer/lib/icon-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

// ============================================================================
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
})
})
4 changes: 2 additions & 2 deletions packages/desktop/packages/shared/src/config/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
Loading