fix: harden perf cleanup edge cases (#2712)

src/main/ipc/pty.test.ts

@@ -3530,5 +3530,45 @@ describe('registerPtyHandlers', () => {
         vi.useRealTimers()
       }
     })
+
+    it('serializes overlapping headless snapshots for the same PTY', async () => {
+      let resolveFirst!: (snapshot: { data: string; cols: number; rows: number }) => void
+      let resolveSecond!: (snapshot: { data: string; cols: number; rows: number }) => void
+      const runtime = {
+        setPtyController: vi.fn(),
+        onPtySpawned: vi.fn(),
+        onPtyData: vi.fn(),
+        onPtyExit: vi.fn(),
+        preAllocateHandleForPty: vi.fn(),
+        serializeHeadlessTerminalBufferForRenderer: vi
+          .fn()
+          .mockImplementationOnce(
+            () =>
+              new Promise<{ data: string; cols: number; rows: number }>((resolve) => {
+                resolveFirst = resolve
+              })
+          )
+          .mockImplementationOnce(
+            () =>
+              new Promise<{ data: string; cols: number; rows: number }>((resolve) => {
+                resolveSecond = resolve
+              })
+          )
+      }
+      handlers.clear()
+      registerPtyHandlers(mainWindow as never, runtime as never)
+
+      const first = handlers.get('pty:serializeHeadlessBuffer')!(null, { id: 'pty-1' })
+      const second = handlers.get('pty:serializeHeadlessBuffer')!(null, { id: 'pty-1' })
+      expect(runtime.serializeHeadlessTerminalBufferForRenderer).toHaveBeenCalledTimes(1)
+
+      resolveFirst({ data: 'first', cols: 80, rows: 24 })
+      await expect(first).resolves.toEqual({ data: 'first', cols: 80, rows: 24 })
+      await Promise.resolve()
+      expect(runtime.serializeHeadlessTerminalBufferForRenderer).toHaveBeenCalledTimes(2)
+
+      resolveSecond({ data: 'second', cols: 80, rows: 24 })
+      await expect(second).resolves.toEqual({ data: 'second', cols: 80, rows: 24 })
+    })
   })
 })

src/main/ipc/pty.ts

