fix(website): bind app art failures to url generations

[bolichen97]

Summary

• re-arm the screenshot and hero failure latches during render instead of in a passive reset effect, so an image that fails before an effect would run keeps its real error instead of being re-shown at a dead URL
• because the re-arm happens on every URL transition, returning to an earlier generation (theme flip back, refetch) retries rather than restoring a stale failure
• a failure carries the generation that produced it, so a superseded onError is dropped instead of resurrecting a cleared set

Determinism evidence

• the two effect-order regressions fail 2/2 against the unpatched production components
• the two A→B→A regressions (light art fails → dark theme → back to light) also fail 2/2 against the pre-fix components, and pass after
• state remains bounded to one current generation; no retries, sleeps, timeout changes, or weakened assertions were added

Validation

• AppDetailArtLocalFallback.test.tsx: 27 passed
• all six AppDetailPage* test files: 96 passed
• tsc -b: passed

Why no screenshot: this is a state-sequencing fix inside two existing image-error latches. It introduces no new markup, styling, copy or layout — every rendered state (image shown, swapped to the local fallback, hidden) already existed and is pixel-identical. What changes is which of those pre-existing states is reached after a specific effect-order / theme-flip sequence, which a still frame cannot show; the four regression tests above assert it directly.

Overlap audit

• feat(dashboard): show repo contributors on the app Details panel #6646 changes the contributors UI in the same page file; merge-tree is clean and the behavior is disjoint
• fix(dashboard): one stated rule for small-size spinners #4085 changes spinner presentation in the same page file; this hunk auto-merges, while that PR remains conflicted elsewhere

This is the independent owner fix for the full-suite failure observed on #5248; #5248 itself is intentionally unchanged.

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of fd77794a4552d4a4d3d0954d884f7895c814940e and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] fd77794

False positive or not applicable? A repository writer can comment:
/ai-review override gpt fd77794a4552d4a4d3d0954d884f7895c814940e: <one-sentence reason>

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of fd77794a4552d4a4d3d0954d884f7895c814940e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

No user-facing surface changes here — the diff is purely internal image-failure latch bookkeeping in AppDetailPage.tsx (no new strings, layout, controls, or screenshots), and the behavior change it does ship is a UX win: dead screenshot/hero URLs no longer flash back after a theme flip, and returning to a previously failed list retries instead of staying blank. The hidden-on-double-failure terminal state is pre-existing, not introduced here.

UX-Verdict: PASS

Invisible-chrome fix: failed app art now hides reliably and retries on return, with no new copy, controls, or layout to misread.

[UX-REVIEWED] fd77794

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of fd77794a4552d4a4d3d0954d884f7895c814940e — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Render-time re-arm with generation-tokened failures is React's documented fix for exactly this effect-order race; scoped, root-cause, and regression-tested against the unpatched components.

Suggestions

• The same latch-plus-generation logic is now hand-rolled twice in this file (set-shaped for the gallery, boolean-shaped for the hero), while components/AppIcon.tsx — the pattern these latches originally mirrored — still resets in a passive effect and so still carries the pre-fix bug class. A shared useImageFailureLatch hook serving both copies here, with AppIcon adopting it as a follow-up, would stop the two patterns from diverging.

[DESIGN-REVIEWED] fd77794

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of fd77794a4552d4a4d3d0954d884f7895c814940e — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. The change is a state-sequencing fix in two image-failure latches, and the repo carries three more sites with the identical effect-reset pattern the PR's own comments call the bug. Producing the review now.

First-Principles-Verdict: CONCERNS

The fix is real and correctly aimed, but the effect-reset pattern it calls a bug survives verbatim in three sibling art latches this change leaves unfixed.

What this change ships

Intent: a failed app image should keep its real failure (swap/hide) instead of being re-shown at a dead URL after a race between the error and the reset effect — a FIX.

1. A screenshot that fails before effects run stays failed instead of re-showing the dead URL — justified (the fix).
2. The hero banner gets the same pre-effect failure fix — justified.
3. An error event from a superseded URL generation is dropped, not resurrected — justified, declared.
4. Returning to a previously failed URL (theme flip back, refetch) retries — declared; preserves prior behavior by construction.
5. Four regression tests pinning the effect-order and A→B→A sequences — justified.

Watch

• Point patch with counted siblings: grepping set\w*Failed\(false\)|setFailed\(''\) inside useEffect resets keyed on an art URL finds 3 unfixed instances of the same root cause — website/src/components/AppIcon.tsx:144-145 and :167 (the two-latch shape the deleted comment says this gallery mirrored), website/src/components/appstore/useHeroArt.ts:256, website/src/components/appstore/useEditorialArt.ts:39. Each can erase a pre-effect failure the same way. The description scopes itself to fix(channels): per-command trust tiers on approval cards (#5231) #5248's two components, so this is accepted-and-deferred — but the siblings should be named somewhere a human sees them.

Subtractions

• Delete the mismatch-masking reads in AppDetailPage.tsx (the primaryFailed/fallbackFailed ternaries at the gallery, and the latch.src === src && … guards in HeroBanner): the render-phase setFailures/setLatch forces an immediate re-render before commit, so a committed render never sees mismatched keys — read failures.primary/failures.fallback/latch.primaryFailed directly (~10 lines). The generation check inside the onError updaters is the reachable one; keep that.

[FIRST-PRINCIPLES-REVIEWED] fd77794

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed fd77794a4552d4a4d3d0954d884f7895c814940e — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] fd77794

Verdict parsed from the review's SHA-scoped output markers for commit fd77794a4552d4a4d3d0954d884f7895c814940e.

False positive or not applicable? A repository writer can comment:
/ai-review override fable fd77794a4552d4a4d3d0954d884f7895c814940e: <one-sentence reason>

[chenmingwei23]

Tier 1 auto-approve: fix (2 files): bind AppDetail screenshot failure latches to the URL generation that rendered them; clear root cause. Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST (Semgrep + CodeQL) annotations/alerts clean, security checklist all-NO, AI reviewers green.

[iamwhatever]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: single root cause -- the screenshot failure latches were reset in a passive useEffect, so an image that failed before the effect ran had its real failure erased and the dead URL re-shown; the latches are now re-armed during render and keyed to the URL generation that produced them.