improvement(favicons): resolve platform tenants no provider can see, retry timeout-derived icons, no miss flash

apps/sim/components/favicon.tsx

@@ -2,8 +2,16 @@
 
 import { type ReactNode, useState } from 'react'
 import { cn } from '@sim/emcn'
+import { LRUCache } from 'lru-cache'
 import { faviconSrc } from '@/lib/favicon/url'
 
+/**
+ * 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: 5 * 60 * 1000 })
+
 interface FaviconProps {
   /** The site's hostname; without one the fallback renders. */
   host: string | null | undefined
@@ -20,15 +28,18 @@ interface FaviconProps {
  */
 export function Favicon({ host, fallback, className }: FaviconProps) {
   const [failedHost, setFailedHost] = useState<string | null>(null)
-  if (!host || failedHost === host) return fallback
+  if (!host || failedHost === host || knownMisses.has(host)) return fallback
   return (
     <img
       src={faviconSrc(host)}
       alt=''
       loading='lazy'
       decoding='async'
       className={cn('rounded-sm', className)}
-      onError={() => setFailedHost(host)}
+      onError={() => {
+        knownMisses.set(host, true)
+        setFailedHost(host)
+      }}
     />
   )
 }

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

@@ -40,6 +40,79 @@ describe('faviconHostCandidates', () => {
     ])
   })
 
