Fix broken project logos: validate candidates + more sources + self-heal - #70
Merged
Merged
Conversation
Broken dashboard logos had two causes: discoverLogoUrl stored the first candidate by weight without checking it loads (so a 404'ing apple-touch-icon became a broken <img>), and ProjectLogo was server-rendered with no error fallback (broken URLs showed as broken images). - discoverLogo now VALIDATES each candidate (real request → 2xx + image content-type, or a clear image extension) and returns the first that actually loads, else null. Also looks at more sources: web-app-manifest icons (often the best brand mark), mask-icon, and respects <base href>. "any"-sized icons rank large. - ProjectLogo is now a client component: falls back to the letter avatar on a broken image AND fires a one-shot refetchProjectLogo so a stale/404 logo_url self-heals (re-discovery overwrites it, or clears it to the letter avatar). - refetchProjectLogo re-discovers even when a logo is already set (the null-only backfill couldn't fix already-broken ones). Tests: candidate validation/fallback, highest-weight preference, null when nothing loads, extension-based acceptance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Broken logos on
/dashboardhad two root causes, both fixed here.1.
discoverLogoUrlnever checked its pick actually loadsIt returned the highest-weight candidate without verifying it — so a linked
apple-touch-icon(or og:image) that 404s got stored and rendered as a broken<img>.Now it validates each candidate (real request →
2xx+image/*content-type, or a clear image extension on 2xx) and returns the first that actually loads, elsenull. It also looks harder for a real logo:<link rel="manifest">→icons[], sized) — often the best brand mark<base href>, and treatssizes="any"as large2.
ProjectLogohad no error fallbackIt was server-rendered
<img>, so a broken URL just showed broken. It's now a client component: on error it falls back to the letter avatar and fires a one-shotrefetchProjectLogo, so a stale/404logo_urlself-heals — re-discovery overwrites it with a working one, or clears it to a clean letter avatar. (The old backfill only ran fornulllogos, so it couldn't fix already-broken ones.)Verification
tsc --noEmitcleantests/discover-logo.test.ts(4): skips a broken candidate → next valid one; prefers highest-weight when it loads;nullwhen nothing loads; accepts an image served with a generic content-type by extensionExisting broken tiles fix themselves on next dashboard view (onError → refetch); new projects only ever store a validated URL.
🤖 Generated with Claude Code