mirror of
https://github.com/NanmiCoder/claude-code-haha.git
synced 2026-10-10 11:53:10 +08:00
fix(computer-use): load app icons through the authenticated channel
The settings page pointed an `<img src>` straight at `/api/computer-use/app-icon`
and got nothing. The packaged renderer is loaded with `loadFile`, so the page
origin is `file://` and that image is a cross-origin subresource: it can carry
neither the Authorization header nor a trusted Origin, and the server's
fetch-metadata policy refuses exactly that shape. Reproduced with curl —
`Sec-Fetch-Site: cross-site` + `Sec-Fetch-Mode: no-cors` turns a 200 into
`401 Missing H5 access token`. The same request without those headers succeeds,
which is why it looked fine from a terminal.
Icons now come through `apiGetBlob`, the credential path every other call
already uses, and reach the DOM as blob URLs (the CSP already allows `blob:`).
Two things this forces that the `<img>` version got for free:
- Caching, including the misses. `null` means "this bundle ships no icon";
without caching that, a list re-render re-requests every iconless bundle.
- A concurrency ceiling. Opening the picker renders every installed
application at once — 208 on the dev machine — so an uncapped fetch is one
request and one server-side `sips` per row, simultaneously.
`getAppIconUrl` is gone; `loadAppIcon` returns a blob URL or null. The letter
tile now covers both "loading" and "no icon", which is what the picker should
show either way — a spinner per row would read as broken on the utilities that
genuinely have no icon.
This commit is contained in:
@@ -197,6 +197,40 @@ function sanitizeDiagnosticValue(value: unknown): unknown {
|
||||
return value
|
||||
}
|
||||
|
||||
/**
|
||||
* Read binary content through the same authenticated channel as `api.get`.
|
||||
*
|
||||
* Pointing an `<img src>` straight at an API endpoint does not work in the
|
||||
* packaged app: the renderer is loaded with `loadFile`, so the page origin is
|
||||
* `file://` and the image is a cross-origin subresource that can carry neither
|
||||
* the Authorization header nor a trusted Origin. The server's fetch-metadata
|
||||
* policy refuses exactly that shape (verified in a real `file://` page: the
|
||||
* image fires `error`). Fetching the bytes here and handing the DOM a blob URL
|
||||
* uses the credential path that already works for every other call.
|
||||
*/
|
||||
export async function apiGetBlob(path: string, options?: ApiRequestOptions): Promise<Blob> {
|
||||
const controller = new AbortController()
|
||||
const timeoutMs = options?.timeout ?? DEFAULT_REQUEST_TIMEOUT_MS
|
||||
const timeout = setTimeout(() => controller.abort(), timeoutMs)
|
||||
const abortFromCaller = () => controller.abort(options?.signal?.reason)
|
||||
if (options?.signal?.aborted) abortFromCaller()
|
||||
else options?.signal?.addEventListener('abort', abortFromCaller, { once: true })
|
||||
try {
|
||||
const res = await fetch(`${baseUrl}${path}`, {
|
||||
method: 'GET',
|
||||
headers: buildHeaders(),
|
||||
signal: controller.signal,
|
||||
})
|
||||
if (!res.ok) {
|
||||
throw new ApiError(res.status, await res.text().catch(() => ''))
|
||||
}
|
||||
return await res.blob()
|
||||
} finally {
|
||||
clearTimeout(timeout)
|
||||
options?.signal?.removeEventListener('abort', abortFromCaller)
|
||||
}
|
||||
}
|
||||
|
||||
export const api = {
|
||||
get: <T>(path: string, options?: ApiRequestOptions) => request<T>('GET', path, undefined, options),
|
||||
post: <T>(path: string, body?: unknown, options?: ApiRequestOptions) => request<T>('POST', path, body, options),
|
||||
|
||||
@@ -5,10 +5,11 @@ const apiMock = vi.hoisted(() => ({
|
||||
post: vi.fn(),
|
||||
put: vi.fn(),
|
||||
}))
|
||||
const apiGetBlobMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('./client', () => ({ api: apiMock }))
|
||||
vi.mock('./client', () => ({ api: apiMock, apiGetBlob: apiGetBlobMock }))
|
||||
|
||||
import { computerUseApi } from './computerUse'
|
||||
import { computerUseApi, __resetAppIconCacheForTests } from './computerUse'
|
||||
|
||||
describe('computerUseApi', () => {
|
||||
beforeEach(() => {
|
||||
@@ -16,6 +17,8 @@ describe('computerUseApi', () => {
|
||||
apiMock.get.mockResolvedValue({})
|
||||
apiMock.post.mockResolvedValue({})
|
||||
apiMock.put.mockResolvedValue({})
|
||||
apiGetBlobMock.mockResolvedValue(new Blob(['png']))
|
||||
__resetAppIconCacheForTests()
|
||||
})
|
||||
|
||||
it('routes every Computer Use settings request through the expected API contract', async () => {
|
||||
@@ -58,3 +61,96 @@ describe('computerUseApi', () => {
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Icons go through `apiGetBlob`, not an `<img src>`, because the packaged
|
||||
* renderer is a `file://` page whose cross-origin image loads the server
|
||||
* refuses. That makes every icon a real request, so the caching and the
|
||||
* concurrency ceiling below are what keep opening the picker from firing one
|
||||
* request — and one server-side rasterisation — per installed application.
|
||||
*/
|
||||
describe('computerUseApi.loadAppIcon', () => {
|
||||
beforeEach(() => {
|
||||
// This is a sibling describe, so it does not inherit the reset above and
|
||||
// has to do its own — the icon cache and the call counts both persist
|
||||
// across cases otherwise.
|
||||
vi.clearAllMocks()
|
||||
__resetAppIconCacheForTests()
|
||||
apiGetBlobMock.mockResolvedValue(new Blob(['png']))
|
||||
// jsdom has no object-URL implementation.
|
||||
globalThis.URL.createObjectURL = vi.fn(() => 'blob:stub') as never
|
||||
})
|
||||
|
||||
it('requests the icon by bundle id and hands back a blob URL', async () => {
|
||||
const url = await computerUseApi.loadAppIcon('com.example.App')
|
||||
|
||||
expect(url).toBe('blob:stub')
|
||||
expect(apiGetBlobMock).toHaveBeenCalledWith(
|
||||
'/api/computer-use/app-icon?bundleId=com.example.App&size=72',
|
||||
)
|
||||
})
|
||||
|
||||
it('percent-encodes a bundle id so it cannot alter the query', async () => {
|
||||
await computerUseApi.loadAppIcon('weird&size=999#x')
|
||||
|
||||
expect(apiGetBlobMock).toHaveBeenCalledWith(
|
||||
'/api/computer-use/app-icon?bundleId=weird%26size%3D999%23x&size=72',
|
||||
)
|
||||
})
|
||||
|
||||
it('fetches once per bundle no matter how many rows ask', async () => {
|
||||
const urls = await Promise.all([
|
||||
computerUseApi.loadAppIcon('com.example.App'),
|
||||
computerUseApi.loadAppIcon('com.example.App'),
|
||||
computerUseApi.loadAppIcon('com.example.App'),
|
||||
])
|
||||
await computerUseApi.loadAppIcon('com.example.App')
|
||||
|
||||
expect(apiGetBlobMock).toHaveBeenCalledTimes(1)
|
||||
expect(new Set(urls)).toEqual(new Set(['blob:stub']))
|
||||
})
|
||||
|
||||
it('resolves to null on failure and does not retry it', async () => {
|
||||
apiGetBlobMock.mockRejectedValue(new Error('404'))
|
||||
|
||||
expect(await computerUseApi.loadAppIcon('com.example.NoIcon')).toBeNull()
|
||||
expect(await computerUseApi.loadAppIcon('com.example.NoIcon')).toBeNull()
|
||||
// A re-rendered list would otherwise re-request every iconless bundle.
|
||||
expect(apiGetBlobMock).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('caps how many icon requests are in flight at once', async () => {
|
||||
let inFlight = 0
|
||||
let peak = 0
|
||||
const release: Array<() => void> = []
|
||||
apiGetBlobMock.mockImplementation(() => {
|
||||
inFlight += 1
|
||||
peak = Math.max(peak, inFlight)
|
||||
return new Promise(resolve => {
|
||||
release.push(() => {
|
||||
inFlight -= 1
|
||||
resolve(new Blob(['png']))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
const pending = Promise.all(
|
||||
Array.from({ length: 30 }, (_, i) =>
|
||||
computerUseApi.loadAppIcon(`com.example.App${i}`),
|
||||
),
|
||||
)
|
||||
|
||||
// Drain in waves: each release frees a slot for a queued request. Yield to
|
||||
// the macrotask queue between waves so the freed slot is actually taken
|
||||
// before the next measurement.
|
||||
for (let step = 0; step < 60; step += 1) {
|
||||
if (release.length === 0 && apiGetBlobMock.mock.calls.length >= 30) break
|
||||
release.splice(0).forEach(fn => fn())
|
||||
await new Promise(resolve => setTimeout(resolve, 0))
|
||||
}
|
||||
await pending
|
||||
|
||||
expect(peak).toBeLessThanOrEqual(6)
|
||||
expect(apiGetBlobMock).toHaveBeenCalledTimes(30)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,4 +1,75 @@
|
||||
import { api, getApiUrl } from './client'
|
||||
import { api, apiGetBlob } from './client'
|
||||
|
||||
/**
|
||||
* Opening the picker renders every installed application at once. Without a
|
||||
* ceiling that is one request — and one `sips` subprocess on the server — per
|
||||
* row, all at the same instant. Six keeps the list filling visibly while the
|
||||
* machine stays responsive.
|
||||
*/
|
||||
const ICON_CONCURRENCY = 6
|
||||
let activeIconRequests = 0
|
||||
const iconWaiters: Array<() => void> = []
|
||||
|
||||
async function withIconSlot<T>(task: () => Promise<T>): Promise<T> {
|
||||
if (activeIconRequests >= ICON_CONCURRENCY) {
|
||||
await new Promise<void>(resolve => iconWaiters.push(resolve))
|
||||
}
|
||||
activeIconRequests += 1
|
||||
try {
|
||||
return await task()
|
||||
} finally {
|
||||
activeIconRequests -= 1
|
||||
iconWaiters.shift()?.()
|
||||
}
|
||||
}
|
||||
|
||||
/** Resolved icons, including the misses — null means "this bundle has none". */
|
||||
const iconCache = new Map<string, string | null>()
|
||||
const iconRequests = new Map<string, Promise<string | null>>()
|
||||
|
||||
function loadAppIconUrl(bundleId: string, size: number): Promise<string | null> {
|
||||
const key = `${bundleId}:${size}`
|
||||
const cached = iconCache.get(key)
|
||||
// `undefined` is "never asked"; `null` is a cached miss and must not retry —
|
||||
// a list that re-renders would otherwise re-request every iconless bundle.
|
||||
if (cached !== undefined) return Promise.resolve(cached)
|
||||
|
||||
const pending = iconRequests.get(key)
|
||||
if (pending) return pending
|
||||
|
||||
const request = withIconSlot(async () => {
|
||||
try {
|
||||
const blob = await apiGetBlob(
|
||||
`/api/computer-use/app-icon?bundleId=${encodeURIComponent(bundleId)}&size=${size}`,
|
||||
)
|
||||
const url = URL.createObjectURL(blob)
|
||||
iconCache.set(key, url)
|
||||
return url
|
||||
} catch {
|
||||
iconCache.set(key, null)
|
||||
return null
|
||||
} finally {
|
||||
iconRequests.delete(key)
|
||||
}
|
||||
})
|
||||
iconRequests.set(key, request)
|
||||
return request
|
||||
}
|
||||
|
||||
/**
|
||||
* Test hook: forget cached icons and release the concurrency gate.
|
||||
*
|
||||
* The gate counter and its waiter queue have to be reset too. They are module
|
||||
* state that no production path ever rewinds, so a case that leaves a request
|
||||
* in flight would otherwise hand the next case a permanently consumed slot —
|
||||
* and after enough of them, a queue that never drains.
|
||||
*/
|
||||
export function __resetAppIconCacheForTests(): void {
|
||||
iconCache.clear()
|
||||
iconRequests.clear()
|
||||
activeIconRequests = 0
|
||||
iconWaiters.splice(0).forEach(resume => resume())
|
||||
}
|
||||
|
||||
export type ComputerUseStatus = {
|
||||
platform: string
|
||||
@@ -111,17 +182,23 @@ export const computerUseApi = {
|
||||
return api.post<{ ok: true }>('/api/computer-use/open-settings', { pane })
|
||||
},
|
||||
/**
|
||||
* URL of an installed app's own icon, for use as an `<img src>`.
|
||||
* A blob URL for an installed app's own icon, or null when it has none.
|
||||
*
|
||||
* macOS-only, and 404s when the bundle declares no icon — callers render a
|
||||
* letter placeholder on error rather than treating that as a failure. The
|
||||
* parameter is a bundle id because the server resolves it against the
|
||||
* installed-app list; there is deliberately no way to ask for a path.
|
||||
* Not an `<img src>` pointing at the endpoint: the packaged renderer is a
|
||||
* `file://` page, so that request is a cross-origin subresource the server
|
||||
* refuses (see `apiGetBlob`). The bytes come through the authenticated
|
||||
* channel instead and reach the DOM as a blob.
|
||||
*
|
||||
* macOS-only. A bundle with no icon resolves to null, which is an ordinary
|
||||
* outcome — the caller shows a letter tile.
|
||||
*
|
||||
* Results are cached per (bundle, size) and blob URLs are intentionally not
|
||||
* revoked: the cache hands out the same URL for the lifetime of the window,
|
||||
* so the count is bounded by the number of installed applications rather
|
||||
* than by how many times a list is rendered.
|
||||
*/
|
||||
getAppIconUrl(bundleId: string, size = 72) {
|
||||
return getApiUrl(
|
||||
`/api/computer-use/app-icon?bundleId=${encodeURIComponent(bundleId)}&size=${size}`,
|
||||
)
|
||||
loadAppIcon(bundleId: string, size = 72): Promise<string | null> {
|
||||
return loadAppIconUrl(bundleId, size)
|
||||
},
|
||||
/**
|
||||
* macOS-only. Spawns the native `cu-helper request-access` permission card and
|
||||
|
||||
@@ -14,7 +14,7 @@ const computerUseApiMock = vi.hoisted(() => ({
|
||||
runSetup: vi.fn(),
|
||||
openSettings: vi.fn(),
|
||||
openPermissionCard: vi.fn(),
|
||||
getAppIconUrl: vi.fn(),
|
||||
loadAppIcon: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('../api/computerUse', () => ({
|
||||
@@ -75,10 +75,8 @@ describe('ComputerUseSettings', () => {
|
||||
computerUseApiMock.runSetup.mockReset()
|
||||
computerUseApiMock.openSettings.mockReset()
|
||||
computerUseApiMock.openPermissionCard.mockReset()
|
||||
computerUseApiMock.getAppIconUrl.mockReset()
|
||||
computerUseApiMock.getAppIconUrl.mockImplementation(
|
||||
(bundleId: string) => `/api/computer-use/app-icon?bundleId=${bundleId}`,
|
||||
)
|
||||
computerUseApiMock.loadAppIcon.mockReset()
|
||||
computerUseApiMock.loadAppIcon.mockResolvedValue(null)
|
||||
Reflect.deleteProperty(window, 'desktopHost')
|
||||
|
||||
computerUseApiMock.getStatus.mockResolvedValue(readyStatus)
|
||||
@@ -336,43 +334,51 @@ describe('ComputerUseSettings', () => {
|
||||
],
|
||||
}
|
||||
|
||||
it('points each row at the icon endpoint for its bundle id', async () => {
|
||||
it('renders the icon it loaded for the row bundle id', async () => {
|
||||
computerUseApiMock.getStatus.mockResolvedValue(nativeStatus)
|
||||
computerUseApiMock.getAuthorizedApps.mockResolvedValue(authorizedConfig)
|
||||
computerUseApiMock.loadAppIcon.mockResolvedValue('blob:icon-preview')
|
||||
|
||||
render(<ComputerUseSettings />)
|
||||
await screen.findByText('Preview')
|
||||
|
||||
const image = document.querySelector('img[src*="app-icon"]')
|
||||
expect(image).not.toBeNull()
|
||||
expect(image?.getAttribute('src')).toContain('com.example.Preview')
|
||||
// The picker is a long scroller; eager loading would fetch and
|
||||
// rasterise every row that is nowhere near the viewport.
|
||||
expect(image?.getAttribute('loading')).toBe('lazy')
|
||||
expect(computerUseApiMock.getAppIconUrl).toHaveBeenCalledWith(
|
||||
'com.example.Preview',
|
||||
)
|
||||
await waitFor(() => {
|
||||
expect(document.querySelector('img[src="blob:icon-preview"]')).not.toBeNull()
|
||||
})
|
||||
expect(computerUseApiMock.loadAppIcon).toHaveBeenCalledWith('com.example.Preview')
|
||||
// The letter tile is the fallback, so it must be gone once the icon
|
||||
// arrives — otherwise both would render.
|
||||
expect(screen.queryByText('P')).toBeNull()
|
||||
})
|
||||
|
||||
it('falls back to the letter tile when the icon fails to load', async () => {
|
||||
it('keeps the letter tile when the bundle has no icon', async () => {
|
||||
computerUseApiMock.getStatus.mockResolvedValue(nativeStatus)
|
||||
computerUseApiMock.getAuthorizedApps.mockResolvedValue(authorizedConfig)
|
||||
// null is the ordinary "this bundle ships no icon" answer.
|
||||
computerUseApiMock.loadAppIcon.mockResolvedValue(null)
|
||||
|
||||
render(<ComputerUseSettings />)
|
||||
await screen.findByText('Preview')
|
||||
|
||||
const image = document.querySelector('img[src*="app-icon"]')
|
||||
expect(image).not.toBeNull()
|
||||
// A bundle with no icon 404s, which reaches the DOM as an error event.
|
||||
expect(screen.queryByText('P')).toBeNull()
|
||||
await waitFor(() => expect(screen.getByText('P')).toBeInTheDocument())
|
||||
expect(document.querySelector('img')).toBeNull()
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.error(image as Element)
|
||||
await Promise.resolve()
|
||||
it('never points an img straight at the endpoint', async () => {
|
||||
// The packaged renderer is a file:// page, so a cross-origin <img> to
|
||||
// /api/... is refused by the server and silently shows nothing. Icons
|
||||
// must arrive as blob URLs through the authenticated channel.
|
||||
computerUseApiMock.getStatus.mockResolvedValue(nativeStatus)
|
||||
computerUseApiMock.getAuthorizedApps.mockResolvedValue(authorizedConfig)
|
||||
computerUseApiMock.loadAppIcon.mockResolvedValue('blob:icon-preview')
|
||||
|
||||
render(<ComputerUseSettings />)
|
||||
await screen.findByText('Preview')
|
||||
await waitFor(() => {
|
||||
expect(document.querySelector('img[src="blob:icon-preview"]')).not.toBeNull()
|
||||
})
|
||||
|
||||
expect(document.querySelector('img[src*="app-icon"]')).toBeNull()
|
||||
expect(screen.getByText('P')).toBeInTheDocument()
|
||||
expect(document.querySelector('img[src*="/api/"]')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -741,31 +741,43 @@ type Translate = ReturnType<typeof useTranslation>
|
||||
* failure — that is what the letter tile is for.
|
||||
*/
|
||||
function AppIcon({ name, bundleId }: { name: string; bundleId?: string }) {
|
||||
const [failed, setFailed] = useState(false)
|
||||
const [iconUrl, setIconUrl] = useState<string | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
setFailed(false)
|
||||
if (!bundleId) {
|
||||
setIconUrl(null)
|
||||
return
|
||||
}
|
||||
let cancelled = false
|
||||
setIconUrl(null)
|
||||
void computerUseApi.loadAppIcon(bundleId).then(url => {
|
||||
if (!cancelled) setIconUrl(url)
|
||||
})
|
||||
return () => {
|
||||
cancelled = true
|
||||
}
|
||||
}, [bundleId])
|
||||
|
||||
const tileClass =
|
||||
'flex h-9 w-9 flex-shrink-0 items-center justify-center rounded-[10px] border border-[var(--color-border)] bg-[var(--color-surface-container-high)] shadow-[inset_0_1px_0_rgba(255,255,255,0.04)]'
|
||||
|
||||
if (bundleId && !failed) {
|
||||
if (iconUrl) {
|
||||
return (
|
||||
<div className={tileClass}>
|
||||
<img
|
||||
src={computerUseApi.getAppIconUrl(bundleId)}
|
||||
src={iconUrl}
|
||||
alt=""
|
||||
aria-hidden="true"
|
||||
draggable={false}
|
||||
loading="lazy"
|
||||
onError={() => setFailed(true)}
|
||||
className="block h-7 w-7 object-contain"
|
||||
/>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
// Shown both while the icon is in flight and when the bundle has none. The
|
||||
// letter is a stable placeholder rather than a spinner, so a list of
|
||||
// iconless utilities does not read as permanently loading.
|
||||
const letter = name.trim().charAt(0).toUpperCase() || '?'
|
||||
return (
|
||||
<div className={`${tileClass} text-[13px] font-semibold text-[var(--color-text-secondary)]`}>
|
||||
|
||||
Reference in New Issue
Block a user