+  it.each([
+    ['portal.awsapps.com', ['portal.awsapps.com', 'awsapps.com', 'aws.amazon.com']],
+    ['acme.coupahost.com', ['acme.coupahost.com', 'coupahost.com', 'coupa.com']],
+    [
+      'docs.acme.gitlab-dedicated.site',
+      [
+        'docs.acme.gitlab-dedicated.site',
+        'acme.gitlab-dedicated.site',
+        'gitlab-dedicated.site',
+        'gitlab.com',
+      ],
+    ],
+    [
+      'acme.gitlab-dedicated.com',
+      ['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'],
+    ],
+  ])(
+    'ends a platform tenant %s at the platform, after its own host and parents',
+    (host, expected) => {
+      expect(faviconHostCandidates(host)).toEqual(expected)
+    }
+  )
+
+  it.each([
+    ['gitlab.acme-internal.io', 'gitlab.com'],
+    ['jira.acme-internal.io', 'atlassian.com'],
+    ['testrail.qa.acme-internal.io', 'testrail.com'],
+  ])('ends a self-hosted %s at its product', (host, product) => {
+    expect(faviconHostCandidates(host).at(-1)).toBe(product)
+  })
+
+  it.each([
+    'mygitlab.acme-internal.io',
+    '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')
+    expect(candidates).not.toContain('aws.amazon.com')
+    expect(candidates.length).toBeGreaterThan(0)
+  })
+
+  it.each(['gitlab.corp', 'gitlab', 'jira.local'])(
+    'still offers nothing for internal %s',
+    (host) => {
+      expect(faviconHostCandidates(host)).toEqual([])
+    }
+  )
+
+  it('adds nothing for a platform whose parent the walk already reaches', () => {
+    expect(faviconHostCandidates('acme.my.salesforce.com')).toEqual([
+      'acme.my.salesforce.com',
+      'my.salesforce.com',
+      'salesforce.com',
+    ])
+  })
+
+  it('never repeats a host the walk already reaches', () => {
+    expect(faviconHostCandidates('gitlab.gitlab.com')).toEqual(['gitlab.gitlab.com', 'gitlab.com'])
+  })
+
   it('tries at most three parents', () => {
     expect(faviconHostCandidates('a.b.c.d.e.example.com')).toEqual([
       'a.b.c.d.e.example.com',

apps/sim/lib/favicon/hosts.ts

@@ -1,13 +1,16 @@
 import { parse } from 'tldts'
+import { platformSite } from '@/lib/favicon/platforms'
 
 const MAX_PARENT_HOSTS = 3
 const HOST_SHAPE = /^[^\s/?#@\\]+$/
 
 /**
  * Hosts to ask a favicon provider about, most specific first. A private tenant such as
  * `acme.atlassian.net` is invisible to providers, but its platform is not, so each parent
- * domain follows — at most three, stopping before the ICANN public suffix (`gitlab.io` is a valid last step;
- * `io` and `co.uk` are not). Internal hosts — IP literals, single labels, and names outside
+ * domain follows — at most three, stopping before the ICANN public suffix (`gitlab.io` is a valid
+ * last step; `io` and `co.uk` are not). A host on a platform no provider can see even at the
+ * parent (`x.awsapps.com`, a self-hosted `gitlab.<company>`) ends at the platform's own site, after
+ * its own host and parents so a tenant's custom icon still wins. Internal hosts — IP literals, single labels, and names outside
  * the public DNS — yield nothing, so they never leave the server, and neither does a name with
  * characters DNS can't hold, which could otherwise reshape a provider's URL.
  */
@@ -23,7 +26,9 @@ export function faviconHostCandidates(input: string): string[] {
   if (isIp || !isIcann || !publicSuffix || /(^|\.)arpa$/.test(publicSuffix)) return []
   const labels = hostname.split('.')
   const candidateCount = labels.length - publicSuffix.split('.').length
-  return labels
+  const candidates = labels
     .slice(0, Math.min(candidateCount, MAX_PARENT_HOSTS + 1))
     .map((_, index) => labels.slice(index).join('.'))
+  const platform = platformSite(hostname)
+  return platform && !candidates.includes(platform) ? [...candidates, platform] : candidates
 }

apps/sim/lib/favicon/platforms.ts

@@ -0,0 +1,52 @@
+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
+ * platform whose parent already resolves (`salesforce.com`, `databricks.com`, `atlassian.net`)
+ * needs no entry. Generic hosting domains (`herokuapp.com`, `azurewebsites.net`,
+ * `cloudfront.net`) are deliberately absent: they serve other companies' apps, and the host's
+ * logo would misrepresent them. GitLab Pages on Dedicated (`gitlab-dedicated.site`) maps like
+ * `gitlab.io`, whose parent already shows GitLab's icon.
+ */
+const PLATFORM_SITES: ReadonlyMap<string, string> = new Map([
+  ['awsapps.com', 'aws.amazon.com'],
+  ['azuredatabricks.net', 'databricks.com'],
+  ['coupacloud.com', 'coupa.com'],
+  ['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'],
+  ['gitlab-dedicated.systems', 'gitlab.com'],
+  ['jamacloud.com', 'jamasoftware.com'],
+  ['oktapreview.com', 'okta.com'],
+  ['testrail.io', 'testrail.com'],
+  ['ultipro.com', 'ukg.com'],
+])
+
+/**
+ * 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'],
+  ['grafana', 'grafana.com'],
+  ['jenkins', 'jenkins.io'],
+  ['jira', 'atlassian.com'],
+  ['testrail', 'testrail.com'],
+])
+
+/** The public site of the platform a host belongs to, matched on whole labels, or `null`. */
+export function platformSite(hostname: string): string | null {
+  const labels = hostname.split('.')
+  for (let index = 0; index < labels.length; index++) {
+    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 {
+  inputValidationMock,
+  inputValidationMockFns,
+} from '@sim/testing/mocks/input-validation.mock'
+import { createMockRedis } from '@sim/testing/mocks/redis.mock'
+import { redisConfigMockFns } from '@sim/testing/mocks/redis-config.mock'
+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

@@ -12,8 +12,15 @@ const PROVIDER_TIMEOUT_MS = 1500
 const RESOLVE_DEADLINE_MS = 4000
 const HIT_TTL_SECONDS = 30 * 24 * 60 * 60
 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 = '-'
 
@@ -28,13 +35,13 @@ const PROVIDERS: readonly ((host: string) => string)[] = [
 ]
 
 type FaviconResolution =
-  | { kind: 'hit'; contentType: string; bytes: Buffer }
+  | { kind: 'hit'; contentType: string; bytes: Buffer; provisional?: true }
   | { kind: 'miss' }
   | { kind: 'unavailable' }
 
 const inFlight = new Map<string, Promise<FaviconResolution>>()
 
-const cacheKey = (host: string) => `favicon:v2:${host}`
+const cacheKey = (host: string) => `favicon:v3:${host}`
 
 /** A host's key holds the result of the walk that starts at that host. */
 async function readCache(host: string): Promise<FaviconResolution | null> {
@@ -44,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
@@ -57,7 +69,15 @@ async function writeCache(host: string, resolution: FaviconResolution): Promise<
   if (!redis) return
   const [value, ttl] =
     resolution.kind === 'hit'
-      ? ([resolution.bytes, 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)
@@ -101,29 +121,41 @@ async function askProviders(host: string, signal: AbortSignal): Promise<FaviconR
 
 /**
  * The most specific host first, so a tenant's own icon beats its platform's. A parent already
- * resolved for a sibling subdomain is reused from the cache instead of asked again.
+ * resolved for a sibling subdomain is reused from the cache instead of asked again. A parent's
+ * icon reached only because a more specific lookup was unavailable is `provisional`: the host's
+ * own icon may exist, so it is not remembered for long.
  */
 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) return cached.kind === 'miss' && 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)
-      return result
+      return unavailable ? { ...result, provisional: true } : result
     }
     if (result.kind === 'unavailable') unavailable = true
   }
   return unavailable ? { kind: 'unavailable' } : { kind: 'miss' }
 }
 
 /**
- * The icon for a host, or its nearest parent domain's, from public icon providers — the
- * destination itself is never contacted, so a host behind a login still gets its platform's
- * icon. Results are cached per host (30 days for an icon, a day for none, minutes after an
- * outage), and concurrent lookups of one host in this process share a single resolution.
+ * The icon for a host, or its nearest parent domain's, from public icon providers — Sim never
+ * contacts the destination itself (a provider may, from its own servers), so a host behind a
+ * login still gets its platform's icon. Results are cached per host (30 days for an icon, a day
+ * for none or for a parent's icon chosen after a timeout, minutes after an outage), and
+ * concurrent lookups of one host in this process share a single resolution.
  */
 export async function resolveFavicon(host: string): Promise<FaviconResolution> {
   const candidates = faviconHostCandidates(host)

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