@@ -658,8 +658,11 @@ export function registerPtyHandlers(
   // reduces IPC round-trips from hundreds/sec to ~120/sec under high
   // throughput. Keystroke echo/redraws bypass this below because agent TUIs
   // already spend tens of ms producing their redraw.
+  type HeadlessRendererSnapshot = { data: string; cols: number; rows: number } | null
+
   const pendingData = new Map<string, string>()
-  const headlessSnapshotHeldPtyIds = new Set<string>()
+  const headlessSnapshotHoldCounts = new Map<string, number>()
+  const headlessSnapshotQueues = new Map<string, Promise<HeadlessRendererSnapshot>>()
   const trustedTerminalHandleEnv = new Set<string>()
   let flushTimer: ReturnType<typeof setTimeout> | null = null
   const PTY_BATCH_INTERVAL_MS = 8
@@ -670,7 +673,7 @@ export function registerPtyHandlers(
 
   const hasFlushablePendingData = (): boolean => {
     for (const id of pendingData.keys()) {
-      if (!headlessSnapshotHeldPtyIds.has(id)) {
+      if (!headlessSnapshotHoldCounts.has(id)) {
         return true
       }
     }
@@ -684,7 +687,7 @@ export function registerPtyHandlers(
       return
     }
     for (const [id, data] of pendingData) {
-      if (headlessSnapshotHeldPtyIds.has(id)) {
+      if (headlessSnapshotHoldCounts.has(id)) {
         continue
       }
       mainWindow.webContents.send('pty:data', { id, data })
@@ -720,6 +723,62 @@ export function registerPtyHandlers(
     return data
   }
 
+  const captureHeadlessSnapshotForRenderer = async (
+    id: string,
+    opts: { scrollbackRows?: number }
+  ): Promise<HeadlessRendererSnapshot> => {
+    if (!runtime) {
+      return null
+    }
+    // Why: hidden-pane reveal uses the headless snapshot as the authoritative
+    // paint. Hold any ≤8ms main-process PTY batches while serializing: a
+    // successful headless snapshot already contains them, while a null
+    // snapshot must release them so the renderer fallback can replay them.
+    const pendingBeforeSnapshot = takePendingDataForPty(id)
+    const holdCount = headlessSnapshotHoldCounts.get(id) ?? 0
+    headlessSnapshotHoldCounts.set(id, holdCount + 1)
+    const releaseSnapshotHold = (): void => {
+      const current = headlessSnapshotHoldCounts.get(id) ?? 0
+      if (current <= 1) {
+        headlessSnapshotHoldCounts.delete(id)
+        return
+      }
+      headlessSnapshotHoldCounts.set(id, current - 1)
+    }
+    let snapshot: HeadlessRendererSnapshot
+    try {
+      snapshot = await runtime.serializeHeadlessTerminalBufferForRenderer(id, opts)
+    } catch (err) {
+      const pendingDuringSnapshot = takePendingDataForPty(id)
+      releaseSnapshotHold()
+      sendPtyDataToRenderer(id, pendingBeforeSnapshot + pendingDuringSnapshot)
+      throw err
+    }
+    const pendingDuringSnapshot = takePendingDataForPty(id)
+    releaseSnapshotHold()
+    if (!snapshot) {
+      sendPtyDataToRenderer(id, pendingBeforeSnapshot + pendingDuringSnapshot)
+    }
+    return snapshot
+  }
+
+  const queueHeadlessSnapshotForRenderer = (
+    id: string,
+    opts: { scrollbackRows?: number }
+  ): Promise<HeadlessRendererSnapshot> => {
+    const previous = headlessSnapshotQueues.get(id)
+    const next = previous
+      ? previous.catch(() => null).then(() => captureHeadlessSnapshotForRenderer(id, opts))
+      : captureHeadlessSnapshotForRenderer(id, opts)
+    const tracked = next.finally(() => {
+      if (headlessSnapshotQueues.get(id) === tracked) {
+        headlessSnapshotQueues.delete(id)
+      }
+    })
+    headlessSnapshotQueues.set(id, tracked)
+    return tracked
+  }
+
   // Why: extracted so the "Restart daemon" flow can rebind against the fresh
   // adapter after replaceDaemonProvider runs. Both the startup registration
   // and the post-restart rebind go through the same code path — no risk of
@@ -758,7 +817,7 @@ export function registerPtyHandlers(
         nextData.length <= INTERACTIVE_OUTPUT_MAX_CHARS &&
         lastInputAt !== undefined &&
         performance.now() - lastInputAt <= INTERACTIVE_OUTPUT_WINDOW_MS
-      if (isInteractiveOutput && !headlessSnapshotHeldPtyIds.has(payload.id)) {
+      if (isInteractiveOutput && !headlessSnapshotHoldCounts.has(payload.id)) {
         pendingData.delete(payload.id)
         clearFlushTimerIfIdle()
         // Why: agent TUIs redraw small prompt regions after every keystroke.
@@ -1868,27 +1927,7 @@ export function registerPtyHandlers(
       ) {
         opts.scrollbackRows = Math.floor(args.scrollbackRows)
       }
-      // Why: hidden-pane reveal uses the headless snapshot as the authoritative
-      // paint. Hold any ≤8ms main-process PTY batches while serializing: a
-      // successful headless snapshot already contains them, while a null
-      // snapshot must release them so the renderer fallback can replay them.
-      const pendingBeforeSnapshot = takePendingDataForPty(args.id)
-      headlessSnapshotHeldPtyIds.add(args.id)
-      let snapshot: { data: string; cols: number; rows: number } | null
-      try {
-        snapshot = await runtime.serializeHeadlessTerminalBufferForRenderer(args.id, opts)
-      } catch (err) {
-        const pendingDuringSnapshot = takePendingDataForPty(args.id)
-        headlessSnapshotHeldPtyIds.delete(args.id)
-        sendPtyDataToRenderer(args.id, pendingBeforeSnapshot + pendingDuringSnapshot)
-        throw err
-      }
-      const pendingDuringSnapshot = takePendingDataForPty(args.id)
-      headlessSnapshotHeldPtyIds.delete(args.id)
-      if (!snapshot) {
-        sendPtyDataToRenderer(args.id, pendingBeforeSnapshot + pendingDuringSnapshot)
-      }
-      return snapshot
+      return queueHeadlessSnapshotForRenderer(args.id, opts)
     }
   )
 

src/main/ipc/shell.ts

@@ -11,8 +11,7 @@ import { getSpawnArgsForWindows } from '../win32-utils'
 export const EXTERNAL_EDITOR_CLI_COMMAND = 'code'
 
 const REPO_ICON_IMAGE_MIME_TYPES: Record<string, string> = {
-  '.png': 'image/png',
-  '.svg': 'image/svg+xml'
+  '.png': 'image/png'
 }
 
 async function pathExists(pathValue: string): Promise<boolean> {
@@ -248,7 +247,7 @@ export function registerShellHandlers(): void {
     async (): Promise<{ dataUrl: string; fileName: string } | null> => {
       const result = await dialog.showOpenDialog({
         properties: ['openFile'],
-        filters: [{ name: 'Repo icon images', extensions: ['png', 'svg'] }]
+        filters: [{ name: 'Repo icon images', extensions: ['png'] }]
       })
       if (result.canceled || result.filePaths.length === 0) {
         return null
@@ -258,7 +257,7 @@ export function registerShellHandlers(): void {
       const extension = extname(filePath).toLowerCase()
       const mimeType = REPO_ICON_IMAGE_MIME_TYPES[extension]
       if (!mimeType) {
-        throw new Error('Repo icons must be PNG or SVG files.')
+        throw new Error('Repo icons must be PNG files.')
       }
 
       const stats = await stat(filePath)

src/main/persistence.test.ts

@@ -1376,6 +1376,39 @@ describe('Store', () => {
     expect(store.getRepo('r1')!.displayName).toBe('renamed')
   })
 
+  it('updateRepo drops repo icons that fail shared sanitization', async () => {
+    const store = await createStore()
+    store.addRepo(makeRepo())
+
+    const updated = store.updateRepo('r1', {
+      repoIcon: {
+        type: 'image',
+        source: 'upload',
+        src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4='
+      } as never
+    })
+
+    expect(updated).not.toBeNull()
+    expect(updated!.repoIcon).toBeUndefined()
+    expect(store.getRepo('r1')!.repoIcon).toBeUndefined()
+  })
+
+  it('getRepo does not expose invalid persisted repo icons', async () => {
+    const store = await createStore()
+    store.addRepo(
+      makeRepo({
+        repoIcon: {
+          type: 'image',
+          source: 'upload',
+          src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4='
+        } as never
+      })
+    )
+
+    expect(store.getRepo('r1')!.repoIcon).toBeUndefined()
+    expect(store.getRepos()[0]!.repoIcon).toBeUndefined()
+  })
+
   it('updateRepo returns null for nonexistent id', async () => {
     const store = await createStore()
     expect(store.updateRepo('nope', { displayName: 'x' })).toBeNull()

src/main/persistence.ts

@@ -93,6 +93,7 @@ import {
   normalizeWorkspaceStatuses
 } from '../shared/workspace-statuses'
 import { isLegacyRepoForExternalWorktreeVisibility } from '../shared/worktree-ownership'
+import { sanitizeRepoIcon } from '../shared/repo-icon'
 
 function encrypt(plaintext: string): string {
   if (!plaintext || !safeStorage.isEncryptionAvailable()) {
@@ -415,6 +416,21 @@ function readLegacySidekickFlag(parsed: PersistedState | undefined): boolean | u
   return (parsed?.settings as { experimentalSidekick?: boolean } | undefined)?.experimentalSidekick
 }
 
+function sanitizeRepoUpdatesForPersistence<T extends Partial<Pick<Repo, 'repoIcon'>>>(
+  updates: T
+): T {
+  const sanitized = { ...updates }
+  if ('repoIcon' in sanitized) {
+    const repoIcon = sanitizeRepoIcon(sanitized.repoIcon)
+    if (repoIcon === undefined) {
+      delete sanitized.repoIcon
+    } else {
+      sanitized.repoIcon = repoIcon
+    }
+  }
+  return sanitized
+}
+
 function expandFloatingWorkspaceHomePath(input: string, home: string): string {
   if (input === '~') {
     return home
@@ -2057,8 +2073,10 @@ export class Store {
     if (!repo) {
       return null
     }
+    const sanitizedUpdates = sanitizeRepoUpdatesForPersistence(updates)
     const externalWorktreeVisibilityLegacy =
-      'externalWorktreeVisibility' in updates && repo.externalWorktreeVisibilityLegacy === undefined
+      'externalWorktreeVisibility' in sanitizedUpdates &&
+      repo.externalWorktreeVisibilityLegacy === undefined
         ? isLegacyRepoForExternalWorktreeVisibility(repo)
         : undefined
     // Why: `issueSourcePreference === undefined` in the patch means "reset to
@@ -2066,15 +2084,18 @@ export class Store {
     // stale explicit value via Object.assign's skip-on-undefined behavior).
     // Without this delete branch, toggling explicit → auto would silently
     // leave the old preference in place on disk.
-    if ('issueSourcePreference' in updates && updates.issueSourcePreference === undefined) {
+    if (
+      'issueSourcePreference' in sanitizedUpdates &&
+      sanitizedUpdates.issueSourcePreference === undefined
+    ) {
       delete repo.issueSourcePreference
-      const { issueSourcePreference: _drop, ...rest } = updates
+      const { issueSourcePreference: _drop, ...rest } = sanitizedUpdates
       Object.assign(repo, rest)
     } else {
-      Object.assign(repo, updates)
+      Object.assign(repo, sanitizedUpdates)
     }
     if (
-      'externalWorktreeVisibility' in updates &&
+      'externalWorktreeVisibility' in sanitizedUpdates &&
       repo.externalWorktreeVisibilityLegacy === undefined
     ) {
       // Why: old persisted repos have no explicit marker. Stamp it the first
@@ -2086,6 +2107,8 @@ export class Store {
   }
 
   private hydrateRepo(repo: Repo): Repo {
+    const { repoIcon: rawRepoIcon, ...repoWithoutIcon } = repo
+    const repoIcon = sanitizeRepoIcon(rawRepoIcon)
     const gitUsername = isFolderRepo(repo)
       ? ''
       : (this.gitUsernameCache.get(repo.path) ??
@@ -2096,7 +2119,8 @@ export class Store {
         })())
 
     return {
-      ...repo,
+      ...repoWithoutIcon,
+      ...(repoIcon !== undefined ? { repoIcon } : {}),
       kind: isFolderRepo(repo) ? 'folder' : 'git',
       gitUsername,
       hookSettings: {

src/renderer/src/components/settings/RepositoryIconPicker.tsx

@@ -230,7 +230,7 @@ export function RepositoryIconPicker({
               onClick={handleUploadImage}
             >
               <Image className="size-3.5" />
-              Upload PNG/SVG
+              Upload PNG
             </Button>
             <Button
               type="button"
@@ -262,7 +262,7 @@ export function RepositoryIconPicker({
               Favicon
             </Button>
           </div>
-          <p className="text-xs text-muted-foreground">PNG/SVG uploads must be 256KB or smaller.</p>
+          <p className="text-xs text-muted-foreground">PNG uploads must be 256KB or smaller.</p>
         </TabsContent>
       </Tabs>
     </div>

src/renderer/src/store/slices/repos-update-serialization.test.ts

@@ -127,4 +127,21 @@ describe('repo update serialization', () => {
       errorSpy.mockRestore()
     }
   })
+
+  it('does not apply repo icons that fail shared sanitization', async () => {
+    reposUpdate.mockResolvedValueOnce(undefined)
+    const store = createTestStore()
+    store.setState({ repos: [localRepo] })
+
+    await store.getState().updateRepo(localRepo.id, {
+      repoIcon: {
+        type: 'image',
+        source: 'upload',
+        src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4='
+      } as never
+    })
+
+    expect(reposUpdate).toHaveBeenCalledWith({ repoId: localRepo.id, updates: {} })
+    expect(store.getState().repos[0]?.repoIcon).toBeUndefined()
+  })
 })

src/renderer/src/store/slices/repos.ts

@@ -7,6 +7,7 @@ import { toast } from 'sonner'
 import type { AppState } from '../types'
 import type { Repo } from '../../../../shared/types'
 import { isGitRepoKind } from '../../../../shared/repo-kind'
+import { sanitizeRepoIcon } from '../../../../shared/repo-icon'
 import { getRepoIdFromWorktreeId } from './worktree-helpers'
 import { callRuntimeRpc, getActiveRuntimeTarget } from '../../runtime/runtime-rpc-client'
 import { buildDismissedOnboardingFolderAgentStartup } from '@/lib/onboarding-folder-agent-startup'
@@ -29,6 +30,19 @@ type RepoUpdate = Partial<
   >
 >
 
+function sanitizeRepoUpdate(updates: RepoUpdate): RepoUpdate {
+  const sanitized = { ...updates }
+  if ('repoIcon' in sanitized) {
+    const repoIcon = sanitizeRepoIcon(sanitized.repoIcon)
+    if (repoIcon === undefined) {
+      delete sanitized.repoIcon
+    } else {
+      sanitized.repoIcon = repoIcon
+    }
+  }
+  return sanitized
+}
+
 const updateRepoChainsByStore = new WeakMap<() => AppState, Map<string, Promise<boolean>>>()
 
 function getRepoUpdateChains(get: () => AppState): Map<string, Promise<boolean>> {
@@ -345,12 +359,18 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
     const updateRepoChains = getRepoUpdateChains(get)
     const applyRepoUpdate = async () => {
       try {
+        const sanitizedUpdates = sanitizeRepoUpdate(updates)
         const target = getActiveRuntimeTarget(get().settings)
         await (target.kind === 'local'
-          ? window.api.repos.update({ repoId, updates })
-          : callRuntimeRpc(target, 'repo.update', { repo: repoId, updates }, { timeoutMs: 15_000 }))
+          ? window.api.repos.update({ repoId, updates: sanitizedUpdates })
+          : callRuntimeRpc(
+              target,
+              'repo.update',
+              { repo: repoId, updates: sanitizedUpdates },
+              { timeoutMs: 15_000 }
+            ))
         set((s) => ({
-          repos: s.repos.map((r) => (r.id === repoId ? { ...r, ...updates } : r))
+          repos: s.repos.map((r) => (r.id === repoId ? { ...r, ...sanitizedUpdates } : r))
         }))
         return true
       } catch (err) {

src/shared/repo-icon.test.ts

@@ -24,6 +24,28 @@ describe('sanitizeRepoIcon', () => {
       source: 'github',
       label: 'stablyai/orca'
     })
+    expect(
+      sanitizeRepoIcon({
+        type: 'image',
+        src: 'https://www.google.com/s2/favicons?domain=example.com&sz=64',
+        source: 'favicon'
+      })
+    ).toEqual({
+      type: 'image',
+      src: 'https://www.google.com/s2/favicons?domain=example.com&sz=64',
+      source: 'favicon'
+    })
+    expect(
+      sanitizeRepoIcon({
+        type: 'image',
+        src: 'data:image/png;base64,aGVsbG8=',
+        source: 'upload'
+      })
+    ).toEqual({
+      type: 'image',
+      src: 'data:image/png;base64,aGVsbG8=',
+      source: 'upload'
+    })
   })
 
   it('keeps null as an explicit reset', () => {
@@ -45,5 +67,19 @@ describe('sanitizeRepoIcon', () => {
         source: 'upload'
       })
     ).toBeUndefined()
+    expect(
+      sanitizeRepoIcon({
+        type: 'image',
+        src: 'data:image/svg+xml;base64,PHN2Zz48L3N2Zz4=',
+        source: 'upload'
+      })
+    ).toBeUndefined()
+    expect(
+      sanitizeRepoIcon({
+        type: 'image',
+        src: 'https://example.com/icon.png',
+        source: 'github'
+      })
+    ).toBeUndefined()
   })
 })

src/shared/repo-icon.ts

@@ -9,13 +9,29 @@ export const MAX_REPO_ICON_UPLOAD_BYTES = 256 * 1024
 export const MAX_REPO_ICON_DATA_URL_LENGTH = 400 * 1024
 
 const LUCIDE_ICON_NAME_PATTERN = /^[A-Za-z][A-Za-z0-9]*$/
-const IMAGE_SOURCE_IDS = new Set(['upload', 'favicon', 'github'])
+const isRepoIconImageSource = (value: string): value is RepoIconImageSource =>
+  value === 'upload' || value === 'favicon' || value === 'github'
 
-function isSupportedImageSrc(src: string): boolean {
-  return (
-    /^https:\/\/[^\s]+$/i.test(src) ||
-    /^data:image\/(?:png|svg\+xml);base64,[A-Za-z0-9+/=\s]+$/i.test(src)
-  )
+function isSupportedImageSrc(src: string, source: RepoIconImageSource): boolean {
+  if (source === 'upload') {
+    return /^data:image\/png;base64,[A-Za-z0-9+/=\s]+$/i.test(src)
+  }
+
+  let url: URL
+  try {
+    url = new URL(src)
+  } catch {
+    return false
+  }
+  if (url.protocol !== 'https:') {
+    return false
+  }
+
+  if (source === 'github') {
+    return url.hostname === 'github.com' && /^\/[^/?#]+\.png$/i.test(url.pathname)
+  }
+
+  return url.hostname === 'www.google.com' && url.pathname === '/s2/favicons'
 }
 
 export function sanitizeRepoIcon(value: unknown): RepoIcon | null | undefined {
@@ -49,10 +65,10 @@ export function sanitizeRepoIcon(value: unknown): RepoIcon | null | undefined {
   if (candidate.type === 'image') {
     const src = typeof candidate.src === 'string' ? candidate.src.trim() : ''
     const source = typeof candidate.source === 'string' ? candidate.source : ''
-    if (!IMAGE_SOURCE_IDS.has(source) || src.length > MAX_REPO_ICON_DATA_URL_LENGTH) {
+    if (!isRepoIconImageSource(source) || src.length > MAX_REPO_ICON_DATA_URL_LENGTH) {
       return undefined
     }
-    if (!isSupportedImageSrc(src)) {
+    if (!isSupportedImageSrc(src, source)) {
       return undefined
     }
     const label = typeof candidate.label === 'string' ? candidate.label.trim().slice(0, 80) : ''