fix(favicons): keep provisional icons provisional through reuse, walk past cached parent misses, skip hosting suffixes

A cached hit now records whether it was definite or provisional, so a host reusing a
parent's timeout-derived icon inherits the one-day lifetime instead of a month. A cached
miss at a parent skips only the candidates that parent's own walk covered, so a product
fallback derived from the original host is still tried. The self-hosted product label
no longer applies on a Public Suffix List private (hosting) suffix. Adds the Databricks
military suffix the repo already recognizes, and the client remembers a failed icon only
as long as the route retries an outage.

Author: waleedlatif1

apps/sim/components/favicon.tsx

@@ -6,10 +6,11 @@ import { LRUCache } from 'lru-cache'
 import { faviconSrc } from '@/lib/favicon/url'
 
 /**
- * Hosts with no icon, so an icon that mounts again renders its fallback at once instead of
- * asking again and showing an empty slot first. Expires with the route's caching of a miss.
+ * Hosts whose icon just failed, so an icon that mounts again renders its fallback at once
+ * instead of an empty slot first. An `<img>` can't tell no icon from a provider outage, so this
+ * lasts only as long as the route retries an outage; a confirmed miss stays in the HTTP cache.
  */
-const knownMisses = new LRUCache<string, true>({ max: 500, ttl: 60 * 60 * 1000 })
+const knownMisses = new LRUCache<string, true>({ max: 500, ttl: 5 * 60 * 1000 })
 
 interface FaviconProps {
   /** The site's hostname; without one the fallback renders. */

apps/sim/lib/favicon/hosts.test.ts

@@ -57,6 +57,10 @@ describe('faviconHostCandidates', () => {
       ['acme.gitlab-dedicated.com', 'gitlab-dedicated.com', 'gitlab.com'],
     ],
     ['acme.testrail.io', ['acme.testrail.io', 'testrail.io', 'testrail.com']],
+    [
+      'acme.databricksapps.mil',
+      ['acme.databricksapps.mil', 'databricksapps.mil', 'databricks.com'],
+    ],
     [
       'dbc-1.cloud.databricks.us',
       ['dbc-1.cloud.databricks.us', 'cloud.databricks.us', 'databricks.us', 'databricks.com'],
@@ -81,6 +85,8 @@ describe('faviconHostCandidates', () => {
     'docs.gitlab.acme-internal.io',
     'notawsapps.com',
     'acme.herokuapp.com',
+    'gitlab.azurewebsites.net',
+    'gitlab.acme.vercel.app',
   ])('adds no platform for %s, which only resembles one', (host) => {
     const candidates = faviconHostCandidates(host)
     expect(candidates).not.toContain('gitlab.com')

apps/sim/lib/favicon/platforms.ts

@@ -1,3 +1,5 @@
+import { parse } from 'tldts'
+
 /**
  * Platforms whose tenant hosts no icon provider can see — not even at any parent the walk
  * reaches — mapped to the platform's own site, so a tenant still shows the product it is. A
@@ -14,6 +16,7 @@ const PLATFORM_SITES: ReadonlyMap<string, string> = new Map([
   ['coupahost.com', 'coupa.com'],
   ['databricks.us', 'databricks.com'],
   ['databricksapps.com', 'databricks.com'],
+  ['databricksapps.mil', 'databricks.com'],
   ['databricksapps.us', 'databricks.com'],
   ['gitlab-dedicated.com', 'gitlab.com'],
   ['gitlab-dedicated.site', 'gitlab.com'],
@@ -24,7 +27,10 @@ const PLATFORM_SITES: ReadonlyMap<string, string> = new Map([
   ['ultipro.com', 'ukg.com'],
 ])
 
-/** Self-hosted tools conventionally served at `<product>.<company domain>`. */
+/**
+ * Self-hosted tools conventionally served at `<product>.<company domain>` — only on a company's
+ * own domain, never on a hosting platform's suffix (`gitlab.azurewebsites.net` is someone's app).
+ */
 const SELF_HOSTED_PRODUCTS: ReadonlyMap<string, string> = new Map([
   ['confluence', 'atlassian.com'],
   ['gitlab', 'gitlab.com'],
@@ -41,5 +47,6 @@ export function platformSite(hostname: string): string | null {
     const site = PLATFORM_SITES.get(labels.slice(index).join('.'))
     if (site) return site
   }
+  if (parse(hostname, { allowPrivateDomains: true }).isPrivate) return null
   return SELF_HOSTED_PRODUCTS.get(labels[0]) ?? null
 }

apps/sim/lib/favicon/resolve.server.test.ts

@@ -0,0 +1,74 @@
+import {
+  createMockRedis,
+  inputValidationMock,
+  inputValidationMockFns,
+  redisConfigMockFns,
+} from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { resolveFavicon } from '@/lib/favicon/resolve.server'
+
+vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock)
+
+const PNG = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0])
+const DAY = 24 * 60 * 60
+
+type Answer = 'icon' | 'none' | 'timeout'
+
+/** Each host's answer at both providers; any host not listed has no icon. */
+let answers: Record<string, Answer> = {}
+let stored: Map<string, { value: Buffer; ttl: number }>
+
+function hostOf(url: string): string {
+  const match = url.match(/url=https:\/\/([^&]+)&/) ?? url.match(/\/ip3\/(.+)\.ico$/)
+  if (!match) throw new Error(`Unexpected provider URL ${url}`)
+  return match[1]
+}
+
+beforeEach(() => {
+  answers = {}
+  stored = new Map()
+  const redis = createMockRedis()
+  redis.getBuffer.mockImplementation(async (key: string) => stored.get(key)?.value ?? null)
+  redis.set.mockImplementation(
+    async (key: string, value: Buffer | string, _ex: string, ttl: number) => {
+      stored.set(key, { value: Buffer.from(value), ttl })
+      return 'OK'
+    }
+  )
+  redisConfigMockFns.mockGetRedisClient.mockReturnValue(redis)
+  inputValidationMockFns.mockSecureFetchWithValidation.mockImplementation(async (url: string) => {
+    const answer = answers[hostOf(url)] ?? 'none'
+    if (answer === 'timeout') throw new Error('timed out')
+    return {
+      status: answer === 'icon' ? 200 : 404,
+      arrayBuffer: async () => PNG.buffer.slice(PNG.byteOffset, PNG.byteOffset + PNG.byteLength),
+      body: { cancel: async () => {} },
+    }
+  })
+})
+
+const ttlOf = (host: string) => stored.get(`favicon:v3:${host}`)?.ttl
+
+describe('resolveFavicon', () => {
+  it('keeps a parent icon chosen after a timeout provisional when a child host reuses it', async () => {
+    answers = { 'gitlab.acme-internal.io': 'timeout', 'gitlab.com': 'icon' }
+    expect((await resolveFavicon('gitlab.acme-internal.io')).kind).toBe('hit')
+    expect(ttlOf('gitlab.acme-internal.io')).toBe(DAY)
+
+    expect((await resolveFavicon('docs.gitlab.acme-internal.io')).kind).toBe('hit')
+    expect(ttlOf('docs.gitlab.acme-internal.io')).toBe(DAY)
+  })
+
+  it('still reaches the product fallback after its parent was cached as a miss', async () => {
+    answers = { 'atlassian.com': 'icon' }
+    expect((await resolveFavicon('acme-internal.io')).kind).toBe('miss')
+
+    expect((await resolveFavicon('jira.acme-internal.io')).kind).toBe('hit')
+  })
+
+  it('remembers a definite icon for a month', async () => {
+    answers = { 'acme.example.com': 'icon' }
+    expect((await resolveFavicon('acme.example.com')).kind).toBe('hit')
+    expect(ttlOf('acme.example.com')).toBe(30 * DAY)
+  })
+})

apps/sim/lib/favicon/resolve.server.ts

@@ -15,7 +15,12 @@ const MISS_TTL_SECONDS = 24 * 60 * 60
 /** A parent's icon chosen because the host's own lookup timed out: retried a day later, not a month. */
 const PROVISIONAL_HIT_TTL_SECONDS = 24 * 60 * 60
 const UNAVAILABLE_TTL_SECONDS = 5 * 60
-/** Cached values: an icon's bytes, or one of these markers. */
+/**
+ * Cached values: a hit is one prefix byte — definite or provisional, which a host reusing it
+ * must inherit — followed by the icon; the markers stand for no icon and for an outage.
+ */
+const DEFINITE_HIT_PREFIX = 'h'
+const PROVISIONAL_HIT_PREFIX = 'p'
 const MISS_MARKER = ''
 const UNAVAILABLE_MARKER = '-'
 
@@ -46,8 +51,13 @@ async function readCache(host: string): Promise<FaviconResolution | null> {
     const cached = await redis.getBuffer(cacheKey(host))
     if (cached === null) return null
     if (cached.toString() === UNAVAILABLE_MARKER) return { kind: 'unavailable' }
-    const result = classifyProviderResponse(200, cached)
-    return result.kind === 'hit' ? { ...result, bytes: cached } : { kind: 'miss' }
+    const prefix = cached.subarray(0, 1).toString()
+    const bytes = cached.subarray(1)
+    const result = classifyProviderResponse(200, bytes)
+    if (result.kind !== 'hit') return { kind: 'miss' }
+    return prefix === PROVISIONAL_HIT_PREFIX
+      ? { ...result, bytes, provisional: true }
+      : { ...result, bytes }
   } catch (error) {
     logger.warn('Favicon cache read failed', { error })
     return null
@@ -59,10 +69,15 @@ async function writeCache(host: string, resolution: FaviconResolution): Promise<
   if (!redis) return
   const [value, ttl] =
     resolution.kind === 'hit'
-      ? ([
-          resolution.bytes,
-          resolution.provisional ? PROVISIONAL_HIT_TTL_SECONDS : HIT_TTL_SECONDS,
-        ] as const)
+      ? resolution.provisional
+        ? ([
+            Buffer.concat([Buffer.from(PROVISIONAL_HIT_PREFIX), resolution.bytes]),
+            PROVISIONAL_HIT_TTL_SECONDS,
+          ] as const)
+        : ([
+            Buffer.concat([Buffer.from(DEFINITE_HIT_PREFIX), resolution.bytes]),
+            HIT_TTL_SECONDS,
+          ] as const)
       : resolution.kind === 'miss'
         ? ([MISS_MARKER, MISS_TTL_SECONDS] as const)
         : ([UNAVAILABLE_MARKER, UNAVAILABLE_TTL_SECONDS] as const)
@@ -112,11 +127,19 @@ async function askProviders(host: string, signal: AbortSignal): Promise<FaviconR
  */
 async function walk(candidates: string[], signal: AbortSignal): Promise<FaviconResolution> {
   let unavailable = false
+  const covered = new Set<string>()
   for (const [index, host] of candidates.entries()) {
+    if (covered.has(host)) continue
     const parent = index > 0
     const cached = parent ? await readCache(host) : null
-    if (cached?.kind === 'hit') return unavailable ? { ...cached, provisional: true } : cached
-    if (cached) return unavailable ? { kind: 'unavailable' } : cached
+    if (cached?.kind === 'hit') {
+      return unavailable || cached.provisional ? { ...cached, provisional: true } : cached
+    }
+    if (cached) {
+      if (cached.kind === 'unavailable') unavailable = true
+      for (const reached of faviconHostCandidates(host)) covered.add(reached)
+      continue
+    }
     const result = await askProviders(host, signal)
     if (result.kind === 'hit') {
       if (parent) await writeCache(host, result)

packages/testing/src/mocks/redis.mock.ts

@@ -30,6 +30,7 @@ export function createMockRedis() {
 
     // Key operations
     get: vi.fn().mockResolvedValue(null),
+    getBuffer: vi.fn().mockResolvedValue(null),
     set: vi.fn().mockResolvedValue('OK'),
     del: vi.fn().mockResolvedValue(1),
     exists: vi.fn().mockResolvedValue(0),