From ec7b31c79cfbb0dcb3e429bc8aad35caee404ef6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 30 Oct 2026 21:23:15 -0700 Subject: [PATCH 1/4] improvement(favicons): resolve platform tenants no provider can see, and retry icons chosen after a timeout A tenant on a platform whose domain has no icon at either provider, even at the parent (AWS sign-in portals, Coupa, GitLab Dedicated, Databricks apps, Salesforce, Okta preview, UKG, Jama), and a self-hosted gitlab./jira./testrail. host, now end their candidate walk at the platform's own site, after their own host and parents so a tenant's custom icon still wins. A parent's icon reached only because a more specific lookup timed out is cached for a day instead of thirty, since the host's own icon may exist. --- apps/sim/lib/favicon/hosts.test.ts | 54 ++++++++++++++++++++++++++ apps/sim/lib/favicon/hosts.ts | 11 ++++-- apps/sim/lib/favicon/platforms.ts | 40 +++++++++++++++++++ apps/sim/lib/favicon/resolve.server.ts | 18 ++++++--- 4 files changed, 115 insertions(+), 8 deletions(-) create mode 100644 apps/sim/lib/favicon/platforms.ts diff --git a/apps/sim/lib/favicon/hosts.test.ts b/apps/sim/lib/favicon/hosts.test.ts index adf37453d92..aabb48c7684 100644 --- a/apps/sim/lib/favicon/hosts.test.ts +++ b/apps/sim/lib/favicon/hosts.test.ts @@ -40,6 +40,60 @@ 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.lightning.force.com', + ['acme.lightning.force.com', 'lightning.force.com', 'force.com', 'salesforce.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', + ])('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('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', diff --git a/apps/sim/lib/favicon/hosts.ts b/apps/sim/lib/favicon/hosts.ts index 5615f58a256..c7b85840f29 100644 --- a/apps/sim/lib/favicon/hosts.ts +++ b/apps/sim/lib/favicon/hosts.ts @@ -1,4 +1,5 @@ import { parse } from 'tldts' +import { platformSite } from '@/lib/favicon/platforms' const MAX_PARENT_HOSTS = 3 const HOST_SHAPE = /^[^\s/?#@\\]+$/ @@ -6,8 +7,10 @@ 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.`) 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 } diff --git a/apps/sim/lib/favicon/platforms.ts b/apps/sim/lib/favicon/platforms.ts new file mode 100644 index 00000000000..401042bac20 --- /dev/null +++ b/apps/sim/lib/favicon/platforms.ts @@ -0,0 +1,40 @@ +/** + * Platforms whose tenant hosts no icon provider can see — not even at the parent domain — mapped + * to the platform's own site, so a tenant still shows the product it is. 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. + */ +const PLATFORM_SITES: ReadonlyMap = new Map([ + ['awsapps.com', 'aws.amazon.com'], + ['azuredatabricks.net', 'databricks.com'], + ['cloud.databricks.com', 'databricks.com'], + ['coupahost.com', 'coupa.com'], + ['databricksapps.com', 'databricks.com'], + ['gitlab-dedicated.site', 'gitlab.com'], + ['jamacloud.com', 'jamasoftware.com'], + ['lightning.force.com', 'salesforce.com'], + ['my.salesforce.com', 'salesforce.com'], + ['oktapreview.com', 'okta.com'], + ['ultipro.com', 'ukg.com'], +]) + +/** Self-hosted tools conventionally served at `.`. */ +const SELF_HOSTED_PRODUCTS: ReadonlyMap = new Map([ + ['confluence', 'atlassian.com'], + ['gitlab', 'gitlab.com'], + ['grafana', 'grafana.com'], + ['jenkins', 'jenkins.io'], + ['jira', 'atlassian.com'], + ['sentry', 'sentry.io'], + ['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 + } + return SELF_HOSTED_PRODUCTS.get(labels[0]) ?? null +} diff --git a/apps/sim/lib/favicon/resolve.server.ts b/apps/sim/lib/favicon/resolve.server.ts index 26c7c08f638..181710f2498 100644 --- a/apps/sim/lib/favicon/resolve.server.ts +++ b/apps/sim/lib/favicon/resolve.server.ts @@ -12,6 +12,8 @@ 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. */ const MISS_MARKER = '' @@ -28,7 +30,7 @@ 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' } @@ -57,7 +59,10 @@ 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.bytes, + resolution.provisional ? PROVISIONAL_HIT_TTL_SECONDS : HIT_TTL_SECONDS, + ] as const) : resolution.kind === 'miss' ? ([MISS_MARKER, MISS_TTL_SECONDS] as const) : ([UNAVAILABLE_MARKER, UNAVAILABLE_TTL_SECONDS] as const) @@ -101,18 +106,21 @@ async function askProviders(host: string, signal: AbortSignal): Promise { let unavailable = false for (const [index, host] of candidates.entries()) { 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: true } : cached + if (cached) return unavailable ? { kind: 'unavailable' } : cached 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 } From 1d1506280b50ce192e756486c27b3cd2b28914c6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 30 Oct 2026 21:32:17 -0700 Subject: [PATCH 2/4] improvement(favicons): correct the platform table from vendor docs and keep misses from flashing GitLab Dedicated instances live on gitlab-dedicated.com (and .systems pre-production); .site is their Pages domain, kept to match gitlab.io. Adds coupacloud.com, databricks.us, databricksapps.us, and testrail.io, each a suffix both providers miss at every level; drops entries the parent walk already resolves and the self-hosted sentry label, whose name collides with other products. A host with no icon is remembered client-side for the route's miss lifetime, so a remounted icon shows its fallback without an empty frame, and the cache key moves to v3 so stale misses for newly mapped platforms clear on deploy. --- apps/sim/components/favicon.tsx | 14 ++++++++++++-- apps/sim/lib/favicon/hosts.test.ts | 17 +++++++++++++++-- apps/sim/lib/favicon/platforms.ts | 21 +++++++++++++-------- apps/sim/lib/favicon/resolve.server.ts | 11 ++++++----- 4 files changed, 46 insertions(+), 17 deletions(-) diff --git a/apps/sim/components/favicon.tsx b/apps/sim/components/favicon.tsx index 53a192c8d59..fc5ce30e5f0 100644 --- a/apps/sim/components/favicon.tsx +++ b/apps/sim/components/favicon.tsx @@ -2,8 +2,15 @@ import { type ReactNode, useState } from 'react' import { cn } from '@sim/emcn' +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. + */ +const knownMisses = new LRUCache({ max: 500, ttl: 60 * 60 * 1000 }) + interface FaviconProps { /** The site's hostname; without one the fallback renders. */ host: string | null | undefined @@ -20,7 +27,7 @@ interface FaviconProps { */ export function Favicon({ host, fallback, className }: FaviconProps) { const [failedHost, setFailedHost] = useState(null) - if (!host || failedHost === host) return fallback + if (!host || failedHost === host || knownMisses.has(host)) return fallback return ( setFailedHost(host)} + onError={() => { + knownMisses.set(host, true) + setFailedHost(host) + }} /> ) } diff --git a/apps/sim/lib/favicon/hosts.test.ts b/apps/sim/lib/favicon/hosts.test.ts index aabb48c7684..c98e0cd1f07 100644 --- a/apps/sim/lib/favicon/hosts.test.ts +++ b/apps/sim/lib/favicon/hosts.test.ts @@ -53,8 +53,13 @@ describe('faviconHostCandidates', () => { ], ], [ - 'acme.lightning.force.com', - ['acme.lightning.force.com', 'lightning.force.com', 'force.com', 'salesforce.com'], + 'acme.gitlab-dedicated.com', + ['acme.gitlab-dedicated.com', 'gitlab-dedicated.com', 'gitlab.com'], + ], + ['acme.testrail.io', ['acme.testrail.io', 'testrail.io', 'testrail.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', @@ -90,6 +95,14 @@ describe('faviconHostCandidates', () => { } ) + 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']) }) diff --git a/apps/sim/lib/favicon/platforms.ts b/apps/sim/lib/favicon/platforms.ts index 401042bac20..aed73e93adf 100644 --- a/apps/sim/lib/favicon/platforms.ts +++ b/apps/sim/lib/favicon/platforms.ts @@ -1,20 +1,26 @@ /** - * Platforms whose tenant hosts no icon provider can see — not even at the parent domain — mapped - * to the platform's own site, so a tenant still shows the product it is. 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. + * 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 = new Map([ ['awsapps.com', 'aws.amazon.com'], ['azuredatabricks.net', 'databricks.com'], - ['cloud.databricks.com', 'databricks.com'], + ['coupacloud.com', 'coupa.com'], ['coupahost.com', 'coupa.com'], + ['databricks.us', 'databricks.com'], ['databricksapps.com', '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'], - ['lightning.force.com', 'salesforce.com'], - ['my.salesforce.com', 'salesforce.com'], ['oktapreview.com', 'okta.com'], + ['testrail.io', 'testrail.com'], ['ultipro.com', 'ukg.com'], ]) @@ -25,7 +31,6 @@ const SELF_HOSTED_PRODUCTS: ReadonlyMap = new Map([ ['grafana', 'grafana.com'], ['jenkins', 'jenkins.io'], ['jira', 'atlassian.com'], - ['sentry', 'sentry.io'], ['testrail', 'testrail.com'], ]) diff --git a/apps/sim/lib/favicon/resolve.server.ts b/apps/sim/lib/favicon/resolve.server.ts index 181710f2498..cbed4b7a130 100644 --- a/apps/sim/lib/favicon/resolve.server.ts +++ b/apps/sim/lib/favicon/resolve.server.ts @@ -36,7 +36,7 @@ type FaviconResolution = const inFlight = new Map>() -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 { @@ -128,10 +128,11 @@ async function walk(candidates: string[], signal: AbortSignal): Promise { const candidates = faviconHostCandidates(host) From 4d7d45140ebe2b2686adcf50ac08802e6be58c27 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 30 Oct 2026 21:46:08 -0700 Subject: [PATCH 3/4] 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. --- apps/sim/components/favicon.tsx | 7 +- apps/sim/lib/favicon/hosts.test.ts | 6 ++ apps/sim/lib/favicon/platforms.ts | 9 ++- apps/sim/lib/favicon/resolve.server.test.ts | 74 +++++++++++++++++++++ apps/sim/lib/favicon/resolve.server.ts | 41 +++++++++--- packages/testing/src/mocks/redis.mock.ts | 1 + 6 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 apps/sim/lib/favicon/resolve.server.test.ts diff --git a/apps/sim/components/favicon.tsx b/apps/sim/components/favicon.tsx index fc5ce30e5f0..89cf0ab72b1 100644 --- a/apps/sim/components/favicon.tsx +++ b/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 `` 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({ max: 500, ttl: 60 * 60 * 1000 }) +const knownMisses = new LRUCache({ max: 500, ttl: 5 * 60 * 1000 }) interface FaviconProps { /** The site's hostname; without one the fallback renders. */ diff --git a/apps/sim/lib/favicon/hosts.test.ts b/apps/sim/lib/favicon/hosts.test.ts index c98e0cd1f07..c47ced23dd2 100644 --- a/apps/sim/lib/favicon/hosts.test.ts +++ b/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') diff --git a/apps/sim/lib/favicon/platforms.ts b/apps/sim/lib/favicon/platforms.ts index aed73e93adf..93e54075803 100644 --- a/apps/sim/lib/favicon/platforms.ts +++ b/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 = 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 = new Map([ ['ultipro.com', 'ukg.com'], ]) -/** Self-hosted tools conventionally served at `.`. */ +/** + * Self-hosted tools conventionally served at `.` — 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 = 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 } diff --git a/apps/sim/lib/favicon/resolve.server.test.ts b/apps/sim/lib/favicon/resolve.server.test.ts new file mode 100644 index 00000000000..7ff867168d6 --- /dev/null +++ b/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 = {} +let stored: Map + +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) + }) +}) diff --git a/apps/sim/lib/favicon/resolve.server.ts b/apps/sim/lib/favicon/resolve.server.ts index cbed4b7a130..53caf74f274 100644 --- a/apps/sim/lib/favicon/resolve.server.ts +++ b/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 { 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 { let unavailable = false + const covered = new Set() 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) diff --git a/packages/testing/src/mocks/redis.mock.ts b/packages/testing/src/mocks/redis.mock.ts index f78b5f2024e..947165d11f0 100644 --- a/packages/testing/src/mocks/redis.mock.ts +++ b/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), From ebd362e517514dcc8e9b8d0b80ab0762e47b3ea6 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 30 Oct 2026 21:58:44 -0700 Subject: [PATCH 4/4] chore(favicons): import resolver test mocks from their own files --- apps/sim/lib/favicon/resolve.server.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/favicon/resolve.server.test.ts b/apps/sim/lib/favicon/resolve.server.test.ts index 7ff867168d6..cc89109539d 100644 --- a/apps/sim/lib/favicon/resolve.server.test.ts +++ b/apps/sim/lib/favicon/resolve.server.test.ts @@ -1,9 +1,9 @@ import { - createMockRedis, inputValidationMock, inputValidationMockFns, - redisConfigMockFns, -} from '@sim/testing' +} 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'