From f490fcc9da658da303a4d7d8951081343a936361 Mon Sep 17 00:00:00 2001
From: openhands <openhands@all-hands.dev>
Date: Thu, 24 Sep 2026 23:34:54 +0200
Subject: [PATCH] fix(imaging): stop caching fallback renders

A fallback render drops the requested effect and is only a degraded
stand-in, so writing it to the 30 day disk cache kept serving the worse
image long after the local renderer recovered. Cache primary renders only
and let the next request pick up the real render.
---
 src/lib/imager-upstream.ts | 10 +++---
 src/lib/imager.test.ts     | 71 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/src/lib/imager-upstream.ts b/src/lib/imager-upstream.ts
index 321a1c58..fecf5a6c 100644
--- a/src/lib/imager-upstream.ts
+++ b/src/lib/imager-upstream.ts
@@ -133,13 +133,11 @@ export async function fetchAvatarImage(
 		buildFallbackParams(params),
 		FALLBACK_TIMEOUT_MS,
 	);
+	// Deliberately not cached: a fallback render drops the effect and is only a
+	// degraded stand-in, so persisting it would keep serving the worse image
+	// for the whole cache lifetime. The local renderer caches its own renders,
+	// so the next request for this figure is cheap and returns the real one.
 	if (fallbackResult) {
-		await writeImagingCache(
-			avatarCacheDir(),
-			cacheKey,
-			fallbackResult.body,
-			fallbackResult.contentType,
-		);
 		return { ...fallbackResult, source: "fallback" };
 	}
 
diff --git a/src/lib/imager.test.ts b/src/lib/imager.test.ts
index af763267..e99ad5fd 100644
--- a/src/lib/imager.test.ts
+++ b/src/lib/imager.test.ts
@@ -1,7 +1,14 @@
 // @ts-nocheck
+import { mkdtempSync, readdirSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { getAvatarUrl } from "./imager";
-import { FALLBACK_TIMEOUT_MS, PRIMARY_TIMEOUT_MS } from "./imager-upstream";
+import {
+	FALLBACK_TIMEOUT_MS,
+	fetchAvatarImage,
+	PRIMARY_TIMEOUT_MS,
+} from "./imager-upstream";
 
 afterEach(() => {
 	vi.unstubAllEnvs();
@@ -73,3 +80,65 @@ describe("upstream render budgets", () => {
 		expect(FALLBACK_TIMEOUT_MS).toBeLessThan(PRIMARY_TIMEOUT_MS);
 	});
 });
+
+describe("fetchAvatarImage caching", () => {
+	let cacheRoot = "";
+
+	beforeEach(() => {
+		cacheRoot = mkdtempSync(join(tmpdir(), "imager-cache-"));
+		vi.stubEnv("IMAGING_CACHE_ROOT", cacheRoot);
+		vi.stubEnv("IMAGER_URL", "http://primary.invalid/avatarimage");
+		vi.stubEnv("IMAGING_UPSTREAM_URL", "http://primary.invalid/avatarimage");
+		vi.stubEnv("APP_URL", "https://cms.example");
+	});
+
+	afterEach(() => {
+		vi.unstubAllGlobals();
+		rmSync(cacheRoot, { recursive: true, force: true });
+	});
+
+	const params = () =>
+		new URLSearchParams({
+			figure: "hd-180-1",
+			direction: "2",
+			head_direction: "3",
+			size: "m",
+			img_format: "png",
+		});
+
+	it("persists renders from the primary renderer", async () => {
+		vi.stubGlobal(
+			"fetch",
+			vi.fn(
+				async () =>
+					new Response(new Uint8Array([1, 2, 3]), {
+						headers: { "content-type": "image/png" },
+					}),
+			),
+		);
+
+		const first = await fetchAvatarImage("https://cms.example", params());
+		expect(first.source).toBe("primary");
+		expect(readdirSync(join(cacheRoot, "avatars")).length).toBe(2);
+
+		const second = await fetchAvatarImage("https://cms.example", params());
+		expect(second.source).toBe("cache");
+	});
+
+	it("never persists a fallback render", async () => {
+		vi.stubGlobal(
+			"fetch",
+			vi.fn(async (url: string | URL) =>
+				String(url).includes("primary.invalid")
+					? new Response(null, { status: 500 })
+					: new Response(new Uint8Array([4, 5, 6]), {
+							headers: { "content-type": "image/png" },
+						}),
+			),
+		);
+
+		const result = await fetchAvatarImage("https://cms.example", params());
+		expect(result.source).toBe("fallback");
+		expect(() => readdirSync(join(cacheRoot, "avatars"))).toThrow();
+	});
+});